From 76e4e8cfe87e3ce0e8d3f37e9e624e0f7d57498c Mon Sep 17 00:00:00 2001 From: Jody Bailey <110463597+JodyBaileyy@users.noreply.github.com> Date: Thu, 8 Jun 2023 16:29:52 +0200 Subject: [PATCH] feat: Added separate url for only returning amplitude recommendations (#32398) * feat: Added separate url for only returning amplitude recommendations * chore: added extra line between serializers * chore: changed name of methods * chore: updated and added docstrings --- .../learner_recommendations/serializers.py | 7 +++ .../tests/test_data.py | 5 +++ .../tests/test_serializers.py | 37 ++++++++++++++-- .../tests/test_views.py | 44 +++++++++++++++++-- .../learner_recommendations/urls.py | 3 ++ .../learner_recommendations/views.py | 44 +++++++++++++++---- 6 files changed, 123 insertions(+), 17 deletions(-) diff --git a/lms/djangoapps/learner_recommendations/serializers.py b/lms/djangoapps/learner_recommendations/serializers.py index 360df532f9..75328e9d06 100644 --- a/lms/djangoapps/learner_recommendations/serializers.py +++ b/lms/djangoapps/learner_recommendations/serializers.py @@ -93,6 +93,13 @@ class CrossProductRecommendationsSerializer(serializers.Serializer): ) +class AmplitudeRecommendationsSerializer(serializers.Serializer): + """Serializer for Amplitude recommendations for Learner Dashboard""" + amplitudeCourses = serializers.ListField( + child=LearnerDashboardProductRecommendationsSerializer(), allow_empty=True + ) + + class CrossProductAndAmplitudeRecommendationsSerializer(serializers.Serializer): """ Cross product recommendation courses and diff --git a/lms/djangoapps/learner_recommendations/tests/test_data.py b/lms/djangoapps/learner_recommendations/tests/test_data.py index 9fc950115c..2bd4d7350a 100644 --- a/lms/djangoapps/learner_recommendations/tests/test_data.py +++ b/lms/djangoapps/learner_recommendations/tests/test_data.py @@ -142,6 +142,7 @@ def get_general_recommendations(): return courses + mock_amplitude_and_cross_product_course_data = { "crossProductCourses": mock_cross_product_data, "amplitudeCourses": mock_amplitude_data @@ -151,6 +152,10 @@ mock_cross_product_course_data = { "courses": mock_course_data } +mock_amplitude_course_data = { + "amplitudeCourses": mock_amplitude_data +} + mock_cross_product_recommendation_keys = { "edx+HL0": ["edx+HL1", "edx+HL2"], "edx+BZ0": ["edx+BZ1", "edx+BZ2"], diff --git a/lms/djangoapps/learner_recommendations/tests/test_serializers.py b/lms/djangoapps/learner_recommendations/tests/test_serializers.py index 0d905b6952..17cb9f1523 100644 --- a/lms/djangoapps/learner_recommendations/tests/test_serializers.py +++ b/lms/djangoapps/learner_recommendations/tests/test_serializers.py @@ -7,11 +7,13 @@ from django.test import TestCase from lms.djangoapps.learner_recommendations.serializers import ( DashboardRecommendationsSerializer, CrossProductRecommendationsSerializer, - CrossProductAndAmplitudeRecommendationsSerializer + CrossProductAndAmplitudeRecommendationsSerializer, + AmplitudeRecommendationsSerializer ) from lms.djangoapps.learner_recommendations.tests.test_data import ( mock_amplitude_and_cross_product_course_data, - mock_cross_product_course_data + mock_cross_product_course_data, + mock_amplitude_course_data ) @@ -91,8 +93,8 @@ class TestDashboardRecommendationsSerializer(TestCase): class TestCrossProductRecommendationsSerializers(TestCase): """ - Tests for the CrossProductRecommendationsSerializer - and CrossProductAndAmplitudeRecommendations Serializer + Tests for the CrossProductRecommendationsSerializer, + AmplitudeRecommendationsSerializer, and CrossProductAndAmplitudeRecommendations Serializer """ def mock_recommended_courses(self, num_of_courses=2, amplitude_courses=False): @@ -159,6 +161,19 @@ class TestCrossProductRecommendationsSerializers(TestCase): mock_cross_product_course_data ) + def test_successful_amplitude_recommendations_serialization(self): + """Test the course data serializes correctly for AmplitudeRecommendationsSerializer""" + courses = self.mock_recommended_courses(num_of_courses=4) + + serialized_data = AmplitudeRecommendationsSerializer({ + "amplitudeCourses": courses + }).data + + self.assertDictEqual( + serialized_data, + mock_amplitude_course_data + ) + def test_successful_cross_product_and_amplitude_recommendations_serializer(self): """Test that course data serializes correctly for CrossProductAndAmplitudeRecommendationSerializer""" @@ -189,6 +204,20 @@ class TestCrossProductRecommendationsSerializers(TestCase): }, ) + def test_no_amplitude_courses_serialization(self): + """Tests that empty course data for AmplitudeRecommendationsSerializer serializes properly""" + + serialized_data = AmplitudeRecommendationsSerializer({ + "amplitudeCourses": [], + }).data + + self.assertDictEqual( + serialized_data, + { + "amplitudeCourses": [], + }, + ) + def test_no_amplitude_and_cross_product_and_course_serialization(self): """Tests that empty course data for CrossProductRecommendationsSerializer serializes properly""" diff --git a/lms/djangoapps/learner_recommendations/tests/test_views.py b/lms/djangoapps/learner_recommendations/tests/test_views.py index 9fed293f73..37d86a5ecf 100644 --- a/lms/djangoapps/learner_recommendations/tests/test_views.py +++ b/lms/djangoapps/learner_recommendations/tests/test_views.py @@ -360,13 +360,18 @@ class TestProductRecommendationsView(APITestCase): self.amplitude_location_restriction_keys = self.amplitude_keys[0:3] self.cross_product_location_restriction_keys = self.associated_course_keys[0] - def _get_url(self, course_key): + def _get_url(self, course_key=None): """ - Returns the url with a sepcific course id + Returns the product recommendations url with or without the course key """ + if course_key: + return reverse_lazy( + "learner_recommendations:product_recommendations", + kwargs={'course_id': f'course-v1:{course_key}+Test_Course'} + ) + return reverse_lazy( - "learner_recommendations:product_recommendations", - kwargs={'course_id': f'course-v1:{course_key}+Test_Course'} + "learner_recommendations:product_recommendations_amplitude_only" ) def _get_product_recommendations(self, course_keys, keys_with_restriction=None): @@ -639,6 +644,37 @@ class TestProductRecommendationsView(APITestCase): self.assertEqual(len(cross_product_course_data), 0) self.assertEqual(len(amplitude_course_data), 4) + @mock.patch("lms.djangoapps.learner_recommendations.utils._get_user_enrolled_course_keys") + @mock.patch("lms.djangoapps.learner_recommendations.utils.get_course_data") + @mock.patch("lms.djangoapps.learner_recommendations.views.get_amplitude_course_recommendations") + @mock.patch("lms.djangoapps.learner_recommendations.views.country_code_from_ip") + def test_amplitude_only_url_response( + self, + country_code_from_ip_mock, + get_amplitude_course_recommendations_mock, + get_course_data_mock, + get_user_enrolled_course_keys_mock + ): + """ + Verify that if no course key was provided in the url, + only 1 field for amplitude courses are sent back + """ + + country_code_from_ip_mock.return_value = "za" + get_user_enrolled_course_keys_mock.return_value = self.enrolled_course_run_keys + get_amplitude_course_recommendations_mock.return_value = [False, True, self.amplitude_keys] + + mock_amplitude_course_data = self._get_product_recommendations(self.amplitude_keys) + get_course_data_mock.side_effect = mock_amplitude_course_data + + response = self.client.get(self._get_url()) + response_content = json.loads(response.content) + amplitude_course_data = response_content["amplitudeCourses"] + + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response_content), 1) + self.assertEqual(len(amplitude_course_data), 4) + @ddt.ddt class TestDashboardRecommendationsApiView(TestRecommendationsBase): diff --git a/lms/djangoapps/learner_recommendations/urls.py b/lms/djangoapps/learner_recommendations/urls.py index bd351614e7..3966255bde 100644 --- a/lms/djangoapps/learner_recommendations/urls.py +++ b/lms/djangoapps/learner_recommendations/urls.py @@ -16,6 +16,9 @@ urlpatterns = [ re_path(fr'^cross_product/{settings.COURSE_ID_PATTERN}/$', views.CrossProductRecommendationsView.as_view(), name='cross_product_recommendations'), + re_path(r'^product_recommendations/$', + views.ProductRecommendationsView.as_view(), + name='product_recommendations_amplitude_only'), re_path(fr'^product_recommendations/{settings.COURSE_ID_PATTERN}/$', views.ProductRecommendationsView.as_view(), name='product_recommendations'), diff --git a/lms/djangoapps/learner_recommendations/views.py b/lms/djangoapps/learner_recommendations/views.py index 953518018c..414c9975cd 100644 --- a/lms/djangoapps/learner_recommendations/views.py +++ b/lms/djangoapps/learner_recommendations/views.py @@ -39,6 +39,7 @@ from lms.djangoapps.learner_recommendations.serializers import ( DashboardRecommendationsSerializer, CrossProductAndAmplitudeRecommendationsSerializer, CrossProductRecommendationsSerializer, + AmplitudeRecommendationsSerializer, ) log = logging.getLogger(__name__) @@ -198,6 +199,7 @@ class ProductRecommendationsView(APIView): """ **Example Request** + GET api/learner_recommendations/product_recommendations/ GET api/learner_recommendations/product_recommendations/{course_id}/ """ @@ -265,17 +267,12 @@ class ProductRecommendationsView(APIView): return filtered_cross_product_courses - def get(self, request, course_id): + def _cross_product_recommendations_response(self, course_key, user, user_country_code): """ - Returns cross product and amplitude recommendation courses + Helper for collecting and forming a response for + cross product and Amplitude recommendations """ - - ip_address = get_client_ip(request)[0] - user_country_code = country_code_from_ip(ip_address).upper() - course_locator = CourseKey.from_string(course_id) - course_key = f'{course_locator.org}+{course_locator.course}' - - amplitude_recommendations = self._get_amplitude_recommendations(request.user, user_country_code) + amplitude_recommendations = self._get_amplitude_recommendations(user, user_country_code) cross_product_recommendations = self._get_cross_product_recommendations(course_key, user_country_code) return Response( @@ -288,6 +285,35 @@ class ProductRecommendationsView(APIView): status=200 ) + def _amplitude_recommendations_response(self, user, user_country_code): + """ + Helper for collecting and forming a response for Amplitude recommendations only + """ + amplitude_recommendations = self._get_amplitude_recommendations(user, user_country_code) + + return Response( + AmplitudeRecommendationsSerializer({ + "amplitudeCourses": amplitude_recommendations + }).data, + status=200 + ) + + def get(self, request, course_id=None): + """ + Returns cross product and Amplitude recommendation courses if a course id is included, + otherwise, returns only Amplitude recommendations + """ + + ip_address = get_client_ip(request)[0] + user_country_code = country_code_from_ip(ip_address).upper() + + if course_id: + course_locator = CourseKey.from_string(course_id) + course_key = f'{course_locator.org}+{course_locator.course}' + return self._cross_product_recommendations_response(course_key, request.user, user_country_code) + + return self._amplitude_recommendations_response(request.user, user_country_code) + class DashboardRecommendationsApiView(APIView): """