Skip to content

Commit

Permalink
quality fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
zawan-ila committed Jun 8, 2024
1 parent 57bf1f1 commit 3849a0f
Show file tree
Hide file tree
Showing 27 changed files with 148 additions and 86 deletions.
4 changes: 3 additions & 1 deletion course_discovery/apps/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2308,7 +2308,9 @@ def prefetch_queryset(cls, partner, course_runs=None):
queryset = Pathway.objects.filter(partner=partner)

return queryset.prefetch_related(
Prefetch('programs', queryset=MinimalProgramSerializer.prefetch_queryset(partner=partner, course_runs=course_runs)),
Prefetch('programs', queryset=MinimalProgramSerializer.prefetch_queryset(
partner=partner, course_runs=course_runs
)),
)

class Meta:
Expand Down
15 changes: 9 additions & 6 deletions course_discovery/apps/api/tests/test_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2347,9 +2347,9 @@ def test_detail_fields_in_response(self, is_post_request):
'staff': MinimalPersonSerializer(course_run.staff, many=True,
context={'request': request}).data,
'content_language': course_run.language.code if course_run.language else None,
'restriction_type': course_run.restricted_run.restriction_type if hasattr(course_run, 'restricted_run') else None


'restriction_type': (
course_run.restricted_run.restriction_type if hasattr(course_run, 'restricted_run') else None
)
}],
'uuid': str(course.uuid),
'subjects': [subject.name for subject in course.subjects.all()],
Expand Down Expand Up @@ -2420,7 +2420,9 @@ def get_expected_data(cls, course, course_run, course_skill, seat):
'estimated_hours': get_course_run_estimated_hours(course_run),
'first_enrollable_paid_seat_price': course_run.first_enrollable_paid_seat_price or 0.0,
'is_enrollable': course_run.is_enrollable,
'restriction_type': course_run.restricted_run.restriction_type if hasattr(course_run, 'restricted_run') else None
'restriction_type': (
course_run.restricted_run.restriction_type if hasattr(course_run, 'restricted_run') else None
)
}],
'uuid': str(course.uuid),
'subjects': [subject.name for subject in course.subjects.all()],
Expand Down Expand Up @@ -2552,8 +2554,9 @@ def get_expected_data(cls, course_run, course_skill, request):
'first_enrollable_paid_seat_sku': course_run.first_enrollable_paid_seat_sku(),
'first_enrollable_paid_seat_price': course_run.first_enrollable_paid_seat_price,
'is_enrollable': course_run.is_enrollable,
'restriction_type': course_run.restricted_run.restriction_type if hasattr(course_run, 'restricted_run') else None,

'restriction_type': (
course_run.restricted_run.restriction_type if hasattr(course_run, 'restricted_run') else None
)
}


Expand Down
6 changes: 4 additions & 2 deletions course_discovery/apps/api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@

from course_discovery.apps.core.api_client.lms import LMSAPIClient
from course_discovery.apps.core.utils import serialize_datetime
from course_discovery.apps.course_metadata.models import CourseRun
from course_discovery.apps.course_metadata.choices import CourseRunRestrictionType
from course_discovery.apps.course_metadata.models import CourseRun

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -199,10 +199,12 @@ def increment_character(character):
"""
return chr(ord(character) + 1) if character != 'z' else 'a'


def get_excluded_restriction_types(request):
include_restricted=request.query_params.get('include_restricted', '').split(',')
include_restricted = request.query_params.get('include_restricted', '').split(',')
return list(set(CourseRunRestrictionType.values) - set(include_restricted))


class StudioAPI:
"""
A convenience class for talking to the Studio API - designed to allow subclassing by the publisher django app,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from course_discovery.apps.course_metadata.choices import CourseRunStatus
from course_discovery.apps.course_metadata.models import Course, CourseType
from course_discovery.apps.course_metadata.tests.factories import (
CourseRunFactory, SeatFactory, SeatTypeFactory, SubjectFactory, RestrictedCourseRunFactory
CourseRunFactory, RestrictedCourseRunFactory, SeatFactory, SeatTypeFactory, SubjectFactory
)
from course_discovery.conftest import get_course_run_states

