From 3943338b114690481692dd99979e27d2afd4137d Mon Sep 17 00:00:00 2001 From: Muhammad Afaq Shuaib <78806673+AfaqShuaib09@users.noreply.github.com> Date: Thu, 14 Mar 2024 02:05:57 +0500 Subject: [PATCH] feat: add active filter on course level (#4284) * feat: add active filter on course level [WIP] * chore: add file doc strings --- .../apps/api/tests/test_serializers.py | 6 ++-- .../apps/api/v1/tests/test_views/mixins.py | 19 ++++++++--- .../api/v1/tests/test_views/test_search.py | 11 +++++-- .../apps/course_metadata/models.py | 33 +++++++++++++++++++ .../search_indexes/documents/course.py | 3 +- .../search_indexes/serializers/course.py | 23 ++++++++++++- .../ietf_language_tags/tests/test_utils.py | 29 ++++++++++++++++ .../apps/ietf_language_tags/utils.py | 14 ++++++++ 8 files changed, 126 insertions(+), 12 deletions(-) create mode 100644 course_discovery/apps/ietf_language_tags/tests/test_utils.py create mode 100644 course_discovery/apps/ietf_language_tags/utils.py diff --git a/course_discovery/apps/api/tests/test_serializers.py b/course_discovery/apps/api/tests/test_serializers.py index 92a8632c8f..2529494c05 100644 --- a/course_discovery/apps/api/tests/test_serializers.py +++ b/course_discovery/apps/api/tests/test_serializers.py @@ -2230,7 +2230,7 @@ def test_exclude_expired_course_run(self, is_post_request): ) course.course_runs.add(course_run) course.save() - seat = SeatFactory(course_run=course_run) + course_skill = CourseSkillsFactory( course_key=course.key ) @@ -2250,9 +2250,9 @@ def test_exclude_expired_course_run(self, is_post_request): 'subjects': [subject.name for subject in course.subjects.all()], 'languages': [ serialize_language(course_run.language) for course_run in course.course_runs.all() - if course_run.language + if course_run.language and course.is_active ], - 'seat_types': [seat.type.slug], + 'seat_types': [], 'skill_names': [course_skill.skill.name], 'skills': [ { diff --git a/course_discovery/apps/api/v1/tests/test_views/mixins.py b/course_discovery/apps/api/v1/tests/test_views/mixins.py index 312e6c6477..fd59b60af7 100644 --- a/course_discovery/apps/api/v1/tests/test_views/mixins.py +++ b/course_discovery/apps/api/v1/tests/test_views/mixins.py @@ -22,8 +22,13 @@ class SerializationMixin: - def _get_request(self, format=None): + def _get_request(self, format=None, query_params=None): if getattr(self, 'request', None): + if query_params: + mutable_query_dict = self.request.GET.copy() + mutable_query_dict.update(query_params) + self.request.GET = mutable_query_dict + request = self.request else: query_data = {} @@ -41,8 +46,12 @@ def _get_request(self, format=None): # DRF issue: https://github.com/encode/django-rest-framework/issues/6488 return APIView().initialize_request(request) - def _serialize_object(self, serializer, obj, many=False, format=None, extra_context=None): - context = {'request': self._get_request(format)} + def _serialize_object(self, serializer, obj, many=False, format=None, extra_context=None, query_params=None): + context = {} + if query_params: + context = {'request': self._get_request(format, query_params)} + else: + context = {'request': self._get_request(format)} if extra_context: context.update(extra_context) return serializer(obj, many=many, context=context).data @@ -56,9 +65,9 @@ def serialize_catalog(self, catalog, many=False, format=None, extra_context=None def serialize_course(self, course, many=False, format=None, extra_context=None): return self._serialize_object(serializers.CourseWithProgramsSerializer, course, many, format, extra_context) - def serialize_course_search(self, course, serializer=None): + def serialize_course_search(self, course, serializer=None, query_params=None): obj = self._get_search_result(CourseDocument, **{'key.raw': course.key}) - return self._serialize_object(serializer or CourseSearchDocumentSerializer, obj) + return self._serialize_object(serializer or CourseSearchDocumentSerializer, obj, query_params=query_params) def serialize_course_run(self, run, many=False, format=None, extra_context=None): return self._serialize_object(serializers.CourseRunWithProgramsSerializer, run, many, format, extra_context) diff --git a/course_discovery/apps/api/v1/tests/test_views/test_search.py b/course_discovery/apps/api/v1/tests/test_views/test_search.py index 5743053654..5e0213a20d 100644 --- a/course_discovery/apps/api/v1/tests/test_views/test_search.py +++ b/course_discovery/apps/api/v1/tests/test_views/test_search.py @@ -266,6 +266,11 @@ def setUp(self): super().setUp() self.desired_key = 'course-v1:edx+DemoX+2018' self.regular_key = 'course-v1:edx+TeamX+2019' + # request.GET is immutable, so we need to make a copy of it and then clear it + # Clearing the request.GET is necessary to avoid the query parameters being passed to the next request + temp_req_dict = self.request.GET.copy() + temp_req_dict.clear() + self.request.GET = temp_req_dict def get_response(self, query=None, endpoint=None): path = endpoint or self.faceted_path @@ -504,7 +509,7 @@ def test_results_filtered_by_default_partner(self, short_code): self.serialize_program_search(other_program), ] - @ddt.data((True, 7), (False, 7)) + @ddt.data((True, 7), (False, 8)) @ddt.unpack def test_query_count_exclude_expired_course_run(self, exclude_expired, expected_queries): """ Verify that there is no query explosion when excluding expired course runs. """ @@ -534,7 +539,9 @@ def test_query_count_exclude_expired_course_run(self, exclude_expired, expected_ self.serialize_program_search(program), # We need to render the json, and then parse it again, to get all of the formatted # data the same as the data coming out of search. - json.loads(JSONRenderer().render(self.serialize_course_search(course_run.course)).decode('utf-8')), + json.loads(JSONRenderer().render( + self.serialize_course_search(course_run.course, query_params={'exclude_expired_course_run': 'True'}) + if exclude_expired else self.serialize_course_search(course_run.course)).decode('utf-8')), ] self.assertCountEqual(response_data['results'], expected) diff --git a/course_discovery/apps/course_metadata/models.py b/course_discovery/apps/course_metadata/models.py index da8ac64678..2ee6b92cb0 100644 --- a/course_discovery/apps/course_metadata/models.py +++ b/course_discovery/apps/course_metadata/models.py @@ -64,6 +64,7 @@ push_tracks_to_lms_for_course_run, set_official_state, subtract_deadline_delta ) from course_discovery.apps.ietf_language_tags.models import LanguageTag +from course_discovery.apps.ietf_language_tags.utils import serialize_language from course_discovery.apps.publisher.utils import VALID_CHARS_IN_COURSE_NUM_AND_ORG_KEY logger = logging.getLogger(__name__) @@ -1680,6 +1681,23 @@ def course_ends(self): else: return _('Past') + def languages(self, exclude_inactive_runs=False): + """ + Returns a set of all languages used in this course. + + Arguments: + exclude_inactive_runs (bool): whether to exclude inactive runs + """ + if exclude_inactive_runs: + return list({ + serialize_language(course_run.language) for course_run in self.course_runs.all() + if course_run.is_active and course_run.language is not None + }) + return list({ + serialize_language(course_run.language) for course_run in self.course_runs.all() + if course_run.language is not None + }) + @property def first_enrollable_paid_seat_price(self): """ @@ -1707,6 +1725,13 @@ def course_run_statuses(self): statuses = set() return sorted(list(get_course_run_statuses(statuses, self.course_runs.all()))) + @property + def is_active(self): + """ + Returns true if any of the course runs of the course is active + """ + return any(course_run.is_active for course_run in self.course_runs.all()) + def unpublish_inactive_runs(self, published_runs=None): """ Find old course runs that are no longer active but still published, these will be unpublished. @@ -2850,6 +2875,14 @@ def is_marketable(self): is_published = self.status == CourseRunStatus.Published return is_published and self.seats.exists() and bool(self.marketing_url) + @property + def is_active(self): + """ + Returns True if the course run is active, meaning it is both enrollable, marketable and has not ended. + - `has_ended()`: method of the CourseRun Model to check if the course end date has passed. + """ + return self.is_enrollable and self.is_marketable and not self.has_ended() + def complete_review_phase(self, has_ofac_restrictions, ofac_comment): """ Complete the review phase of the course run by marking status as reviewed and adding diff --git a/course_discovery/apps/course_metadata/search_indexes/documents/course.py b/course_discovery/apps/course_metadata/search_indexes/documents/course.py index 2868e12dcd..3e2c5ae5a3 100644 --- a/course_discovery/apps/course_metadata/search_indexes/documents/course.py +++ b/course_discovery/apps/course_metadata/search_indexes/documents/course.py @@ -123,7 +123,8 @@ def prepare_prerequisites(self, obj): return [prerequisite.name for prerequisite in obj.prerequisites.all()] def get_queryset(self): - return super().get_queryset().prefetch_related('course_runs__seats__type', 'course_runs__type') + return super().get_queryset().prefetch_related( + 'course_runs__seats__type', 'course_runs__type', 'course_runs__language').select_related('partner') def prepare_course_type(self, obj): return obj.type.slug diff --git a/course_discovery/apps/course_metadata/search_indexes/serializers/course.py b/course_discovery/apps/course_metadata/search_indexes/serializers/course.py index 563382f7b8..317441ef01 100644 --- a/course_discovery/apps/course_metadata/search_indexes/serializers/course.py +++ b/course_discovery/apps/course_metadata/search_indexes/serializers/course.py @@ -50,6 +50,7 @@ class CourseSearchDocumentSerializer(ModelObjectDocumentSerializerMixin, DateTim skills = serializers.SerializerMethodField() end_date = serializers.SerializerMethodField() course_ends = serializers.SerializerMethodField() + languages = serializers.SerializerMethodField() def course_run_detail(self, request, detail_fields, course_run): course_run_detail = { @@ -114,8 +115,28 @@ def should_include_course_run(course_run, params, exclude_expired): if should_include_course_run(course_run, query_params, exclude_expired) ] + def get_languages(self, result): + request = self.context['request'] + exclude_non_active_languages = request.GET.get('exclude_expired_course_run') + if request.method == 'POST': + exclude_non_active_languages = request.POST.get('exclude_expired_course_run', exclude_non_active_languages) + + return result.object.languages(exclude_non_active_languages) + def get_seat_types(self, result): - seat_types = [seat.slug for course_run in result.object.course_runs.all() for seat in course_run.seat_types] + now = datetime.datetime.now(pytz.UTC) + request = self.context['request'] + exclude_expired = request.GET.get('exclude_expired_course_run') + if request.method == 'POST': + exclude_expired = request.POST.get('exclude_expired_course_run', exclude_expired) + if exclude_expired: + # if course_run is active then add course_run.seat_types to seat_types + seat_types = [ + seat.slug for course_run in result.object.course_runs.all() + if course_run.end is None or course_run.end > now for seat in course_run.seat_types + ] + else: + seat_types = [seat.slug for course_run in result.object.course_runs.all() for seat in course_run.seat_types] return list(set(seat_types)) def get_skill_names(self, result): diff --git a/course_discovery/apps/ietf_language_tags/tests/test_utils.py b/course_discovery/apps/ietf_language_tags/tests/test_utils.py new file mode 100644 index 0000000000..f15e53f843 --- /dev/null +++ b/course_discovery/apps/ietf_language_tags/tests/test_utils.py @@ -0,0 +1,29 @@ +""" Tests for the ietf_language_tags utility methods """ + +import ddt +import pytest +from django.test import TestCase + +from course_discovery.apps.ietf_language_tags.models import LanguageTag +from course_discovery.apps.ietf_language_tags.utils import serialize_language + + +@ddt.ddt +@pytest.mark.django_db +class SerializeLanguageTest(TestCase): + """ + Tests for serialize_language method + """ + + def setUp(self): + super().setUp() + self.language_1 = LanguageTag.objects.filter(code__startswith='zh').first() + self.language_2 = LanguageTag.objects.filter(code__startswith='en').first() + + def test_serialize_language__with_language(self): + """ + Test that serialize_language method returns the language name + if the language code starts with 'zh' else returns the macrolanguage. + """ + assert serialize_language(self.language_1) == self.language_1.name + assert serialize_language(self.language_2) == self.language_2.macrolanguage diff --git a/course_discovery/apps/ietf_language_tags/utils.py b/course_discovery/apps/ietf_language_tags/utils.py new file mode 100644 index 0000000000..41d94f7838 --- /dev/null +++ b/course_discovery/apps/ietf_language_tags/utils.py @@ -0,0 +1,14 @@ +""" +This module contains utility functions for the ietf_language_tags app +""" + + +def serialize_language(language): + """ + Given a language object, it returns the language name if the language code starts with 'zh' else returns the + macrolanguage. + """ + if language.code.startswith('zh'): + return language.name + + return language.macrolanguage