From 278dbba646531ab1d8efc13e807becdbc20c3c08 Mon Sep 17 00:00:00 2001 From: Syed Sajjad Hussain Shah <52817156+syedsajjadkazmii@users.noreply.github.com> Date: Fri, 17 Mar 2023 10:04:09 +0500 Subject: [PATCH] fix: fix CS50 enrollment and filtering issue for recommendations (#31916) VAN-1314 Co-authored-by: Syed Sajjad Hussain Shah --- .../tests/test_utils.py | 44 ++----- .../learner_recommendations/utils.py | 118 +++--------------- .../learner_recommendations/views.py | 8 +- 3 files changed, 25 insertions(+), 145 deletions(-) diff --git a/lms/djangoapps/learner_recommendations/tests/test_utils.py b/lms/djangoapps/learner_recommendations/tests/test_utils.py index 410689b8a6..b1a9a7b48c 100644 --- a/lms/djangoapps/learner_recommendations/tests/test_utils.py +++ b/lms/djangoapps/learner_recommendations/tests/test_utils.py @@ -84,10 +84,6 @@ class TestFilterRecommendedCourses(ModuleStoreTestCase): def setUp(self): super().setUp() self.user = UserFactory() - self.unrestricted_course_keys = [ - "MITx+6.00.1x", - "IBM+PY0101EN", - ] self.recommended_course_keys = [ "MITx+6.00.1x", "IBM+PY0101EN", @@ -100,36 +96,10 @@ class TestFilterRecommendedCourses(ModuleStoreTestCase): "NYUx+FCS.NET.1", "MichinX+101x", ] - self.course_run_keys = [ - "course-v1:MITx+6.00.1x+Run_0", - "course-v1:IBM+PY0101EN+Run_0", - "course-v1:HarvardX+CS50P+Run_0", - "course-v1:UQx+IELTSx+Run_0", - "course-v1:HarvardX+CS50x+Run_0", - "course-v1:Harvard+CS50z+Run_0", - "course-v1:BabsonX+EPS03x+Run_0", - "course-v1:TUMx+QPLS2x+Run_0", - "course-v1:NYUx+FCS.NET.1+Run_0", - "course-v1:MichinX+101x+Run_0", - ] - self.course_keys_with_active_course_runs = [ - "MITx+6.00.1x", - "IBM+PY0101EN", - "HarvardX+CS50P", - "UQx+IELTSx", - "HarvardX+CS50x", - "Harvard+CS50z", - "BabsonX+EPS03x", - "TUMx+QPLS2x", - ] - self.enrolled_course_run_keys = [ - "course-v1:HarvardX+CS50x+Run_0", - "course-v1:Harvard+CS50z+Run_0", - "course-v1:BabsonX+EPS03x+Run_0", - "course-v1:TUMx+QPLS2x+Run_0", - "course-v1:NYUx+FCS.NET.1+Run_0", - "course-v1:MichinX+101x+Run_0", - ] + self.unrestricted_course_keys = self.recommended_course_keys[0:2] + self.course_run_keys = [f"course-v1:{course_key}+Run_0" for course_key in self.recommended_course_keys] + self.course_keys_with_active_course_runs = self.recommended_course_keys[0:8] + self.enrolled_course_run_keys = self.course_run_keys[4:10] def _mock_get_course_data(self, course_id, fields=None, querystring=None): # pylint: disable=unused-argument """ @@ -157,7 +127,7 @@ class TestFilterRecommendedCourses(ModuleStoreTestCase): { "course_runs": [ { - "key": "course-v1:MITx+6.00.1x+Run_0", + "key": f"course-v1:{course_id}+Run_0", } ] } @@ -193,12 +163,12 @@ class TestFilterRecommendedCourses(ModuleStoreTestCase): Test that if the "request course" is one of the recommended courses, we filter that from the final recommendation list. """ - request_course = self.recommended_course_keys[0] + request_course = self.course_run_keys[0] mocked_get_course_data.side_effect = self._mock_get_course_data filtered_courses = filter_recommended_courses( self.user, self.recommended_course_keys, - request_course=request_course, + request_course_key=request_course, ) assert all(course["course_key"] != request_course for course in filtered_courses) is True diff --git a/lms/djangoapps/learner_recommendations/utils.py b/lms/djangoapps/learner_recommendations/utils.py index 8efe062dff..b6bcb1ce35 100644 --- a/lms/djangoapps/learner_recommendations/utils.py +++ b/lms/djangoapps/learner_recommendations/utils.py @@ -40,20 +40,19 @@ class AlgoliaClient: return cls.algolia_client -def _remove_user_enrolled_course_keys(user, course_keys): +def _get_user_enrolled_course_keys(user): """ - Remove the course keys a user is already enrolled in - and returns enrollable course keys. + Returns course ids in which the user is enrolled in. """ - user_enrolled_course_keys = set() course_enrollments = CourseEnrollment.enrollments_for_user(user) + return [str(course_enrollment.course_id) for course_enrollment in course_enrollments] - for course_enrollment in course_enrollments: - course_key = f"{course_enrollment.course_id.org}+{course_enrollment.course_id.course}" - user_enrolled_course_keys.add(course_key) - enrollable_course_keys = [course_key for course_key in course_keys if course_key not in user_enrolled_course_keys] - return enrollable_course_keys +def _is_enrolled_in_course(course_runs, enrolled_course_keys): + """ + Returns True if a user is enrolled in any course run of the course else false. + """ + return any(course_run.get("key", None) in enrolled_course_keys for course_run in course_runs) def _has_country_restrictions(product, user_country): @@ -85,34 +84,6 @@ def _has_country_restrictions(product, user_country): return user_country in block_list or (bool(allow_list) and user_country not in allow_list) -def _get_program_duration(weeks): - """ - Helper method that returns the program duration in textual form. - """ - total_months = round(weeks / 4) - - if total_months < 1: - return f'{total_months} weeks' - - if 1 <= total_months < 12: - return f'{total_months} months' - - total_years = round(total_months / 12) - total_remainder_months = round(total_months % 12) - - if total_remainder_months == 0: - return f'{total_years} years' - - if total_years == 1 and total_remainder_months == 1: - return f'1 year {total_remainder_months} months' - - if total_remainder_months == 1: - return f'{total_years} years 1 months' - - else: - return f'{total_years} years {total_remainder_months} months' - - def get_amplitude_course_recommendations(user_id, recommendation_id): """ Get personalized recommendations from Amplitude. @@ -178,7 +149,7 @@ def filter_recommended_courses( unfiltered_course_keys, recommendation_count=10, user_country_code=None, - request_course=None, + request_course_key=None, ): """ Returns the filtered course recommendations. The unfiltered course keys @@ -192,7 +163,7 @@ def filter_recommended_courses( unfiltered_course_keys: recommended course keys that needs to be filtered recommendation_count: the maximum count of recommendations to be returned user_country_code: if provided, will apply location restrictions to recommendations - request_course: if provided, will filter out that course from recommendations (used for course about page) + request_course_key: if provided, will filter out that course from recommendations (used for course about page) Returns: filtered_recommended_courses (list): A list of filtered course objects. @@ -211,17 +182,13 @@ def filter_recommended_courses( "programs", ] - # Remove the course keys a user is already enrolled in - enrollable_course_keys = _remove_user_enrolled_course_keys(user, unfiltered_course_keys) - + # Filter out enrolled courses . + course_keys_to_filter_out = _get_user_enrolled_course_keys(user) # If user is seeing the recommendations on a course about page, filter that course out of recommendations - recommended_course_keys = [ - course_key - for course_key in enrollable_course_keys - if course_key != request_course - ] + if request_course_key: + course_keys_to_filter_out.append(request_course_key) - for course_id in recommended_course_keys: + for course_id in unfiltered_course_keys: if len(filtered_recommended_courses) >= recommendation_count: break @@ -229,62 +196,9 @@ def filter_recommended_courses( if ( course_data and course_data.get("course_runs", []) + and not _is_enrolled_in_course(course_data.get("course_runs", []), course_keys_to_filter_out) and not _has_country_restrictions(course_data, user_country_code) ): filtered_recommended_courses.append(course_data) return filtered_recommended_courses - - -def get_programs_based_on_course(request_course, country_code, user): - """ - Returns a program for the course. If a course is part of multiple programs, - this function returns the program with the highest price. - """ - max_price, max_price_program = 0, {} - programs = get_programs(course=request_course) - - if not programs: - return None - - for program in programs: - if program.get('status') != 'active' or _has_country_restrictions(program, country_code): - continue - - price = program['price_ranges'][0]['total'] - if price > max_price: - if fetch_program_enrollments_by_student(program_uuids=[program.get('uuid')], user=user).exists(): - continue - - course_keys = [ - course['key'] - for course in program.get('courses', []) - if course.get('key') and course.get('key') != request_course - ] - if _remove_user_enrolled_course_keys(user, course_keys): - max_price_program = program - max_price = price - - if not max_price_program: - return None - - course_pacing_type, total_weeks_to_complete = '', 0 - for course in max_price_program.get('courses'): - for course_run in course.get('course_runs'): - if course_run.get('status') == 'published': - if not course_pacing_type: - course_pacing_type = course_run.get("pacing_type") - total_weeks_to_complete += int(course_run.get("weeks_to_complete")) - - program_upsell = { - "title": max_price_program.get('title'), - "marketing_url": max_price_program.get('marketing_url'), - "courses_count": len(max_price_program.get('courses')), - "pacing_type": course_pacing_type, - "weeks_to_complete": _get_program_duration(total_weeks_to_complete), - "min_hours": max_price_program.get('min_hours_effort_per_week'), - "max_hours": max_price_program.get('max_hours_effort_per_week'), - "type": max_price_program.get('type'), - } - - return program_upsell diff --git a/lms/djangoapps/learner_recommendations/views.py b/lms/djangoapps/learner_recommendations/views.py index cb6ef802ce..39375a0804 100644 --- a/lms/djangoapps/learner_recommendations/views.py +++ b/lms/djangoapps/learner_recommendations/views.py @@ -10,7 +10,6 @@ from edx_rest_framework_extensions.auth.session.authentication import ( SessionAuthenticationAllowInactiveUser, ) from django.core.exceptions import PermissionDenied -from opaque_keys.edx.keys import CourseKey from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView @@ -65,8 +64,7 @@ class AmplitudeRecommendationsView(APIView): def get(self, request, course_id): """ Returns - - Amplitude course recommendations - - Upsell program if the requesting course has a related program + - Amplitude course recommendations for course about page """ if not enable_course_about_page_recommendations(): return Response(status=404) @@ -75,8 +73,6 @@ class AmplitudeRecommendationsView(APIView): raise PermissionDenied() user = request.user - course_locator = CourseKey.from_string(course_id) - course_key = f'{course_locator.org}+{course_locator.course}' try: is_control, has_is_control, course_keys = get_amplitude_course_recommendations( @@ -95,7 +91,7 @@ class AmplitudeRecommendationsView(APIView): user, course_keys, user_country_code=user_country_code, - request_course=course_key, + request_course_key=course_id, recommendation_count=self.recommendations_count )