fix: fix CS50 enrollment and filtering issue for recommendations (#31916)
VAN-1314 Co-authored-by: Syed Sajjad Hussain Shah <syed.sajjad@H7FKF7K6XD.local>
This commit is contained in:
committed by
GitHub
parent
463b64d349
commit
278dbba646
@@ -84,10 +84,6 @@ class TestFilterRecommendedCourses(ModuleStoreTestCase):
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
self.user = UserFactory()
|
||||
self.unrestricted_course_keys = [
|
||||
"MITx+6.00.1x",
|
||||
"IBM+PY0101EN",
|
||||
]
|
||||
self.recommended_course_keys = [
|
||||
"MITx+6.00.1x",
|
||||
"IBM+PY0101EN",
|
||||
@@ -100,36 +96,10 @@ class TestFilterRecommendedCourses(ModuleStoreTestCase):
|
||||
"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",
|
||||
]
|
||||
self.course_keys_with_active_course_runs = [
|
||||
"MITx+6.00.1x",
|
||||
"IBM+PY0101EN",
|
||||
"HarvardX+CS50P",
|
||||
"UQx+IELTSx",
|
||||
"HarvardX+CS50x",
|
||||
"Harvard+CS50z",
|
||||
"BabsonX+EPS03x",
|
||||
"TUMx+QPLS2x",
|
||||
]
|
||||
self.enrolled_course_run_keys = [
|
||||
"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",
|
||||
]
|
||||
self.unrestricted_course_keys = self.recommended_course_keys[0:2]
|
||||
self.course_run_keys = [f"course-v1:{course_key}+Run_0" for course_key in self.recommended_course_keys]
|
||||
self.course_keys_with_active_course_runs = self.recommended_course_keys[0:8]
|
||||
self.enrolled_course_run_keys = self.course_run_keys[4:10]
|
||||
|
||||
def _mock_get_course_data(self, course_id, fields=None, querystring=None): # pylint: disable=unused-argument
|
||||
"""
|
||||
@@ -157,7 +127,7 @@ class TestFilterRecommendedCourses(ModuleStoreTestCase):
|
||||
{
|
||||
"course_runs": [
|
||||
{
|
||||
"key": "course-v1:MITx+6.00.1x+Run_0",
|
||||
"key": f"course-v1:{course_id}+Run_0",
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -193,12 +163,12 @@ class TestFilterRecommendedCourses(ModuleStoreTestCase):
|
||||
Test that if the "request course" is one of the recommended courses,
|
||||
we filter that from the final recommendation list.
|
||||
"""
|
||||
request_course = self.recommended_course_keys[0]
|
||||
request_course = self.course_run_keys[0]
|
||||
mocked_get_course_data.side_effect = self._mock_get_course_data
|
||||
filtered_courses = filter_recommended_courses(
|
||||
self.user,
|
||||
self.recommended_course_keys,
|
||||
request_course=request_course,
|
||||
request_course_key=request_course,
|
||||
)
|
||||
|
||||
assert all(course["course_key"] != request_course for course in filtered_courses) is True
|
||||
|
||||
@@ -40,20 +40,19 @@ class AlgoliaClient:
|
||||
return cls.algolia_client
|
||||
|
||||
|
||||
def _remove_user_enrolled_course_keys(user, course_keys):
|
||||
def _get_user_enrolled_course_keys(user):
|
||||
"""
|
||||
Remove the course keys a user is already enrolled in
|
||||
and returns enrollable course keys.
|
||||
Returns course ids in which the user is enrolled in.
|
||||
"""
|
||||
user_enrolled_course_keys = set()
|
||||
course_enrollments = CourseEnrollment.enrollments_for_user(user)
|
||||
return [str(course_enrollment.course_id) for course_enrollment in course_enrollments]
|
||||
|
||||
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 _is_enrolled_in_course(course_runs, enrolled_course_keys):
|
||||
"""
|
||||
Returns True if a user is enrolled in any course run of the course else false.
|
||||
"""
|
||||
return any(course_run.get("key", None) in enrolled_course_keys for course_run in course_runs)
|
||||
|
||||
|
||||
def _has_country_restrictions(product, user_country):
|
||||
@@ -85,34 +84,6 @@ def _has_country_restrictions(product, user_country):
|
||||
return user_country in block_list or (bool(allow_list) and user_country not in allow_list)
|
||||
|
||||
|
||||
def _get_program_duration(weeks):
|
||||
"""
|
||||
Helper method that returns the program duration in textual form.
|
||||
"""
|
||||
total_months = round(weeks / 4)
|
||||
|
||||
if total_months < 1:
|
||||
return f'{total_months} weeks'
|
||||
|
||||
if 1 <= total_months < 12:
|
||||
return f'{total_months} months'
|
||||
|
||||
total_years = round(total_months / 12)
|
||||
total_remainder_months = round(total_months % 12)
|
||||
|
||||
if total_remainder_months == 0:
|
||||
return f'{total_years} years'
|
||||
|
||||
if total_years == 1 and total_remainder_months == 1:
|
||||
return f'1 year {total_remainder_months} months'
|
||||
|
||||
if total_remainder_months == 1:
|
||||
return f'{total_years} years 1 months'
|
||||
|
||||
else:
|
||||
return f'{total_years} years {total_remainder_months} months'
|
||||
|
||||
|
||||
def get_amplitude_course_recommendations(user_id, recommendation_id):
|
||||
"""
|
||||
Get personalized recommendations from Amplitude.
|
||||
@@ -178,7 +149,7 @@ def filter_recommended_courses(
|
||||
unfiltered_course_keys,
|
||||
recommendation_count=10,
|
||||
user_country_code=None,
|
||||
request_course=None,
|
||||
request_course_key=None,
|
||||
):
|
||||
"""
|
||||
Returns the filtered course recommendations. The unfiltered course keys
|
||||
@@ -192,7 +163,7 @@ def filter_recommended_courses(
|
||||
unfiltered_course_keys: recommended course keys that needs to be filtered
|
||||
recommendation_count: the maximum count of recommendations to be returned
|
||||
user_country_code: if provided, will apply location restrictions to recommendations
|
||||
request_course: if provided, will filter out that course from recommendations (used for course about page)
|
||||
request_course_key: if provided, will filter out that course from recommendations (used for course about page)
|
||||
|
||||
Returns:
|
||||
filtered_recommended_courses (list): A list of filtered course objects.
|
||||
@@ -211,17 +182,13 @@ def filter_recommended_courses(
|
||||
"programs",
|
||||
]
|
||||
|
||||
# Remove the course keys a user is already enrolled in
|
||||
enrollable_course_keys = _remove_user_enrolled_course_keys(user, unfiltered_course_keys)
|
||||
|
||||
# Filter out enrolled courses .
|
||||
course_keys_to_filter_out = _get_user_enrolled_course_keys(user)
|
||||
# If user is seeing the recommendations on a course about page, filter that course out of recommendations
|
||||
recommended_course_keys = [
|
||||
course_key
|
||||
for course_key in enrollable_course_keys
|
||||
if course_key != request_course
|
||||
]
|
||||
if request_course_key:
|
||||
course_keys_to_filter_out.append(request_course_key)
|
||||
|
||||
for course_id in recommended_course_keys:
|
||||
for course_id in unfiltered_course_keys:
|
||||
if len(filtered_recommended_courses) >= recommendation_count:
|
||||
break
|
||||
|
||||
@@ -229,62 +196,9 @@ def filter_recommended_courses(
|
||||
if (
|
||||
course_data
|
||||
and course_data.get("course_runs", [])
|
||||
and not _is_enrolled_in_course(course_data.get("course_runs", []), course_keys_to_filter_out)
|
||||
and not _has_country_restrictions(course_data, user_country_code)
|
||||
):
|
||||
filtered_recommended_courses.append(course_data)
|
||||
|
||||
return filtered_recommended_courses
|
||||
|
||||
|
||||
def get_programs_based_on_course(request_course, country_code, user):
|
||||
"""
|
||||
Returns a program for the course. If a course is part of multiple programs,
|
||||
this function returns the program with the highest price.
|
||||
"""
|
||||
max_price, max_price_program = 0, {}
|
||||
programs = get_programs(course=request_course)
|
||||
|
||||
if not programs:
|
||||
return None
|
||||
|
||||
for program in programs:
|
||||
if program.get('status') != 'active' or _has_country_restrictions(program, country_code):
|
||||
continue
|
||||
|
||||
price = program['price_ranges'][0]['total']
|
||||
if price > max_price:
|
||||
if fetch_program_enrollments_by_student(program_uuids=[program.get('uuid')], user=user).exists():
|
||||
continue
|
||||
|
||||
course_keys = [
|
||||
course['key']
|
||||
for course in program.get('courses', [])
|
||||
if course.get('key') and course.get('key') != request_course
|
||||
]
|
||||
if _remove_user_enrolled_course_keys(user, course_keys):
|
||||
max_price_program = program
|
||||
max_price = price
|
||||
|
||||
if not max_price_program:
|
||||
return None
|
||||
|
||||
course_pacing_type, total_weeks_to_complete = '', 0
|
||||
for course in max_price_program.get('courses'):
|
||||
for course_run in course.get('course_runs'):
|
||||
if course_run.get('status') == 'published':
|
||||
if not course_pacing_type:
|
||||
course_pacing_type = course_run.get("pacing_type")
|
||||
total_weeks_to_complete += int(course_run.get("weeks_to_complete"))
|
||||
|
||||
program_upsell = {
|
||||
"title": max_price_program.get('title'),
|
||||
"marketing_url": max_price_program.get('marketing_url'),
|
||||
"courses_count": len(max_price_program.get('courses')),
|
||||
"pacing_type": course_pacing_type,
|
||||
"weeks_to_complete": _get_program_duration(total_weeks_to_complete),
|
||||
"min_hours": max_price_program.get('min_hours_effort_per_week'),
|
||||
"max_hours": max_price_program.get('max_hours_effort_per_week'),
|
||||
"type": max_price_program.get('type'),
|
||||
}
|
||||
|
||||
return program_upsell
|
||||
|
||||
@@ -10,7 +10,6 @@ from edx_rest_framework_extensions.auth.session.authentication import (
|
||||
SessionAuthenticationAllowInactiveUser,
|
||||
)
|
||||
from django.core.exceptions import PermissionDenied
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from rest_framework.permissions import IsAuthenticated
|
||||
from rest_framework.response import Response
|
||||
from rest_framework.views import APIView
|
||||
@@ -65,8 +64,7 @@ class AmplitudeRecommendationsView(APIView):
|
||||
def get(self, request, course_id):
|
||||
"""
|
||||
Returns
|
||||
- Amplitude course recommendations
|
||||
- Upsell program if the requesting course has a related program
|
||||
- Amplitude course recommendations for course about page
|
||||
"""
|
||||
if not enable_course_about_page_recommendations():
|
||||
return Response(status=404)
|
||||
@@ -75,8 +73,6 @@ class AmplitudeRecommendationsView(APIView):
|
||||
raise PermissionDenied()
|
||||
|
||||
user = request.user
|
||||
course_locator = CourseKey.from_string(course_id)
|
||||
course_key = f'{course_locator.org}+{course_locator.course}'
|
||||
|
||||
try:
|
||||
is_control, has_is_control, course_keys = get_amplitude_course_recommendations(
|
||||
@@ -95,7 +91,7 @@ class AmplitudeRecommendationsView(APIView):
|
||||
user,
|
||||
course_keys,
|
||||
user_country_code=user_country_code,
|
||||
request_course=course_key,
|
||||
request_course_key=course_id,
|
||||
recommendation_count=self.recommendations_count
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user