From 395c64b4055505f585c08329d9937aca430b360c Mon Sep 17 00:00:00 2001 From: Kyrylo Kireiev <90455454+KyryloKireiev@users.noreply.github.com> Date: Tue, 19 Mar 2024 18:09:04 +0200 Subject: [PATCH] feat: [AXM-24] Update structure for course enrollments API (#2515) * feat: [AXM-24] Update structure for course enrollments API * style: [AXM-24] Improve code style * fix: [AXM-24] Fix student's latest enrollment filter --- lms/djangoapps/mobile_api/users/tests.py | 209 ++++++++++++++++++++++- lms/djangoapps/mobile_api/users/views.py | 90 ++++++++-- lms/djangoapps/mobile_api/utils.py | 1 + lms/urls.py | 2 +- 4 files changed, 286 insertions(+), 16 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 65b1fba65ce3..08dc255426c2 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -18,6 +18,7 @@ from django.utils.timezone import now from milestones.tests.utils import MilestonesTestCaseMixin from opaque_keys.edx.keys import CourseKey +from rest_framework import status from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.models import CourseEnrollment @@ -27,6 +28,7 @@ from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory from lms.djangoapps.courseware.access_response import MilestoneAccessError, StartDateError, VisibilityError +from lms.djangoapps.courseware.models import StudentModule from lms.djangoapps.mobile_api.models import MobileConfig from lms.djangoapps.mobile_api.testutils import ( MobileAPITestCase, @@ -34,7 +36,7 @@ MobileAuthUserTestMixin, MobileCourseAccessTestMixin ) -from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2, API_V3 +from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2, API_V3, API_V4 from openedx.core.lib.courses import course_image_url from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration from openedx.features.course_duration_limits.models import CourseDurationLimitConfig @@ -406,6 +408,211 @@ def test_pagination_enrollment(self): assert "next" in response.data["enrollments"] assert "previous" in response.data["enrollments"] + def test_student_dont_have_enrollments(self): + """ + Testing modified `UserCourseEnrollmentsList` view with api_version == v4. + """ + self.login() + expected_result = { + 'configs': { + 'iap_configs': {} + }, + 'enrollments': { + 'next': None, + 'previous': None, + 'count': 0, + 'num_pages': 1, + 'current_page': 1, + 'start': 0, + 'results': [] + } + } + + response = self.api_response(api_version=API_V4) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertDictEqual(expected_result, response.data) + self.assertNotIn('primary', response.data) + + def test_student_have_one_enrollment(self): + """ + Testing modified `UserCourseEnrollmentsList` view with api_version == v4. + """ + self.login() + course = CourseFactory.create(org="edx", mobile_available=True) + self.enroll(course.id) + expected_enrollments = { + 'next': None, + 'previous': None, + 'count': 0, + 'num_pages': 1, + 'current_page': 1, + 'start': 0, + 'results': [] + } + + response = self.api_response(api_version=API_V4) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertDictEqual(expected_enrollments, response.data['enrollments']) + self.assertIn('primary', response.data) + self.assertEqual(str(course.id), response.data['primary']['course']['id']) + + def test_student_have_two_enrollments(self): + """ + Testing modified `UserCourseEnrollmentsList` view with api_version == v4. + """ + self.login() + course_first = CourseFactory.create(org="edx", mobile_available=True) + course_second = CourseFactory.create(org="edx", mobile_available=True) + self.enroll(course_first.id) + self.enroll(course_second.id) + + response = self.api_response(api_version=API_V4) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(len(response.data['enrollments']['results']), 1) + self.assertEqual(response.data['enrollments']['count'], 1) + self.assertEqual(response.data['enrollments']['results'][0]['course']['id'], str(course_first.id)) + self.assertIn('primary', response.data) + self.assertEqual(response.data['primary']['course']['id'], str(course_second.id)) + + def test_student_have_more_then_ten_enrollments(self): + """ + Testing modified `UserCourseEnrollmentsList` view with api_version == v4. + """ + self.login() + courses = [CourseFactory.create(org="edx", mobile_available=True) for _ in range(15)] + for course in courses: + self.enroll(course.id) + latest_enrolment = CourseFactory.create(org="edx", mobile_available=True) + self.enroll(latest_enrolment.id) + + response = self.api_response(api_version=API_V4) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['enrollments']['count'], 15) + self.assertEqual(response.data['enrollments']['num_pages'], 2) + self.assertEqual(len(response.data['enrollments']['results']), 10) + self.assertIn('primary', response.data) + self.assertEqual(response.data['primary']['course']['id'], str(latest_enrolment.id)) + + def test_student_have_progress_in_old_course_and_enroll_newest_course(self): + """ + Testing modified `UserCourseEnrollmentsList` view with api_version == v4. + """ + self.login() + old_course = CourseFactory.create(org="edx", mobile_available=True) + self.enroll(old_course.id) + courses = [CourseFactory.create(org="edx", mobile_available=True) for _ in range(5)] + for course in courses: + self.enroll(course.id) + new_course = CourseFactory.create(org="edx", mobile_available=True) + self.enroll(new_course.id) + + response = self.api_response(api_version=API_V4) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['enrollments']['count'], 6) + self.assertEqual(len(response.data['enrollments']['results']), 6) + # check that we have the new_course in primary section + self.assertIn('primary', response.data) + self.assertEqual(response.data['primary']['course']['id'], str(new_course.id)) + + # doing progress in the old_course + StudentModule.objects.create( + student=self.user, + course_id=old_course.id, + module_state_key=old_course.location, + ) + response = self.api_response(api_version=API_V4) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['enrollments']['count'], 6) + self.assertEqual(len(response.data['enrollments']['results']), 6) + # check that now we have the old_course in primary section + self.assertIn('primary', response.data) + self.assertEqual(response.data['primary']['course']['id'], str(old_course.id)) + + # enroll to the newest course + newest_course = CourseFactory.create(org="edx", mobile_available=True) + self.enroll(newest_course.id) + + response = self.api_response(api_version=API_V4) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['enrollments']['count'], 7) + self.assertEqual(len(response.data['enrollments']['results']), 7) + # check that now we have the newest_course in primary section + self.assertIn('primary', response.data) + self.assertEqual(response.data['primary']['course']['id'], str(newest_course.id)) + + def test_student_enrolled_only_not_mobile_available_courses(self): + """ + Testing modified `UserCourseEnrollmentsList` view with api_version == v4. + """ + self.login() + courses = [CourseFactory.create(org="edx", mobile_available=False) for _ in range(3)] + for course in courses: + self.enroll(course.id) + expected_result = { + "configs": { + "iap_configs": {} + }, + "enrollments": { + "next": None, + "previous": None, + "count": 0, + "num_pages": 1, + "current_page": 1, + "start": 0, + "results": [] + } + } + + response = self.api_response(api_version=API_V4) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertDictEqual(expected_result, response.data) + self.assertNotIn('primary', response.data) + + def test_do_progress_in_not_mobile_available_course(self): + """ + Testing modified `UserCourseEnrollmentsList` view with api_version == v4. + """ + self.login() + not_mobile_available = CourseFactory.create(org="edx", mobile_available=False) + self.enroll(not_mobile_available.id) + courses = [CourseFactory.create(org="edx", mobile_available=True) for _ in range(5)] + for course in courses: + self.enroll(course.id) + new_course = CourseFactory.create(org="edx", mobile_available=True) + self.enroll(new_course.id) + + response = self.api_response(api_version=API_V4) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['enrollments']['count'], 5) + self.assertEqual(len(response.data['enrollments']['results']), 5) + # check that we have the new_course in primary section + self.assertIn('primary', response.data) + self.assertEqual(response.data['primary']['course']['id'], str(new_course.id)) + + # doing progress in the not_mobile_available course + StudentModule.objects.create( + student=self.user, + course_id=not_mobile_available.id, + module_state_key=not_mobile_available.location, + ) + response = self.api_response(api_version=API_V4) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['enrollments']['count'], 5) + self.assertEqual(len(response.data['enrollments']['results']), 5) + # check that we have the new_course in primary section in the same way + self.assertIn('primary', response.data) + self.assertEqual(response.data['primary']['course']['id'], str(new_course.id)) + @override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) class TestUserEnrollmentCertificates(UrlResetMixin, MobileAPITestCase, MilestonesTestCaseMixin): diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 049678dcd7ba..d6d77dc1edf6 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -4,6 +4,7 @@ import logging +from typing import List, Optional from completion.exceptions import UnavailableCompletionData from completion.utilities import get_key_to_last_completed_block @@ -29,9 +30,10 @@ from lms.djangoapps.courseware.courses import get_current_child from lms.djangoapps.courseware.model_data import FieldDataCache from lms.djangoapps.courseware.block_render import get_block_for_descriptor +from lms.djangoapps.courseware.models import StudentModule from lms.djangoapps.courseware.views.index import save_positions_recursively_up from lms.djangoapps.mobile_api.models import MobileConfig -from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2, API_V3 +from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2, API_V3, API_V4 from openedx.features.course_duration_limits.access import check_course_expired from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order @@ -263,6 +265,10 @@ class UserCourseEnrollmentsList(generics.ListAPIView): An additional attribute "expiration" has been added to the response, which lists the date when access to the course will expire or null if it doesn't expire. + In v4 we added to the response primary object. Primary object contains the latest user's enrollment + or course where user has the latest progress. Primary object has been cut from user's + enrolments array and inserted into separated section with key `primary`. + **Example Request** GET /api/mobile/v1/users/{username}/course_enrollments/ @@ -343,6 +349,29 @@ def get_serializer_class(self): def get_queryset(self): api_version = self.kwargs.get('api_version') + mobile_available = self.get_mobile_available_enrollments() + + not_duration_limited = ( + enrollment for enrollment in mobile_available + if check_course_expired(self.request.user, enrollment.course) == ACCESS_GRANTED + ) + + if api_version == API_V4: + primary_enrollment_obj = self.get_primary_enrollment_by_latest_enrollment_or_progress() + if primary_enrollment_obj: + mobile_available.remove(primary_enrollment_obj) + + if api_version == API_V05: + # for v0.5 don't return expired courses + return list(not_duration_limited) + else: + # return all courses, with associated expiration + return mobile_available + + def get_mobile_available_enrollments(self) -> List[Optional[CourseEnrollment]]: + """ + Gets list with `CourseEnrollment` for mobile available courses. + """ enrollments = self.queryset.filter( user__username=self.kwargs['username'], is_active=True @@ -357,31 +386,64 @@ def get_queryset(self): enrollment for enrollment in same_org if is_mobile_available_for_user(self.request.user, enrollment.course_overview) ) - not_duration_limited = ( - enrollment for enrollment in mobile_available - if check_course_expired(self.request.user, enrollment.course) == ACCESS_GRANTED - ) - - if api_version == API_V05: - # for v0.5 don't return expired courses - return list(not_duration_limited) - else: - # return all courses, with associated expiration - return list(mobile_available) + return list(mobile_available) def list(self, request, *args, **kwargs): response = super().list(request, *args, **kwargs) api_version = self.kwargs.get('api_version') - if api_version in (API_V2, API_V3): + if api_version in (API_V2, API_V3, API_V4): enrollment_data = { 'configs': MobileConfig.get_structured_configs(), 'enrollments': response.data } + if api_version == API_V4: + primary_enrollment_obj = self.get_primary_enrollment_by_latest_enrollment_or_progress() + if primary_enrollment_obj: + serializer = self.get_serializer(primary_enrollment_obj) + enrollment_data.update({'primary': serializer.data}) + return Response(enrollment_data) return response + def get_primary_enrollment_by_latest_enrollment_or_progress(self) -> Optional[CourseEnrollment]: + """ + Gets primary enrollment obj by latest enrollment or latest progress on the course. + """ + mobile_available = self.get_mobile_available_enrollments() + if not mobile_available: + return None + + mobile_available_course_ids = [enrollment.course_id for enrollment in mobile_available] + + latest_enrollment = self.queryset.filter( + user__username=self.kwargs['username'], + is_active=True, + course__id__in=mobile_available_course_ids, + ).order_by('-created').first() + + if not latest_enrollment: + return None + + latest_progress = StudentModule.objects.filter( + student__username=self.kwargs['username'], + course_id__in=mobile_available_course_ids, + ).order_by('-modified').first() + + if not latest_progress: + return latest_enrollment + + enrollment_with_latest_progress = self.queryset.filter( + course_id=latest_progress.course_id, + user__username=self.kwargs['username'], + ).first() + + if latest_enrollment.created > latest_progress.modified: + return latest_enrollment + else: + return enrollment_with_latest_progress + # pylint: disable=attribute-defined-outside-init @property def paginator(self): @@ -394,7 +456,7 @@ def paginator(self): super().paginator # pylint: disable=expression-not-assigned api_version = self.kwargs.get('api_version') - if self._paginator is None and api_version == API_V3: + if self._paginator is None and api_version in (API_V3, API_V4): self._paginator = DefaultPagination() return self._paginator diff --git a/lms/djangoapps/mobile_api/utils.py b/lms/djangoapps/mobile_api/utils.py index 73a0cfea0827..9204b27ab49b 100644 --- a/lms/djangoapps/mobile_api/utils.py +++ b/lms/djangoapps/mobile_api/utils.py @@ -6,6 +6,7 @@ API_V1 = 'v1' API_V2 = 'v2' API_V3 = 'v3' +API_V4 = 'v4' def parsed_version(version): diff --git a/lms/urls.py b/lms/urls.py index c7a0d49da4d7..666f896ed05d 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -218,7 +218,7 @@ if settings.FEATURES.get('ENABLE_MOBILE_REST_API'): urlpatterns += [ - re_path(r'^api/mobile/(?Pv(3|2|1|0.5))/', include('lms.djangoapps.mobile_api.urls')), + re_path(r'^api/mobile/(?Pv(4|3|2|1|0.5))/', include('lms.djangoapps.mobile_api.urls')), ] urlpatterns += [