diff --git a/lms/djangoapps/learner_recommendations/tests/test_utils.py b/lms/djangoapps/learner_recommendations/tests/test_utils.py index 62e8beb1d6..410689b8a6 100644 --- a/lms/djangoapps/learner_recommendations/tests/test_utils.py +++ b/lms/djangoapps/learner_recommendations/tests/test_utils.py @@ -112,8 +112,26 @@ class TestFilterRecommendedCourses(ModuleStoreTestCase): "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", + ] - def _mock_get_course_data(self, course_id, fields=None): # pylint: disable=unused-argument + def _mock_get_course_data(self, course_id, fields=None, querystring=None): # pylint: disable=unused-argument """ Mocked response for the get_course_data call """ @@ -134,6 +152,17 @@ class TestFilterRecommendedCourses(ModuleStoreTestCase): } ) + if course_id in self.course_keys_with_active_course_runs: + course_data.update( + { + "course_runs": [ + { + "key": "course-v1:MITx+6.00.1x+Run_0", + } + ] + } + ) + return course_data @patch("lms.djangoapps.learner_recommendations.utils.get_course_data") @@ -144,10 +173,10 @@ class TestFilterRecommendedCourses(ModuleStoreTestCase): Tests that given a recommended course list, the filter_recommended_courses method removes the enrolled courses from it. """ - total_enrolled_courses = 6 + total_enrolled_courses = len(self.enrolled_course_run_keys) total_recommendations = len(self.recommended_course_keys) mocked_get_course_data.side_effect = self._mock_get_course_data - for course_run_key in self.course_run_keys[:total_enrolled_courses]: + for course_run_key in self.enrolled_course_run_keys: CourseEnrollmentFactory(course_id=course_run_key, user=self.user) filtered_courses = filter_recommended_courses( @@ -192,3 +221,21 @@ class TestFilterRecommendedCourses(ModuleStoreTestCase): expected_recommendations.append(self._mock_get_course_data(course_key)) assert filtered_courses == expected_recommendations + + @patch("lms.djangoapps.learner_recommendations.utils.get_course_data") + def test_recommend_only_active_courses( + self, + mocked_get_course_data, + ): + """ + Test that courses having no active course runs are filtered out from recommended courses. + """ + mocked_get_course_data.side_effect = self._mock_get_course_data + filtered_courses = filter_recommended_courses( + self.user, self.recommended_course_keys + ) + expected_recommendations = [] + for course_key in self.course_keys_with_active_course_runs: + expected_recommendations.append(self._mock_get_course_data(course_key)) + + assert filtered_courses == expected_recommendations diff --git a/lms/djangoapps/learner_recommendations/tests/test_views.py b/lms/djangoapps/learner_recommendations/tests/test_views.py index b8f75212d2..9e5c639abb 100644 --- a/lms/djangoapps/learner_recommendations/tests/test_views.py +++ b/lms/djangoapps/learner_recommendations/tests/test_views.py @@ -168,7 +168,7 @@ class TestAmplitudeRecommendationsView(APITestCase): Returns the filtered course data """ filtered_course = [] - for course_key in self.recommended_courses: + for course_key in self.recommended_courses[0:4]: filtered_course.append({ "key": course_key, "uuid": "4f8cb2c9-589b-4d1e-88c1-b01a02db3a9c", diff --git a/lms/djangoapps/learner_recommendations/utils.py b/lms/djangoapps/learner_recommendations/utils.py index d202848c5b..f775cd9184 100644 --- a/lms/djangoapps/learner_recommendations/utils.py +++ b/lms/djangoapps/learner_recommendations/utils.py @@ -113,16 +113,6 @@ def _get_program_duration(weeks): return f'{total_years} years {total_remainder_months} months' -def get_active_course_run(course_data): - """Helper method to get course active run""" - active_course_runs = [ - course_run - for course_run in course_data.get("course_runs", []) - if course_run.get("availability") == "Current" - ] - return active_course_runs[0] if active_course_runs else [] - - def get_algolia_courses_recommendation(course_data): """ Get courses recommendation from Algolia search. @@ -237,8 +227,9 @@ def filter_recommended_courses( if len(filtered_recommended_courses) >= recommendation_count: break - course_data = get_course_data(course_id, fields) - if course_data and not _has_country_restrictions(course_data, user_country_code): + course_data = get_course_data(course_id, fields, querystring={'marketable_course_runs_only': 1}) + if (course_data and course_data.get("course_runs", []) + and not _has_country_restrictions(course_data, user_country_code)): filtered_recommended_courses.append(course_data) return filtered_recommended_courses diff --git a/lms/djangoapps/learner_recommendations/views.py b/lms/djangoapps/learner_recommendations/views.py index 0f7b4553eb..28956a8be3 100644 --- a/lms/djangoapps/learner_recommendations/views.py +++ b/lms/djangoapps/learner_recommendations/views.py @@ -27,7 +27,6 @@ from lms.djangoapps.learner_recommendations.utils import ( get_algolia_courses_recommendation, get_amplitude_course_recommendations, filter_recommended_courses, - get_active_course_run, ) from lms.djangoapps.learner_recommendations.serializers import RecommendationsSerializer @@ -126,20 +125,18 @@ class AmplitudeRecommendationsView(APIView): if not (is_control or is_control is None): ip_address = get_client_ip(request)[0] user_country_code = country_code_from_ip(ip_address).upper() - filtered_courses = filter_recommended_courses( - user, course_keys, user_country_code=user_country_code, request_course=course_key, + recommended_courses = filter_recommended_courses( + user, + course_keys, + user_country_code=user_country_code, + request_course=course_key, + recommendation_count=self.recommendations_count ) - for course in filtered_courses: - active_course_run = get_active_course_run(course) - if active_course_run: - course.update({ - "active_course_run": get_active_course_run(course) - }) - recommended_courses.append(course) - - if len(recommended_courses) == self.recommendations_count: - break + for course in recommended_courses: + course.update({ + "active_course_run": course.get("course_runs")[0] + }) self._emit_recommendations_viewed_event( user.id, is_control, recommended_courses diff --git a/openedx/core/djangoapps/catalog/utils.py b/openedx/core/djangoapps/catalog/utils.py index efb34f3465..f15d5d9c07 100644 --- a/openedx/core/djangoapps/catalog/utils.py +++ b/openedx/core/djangoapps/catalog/utils.py @@ -741,13 +741,14 @@ def get_programs_for_organization(organization): return cache.get(PROGRAMS_BY_ORGANIZATION_CACHE_KEY_TPL.format(org_key=organization)) -def get_course_data(course_key_str, fields): +def get_course_data(course_key_str, fields, querystring=None): """ Retrieve information about the course with the given course key. Arguments: course_key_str: key for the course about which we are retrieving information. fields (List, string): The given fields that you want to retrieve from API response. + querystring (dict): Optional query string parameters. Returns: dict with details about specified course. @@ -767,7 +768,8 @@ def get_course_data(course_key_str, fields): cache_key=course_cache_key if catalog_integration.is_cache_enabled else None, long_term_cache=True, many=False, - fields=fields + fields=fields, + querystring=querystring ) if data: return data