Expand Down Expand Up @@ -346,7 +346,7 @@ def test_courses_with_restricted_runs(self, include_restriction_param):
course__title='ABC Test Course With Archived', end=future, enrollment_end=future
)
restricted_course_run = CourseRunFactory.create(
course = course_run.course,
course=course_run.course,
course__title='ABC Test Course With Archived', end=future, enrollment_end=future,
status=CourseRunStatus.Published
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1211,13 +1211,10 @@ def test_list_sorted_by_course_start_date(self):
self.serialize_course_run(CourseRun.objects.all().order_by('start'), many=True)
)


@ddt.data(True, False)
def test_list_include_restricted(self, include_restriction_param):

restricted_run = CourseRunFactory(course__partner=self.partner)
RestrictedCourseRunFactory(course_run=restricted_run, restriction_type='custom-b2c')

url = reverse('api:v1:course_run-list')
if include_restriction_param:
url += '?include_restricted=custom-b2c'
Expand All @@ -1230,11 +1227,11 @@ def test_list_include_restricted(self, include_restriction_param):
if include_restriction_param:
assert restricted_run.key in restrieved_keys
else:
assert not restricted_run.key in restrieved_keys
assert restricted_run.key not in restrieved_keys

@ddt.data(True, False)
def test_list_query_include_restricted(self, include_restriction_param):
course_runs = CourseRunFactory.create_batch(3, title='Some cool title', course__partner=self.partner)
CourseRunFactory.create_batch(3, title='Some cool title', course__partner=self.partner)
CourseRunFactory(title='non-cool title')
restricted_run = CourseRunFactory(title='Some cool title', course__partner=self.partner)
RestrictedCourseRunFactory(course_run=restricted_run, restriction_type='custom-b2c')
Expand Down
32 changes: 25 additions & 7 deletions course_discovery/apps/api/v1/tests/test_views/test_courses.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
from course_discovery.apps.course_metadata.tests.factories import (
CourseEditorFactory, CourseEntitlementFactory, CourseFactory, CourseLocationRestrictionFactory, CourseRunFactory,
CourseTypeFactory, GeoLocationFactory, LevelTypeFactory, OrganizationFactory, ProductValueFactory, ProgramFactory,
SeatFactory, SeatTypeFactory, SourceFactory, SubjectFactory, RestrictedCourseRunFactory
RestrictedCourseRunFactory, SeatFactory, SeatTypeFactory, SourceFactory, SubjectFactory
)
from course_discovery.apps.course_metadata.toggles import IS_SUBDIRECTORY_SLUG_FORMAT_ENABLED
from course_discovery.apps.course_metadata.utils import data_modified_timestamp_update, ensure_draft_world
Expand Down Expand Up @@ -280,8 +280,16 @@ def test_course_runs_are_ordered(self):

@ddt.data(True, False)
def test_course_runs_restriction(self, include_restriction_param):
run_restricted = CourseRunFactory(course=self.course, start=datetime.datetime(2033, 1, 1, tzinfo=pytz.UTC), status=CourseRunStatus.Published)
run_not_restricted = CourseRunFactory(course=self.course, start=datetime.datetime(2033, 1, 1, tzinfo=pytz.UTC), status=CourseRunStatus.Unpublished)
run_restricted = CourseRunFactory(
course=self.course,
start=datetime.datetime(2033, 1, 1, tzinfo=pytz.UTC),
status=CourseRunStatus.Published
)
run_not_restricted = CourseRunFactory(
course=self.course,
start=datetime.datetime(2033, 1, 1, tzinfo=pytz.UTC),
status=CourseRunStatus.Unpublished
)
RestrictedCourseRunFactory(course_run=run_restricted, restriction_type='custom-b2c')
SeatFactory(course_run=run_restricted)
SeatFactory(course_run=run_not_restricted)
Expand All @@ -300,14 +308,24 @@ def test_course_runs_restriction(self, include_restriction_param):
self.assertEqual(response.data['advertised_course_run_uuid'], None)
else:
self.assertEqual(set(response.data['course_run_keys']), {run_not_restricted.key, run_restricted.key})
self.assertEqual(set(response.data['course_run_statuses']), {run_not_restricted.status, run_restricted.status})
self.assertEqual(
set(response.data['course_run_statuses']),
{run_not_restricted.status, run_restricted.status}
)
self.assertEqual(len(response.data['course_runs']), 2)
self.assertEqual(response.data['advertised_course_run_uuid'], run_restricted.uuid)


