diff --git a/lms/djangoapps/learner_dashboard/api/utils.py b/lms/djangoapps/learner_dashboard/api/utils.py deleted file mode 100644 index e116dc6e8a..0000000000 --- a/lms/djangoapps/learner_dashboard/api/utils.py +++ /dev/null @@ -1,28 +0,0 @@ -"""API utils""" - -import requests -from django.conf import settings - - -def get_personalized_course_recommendations(user_id): - """ Get personalize recommendations from Amplitude. """ - headers = { - 'Authorization': f'Api-Key {settings.AMPLITUDE_API_KEY}', - 'Content-Type': 'application/json' - } - params = { - 'user_id': user_id, - 'get_recs': True, - 'rec_id': settings.REC_ID, - } - response = requests.get(settings.AMPLITUDE_URL, params=params, headers=headers) - if response.status_code == 200: - response = response.json() - 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, has_is_control, recommended_course_keys - - 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 118d3c6713..966cb7ded0 100644 --- a/lms/djangoapps/learner_dashboard/api/v0/tests/test_views.py +++ b/lms/djangoapps/learner_dashboard/api/v0/tests/test_views.py @@ -7,6 +7,7 @@ from uuid import uuid4 import ddt from django.core.cache import cache +from django.test import TestCase from django.urls import reverse_lazy from edx_toggles.toggles.testutils import override_waffle_flag from enterprise.models import EnterpriseCourseEnrollment @@ -245,10 +246,9 @@ class TestProgramsView(SharedModuleStoreTestCase, ProgramCacheMixin): @ddt.ddt -class TestCourseRecommendationApiView(SharedModuleStoreTestCase): +class TestCourseRecommendationApiView(TestCase): """Unit tests for the course recommendations on dashboard page.""" - password = "test" url = reverse_lazy("learner_dashboard:v0:courses") GENERAL_RECOMMENDATIONS = [ { @@ -268,7 +268,7 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): def setUp(self): super().setUp() self.user = UserFactory() - self.client.login(username=self.user.username, password=self.password) + self.client.login(username=self.user.username, password="test") self.recommended_courses = [ "MITx+6.00.1x", "IBM+PY0101EN", @@ -282,16 +282,20 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): "MichinX+101x", ] self.general_recommendation_courses = ["HogwartsX+6.00.1x", "MonstersX+SC101EN"] - self.course_data = { - "course_key": "MITx+6.00.1x", - "title": "Introduction to Computer Science and Programming Using Python", - "owners": [ - { - "logo_image_url": "https://discovery/organization/logos/2a73d2ce-c34a-4e08.png" - } - ], - "marketing_url": "https://marketing-site.com/course/introduction-to-computer-science-and-programming", - } + + def _get_filtered_courses(self): + """ + Returns the filtered course data + """ + filtered_course = [] + for course_key in self.recommended_courses[:5]: + filtered_course.append({ + "course_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 @ddt.data( (True, GENERAL_RECOMMENDATIONS), @@ -299,14 +303,14 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): ) @mock.patch("django.conf.settings.GENERAL_RECOMMENDATIONS", GENERAL_RECOMMENDATIONS) @mock.patch( - "lms.djangoapps.learner_dashboard.api.v0.views.get_personalized_course_recommendations" + "lms.djangoapps.learner_dashboard.api.v0.views.get_amplitude_course_recommendations" ) @ddt.unpack def test_amplitude_user_profile_call_failed( self, show_fallback_recommendations, expected_course_list, - mocked_get_personalized_course_recommendations, + get_amplitude_course_recommendations_mock, ): """ Test that if the call to Amplitude user profile API fails, we return the @@ -314,7 +318,7 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): If the fallback recommendations are not configured, an empty course list is returned. """ - mocked_get_personalized_course_recommendations.side_effect = Exception + get_amplitude_course_recommendations_mock.side_effect = Exception with override_waffle_flag( ENABLE_FALLBACK_RECOMMENDATIONS, active=show_fallback_recommendations ): @@ -328,17 +332,17 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): @mock.patch("django.conf.settings.GENERAL_RECOMMENDATIONS", GENERAL_RECOMMENDATIONS) @mock.patch("lms.djangoapps.learner_dashboard.api.v0.views.segment.track") @mock.patch( - "lms.djangoapps.learner_dashboard.api.v0.views.get_personalized_course_recommendations" + "lms.djangoapps.learner_dashboard.api.v0.views.get_amplitude_course_recommendations" ) def test_amplitude_recommended_no_courses( self, - mocked_get_personalized_course_recommendations, + get_amplitude_course_recommendations_mock, segment_mock, ): """ Verify API returns fallback recommendations if no courses are recommended by Amplitude. """ - mocked_get_personalized_course_recommendations.return_value = [False, True, []] + get_amplitude_course_recommendations_mock.return_value = [False, True, []] with override_waffle_flag(ENABLE_FALLBACK_RECOMMENDATIONS, active=True): response = self.client.get(self.url) @@ -362,24 +366,24 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): @mock.patch("lms.djangoapps.learner_dashboard.api.v0.views.segment.track") @mock.patch( - "lms.djangoapps.learner_dashboard.api.v0.views.get_personalized_course_recommendations" + "lms.djangoapps.learner_dashboard.api.v0.views.get_amplitude_course_recommendations" ) - @mock.patch("lms.djangoapps.learner_dashboard.api.v0.views.get_course_data") + @mock.patch("lms.djangoapps.learner_dashboard.api.v0.views.filter_recommended_courses") def test_get_course_recommendations( self, - mocked_get_course_data, - mocked_get_personalized_course_recommendations, + filter_recommended_courses_mock, + get_amplitude_course_recommendations_mock, segment_mock, ): """ Verify API returns course recommendations for users that fall in non-control group. """ - mocked_get_personalized_course_recommendations.return_value = [ + filter_recommended_courses_mock.return_value = self._get_filtered_courses() + get_amplitude_course_recommendations_mock.return_value = [ False, True, self.recommended_courses, ] - mocked_get_course_data.return_value = self.course_data expected_recommendations = 5 response = self.client.get(self.url) @@ -398,62 +402,6 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): }, ) - @mock.patch("lms.djangoapps.learner_dashboard.api.v0.views.segment.track") - @mock.patch( - "lms.djangoapps.learner_dashboard.api.v0.views.get_personalized_course_recommendations" - ) - @mock.patch("lms.djangoapps.learner_dashboard.api.v0.views.get_course_data") - def test_get_enrollable_course_recommendations( - self, - mocked_get_course_data, - mocked_get_personalized_course_recommendations, - segment_mock, - ): - """ - Verify API returns course recommendations for courses in which user is not enrolled. - """ - mocked_get_personalized_course_recommendations.return_value = [ - False, - True, - self.recommended_courses, - ] - mocked_get_course_data.return_value = self.course_data - - recommended_courses = [ - "HarvardX+CS50x", - "BabsonX+EPS03x", - "NYUx+FCS.NET.1", - "MichinX+101x", - ] - 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", - ] - - # enrolling in 6 courses - for course_key in course_keys: - CourseEnrollmentFactory(course_id=course_key, user=self.user) - - response = self.client.get(self.url) - self.assertEqual(response.status_code, 200) - self.assertEqual(response.data.get("is_control"), False) - self.assertEqual(len(response.data.get("courses")), len(recommended_courses)) - - # Verify that the segment event was fired - assert segment_mock.call_args[0][1] == "edx.bi.user.recommendations.viewed" - self.assertEqual( - segment_mock.call_args[0][2], - { - "is_control": False, - "amplitude_recommendations": True, - "course_key_array": recommended_courses, - }, - ) - @ddt.data( (True, False, None), (False, True, False), @@ -461,9 +409,9 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): (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.filter_recommended_courses") @mock.patch( - "lms.djangoapps.learner_dashboard.api.v0.views.get_personalized_course_recommendations" + "lms.djangoapps.learner_dashboard.api.v0.views.get_amplitude_course_recommendations" ) @ddt.unpack def test_recommendations_viewed_segment_event( @@ -471,16 +419,16 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): is_control, has_is_control, expected_is_control, - mocked_get_personalized_course_recommendations, - mocked_get_course_data, + get_amplitude_course_recommendations_mock, + filter_recommended_courses_mock, segment_mock, ): - mocked_get_personalized_course_recommendations.return_value = [ + filter_recommended_courses_mock.return_value = self._get_filtered_courses() + get_amplitude_course_recommendations_mock.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_mock.call_count == 1 diff --git a/lms/djangoapps/learner_dashboard/api/v0/views.py b/lms/djangoapps/learner_dashboard/api/v0/views.py index 14acdc642a..e8c35662a9 100644 --- a/lms/djangoapps/learner_dashboard/api/v0/views.py +++ b/lms/djangoapps/learner_dashboard/api/v0/views.py @@ -22,9 +22,9 @@ from openedx.core.djangoapps.programs.utils import ( get_program_and_course_data, get_program_urls, ) -from openedx.core.djangoapps.catalog.utils import get_course_data -from lms.djangoapps.learner_dashboard.api.utils import ( - get_personalized_course_recommendations, +from lms.djangoapps.learner_recommendations.utils import ( + filter_recommended_courses, + get_amplitude_course_recommendations, ) @@ -403,7 +403,6 @@ 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 fallback_recommendations = ( settings.GENERAL_RECOMMENDATIONS if show_fallback_recommendations() else [] @@ -414,7 +413,7 @@ class CourseRecommendationApiView(APIView): is_control, has_is_control, course_keys, - ) = get_personalized_course_recommendations(user_id) + ) = get_amplitude_course_recommendations(user_id, settings.DASHBOARD_AMPLITUDE_RECOMMENDATION_ID) except Exception as ex: # pylint: disable=broad-except logger.warning(f"Cannot get recommendations from Amplitude: {ex}") return self._general_recommendations_response( @@ -428,32 +427,7 @@ class CourseRecommendationApiView(APIView): user_id, is_control, fallback_recommendations ) - user_enrolled_course_keys = set() - fields = ["title", "owners", "marketing_url"] - - course_enrollments = CourseEnrollment.enrollments_for_user(request.user) - for course_enrollment in course_enrollments: - course_key = f"{course_enrollment.course_id.org}+{course_enrollment.course_id.course}" - user_enrolled_course_keys.add(course_key) - - # Pick 5 course keys, excluding the user's already enrolled course(s). - enrollable_course_keys = [ - course_key - for course_key in course_keys - if course_key not in user_enrolled_course_keys - ][:5] - for course_id in enrollable_course_keys: - course_data = get_course_data(course_id, fields) - if course_data: - recommended_courses.append( - { - "course_key": course_id, - "title": course_data["title"], - "logo_image_url": course_data["owners"][0]["logo_image_url"], - "marketing_url": course_data.get("marketing_url"), - } - ) - + recommended_courses = filter_recommended_courses(request.user, course_keys) if not recommended_courses: return self._general_recommendations_response( user_id, is_control, fallback_recommendations diff --git a/lms/djangoapps/learner_home/recommendations/test_views.py b/lms/djangoapps/learner_home/recommendations/test_views.py index 4e864f6d39..c62d949900 100644 --- a/lms/djangoapps/learner_home/recommendations/test_views.py +++ b/lms/djangoapps/learner_home/recommendations/test_views.py @@ -7,13 +7,11 @@ from unittest import mock from unittest.mock import Mock import ddt +from django.test import TestCase from django.urls import reverse_lazy from edx_toggles.toggles.testutils import override_waffle_flag -from common.djangoapps.student.tests.factories import ( - CourseEnrollmentFactory, - UserFactory, -) +from common.djangoapps.student.tests.factories import UserFactory from common.djangoapps.student.toggles import ENABLE_FALLBACK_RECOMMENDATIONS from lms.djangoapps.learner_home.test_utils import ( random_url, @@ -21,16 +19,12 @@ from lms.djangoapps.learner_home.test_utils import ( from lms.djangoapps.learner_home.recommendations.waffle import ( ENABLE_LEARNER_HOME_AMPLITUDE_RECOMMENDATIONS, ) -from xmodule.modulestore.tests.django_utils import ( - SharedModuleStoreTestCase, -) @ddt.ddt -class TestCourseRecommendationApiView(SharedModuleStoreTestCase): +class TestCourseRecommendationApiView(TestCase): """Unit tests for the course recommendations on learner home page.""" - password = "test" url = reverse_lazy("learner_home:courses") GENERAL_RECOMMENDATIONS = [ @@ -66,7 +60,7 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): def setUp(self): super().setUp() self.user = UserFactory() - self.client.login(username=self.user.username, password=self.password) + self.client.login(username=self.user.username, password="test") self.recommended_courses = [ "MITx+6.00.1x", "IBM+PY0101EN", @@ -91,12 +85,21 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): "course-v1:NYUx+FCS.NET.1+Run_0", "course-v1:MichinX+101x+Run_0", ] - self.course_data = { - "course_key": "MITx+6.00.1x", - "title": "Introduction to Computer Science and Programming Using Python", - "owners": [{"logo_image_url": "https://www.logo_image_url.com"}], - "marketing_url": "https://www.marketing_url.com", - } + + def _get_filtered_courses(self): + """ + Returns the filtered course data + """ + filtered_course = [] + for course_key in self.recommended_courses[:5]: + filtered_course.append({ + "course_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_LEARNER_HOME_AMPLITUDE_RECOMMENDATIONS, active=False) def test_waffle_flag_off(self): @@ -111,15 +114,15 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): @override_waffle_flag(ENABLE_LEARNER_HOME_AMPLITUDE_RECOMMENDATIONS, active=True) @mock.patch("django.conf.settings.GENERAL_RECOMMENDATIONS", GENERAL_RECOMMENDATIONS) @mock.patch( - "lms.djangoapps.learner_home.recommendations.views.get_personalized_course_recommendations" + "lms.djangoapps.learner_home.recommendations.views.get_amplitude_course_recommendations" ) def test_no_recommendations_from_amplitude( - self, mocked_get_personalized_course_recommendations + self, get_amplitude_course_recommendations_mock ): """ Verify API returns general recommendations if no course recommendations from amplitude. """ - mocked_get_personalized_course_recommendations.return_value = [False, True, []] + get_amplitude_course_recommendations_mock.return_value = [False, True, []] response = self.client.get(self.url) self.assertEqual(response.status_code, 200) @@ -135,7 +138,7 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): @override_waffle_flag(ENABLE_LEARNER_HOME_AMPLITUDE_RECOMMENDATIONS, active=True) @mock.patch("django.conf.settings.GENERAL_RECOMMENDATIONS", GENERAL_RECOMMENDATIONS) @mock.patch( - "lms.djangoapps.learner_home.recommendations.views.get_personalized_course_recommendations", + "lms.djangoapps.learner_home.recommendations.views.get_amplitude_course_recommendations", Mock(side_effect=Exception), ) def test_amplitude_api_unexpected_error(self): @@ -155,21 +158,22 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): @override_waffle_flag(ENABLE_LEARNER_HOME_AMPLITUDE_RECOMMENDATIONS, active=True) @mock.patch( - "lms.djangoapps.learner_home.recommendations.views.get_personalized_course_recommendations" + "lms.djangoapps.learner_home.recommendations.views.get_amplitude_course_recommendations" ) - @mock.patch("lms.djangoapps.learner_home.recommendations.views.get_course_data") + @mock.patch("lms.djangoapps.learner_home.recommendations.views.filter_recommended_courses") def test_get_course_recommendations( - self, mocked_get_course_data, mocked_get_personalized_course_recommendations + self, filter_recommended_courses_mock, get_amplitude_course_recommendations_mock ): """ Verify API returns course recommendations. """ - mocked_get_personalized_course_recommendations.return_value = [ + get_amplitude_course_recommendations_mock.return_value = [ False, True, self.recommended_courses, ] - mocked_get_course_data.return_value = self.course_data + + filter_recommended_courses_mock.return_value = self._get_filtered_courses() expected_recommendations_length = 5 response = self.client.get(self.url) @@ -185,15 +189,15 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): @override_waffle_flag(ENABLE_LEARNER_HOME_AMPLITUDE_RECOMMENDATIONS, active=True) @mock.patch("django.conf.settings.GENERAL_RECOMMENDATIONS", GENERAL_RECOMMENDATIONS) @mock.patch( - "lms.djangoapps.learner_home.recommendations.views.get_personalized_course_recommendations" + "lms.djangoapps.learner_home.recommendations.views.get_amplitude_course_recommendations" ) def test_general_recommendations( - self, mocked_get_personalized_course_recommendations + self, get_amplitude_course_recommendations_mock ): """ Test that a user gets general recommendations for the control group. """ - mocked_get_personalized_course_recommendations.return_value = [ + get_amplitude_course_recommendations_mock.return_value = [ True, True, self.recommended_courses, @@ -213,15 +217,15 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): @override_waffle_flag(ENABLE_LEARNER_HOME_AMPLITUDE_RECOMMENDATIONS, active=True) @mock.patch("django.conf.settings.GENERAL_RECOMMENDATIONS", GENERAL_RECOMMENDATIONS) @mock.patch( - "lms.djangoapps.learner_home.recommendations.views.get_personalized_course_recommendations" + "lms.djangoapps.learner_home.recommendations.views.get_amplitude_course_recommendations" ) def test_fallback_recommendations_disabled( - self, mocked_get_personalized_course_recommendations + self, get_amplitude_course_recommendations_mock ): """ Test that a user gets no recommendations for the control group. """ - mocked_get_personalized_course_recommendations.return_value = [ + get_amplitude_course_recommendations_mock.return_value = [ True, True, [], @@ -234,59 +238,26 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): self.assertEqual(response_content.get("isControl"), True) self.assertEqual(response_content.get("courses"), []) - @override_waffle_flag(ENABLE_LEARNER_HOME_AMPLITUDE_RECOMMENDATIONS, active=True) - @mock.patch( - "lms.djangoapps.learner_home.recommendations.views.get_personalized_course_recommendations" - ) - @mock.patch("lms.djangoapps.learner_home.recommendations.views.get_course_data") - def test_get_enrollable_course_recommendations( - self, mocked_get_course_data, mocked_get_personalized_course_recommendations - ): - """ - Verify API returns course recommendations for courses in which user is not enrolled. - """ - mocked_get_personalized_course_recommendations.return_value = [ - False, - True, - self.recommended_courses, - ] - mocked_get_course_data.return_value = self.course_data - expected_recommendations = 4 - # enrolling in 6 courses - for course_run_key in self.course_run_keys[:6]: - CourseEnrollmentFactory(course_id=course_run_key, user=self.user) - - 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) - @override_waffle_flag(ENABLE_FALLBACK_RECOMMENDATIONS, active=True) @override_waffle_flag(ENABLE_LEARNER_HOME_AMPLITUDE_RECOMMENDATIONS, active=True) @mock.patch("django.conf.settings.GENERAL_RECOMMENDATIONS", GENERAL_RECOMMENDATIONS) @mock.patch( - "lms.djangoapps.learner_home.recommendations.views.get_personalized_course_recommendations" + "lms.djangoapps.learner_home.recommendations.views.get_amplitude_course_recommendations" ) - @mock.patch("lms.djangoapps.learner_home.recommendations.views.get_course_data") - def test_no_enrollable_course( - self, mocked_get_course_data, mocked_get_personalized_course_recommendations + @mock.patch("lms.djangoapps.learner_home.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. """ - mocked_get_personalized_course_recommendations.return_value = [ + filter_recommended_courses_mock.return_value = [] + get_amplitude_course_recommendations_mock.return_value = [ False, True, self.recommended_courses, ] - mocked_get_course_data.return_value = self.course_data - - # Enrolling in all courses - for course_run_key in self.course_run_keys: - CourseEnrollmentFactory(course_id=course_run_key, user=self.user) response = self.client.get(self.url) self.assertEqual(response.status_code, 200) @@ -305,9 +276,9 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): (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.filter_recommended_courses") @mock.patch( - "lms.djangoapps.learner_home.recommendations.views.get_personalized_course_recommendations" + "lms.djangoapps.learner_home.recommendations.views.get_amplitude_course_recommendations" ) @override_waffle_flag(ENABLE_LEARNER_HOME_AMPLITUDE_RECOMMENDATIONS, active=True) @ddt.unpack @@ -316,19 +287,19 @@ class TestCourseRecommendationApiView(SharedModuleStoreTestCase): is_control, has_is_control, expected_is_control, - mocked_get_personalized_course_recommendations, - mocked_get_course_data, + get_amplitude_course_recommendations_mock, + filter_recommended_courses_mock, segment_track_mock ): """ Test that Segment event is emitted with desired properties. """ - mocked_get_personalized_course_recommendations.return_value = [ + get_amplitude_course_recommendations_mock.return_value = [ is_control, has_is_control, self.recommended_courses, ] - mocked_get_course_data.return_value = self.course_data + filter_recommended_courses_mock.return_value = self._get_filtered_courses() self.client.get(self.url) assert segment_track_mock.call_count == 1 diff --git a/lms/djangoapps/learner_home/recommendations/utils.py b/lms/djangoapps/learner_home/recommendations/utils.py deleted file mode 100644 index fa0448c6b3..0000000000 --- a/lms/djangoapps/learner_home/recommendations/utils.py +++ /dev/null @@ -1,32 +0,0 @@ -"""API utils""" - -import logging -import requests - -from django.conf import settings - -log = logging.getLogger(__name__) - - -def get_personalized_course_recommendations(user_id): - """Get personalize recommendations from Amplitude.""" - headers = { - "Authorization": f"Api-Key {settings.AMPLITUDE_API_KEY}", - "Content-Type": "application/json", - } - params = { - "user_id": user_id, - "get_recs": True, - "rec_id": settings.REC_ID, - } - response = requests.get(settings.AMPLITUDE_URL, params=params, headers=headers) - if response.status_code == 200: - response = response.json() - 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, has_is_control, recommended_course_keys - - return True, False, [] diff --git a/lms/djangoapps/learner_home/recommendations/views.py b/lms/djangoapps/learner_home/recommendations/views.py index a2339c79cf..a5f8c3b667 100644 --- a/lms/djangoapps/learner_home/recommendations/views.py +++ b/lms/djangoapps/learner_home/recommendations/views.py @@ -13,19 +13,18 @@ from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView -from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.toggles import show_fallback_recommendations from common.djangoapps.track import segment from lms.djangoapps.learner_home.recommendations.serializers import ( CourseRecommendationSerializer, ) -from lms.djangoapps.learner_home.recommendations.utils import ( - get_personalized_course_recommendations, -) from lms.djangoapps.learner_home.recommendations.waffle import ( should_show_learner_home_amplitude_recommendations, ) -from openedx.core.djangoapps.catalog.utils import get_course_data +from lms.djangoapps.learner_recommendations.utils import ( + filter_recommended_courses, + get_amplitude_course_recommendations, +) logger = logging.getLogger(__name__) @@ -54,11 +53,12 @@ class CourseRecommendationApiView(APIView): return Response(status=404) user_id = request.user.id - recommended_courses = [] fallback_recommendations = settings.GENERAL_RECOMMENDATIONS if show_fallback_recommendations() else [] try: - is_control, has_is_control, course_keys = get_personalized_course_recommendations(user_id) + 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 logger.warning(f"Cannot get recommendations from Amplitude: {ex}") return self._general_recommendations_response(user_id, None, fallback_recommendations) @@ -67,30 +67,7 @@ class CourseRecommendationApiView(APIView): if is_control or is_control is None or not course_keys: return self._general_recommendations_response(user_id, is_control, fallback_recommendations) - user_enrolled_course_keys = set() - fields = ["title", "owners", "marketing_url"] - - course_enrollments = CourseEnrollment.enrollments_for_user(request.user) - for course_enrollment in course_enrollments: - course_key = f"{course_enrollment.course_id.org}+{course_enrollment.course_id.course}" - user_enrolled_course_keys.add(course_key) - - # Pick 5 course keys, excluding the user's already enrolled course(s). - enrollable_course_keys = [ - course_key for course_key in course_keys if course_key not in user_enrolled_course_keys - ][:5] - for course_id in enrollable_course_keys: - course_data = get_course_data(course_id, fields) - if course_data: - recommended_courses.append( - { - "course_key": course_id, - "title": course_data["title"], - "logo_image_url": course_data["owners"][0]["logo_image_url"], - "marketing_url": course_data.get("marketing_url"), - } - ) - + recommended_courses = filter_recommended_courses(request.user, course_keys) # If no courses are left after filtering already enrolled courses from # the list of amplitude recommendations, show general recommendations # to the user. diff --git a/lms/djangoapps/learner_recommendations/tests/__init__.py b/lms/djangoapps/learner_recommendations/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/learner_recommendations/tests/test_utils.py b/lms/djangoapps/learner_recommendations/tests/test_utils.py new file mode 100644 index 0000000000..3baf81207c --- /dev/null +++ b/lms/djangoapps/learner_recommendations/tests/test_utils.py @@ -0,0 +1,113 @@ +""" Test Recommendations helpers methods """ +import ddt +from django.test import TestCase +from unittest.mock import Mock, patch + +from common.djangoapps.student.tests.factories import ( + CourseEnrollmentFactory, + UserFactory, +) +from lms.djangoapps.learner_recommendations.utils import ( + filter_recommended_courses, + get_amplitude_course_recommendations, +) +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + + +@ddt.ddt +class TestRecommendationsHelper(TestCase): + """Test course recommendations helper methods.""" + + def setUp(self): + super().setUp() + self.user = UserFactory() + + @ddt.data( + ({}, 0), + ({"userData": {}}, 0), + ({"userData": {"recommendations": []}}, 0), + ( + { + "userData": { + "recommendations": [ + { + "items": ["MITx+6.00.1x", "IBM+PY0101EN", "HarvardX+CS50P"], + "is_control": True, + "has_is_control": False, + } + ], + } + }, + 3, + ), + ) + @patch("lms.djangoapps.learner_recommendations.utils.requests.get") + @ddt.unpack + def test_get_amplitude_course_recommendations_method( + self, mocked_response, expected_recommendations_count, mock_get + ): + """ + Tests the get_amplitude_recommendations method returns course key list. + """ + mock_get.return_value = Mock(status_code=200, json=lambda: mocked_response) + _, _, course_keys = get_amplitude_course_recommendations( + self.user.id, "amplitude-rec-id" + ) + self.assertEqual(len(course_keys), expected_recommendations_count) + + +class TestFilterRecommendedCourses(ModuleStoreTestCase): + """Test for filter_recommended_courses helper method.""" + + def setUp(self): + super().setUp() + self.user = UserFactory() + self.course_data = { + "course_key": "Mocked course key", + "title": "Mocked course title", + "owners": [{"logo_image_url": "https://www.logo_image_url.com"}], + "marketing_url": "https://www.marketing_url.com", + } + self.recommended_course_keys = [ + "MITx+6.00.1x", + "IBM+PY0101EN", + "HarvardX+CS50P", + "UQx+IELTSx", + "HarvardX+CS50x", + "Harvard+CS50z", + "BabsonX+EPS03x", + "TUMx+QPLS2x", + "NYUx+FCS.NET.1", + "MichinX+101x", + ] + self.course_run_keys = [ + "course-v1:MITx+6.00.1x+Run_0", + "course-v1:IBM+PY0101EN+Run_0", + "course-v1:HarvardX+CS50P+Run_0", + "course-v1:UQx+IELTSx+Run_0", + "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", + ] + + @patch("lms.djangoapps.learner_recommendations.utils.get_course_data") + def test_enrolled_courses_are_removed_from_recommendations( + self, mocked_get_course_data + ): + """ + Tests that given a recommended course list, the filter_recommended_courses + method removes the enrolled courses from it. + """ + total_enrolled_courses = 6 + total_recommendations = len(self.recommended_course_keys) + mocked_get_course_data.return_value = self.course_data + for course_run_key in self.course_run_keys[:total_enrolled_courses]: + CourseEnrollmentFactory(course_id=course_run_key, user=self.user) + + filtered_courses = filter_recommended_courses( + self.user, self.recommended_course_keys, total_recommendations + ) + self.assertEqual(len(filtered_courses), total_recommendations - total_enrolled_courses) diff --git a/lms/djangoapps/learner_recommendations/test_views.py b/lms/djangoapps/learner_recommendations/tests/test_views.py similarity index 100% rename from lms/djangoapps/learner_recommendations/test_views.py rename to lms/djangoapps/learner_recommendations/tests/test_views.py diff --git a/lms/djangoapps/learner_recommendations/utils.py b/lms/djangoapps/learner_recommendations/utils.py index 0574d31c02..4601997ca3 100644 --- a/lms/djangoapps/learner_recommendations/utils.py +++ b/lms/djangoapps/learner_recommendations/utils.py @@ -1,13 +1,16 @@ """ Additional utilities for Learner Recommendations. """ - import logging +import requests from algoliasearch.exceptions import RequestException, AlgoliaUnreachableHostException from algoliasearch.search_client import SearchClient from django.conf import settings +from common.djangoapps.student.models import CourseEnrollment +from openedx.core.djangoapps.catalog.utils import get_course_data + log = logging.getLogger(__name__) @@ -36,6 +39,22 @@ class AlgoliaClient: return cls.algolia_client +def _remove_user_enrolled_course_keys(user, course_keys): + """ + Remove the course keys a user is already enrolled in + and returns enrollable course keys. + """ + user_enrolled_course_keys = set() + course_enrollments = CourseEnrollment.enrollments_for_user(user) + + for course_enrollment in course_enrollments: + course_key = f"{course_enrollment.course_id.org}+{course_enrollment.course_id.course}" + user_enrolled_course_keys.add(course_key) + + enrollable_course_keys = [course_key for course_key in course_keys if course_key not in user_enrolled_course_keys] + return enrollable_course_keys + + def get_algolia_courses_recommendation(course_data): """ Get courses recommendation from Algolia search. @@ -76,3 +95,75 @@ def get_algolia_courses_recommendation(course_data): log.warning(f"Unexpected exception while attempting to fetch courses data from Algolia: {str(ex)}") return {} + + +def get_amplitude_course_recommendations(user_id, recommendation_id): + """ + Get personalized recommendations from Amplitude. + + Args: + user_id: The user for which the recommendations need to be pulled + recommendation_id: Amplitude model id + + Returns: + is_control (bool): Control group value for the user + has_is_control (bool): Boolean value indicating if the control group for + the user has been decided. + recommended_course_keys (list): Course keys returned by Amplitude. + """ + headers = { + "Authorization": f"Api-Key {settings.AMPLITUDE_API_KEY}", + "Content-Type": "application/json", + } + params = { + "user_id": user_id, + "get_recs": True, + "rec_id": recommendation_id, + } + response = requests.get(settings.AMPLITUDE_URL, params=params, headers=headers) + if response.status_code == 200: + response = response.json() + 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, has_is_control, recommended_course_keys + + return True, False, [] + + +def filter_recommended_courses(user, unfiltered_course_keys, recommendation_count=5): + """ + Returns the filtered course recommendations. The unfiltered course keys + pass through the following filters: + 1. Remove courses that a user is already enrolled in + 2. + + Returns: + filtered_recommended_courses (list): A list of course objects. Each item has + the following details for a course: + - course key (course_key) + - title + - partner image url (logo_image_url) + - marketing url for the course (marketing_url) + """ + filtered_recommended_courses = [] + fields = ["title", "owners", "marketing_url"] + + # Remove the course keys a user is already enrolled in + enrollable_course_keys = _remove_user_enrolled_course_keys(user, unfiltered_course_keys)[:recommendation_count] + + for course_id in enrollable_course_keys: + course_data = get_course_data(course_id, fields) + if course_data: + filtered_recommended_courses.append( + { + "course_key": course_id, + "title": course_data["title"], + "logo_image_url": course_data["owners"][0]["logo_image_url"], + "marketing_url": course_data.get("marketing_url"), + } + ) + + return filtered_recommended_courses diff --git a/lms/envs/common.py b/lms/envs/common.py index 1821567cc9..8c74c9dc8e 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -4768,7 +4768,7 @@ BRAZE_COURSE_ENROLLMENT_CANVAS_ID = '' ### SETTINGS FOR AMPLITUDE #### AMPLITUDE_URL = '' AMPLITUDE_API_KEY = '' -REC_ID = '' +DASHBOARD_AMPLITUDE_RECOMMENDATION_ID = '' # Keeping this for back compatibility with learner dashboard api GENERAL_RECOMMENDATION = {}