From e3cb59ca2e894513e047cc23bdca7f084b1487f1 Mon Sep 17 00:00:00 2001 From: Jody Bailey <110463597+JodyBaileyy@users.noreply.github.com> Date: Fri, 28 Apr 2023 09:35:03 +0200 Subject: [PATCH] fix: adjusted cross product recommendations course filtering and response (#32119) * fix: adjusted cross product recommendations course filtering and response * chore: added method descriptions * test: adjusted mock course data * chore: removed logs * chore: removed extra empty line * fix: removed redundant active course run logic, added active course run filtering --- .../tests/test_utils.py | 57 ++++++++++++++++++- .../tests/test_views.py | 26 ++++++++- .../learner_recommendations/utils.py | 16 ++++++ .../learner_recommendations/views.py | 19 ++++--- 4 files changed, 106 insertions(+), 12 deletions(-) diff --git a/lms/djangoapps/learner_recommendations/tests/test_utils.py b/lms/djangoapps/learner_recommendations/tests/test_utils.py index 9aa76b7e81..62a483a876 100644 --- a/lms/djangoapps/learner_recommendations/tests/test_utils.py +++ b/lms/djangoapps/learner_recommendations/tests/test_utils.py @@ -11,7 +11,8 @@ from lms.djangoapps.learner_recommendations.utils import ( _has_country_restrictions, filter_recommended_courses, get_amplitude_course_recommendations, - get_cross_product_recommendations + get_cross_product_recommendations, + get_active_course_run ) from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from lms.djangoapps.learner_recommendations.tests.test_data import mock_cross_product_recommendation_keys @@ -226,3 +227,57 @@ class TestGetCrossProductRecommendationsMethod(TestCase): @ddt.unpack def test_get_cross_product_recommendations_method(self, course_key, expected_response): assert get_cross_product_recommendations(course_key) == expected_response + + +class TestGetActiveCourseRunMethod(TestCase): + """Tests for get_active_course_run method""" + + advertised_course_run_uuid = "jh76b2c9-589b-4d1e-88c1-b01a02db3a9c" + + def _mock_get_course_data(self, active_course_run=False): + """ + Returns a course with details based on the status passed + """ + return { + "key": "edx+BLN", + "uuid": "6f8cb2c9-589b-4d1e-88c1-b01a02db3a9c", + "course_runs": [ + { + "key": "course-v1:Test+2023_T1", + "uuid": "hb86b3cf-589b-4d1e-88c1-b01a02db3a9c", + "status": "published", + }, + { + "key": "course-v1:Test+2023_T2", + "uuid": self.advertised_course_run_uuid if active_course_run else "other-uuid", + "status": "published", + } + ], + "course_run_statuses": ["published"], + "advertised_course_run_uuid": self.advertised_course_run_uuid if active_course_run else None, + } + + def test_advertised_course_run_returned(self): + """ + Test that the course run with the uuid that matches the advertised_uuid_course_run_uuid is returned + """ + course = self._mock_get_course_data(active_course_run=True) + active_course_run = get_active_course_run(course) + + self.assertDictEqual( + active_course_run, + { + "key": "course-v1:Test+2023_T2", + "uuid": self.advertised_course_run_uuid, + "status": "published" + } + ) + + def test_no_course_run_returned(self): + """ + Test that if there is no advertised_course_run_uuid value, no course run is returned + """ + course = self._mock_get_course_data(active_course_run=False) + active_course_run = get_active_course_run(course) + + self.assertIsNone(active_course_run) diff --git a/lms/djangoapps/learner_recommendations/tests/test_views.py b/lms/djangoapps/learner_recommendations/tests/test_views.py index d582eaf334..6ebeb93c51 100644 --- a/lms/djangoapps/learner_recommendations/tests/test_views.py +++ b/lms/djangoapps/learner_recommendations/tests/test_views.py @@ -170,7 +170,7 @@ class TestCrossProductRecommendationsView(APITestCase): kwargs={'course_id': f'course-v1:{course_key}+Test_Course'} ) - def _get_recommended_courses(self, num_of_courses_with_restriction=0): + def _get_recommended_courses(self, num_of_courses_with_restriction=0, active_course_run=True): """ Returns an array of 2 discovery courses with or without country restrictions """ @@ -183,6 +183,7 @@ class TestCrossProductRecommendationsView(APITestCase): for course_key in enumerate(self.associated_course_keys): location_restriction = restriction_obj if num_of_courses_with_restriction > 0 else None + advertised_course_run_uuid = "jh76b2c9-589b-4d1e-88c1-b01a02db3a9c" if active_course_run else None courses.append({ "key": course_key[1], @@ -205,9 +206,12 @@ class TestCrossProductRecommendationsView(APITestCase): "key": "course-v1:Test+2023_T2", "marketing_url": "https://www.marketing_url.com", "availability": "Current", + "uuid": "jh76b2c9-589b-4d1e-88c1-b01a02db3a9c", + "status": "published" } ], - "location_restriction": location_restriction + "advertised_course_run_uuid": advertised_course_run_uuid, + "location_restriction": location_restriction, }) if num_of_courses_with_restriction > 0: @@ -308,6 +312,24 @@ class TestCrossProductRecommendationsView(APITestCase): self.assertEqual(response.status_code, 200) self.assertEqual(len(course_data), 0) + @mock.patch("django.conf.settings.CROSS_PRODUCT_RECOMMENDATIONS_KEYS", mock_cross_product_recommendation_keys) + @mock.patch("lms.djangoapps.learner_recommendations.views.get_course_data") + @mock.patch("lms.djangoapps.learner_recommendations.views.country_code_from_ip") + def test_no_active_course_runs_response(self, country_code_from_ip_mock, get_course_data_mock): + """ + Verify that an empty array of courses is returned if courses do not have an active course run. + """ + country_code_from_ip_mock.return_value = "za" + mock_course_data = self._get_recommended_courses(0, active_course_run=False) + get_course_data_mock.side_effect = [mock_course_data[0], mock_course_data[1]] + + response = self.client.get(self._get_url('edx+HL0')) + reponse_content = json.loads(response.content) + course_data = reponse_content["courses"] + + self.assertEqual(response.status_code, 200) + self.assertEqual(len(course_data), 0) + @ddt.ddt class TestDashboardRecommendationsApiView(TestRecommendationsBase): diff --git a/lms/djangoapps/learner_recommendations/utils.py b/lms/djangoapps/learner_recommendations/utils.py index 20f58d743e..1ed6471fd5 100644 --- a/lms/djangoapps/learner_recommendations/utils.py +++ b/lms/djangoapps/learner_recommendations/utils.py @@ -209,3 +209,19 @@ def get_cross_product_recommendations(course_key): Helper method to get associated course keys based on the key passed """ return settings.CROSS_PRODUCT_RECOMMENDATIONS_KEYS.get(course_key) + + +def get_active_course_run(course): + """ + Returns an active course run based on prospectus frontend logic + for what defines an active course run + """ + course_runs = course.get("course_runs") + advertised_course_run_uuid = course.get("advertised_course_run_uuid") + + if advertised_course_run_uuid: + for course_run in course_runs: + if course_run.get("uuid") == advertised_course_run_uuid: + return course_run + + return None diff --git a/lms/djangoapps/learner_recommendations/views.py b/lms/djangoapps/learner_recommendations/views.py index f060b5c4a9..d4787e2e06 100644 --- a/lms/djangoapps/learner_recommendations/views.py +++ b/lms/djangoapps/learner_recommendations/views.py @@ -29,9 +29,10 @@ from lms.djangoapps.learner_recommendations.utils import ( filter_recommended_courses, is_user_enrolled_in_ut_austin_masters_program, _has_country_restrictions, + get_cross_product_recommendations, + get_active_course_run ) from lms.djangoapps.learner_recommendations.serializers import CrossProductRecommendationsSerializer -from lms.djangoapps.learner_recommendations.utils import get_cross_product_recommendations from openedx.core.djangoapps.catalog.utils import get_course_data from lms.djangoapps.learner_recommendations.serializers import ( AboutPageRecommendationsSerializer, @@ -160,9 +161,9 @@ class CrossProductRecommendationsView(APIView): "course_type", "course_runs", "location_restriction", + "advertised_course_run_uuid", ] - query_string = {'marketable_course_runs_only': 1} - course_data = [get_course_data(key, fields, query_string) for key in associated_course_keys] + course_data = [get_course_data(key, fields) for key in associated_course_keys] filtered_courses = [course for course in course_data if course and course.get("course_runs")] ip_address = get_client_ip(request)[0] @@ -171,17 +172,17 @@ class CrossProductRecommendationsView(APIView): unrestricted_courses = [] for course in filtered_courses: - if not _has_country_restrictions(course, user_country_code): + if _has_country_restrictions(course, user_country_code): + continue + + active_course_run = get_active_course_run(course) + if active_course_run: + course.update({"active_course_run": active_course_run}) unrestricted_courses.append(course) if not unrestricted_courses: return self._empty_response() - for course in unrestricted_courses: - course.update({ - "active_course_run": course.get("course_runs")[0] - }) - return Response( CrossProductRecommendationsSerializer( {