From 030267f5ee3e89c3e1fa704bbcfe892a38747064 Mon Sep 17 00:00:00 2001 From: Shafqat Farhan Date: Tue, 3 Jan 2023 04:12:11 +0500 Subject: [PATCH] fix: VAN-1223 - Add is_control property to recommendations viewed event --- lms/djangoapps/learner_dashboard/api/utils.py | 5 ++- .../api/v0/tests/test_views.py | 39 ++++++++++++++-- .../learner_dashboard/api/v0/views.py | 27 ++++++----- .../recommendations/test_views.py | 45 ++++++++++++++++++- .../learner_home/recommendations/utils.py | 5 ++- .../learner_home/recommendations/views.py | 31 ++++++++----- 6 files changed, 122 insertions(+), 30 deletions(-) diff --git a/lms/djangoapps/learner_dashboard/api/utils.py b/lms/djangoapps/learner_dashboard/api/utils.py index ae529c9c5f..85461cdc6f 100644 --- a/lms/djangoapps/learner_dashboard/api/utils.py +++ b/lms/djangoapps/learner_dashboard/api/utils.py @@ -25,9 +25,10 @@ def get_personalized_course_recommendations(user_id): recommendations = response.get('userData', {}).get('recommendations', []) if recommendations: is_control = recommendations[0].get('is_control') + has_is_control = recommendations[0].get('has_is_control') recommended_course_keys = recommendations[0].get('items') - return is_control, recommended_course_keys + return is_control, has_is_control, recommended_course_keys except Exception as ex: # pylint: disable=broad-except log.warning(f'Cannot get recommendations from Amplitude: {ex}') - return True, [] + return True, False, [] diff --git a/lms/djangoapps/learner_dashboard/api/v0/tests/test_views.py b/lms/djangoapps/learner_dashboard/api/v0/tests/test_views.py index ac9f50d1ef..64689fb389 100644 --- a/lms/djangoapps/learner_dashboard/api/v0/tests/test_views.py +++ b/lms/djangoapps/learner_dashboard/api/v0/tests/test_views.py @@ -5,6 +5,7 @@ Unit tests for Learner Dashboard REST APIs and Views from unittest import mock from uuid import uuid4 +import ddt from django.core.cache import cache from django.urls import reverse_lazy from enterprise.models import EnterpriseCourseEnrollment @@ -232,6 +233,7 @@ class TestProgramsView(SharedModuleStoreTestCase, ProgramCacheMixin): self.assertEqual(response.data, []) +@ddt.ddt class TestCourseRecommendationApiView(SharedModuleStoreTestCase): """Unit tests for the course recommendations on dashboard page.""" @@ -259,7 +261,7 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): """ Verify API returns 400 if no course recommendations from amplitude. """ - mocked_get_personalized_course_recommendations.return_value = [False, []] + mocked_get_personalized_course_recommendations.return_value = [False, True, []] mocked_get_course_data.return_value = self.course_data response = self.client.get(self.url) @@ -273,7 +275,7 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): """ Verify API returns course recommendations. """ - mocked_get_personalized_course_recommendations.return_value = [False, self.recommended_courses] + mocked_get_personalized_course_recommendations.return_value = [False, True, self.recommended_courses] mocked_get_course_data.return_value = self.course_data expected_recommendations = 5 @@ -289,7 +291,7 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): """ Verify API returns course recommendations for courses in which user is not enrolled. """ - mocked_get_personalized_course_recommendations.return_value = [False, self.recommended_courses] + mocked_get_personalized_course_recommendations.return_value = [False, True, self.recommended_courses] mocked_get_course_data.return_value = self.course_data course_keys = ['course-v1:IBM+PY0101EN+Run_0', 'course-v1:UQx+IELTSx+Run_0', 'course-v1:MITx+6.00.1x+Run_0', 'course-v1:HarvardX+CS50P+Run_0', 'course-v1:Harvard+CS50z+Run_0', 'course-v1:TUMx+QPLS2x+Run_0'] @@ -302,3 +304,34 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): self.assertEqual(response.status_code, 200) self.assertEqual(response.data.get('is_personalized_recommendation'), True) self.assertEqual(len(response.data.get('courses')), expected_recommendations) + + @ddt.data( + (True, False, None), + (False, True, False), + (False, False, None), + (True, True, True), + ) + @mock.patch('lms.djangoapps.learner_dashboard.api.v0.views.segment.track') + @mock.patch('lms.djangoapps.learner_dashboard.api.v0.views.get_course_data') + @mock.patch('lms.djangoapps.learner_dashboard.api.v0.views.get_personalized_course_recommendations') + @ddt.unpack + def test_recommendations_viewed_segment_event( + self, + is_control, + has_is_control, + expected_is_control, + mocked_get_personalized_course_recommendations, + mocked_get_course_data, + segment_track_mock + ): + mocked_get_personalized_course_recommendations.return_value = [ + is_control, + has_is_control, + self.recommended_courses, + ] + mocked_get_course_data.return_value = self.course_data + 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) diff --git a/lms/djangoapps/learner_dashboard/api/v0/views.py b/lms/djangoapps/learner_dashboard/api/v0/views.py index 48fc8bf1ee..114426a2ea 100644 --- a/lms/djangoapps/learner_dashboard/api/v0/views.py +++ b/lms/djangoapps/learner_dashboard/api/v0/views.py @@ -357,22 +357,14 @@ class CourseRecommendationApiView(APIView): def get(self, request): """ Retrieves course recommendations details of a user in a specified course. """ + recommended_courses = [] user_id = request.user.id - is_control, course_keys = get_personalized_course_recommendations(user_id) - - # Emits an event to track student dashboard page visits. - segment.track( - user_id, - 'edx.bi.user.recommendations.viewed', - { - 'is_personalized_recommendation': not is_control, - } - ) + is_control, has_is_control, course_keys = get_personalized_course_recommendations(user_id) if is_control or not course_keys: + self._emit_recommendations_viewed_event(user_id, is_control, has_is_control, recommended_courses) return Response(status=400) - recommended_courses = [] user_enrolled_course_keys = set() fields = ['title', 'owners', 'marketing_url'] @@ -392,5 +384,18 @@ class CourseRecommendationApiView(APIView): 'logo_image_url': course_data['owners'][0]['logo_image_url'], 'marketing_url': course_data.get('marketing_url') }) + self._emit_recommendations_viewed_event(user_id, is_control, has_is_control, recommended_courses) return Response({'courses': recommended_courses, 'is_personalized_recommendation': not is_control}, status=200) + + def _emit_recommendations_viewed_event(self, user_id, is_control, has_is_control, recommended_courses): + """Emits an event to track student dashboard page visits.""" + segment.track( + user_id, + 'edx.bi.user.recommendations.viewed', + { + 'is_personalized_recommendation': not is_control, + 'is_control': is_control if has_is_control else None, + 'course_key_array': [course['course_key'] for course in recommended_courses], + } + ) diff --git a/lms/djangoapps/learner_home/recommendations/test_views.py b/lms/djangoapps/learner_home/recommendations/test_views.py index 93d05c34d5..63256c322b 100644 --- a/lms/djangoapps/learner_home/recommendations/test_views.py +++ b/lms/djangoapps/learner_home/recommendations/test_views.py @@ -6,6 +6,7 @@ import json from unittest import mock from unittest.mock import Mock +import ddt from django.urls import reverse_lazy from edx_toggles.toggles.testutils import override_waffle_flag @@ -24,6 +25,7 @@ from xmodule.modulestore.tests.django_utils import ( ) +@ddt.ddt class TestCourseRecommendationApiView(SharedModuleStoreTestCase): """Unit tests for the course recommendations on learner home page.""" @@ -115,7 +117,7 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): """ Verify API returns general recommendations if no course recommendations from amplitude. """ - mocked_get_personalized_course_recommendations.return_value = [False, []] + mocked_get_personalized_course_recommendations.return_value = [False, True, []] response = self.client.get(self.url) self.assertEqual(response.status_code, 200) @@ -161,6 +163,7 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): """ mocked_get_personalized_course_recommendations.return_value = [ False, + True, self.recommended_courses, ] mocked_get_course_data.return_value = self.course_data @@ -187,6 +190,7 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): Test that a user gets general recommendations for the control group. """ mocked_get_personalized_course_recommendations.return_value = [ + True, True, self.recommended_courses, ] @@ -214,6 +218,7 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): """ mocked_get_personalized_course_recommendations.return_value = [ False, + True, self.recommended_courses, ] mocked_get_course_data.return_value = self.course_data @@ -244,6 +249,7 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): """ mocked_get_personalized_course_recommendations.return_value = [ False, + True, self.recommended_courses, ] mocked_get_course_data.return_value = self.course_data @@ -261,3 +267,40 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): 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_home.recommendations.views.segment.track") + @mock.patch("lms.djangoapps.learner_home.recommendations.views.get_course_data") + @mock.patch( + "lms.djangoapps.learner_home.recommendations.views.get_personalized_course_recommendations" + ) + @override_waffle_flag(ENABLE_LEARNER_HOME_AMPLITUDE_RECOMMENDATIONS, active=True) + @ddt.unpack + def test_recommendations_viewed_segment_event( + self, + is_control, + has_is_control, + expected_is_control, + mocked_get_personalized_course_recommendations, + mocked_get_course_data, + segment_track_mock + ): + """ + Test that Segment event is emitted with desired properties. + """ + mocked_get_personalized_course_recommendations.return_value = [ + is_control, + has_is_control, + self.recommended_courses, + ] + mocked_get_course_data.return_value = self.course_data + 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) diff --git a/lms/djangoapps/learner_home/recommendations/utils.py b/lms/djangoapps/learner_home/recommendations/utils.py index e6bc937f46..fa0448c6b3 100644 --- a/lms/djangoapps/learner_home/recommendations/utils.py +++ b/lms/djangoapps/learner_home/recommendations/utils.py @@ -25,7 +25,8 @@ def get_personalized_course_recommendations(user_id): recommendations = response.get("userData", {}).get("recommendations", []) if recommendations: is_control = recommendations[0].get("is_control") + has_is_control = recommendations[0].get("has_is_control") recommended_course_keys = recommendations[0].get("items") - return is_control, recommended_course_keys + return is_control, has_is_control, recommended_course_keys - return True, [] + return True, False, [] diff --git a/lms/djangoapps/learner_home/recommendations/views.py b/lms/djangoapps/learner_home/recommendations/views.py index f2ad1420c8..f5b290b900 100644 --- a/lms/djangoapps/learner_home/recommendations/views.py +++ b/lms/djangoapps/learner_home/recommendations/views.py @@ -52,6 +52,7 @@ class CourseRecommendationApiView(APIView): if not should_show_learner_home_amplitude_recommendations(): return Response(status=404) + recommended_courses = [] general_recommendations_response = Response( CourseRecommendationSerializer( { @@ -64,24 +65,17 @@ class CourseRecommendationApiView(APIView): try: user_id = request.user.id - is_control, course_keys = get_personalized_course_recommendations(user_id) + is_control, has_is_control, course_keys = get_personalized_course_recommendations(user_id) except Exception as ex: # pylint: disable=broad-except logger.warning(f"Cannot get recommendations from Amplitude: {ex}") return general_recommendations_response - # Emits an event to track student dashboard page visits. - segment.track( - user_id, - "edx.bi.user.recommendations.viewed", - { - "is_personalized_recommendation": not is_control, - }, - ) - if is_control or not course_keys: + self._emit_recommendations_viewed_event( + user_id, is_control, has_is_control, recommended_courses + ) return general_recommendations_response - recommended_courses = [] user_enrolled_course_keys = set() fields = ["title", "owners", "marketing_url"] @@ -105,6 +99,9 @@ class CourseRecommendationApiView(APIView): "marketing_url": course_data.get("marketing_url"), } ) + self._emit_recommendations_viewed_event( + user_id, is_control, has_is_control, recommended_courses + ) # If no courses are left after filtering already enrolled courses from # the list of amplitude recommendations, show general recommendations @@ -121,3 +118,15 @@ class CourseRecommendationApiView(APIView): ).data, status=200, ) + + def _emit_recommendations_viewed_event(self, user_id, is_control, has_is_control, recommended_courses): + """Emits an event to track Learner Home page visits.""" + segment.track( + user_id, + "edx.bi.user.recommendations.viewed", + { + "is_personalized_recommendation": not is_control, + "is_control": is_control if has_is_control else None, + "course_key_array": [course["course_key"] for course in recommended_courses], + }, + )