def test_course_runs_restriction_param(self):
run_restricted = CourseRunFactory(course=self.course, start=datetime.datetime(2033, 1, 1, tzinfo=pytz.UTC), status=CourseRunStatus.Published)
run_not_restricted = CourseRunFactory(course=self.course, start=datetime.datetime(2033, 1, 1, tzinfo=pytz.UTC), status=CourseRunStatus.Unpublished)
run_restricted = CourseRunFactory(
course=self.course,
start=datetime.datetime(2033, 1, 1, tzinfo=pytz.UTC),
status=CourseRunStatus.Published
)
run_not_restricted = CourseRunFactory(
course=self.course,
start=datetime.datetime(2033, 1, 1, tzinfo=pytz.UTC),
status=CourseRunStatus.Unpublished
)
RestrictedCourseRunFactory(course_run=run_restricted, restriction_type='custom-b2c')
SeatFactory(course_run=run_restricted)

Expand Down
11 changes: 5 additions & 6 deletions course_discovery/apps/api/v1/tests/test_views/test_programs.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import urllib.parse
from unittest import mock

import ddt
import pytest
import pytz
from django.test import RequestFactory
Expand All @@ -14,15 +13,16 @@
from course_discovery.apps.api.v1.views.programs import ProgramViewSet
from course_discovery.apps.core.tests.factories import USER_PASSWORD, UserFactory
from course_discovery.apps.core.tests.helpers import make_image_file
from course_discovery.apps.course_metadata.choices import ProgramStatus, CourseRunStatus
from course_discovery.apps.course_metadata.choices import CourseRunStatus, ProgramStatus
from course_discovery.apps.course_metadata.models import CourseType, Program, ProgramType
from course_discovery.apps.course_metadata.tests.factories import (
CorporateEndorsementFactory, CourseFactory, CourseRunFactory, CurriculumCourseMembershipFactory, CurriculumFactory,
CurriculumProgramMembershipFactory, DegreeAdditionalMetadataFactory, DegreeFactory, EndorsementFactory,
ExpectedLearningItemFactory, JobOutlookItemFactory, OrganizationFactory, PersonFactory, ProgramFactory,
ProgramTypeFactory, VideoFactory, RestrictedCourseRunFactory
ProgramTypeFactory, RestrictedCourseRunFactory, VideoFactory
)


