From 773f97d32452e00400b079066ce983e1c7ff4bc7 Mon Sep 17 00:00:00 2001 From: Mubbshar Anwar <78487564+mubbsharanwar@users.noreply.github.com> Date: Fri, 24 Mar 2023 16:18:57 +0500 Subject: [PATCH] refactor: Reduce redundant code (#31975) Move recommendations related code to learner_recommendations app. VAN-1310 --- .../learner_recommendations/serializers.py | 23 +- .../tests/test_serializers.py | 83 +++++ .../tests/test_views.py | 304 +++++++++++++++++- .../learner_recommendations/toggles.py | 18 ++ .../learner_recommendations/urls.py | 5 +- .../learner_recommendations/views.py | 113 ++++++- 6 files changed, 531 insertions(+), 15 deletions(-) create mode 100644 lms/djangoapps/learner_recommendations/tests/test_serializers.py diff --git a/lms/djangoapps/learner_recommendations/serializers.py b/lms/djangoapps/learner_recommendations/serializers.py index c8c747cdb3..765bbc5d54 100644 --- a/lms/djangoapps/learner_recommendations/serializers.py +++ b/lms/djangoapps/learner_recommendations/serializers.py @@ -39,7 +39,7 @@ class RecommendedCourseSerializer(serializers.Serializer): return f"course/{url_slug}" -class RecommendationsSerializer(serializers.Serializer): +class AboutPageRecommendationsSerializer(serializers.Serializer): """Recommended courses for course about page""" courses = serializers.ListField( @@ -49,3 +49,24 @@ class RecommendationsSerializer(serializers.Serializer): source="is_control", default=None ) + + +class CourseSerializer(serializers.Serializer): + """Serializer for a recommended course from the recommendation engine""" + + courseKey = serializers.CharField(source="course_key") + logoImageUrl = serializers.URLField(source="logo_image_url") + marketingUrl = serializers.URLField(source="marketing_url") + title = serializers.CharField() + + +class DashboardRecommendationsSerializer(serializers.Serializer): + """Recommended courses for learner dashboard""" + + courses = serializers.ListField( + child=CourseSerializer(), allow_empty=True + ) + isControl = serializers.BooleanField( + source="is_control", + default=None + ) diff --git a/lms/djangoapps/learner_recommendations/tests/test_serializers.py b/lms/djangoapps/learner_recommendations/tests/test_serializers.py new file mode 100644 index 0000000000..ff66cadb25 --- /dev/null +++ b/lms/djangoapps/learner_recommendations/tests/test_serializers.py @@ -0,0 +1,83 @@ +"""Tests for serializers for the Learner Recommendations""" + +from uuid import uuid4 + +from django.test import TestCase + +from lms.djangoapps.learner_recommendations.serializers import ( + DashboardRecommendationsSerializer, +) + + +class TestDashboardRecommendationsSerializer(TestCase): + """High-level tests for DashboardRecommendationsSerializer""" + + @classmethod + def mock_recommended_courses(cls, courses_count=2): + """Sample course data""" + + recommended_courses = [] + + for _ in range(courses_count): + recommended_courses.append( + { + "course_key": str(uuid4()), + "logo_image_url": "http://edx.org/images/test.png", + "marketing_url": "http://edx.org/courses/AI", + "title": str(uuid4()), + }, + ) + + return recommended_courses + + def test_no_recommended_courses(self): + """That that data serializes correctly for empty courses list""" + + recommended_courses = self.mock_recommended_courses(courses_count=0) + + output_data = DashboardRecommendationsSerializer( + { + "courses": recommended_courses, + } + ).data + + self.assertDictEqual( + output_data, + { + "courses": [], + "isControl": None, + }, + ) + + def test_happy_path(self): + """Test that data serializes correctly""" + + recommended_courses = self.mock_recommended_courses() + + output_data = DashboardRecommendationsSerializer( + { + "courses": recommended_courses, + "is_control": False, + } + ).data + + self.assertDictEqual( + output_data, + { + "courses": [ + { + "courseKey": recommended_courses[0]["course_key"], + "logoImageUrl": recommended_courses[0]["logo_image_url"], + "marketingUrl": recommended_courses[0]["marketing_url"], + "title": recommended_courses[0]["title"], + }, + { + "courseKey": recommended_courses[1]["course_key"], + "logoImageUrl": recommended_courses[1]["logo_image_url"], + "marketingUrl": recommended_courses[1]["marketing_url"], + "title": recommended_courses[1]["title"], + }, + ], + "isControl": False, + }, + ) diff --git a/lms/djangoapps/learner_recommendations/tests/test_views.py b/lms/djangoapps/learner_recommendations/tests/test_views.py index 1370c588ac..1d3aaf79a0 100644 --- a/lms/djangoapps/learner_recommendations/tests/test_views.py +++ b/lms/djangoapps/learner_recommendations/tests/test_views.py @@ -8,21 +8,17 @@ from edx_toggles.toggles.testutils import override_waffle_flag from rest_framework.test import APITestCase from unittest import mock +import ddt from common.djangoapps.student.tests.factories import UserFactory +from common.djangoapps.student.toggles import ENABLE_FALLBACK_RECOMMENDATIONS from lms.djangoapps.learner_recommendations.toggles import ( ENABLE_COURSE_ABOUT_PAGE_RECOMMENDATIONS, + ENABLE_DASHBOARD_RECOMMENDATIONS, ) -@override_waffle_flag(ENABLE_COURSE_ABOUT_PAGE_RECOMMENDATIONS, active=True) -class TestAmplitudeRecommendationsView(APITestCase): - """Unit tests for the Amplitude recommendations API""" - - url = reverse_lazy( - "learner_recommendations:amplitude_recommendations", - kwargs={'course_id': 'course-v1:test+TestX+Test_Course'} - ) - +class TestRecommendationsBase(APITestCase): + """Recommendations test base class""" def setUp(self): super().setUp() self.user = UserFactory() @@ -40,6 +36,16 @@ class TestAmplitudeRecommendationsView(APITestCase): "MichinX+101x", ] + +@override_waffle_flag(ENABLE_COURSE_ABOUT_PAGE_RECOMMENDATIONS, active=True) +class TestAboutPageRecommendationsView(TestRecommendationsBase): + """Unit tests for the Amplitude recommendations API""" + + url = reverse_lazy( + "learner_recommendations:amplitude_recommendations", + kwargs={'course_id': 'course-v1:test+TestX+Test_Course'} + ) + def _get_filtered_courses(self): """ Returns the filtered course data @@ -144,3 +150,283 @@ class TestAmplitudeRecommendationsView(APITestCase): # Verify that the segment event was fired assert segment_mock.call_count == 1 assert segment_mock.call_args[0][1] == "edx.bi.user.recommendations.viewed" + + +@ddt.ddt +class TestDashboardRecommendationsApiView(TestRecommendationsBase): + """Unit tests for the course recommendations on learner home page.""" + + url = reverse_lazy("learner_recommendations:courses") + + GENERAL_RECOMMENDATIONS = [ + { + "course_key": "HogwartsX+6.00.1x", + "logo_image_url": "http://edx.org/images/test.png", + "marketing_url": "http://edx.org/courses/AI", + "title": "Defense Against the Dark Arts", + }, + { + "course_key": "MonstersX+SC101EN", + "logo_image_url": "http://edx.org/images/test.png", + "marketing_url": "http://edx.org/courses/AI", + "title": "Scaring 101", + }, + ] + + SERIALIZED_GENERAL_RECOMMENDATIONS = [ + { + "courseKey": GENERAL_RECOMMENDATIONS[0]["course_key"], + "logoImageUrl": GENERAL_RECOMMENDATIONS[0]["logo_image_url"], + "marketingUrl": GENERAL_RECOMMENDATIONS[0]["marketing_url"], + "title": GENERAL_RECOMMENDATIONS[0]["title"], + }, + { + "courseKey": GENERAL_RECOMMENDATIONS[1]["course_key"], + "logoImageUrl": GENERAL_RECOMMENDATIONS[1]["logo_image_url"], + "marketingUrl": GENERAL_RECOMMENDATIONS[1]["marketing_url"], + "title": GENERAL_RECOMMENDATIONS[1]["title"], + }, + ] + + def setUp(self): + super().setUp() + self.course_run_keys = [f"course-v1:{course}+Run_0" for course in self.recommended_courses] + + def _get_filtered_courses(self): + """ + Returns the filtered course data + """ + filtered_course = [] + for course_key in self.recommended_courses[:5]: + filtered_course.append({ + "key": course_key, + "title": f"Title for {course_key}", + "logo_image_url": "https://www.logo_image_url.com", + "marketing_url": "https://www.marketing_url.com", + }) + + return filtered_course + + @override_waffle_flag(ENABLE_DASHBOARD_RECOMMENDATIONS, active=False) + def test_waffle_flag_off(self): + """ + Verify API returns 404 if waffle flag is off. + """ + response = self.client.get(self.url) + self.assertEqual(response.status_code, 404) + self.assertEqual(response.data, None) + + @override_waffle_flag(ENABLE_FALLBACK_RECOMMENDATIONS, active=True) + @override_waffle_flag(ENABLE_DASHBOARD_RECOMMENDATIONS, active=True) + @mock.patch("django.conf.settings.GENERAL_RECOMMENDATIONS", GENERAL_RECOMMENDATIONS) + @mock.patch( + "lms.djangoapps.learner_recommendations.views.get_amplitude_course_recommendations" + ) + def test_no_recommendations_from_amplitude( + self, get_amplitude_course_recommendations_mock + ): + """ + Verify API returns general recommendations if no course recommendations from amplitude. + """ + get_amplitude_course_recommendations_mock.return_value = [False, True, []] + + response = self.client.get(self.url) + self.assertEqual(response.status_code, 200) + + response_content = json.loads(response.content) + self.assertEqual(response_content.get("isControl"), False) + self.assertEqual( + response_content.get("courses"), + self.SERIALIZED_GENERAL_RECOMMENDATIONS, + ) + + @override_waffle_flag(ENABLE_FALLBACK_RECOMMENDATIONS, active=True) + @override_waffle_flag(ENABLE_DASHBOARD_RECOMMENDATIONS, active=True) + @mock.patch("django.conf.settings.GENERAL_RECOMMENDATIONS", GENERAL_RECOMMENDATIONS) + @mock.patch( + "lms.djangoapps.learner_recommendations.views.get_amplitude_course_recommendations", + mock.Mock(side_effect=Exception), + ) + def test_amplitude_api_unexpected_error(self): + """ + Test that if the Amplitude API gives an unexpected error, general recommendations are returned. + """ + + response = self.client.get(self.url) + self.assertEqual(response.status_code, 200) + + response_content = json.loads(response.content) + self.assertEqual(response_content.get("isControl"), None) + self.assertEqual( + response_content.get("courses"), + self.SERIALIZED_GENERAL_RECOMMENDATIONS, + ) + + @override_waffle_flag(ENABLE_DASHBOARD_RECOMMENDATIONS, active=True) + @mock.patch( + "lms.djangoapps.learner_recommendations.views.get_amplitude_course_recommendations" + ) + @mock.patch("lms.djangoapps.learner_recommendations.views.filter_recommended_courses") + def test_get_course_recommendations( + self, filter_recommended_courses_mock, get_amplitude_course_recommendations_mock + ): + """ + Verify API returns course recommendations. + """ + get_amplitude_course_recommendations_mock.return_value = [ + False, + True, + self.recommended_courses, + ] + + filter_recommended_courses_mock.return_value = self._get_filtered_courses() + expected_recommendations_length = 5 + + response = self.client.get(self.url) + self.assertEqual(response.status_code, 200) + + response_content = json.loads(response.content) + self.assertEqual(response_content.get("isControl"), False) + self.assertEqual( + len(response_content.get("courses")), expected_recommendations_length + ) + + @override_waffle_flag(ENABLE_FALLBACK_RECOMMENDATIONS, active=True) + @override_waffle_flag(ENABLE_DASHBOARD_RECOMMENDATIONS, active=True) + @mock.patch("django.conf.settings.GENERAL_RECOMMENDATIONS", GENERAL_RECOMMENDATIONS) + @mock.patch( + "lms.djangoapps.learner_recommendations.views.get_amplitude_course_recommendations" + ) + def test_general_recommendations( + self, get_amplitude_course_recommendations_mock + ): + """ + Test that a user gets general recommendations for the control group. + """ + get_amplitude_course_recommendations_mock.return_value = [ + True, + True, + self.recommended_courses, + ] + + response = self.client.get(self.url) + self.assertEqual(response.status_code, 200) + + response_content = json.loads(response.content) + self.assertEqual(response_content.get("isControl"), True) + self.assertEqual( + response_content.get("courses"), + self.SERIALIZED_GENERAL_RECOMMENDATIONS, + ) + + @override_waffle_flag(ENABLE_FALLBACK_RECOMMENDATIONS, active=False) + @override_waffle_flag(ENABLE_DASHBOARD_RECOMMENDATIONS, active=True) + @mock.patch("django.conf.settings.GENERAL_RECOMMENDATIONS", GENERAL_RECOMMENDATIONS) + @mock.patch( + "lms.djangoapps.learner_recommendations.views.get_amplitude_course_recommendations" + ) + def test_fallback_recommendations_disabled( + self, get_amplitude_course_recommendations_mock + ): + """ + Test that a user gets no recommendations for the control group. + """ + get_amplitude_course_recommendations_mock.return_value = [ + True, + True, + [], + ] + + response = self.client.get(self.url) + self.assertEqual(response.status_code, 200) + + response_content = json.loads(response.content) + self.assertEqual(response_content.get("isControl"), True) + self.assertEqual(response_content.get("courses"), []) + + @override_waffle_flag(ENABLE_FALLBACK_RECOMMENDATIONS, active=True) + @override_waffle_flag(ENABLE_DASHBOARD_RECOMMENDATIONS, active=True) + @mock.patch("django.conf.settings.GENERAL_RECOMMENDATIONS", GENERAL_RECOMMENDATIONS) + @mock.patch( + "lms.djangoapps.learner_recommendations.views.get_amplitude_course_recommendations" + ) + @mock.patch("lms.djangoapps.learner_recommendations.views.filter_recommended_courses") + def test_no_recommended_courses_after_filtration( + self, filter_recommended_courses_mock, get_amplitude_course_recommendations_mock + ): + """ + Test that if after filtering already enrolled courses from Amplitude recommendations + we are left with zero personalized recommendations, we return general recommendations. + """ + filter_recommended_courses_mock.return_value = [] + get_amplitude_course_recommendations_mock.return_value = [ + False, + True, + self.recommended_courses, + ] + + response = self.client.get(self.url) + self.assertEqual(response.status_code, 200) + + response_content = json.loads(response.content) + self.assertEqual(response_content.get("isControl"), False) + self.assertEqual( + response_content.get("courses"), + self.SERIALIZED_GENERAL_RECOMMENDATIONS, + ) + + @ddt.data( + (True, False, None), + (False, True, False), + (False, False, None), + (True, True, True), + ) + @mock.patch("lms.djangoapps.learner_recommendations.views.segment.track") + @mock.patch("lms.djangoapps.learner_recommendations.views.filter_recommended_courses") + @mock.patch( + "lms.djangoapps.learner_recommendations.views.get_amplitude_course_recommendations" + ) + @override_waffle_flag(ENABLE_DASHBOARD_RECOMMENDATIONS, active=True) + @ddt.unpack + def test_recommendations_viewed_segment_event( + self, + is_control, + has_is_control, + expected_is_control, + get_amplitude_course_recommendations_mock, + filter_recommended_courses_mock, + segment_track_mock + ): + """ + Test that Segment event is emitted with desired properties. + """ + get_amplitude_course_recommendations_mock.return_value = [ + is_control, + has_is_control, + self.recommended_courses, + ] + filter_recommended_courses_mock.return_value = self._get_filtered_courses() + self.client.get(self.url) + + assert segment_track_mock.call_count == 1 + assert segment_track_mock.call_args[0][1] == "edx.bi.user.recommendations.viewed" + self.assertEqual(segment_track_mock.call_args[0][2]["is_control"], expected_is_control) + + @override_waffle_flag(ENABLE_DASHBOARD_RECOMMENDATIONS, active=True) + @mock.patch( + "lms.djangoapps.learner_recommendations.views.is_user_enrolled_in_ut_austin_masters_program" + ) + def test_no_recommendations_for_masters_program_learners( + self, is_user_enrolled_in_ut_austin_masters_program_mock + ): + """ + Verify API returns no recommendations if a user is enrolled in UT Austin masters program. + """ + is_user_enrolled_in_ut_austin_masters_program_mock.return_value = True + + response = self.client.get(self.url) + self.assertEqual(response.status_code, 200) + + response_content = json.loads(response.content) + self.assertEqual(response_content.get("isControl"), None) + self.assertEqual(response_content.get("courses"), []) diff --git a/lms/djangoapps/learner_recommendations/toggles.py b/lms/djangoapps/learner_recommendations/toggles.py index 8570422617..bfab4ecfe8 100644 --- a/lms/djangoapps/learner_recommendations/toggles.py +++ b/lms/djangoapps/learner_recommendations/toggles.py @@ -21,6 +21,24 @@ ENABLE_COURSE_ABOUT_PAGE_RECOMMENDATIONS = WaffleFlag( f'{WAFFLE_FLAG_NAMESPACE}.enable_course_about_page_recommendations', __name__ ) +# Waffle flag to enable to recommendation panel on learner dashboard +# .. toggle_name: learner_recommendations.enable_dashboard_recommendations +# .. toggle_implementation: WaffleFlag +# .. toggle_default: False +# .. toggle_description: Waffle flag to enable to recommendation panel on learner dashboard +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2023-03-24 +# .. toggle_target_removal_date: None +# .. toggle_warning: None +# .. toggle_tickets: VAN-1310 +ENABLE_DASHBOARD_RECOMMENDATIONS = WaffleFlag( + f"{WAFFLE_FLAG_NAMESPACE}.enable_dashboard_recommendations", __name__ +) + + +def enable_dashboard_recommendations(): + return ENABLE_DASHBOARD_RECOMMENDATIONS.is_enabled() + def enable_course_about_page_recommendations(): return ENABLE_COURSE_ABOUT_PAGE_RECOMMENDATIONS.is_enabled() diff --git a/lms/djangoapps/learner_recommendations/urls.py b/lms/djangoapps/learner_recommendations/urls.py index 2e082c18f4..c934cf0742 100644 --- a/lms/djangoapps/learner_recommendations/urls.py +++ b/lms/djangoapps/learner_recommendations/urls.py @@ -11,6 +11,9 @@ app_name = "learner_recommendations" urlpatterns = [ re_path(fr'^amplitude/{settings.COURSE_ID_PATTERN}/$', - views.AmplitudeRecommendationsView.as_view(), + views.AboutPageRecommendationsView.as_view(), name='amplitude_recommendations'), + re_path(r"^courses/$", + views.DashboardRecommendationsApiView.as_view(), + name="courses"), ] diff --git a/lms/djangoapps/learner_recommendations/views.py b/lms/djangoapps/learner_recommendations/views.py index 39375a0804..a969fe86e0 100644 --- a/lms/djangoapps/learner_recommendations/views.py +++ b/lms/djangoapps/learner_recommendations/views.py @@ -9,26 +9,35 @@ from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthenticat from edx_rest_framework_extensions.auth.session.authentication import ( SessionAuthenticationAllowInactiveUser, ) +from edx_rest_framework_extensions.permissions import NotJwtRestrictedApplication from django.core.exceptions import PermissionDenied from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView from common.djangoapps.track import segment +from common.djangoapps.student.toggles import show_fallback_recommendations from openedx.core.djangoapps.geoinfo.api import country_code_from_ip from openedx.features.enterprise_support.utils import is_enterprise_learner -from lms.djangoapps.learner_recommendations.toggles import enable_course_about_page_recommendations +from lms.djangoapps.learner_recommendations.toggles import ( + enable_course_about_page_recommendations, + enable_dashboard_recommendations, +) from lms.djangoapps.learner_recommendations.utils import ( get_amplitude_course_recommendations, filter_recommended_courses, + is_user_enrolled_in_ut_austin_masters_program, +) +from lms.djangoapps.learner_recommendations.serializers import ( + AboutPageRecommendationsSerializer, + DashboardRecommendationsSerializer, ) -from lms.djangoapps.learner_recommendations.serializers import RecommendationsSerializer log = logging.getLogger(__name__) -class AmplitudeRecommendationsView(APIView): +class AboutPageRecommendationsView(APIView): """ **Example Request** @@ -105,7 +114,7 @@ class AmplitudeRecommendationsView(APIView): ) return Response( - RecommendationsSerializer( + AboutPageRecommendationsSerializer( { "courses": recommended_courses, "is_control": is_control, @@ -113,3 +122,99 @@ class AmplitudeRecommendationsView(APIView): ).data, status=200, ) + + +class DashboardRecommendationsApiView(APIView): + """ + API to get personalized recommendations from Amplitude. + + **Example Request** + + GET /api/learner_recommendations/courses/ + """ + + authentication_classes = ( + JwtAuthentication, + SessionAuthenticationAllowInactiveUser, + ) + permission_classes = (IsAuthenticated, NotJwtRestrictedApplication) + + def get(self, request): + """ + Retrieves course recommendations details. + """ + if not enable_dashboard_recommendations(): + return Response(status=404) + + user_id = request.user.id + + if is_user_enrolled_in_ut_austin_masters_program(request.user): + return self._recommendations_response(user_id, None, [], False) + + fallback_recommendations = settings.GENERAL_RECOMMENDATIONS if show_fallback_recommendations() else [] + + try: + is_control, has_is_control, course_keys = get_amplitude_course_recommendations( + user_id, settings.DASHBOARD_AMPLITUDE_RECOMMENDATION_ID + ) + except Exception as ex: # pylint: disable=broad-except + log.warning(f"Cannot get recommendations from Amplitude: {ex}") + return self._recommendations_response(user_id, None, fallback_recommendations, False) + + is_control = is_control if has_is_control else None + if is_control or is_control is None or not course_keys: + return self._recommendations_response(user_id, is_control, fallback_recommendations, False) + + ip_address = get_client_ip(request)[0] + user_country_code = country_code_from_ip(ip_address).upper() + filtered_courses = filter_recommended_courses( + request.user, course_keys, user_country_code=user_country_code, recommendation_count=5 + ) + # If no courses are left after filtering already enrolled courses from + # the list of amplitude recommendations, show general recommendations + # to the user. + if not filtered_courses: + return self._recommendations_response(user_id, is_control, fallback_recommendations, False) + + recommended_courses = list(map(self._course_data, filtered_courses)) + return self._recommendations_response(user_id, is_control, recommended_courses, True) + + def _emit_recommendations_viewed_event( + self, user_id, is_control, recommended_courses, amplitude_recommendations=True + ): + """Emits an event to track Learner Home page visits.""" + segment.track( + user_id, + "edx.bi.user.recommendations.viewed", + { + "is_control": is_control, + "amplitude_recommendations": amplitude_recommendations, + "course_key_array": [course["course_key"] for course in recommended_courses], + "page": "dashboard", + }, + ) + + def _recommendations_response(self, user_id, is_control, recommended_courses, amplitude_recommendations): + """ Helper method for general recommendations response. """ + self._emit_recommendations_viewed_event( + user_id, is_control, recommended_courses, amplitude_recommendations + ) + return Response( + DashboardRecommendationsSerializer( + { + "courses": recommended_courses, + "is_control": is_control, + } + ).data, + status=200, + ) + + def _course_data(self, course): + """Helper method for personalized recommendation response""" + return { + "course_key": course.get("key"), + "title": course.get("title"), + "logo_image_url": course.get("owners")[0]["logo_image_url"] if course.get( + "owners") else "", + "marketing_url": course.get("marketing_url"), + }