@pytest.mark.django_db
@pytest.mark.usefixtures('django_cache')
class TestProgramViewSet(SerializationMixin):
Expand Down Expand Up @@ -55,9 +55,9 @@ def create_program(self, courses=None, program_type=None, include_restricted_run
if courses is None:
courses = [CourseFactory(partner=self.partner)]
course_run = CourseRunFactory(course=courses[0], staff=[person])

if include_restricted_run:
RestrictedCourseRunFactory(course_run=course_run, restriction_type='custom-b2c')
RestrictedCourseRunFactory(course_run=course_run, restriction_type='custom-b2c') # pylint: disable=possibly-used-before-assignment

if program_type is None:
program_type = ProgramTypeFactory()
Expand Down Expand Up @@ -234,7 +234,6 @@ def test_list_restricted_runs(self, include_restriction_param):
assert not resp.data['results'][0]['courses'][0]['course_run_statuses']
assert resp.data['results'][0]['course_run_statuses'] == []


def test_extended_query_param_fields(self):
""" Verify that the `extended` query param will result in an extended amount of fields returned. """
for _ in range(3):
Expand Down
28 changes: 15 additions & 13 deletions course_discovery/apps/api/v1/tests/test_views/test_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import sys
import urllib.parse
import uuid
from itertools import product

import ddt
import factory
Expand All @@ -24,10 +23,10 @@
CourseRunSearchDocumentSerializer, CourseRunSearchModelSerializer, LimitedAggregateSearchSerializer
)
from course_discovery.apps.course_metadata.tests.factories import (
CourseFactory, CourseRunFactory, OrganizationFactory, PersonFactory, PositionFactory, ProgramFactory, SeatFactory, RestrictedCourseRunFactory, SeatTypeFactory
CourseFactory, CourseRunFactory, OrganizationFactory, PersonFactory, PositionFactory, ProgramFactory,
RestrictedCourseRunFactory, SeatFactory, SeatTypeFactory
)
from course_discovery.apps.ietf_language_tags.utils import serialize_language

from course_discovery.apps.learner_pathway.models import LearnerPathway
from course_discovery.apps.learner_pathway.tests.factories import LearnerPathwayStepFactory
from course_discovery.apps.publisher.tests import factories as publisher_factories
Expand Down Expand Up @@ -124,10 +123,10 @@ def test_search(self, path, serializer):
self.assert_successful_search(path=path, serializer=serializer)

@ddt.data(
[list_path, True],
[list_path, False],
[detailed_path, True],
[detailed_path, False]
[list_path, True],
[list_path, False],
[detailed_path, True],
[detailed_path, False]
)
@ddt.unpack
def test_search_restricted_runs(self, path, include_restriction_param):
Expand All @@ -151,7 +150,6 @@ def test_search_restricted_runs(self, path, include_restriction_param):
assert not response.data['results']
assert response.data['count'] == 0


def test_faceted_search(self):
""" Verify the view returns results and facets. """
course_run, response_data = self.assert_successful_search(path=self.faceted_path)
Expand Down Expand Up @@ -343,7 +341,6 @@ def process_response(self, response):
assert objects['count'] > 0
return objects


@ddt.data(True, False)
def test_results_restricted_runs(self, include_restriced_param):
CourseFactory(
Expand Down Expand Up @@ -379,7 +376,11 @@ def test_results_restricted_runs(self, include_restriced_param):
endpoint = self.list_path
if include_restriced_param:
endpoint += '?include_restricted=custom-b2c'
response = self.get_response(query={'key.raw': self.desired_key}, endpoint=endpoint, path_has_params=include_restriced_param)
response = self.get_response(
query={'key.raw': self.desired_key},
endpoint=endpoint,
path_has_params=include_restriced_param
)

assert response.status_code == 200
response_data = response.json()
Expand All @@ -389,12 +390,13 @@ def test_results_restricted_runs(self, include_restriced_param):
assert len(response_data["results"][0]["course_runs"]) == 1
assert set(response_data["results"][0]["languages"]) == {serialize_language(course_run.language)}
assert set(response_data["results"][0]["seat_types"]) == {'audit'}
else:
else:
assert len(response_data["results"][0]["course_runs"]) == 2
assert set(response_data["results"][0]["languages"]) == {serialize_language(course_run.language), serialize_language(course_run_restricted.language)}
assert set(response_data["results"][0]["languages"]) == {
serialize_language(course_run.language), serialize_language(course_run_restricted.language)
}
assert set(response_data["results"][0]["seat_types"]) == {'audit', 'verified'}


def test_results_only_include_specific_key_objects(self):
""" Verify the search results only include items with 'key' set to 'course:edX+DemoX'. """

Expand Down
5 changes: 3 additions & 2 deletions course_discovery/apps/api/v1/views/course_runs.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
from course_discovery.apps.api.pagination import ProxiedPagination
from course_discovery.apps.api.permissions import IsCourseRunEditorOrDjangoOrReadOnly
from course_discovery.apps.api.serializers import MetadataWithRelatedChoices
from course_discovery.apps.api.utils import StudioAPI, get_query_param, reviewable_data_has_changed, get_excluded_restriction_types
from course_discovery.apps.api.utils import (
StudioAPI, get_excluded_restriction_types, get_query_param, reviewable_data_has_changed
)
from course_discovery.apps.api.v1.exceptions import EditableAndQUnsupported
from course_discovery.apps.core.utils import SearchQuerySetWrapper
from course_discovery.apps.course_metadata.choices import CourseRunStatus
Expand Down Expand Up @@ -99,7 +101,6 @@ def get_queryset(self):

if self.request.method == 'GET':
queryset = queryset.exclude(restricted_run__restriction_type__in=excluded_restriction_types)

if q:
queryset = SearchQuerySetWrapper(
CourseRun.search(q).filter('term', partner=partner.short_code).exclude(
Expand Down
8 changes: 6 additions & 2 deletions course_discovery/apps/api/v1/views/courses.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
from course_discovery.apps.api.pagination import ProxiedPagination
from course_discovery.apps.api.permissions import IsCourseEditorOrReadOnly
from course_discovery.apps.api.serializers import CourseEntitlementSerializer, MetadataWithType
from course_discovery.apps.api.utils import decode_image_data, get_query_param, reviewable_data_has_changed, get_excluded_restriction_types
from course_discovery.apps.api.utils import (
decode_image_data, get_excluded_restriction_types, get_query_param, reviewable_data_has_changed
)
from course_discovery.apps.api.v1.exceptions import EditableAndQUnsupported
from course_discovery.apps.api.v1.views.course_runs import CourseRunViewSet
from course_discovery.apps.course_metadata.choices import CourseRunStatus, ProgramStatus
Expand Down Expand Up @@ -126,7 +128,9 @@ def get_queryset(self):
if q:
queryset = Course.search(q, queryset=queryset)
course_runs = CourseRun.objects.exclude(restricted_run__restriction_type__in=excluded_restriction_types)
queryset = self.get_serializer_class().prefetch_queryset(queryset=queryset, partner=partner, course_runs=course_runs)
queryset = self.get_serializer_class().prefetch_queryset(
queryset=queryset, partner=partner, course_runs=course_runs
)
else:
if edit_mode:
course_runs = CourseRun.objects.filter_drafts(course__partner=partner)
Expand Down
9 changes: 6 additions & 3 deletions course_discovery/apps/api/v1/views/pathways.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
from rest_framework import viewsets

from course_discovery.apps.api import serializers
from course_discovery.apps.api.utils import get_excluded_restriction_types
from course_discovery.apps.api.cache import CompressedCacheResponseMixin
from course_discovery.apps.api.permissions import ReadOnlyByPublisherUser

from course_discovery.apps.api.utils import get_excluded_restriction_types
from course_discovery.apps.course_metadata.models import CourseRun


class PathwayViewSet(CompressedCacheResponseMixin, viewsets.ReadOnlyModelViewSet):
permission_classes = (ReadOnlyByPublisherUser,)
serializer_class = serializers.PathwaySerializer
Expand All @@ -16,5 +16,8 @@ def get_queryset(self):
excluded_restriction_types = get_excluded_restriction_types(self.request)
course_runs = CourseRun.objects.exclude(restricted_run__restriction_type__in=excluded_restriction_types)

queryset = self.get_serializer_class().prefetch_queryset(partner=self.request.site.partner, course_runs=course_runs)
queryset = self.get_serializer_class().prefetch_queryset(
partner=self.request.site.partner,
course_runs=course_runs
)
return queryset.order_by('created')
Loading

0 comments on commit 3849a0f

Please sign in to comment.