From 348fdc375e3d2dc8fd977740618a6e67182223d1 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 01/18] 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 48b0aecaeb88..99288e25b519 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -221,7 +221,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 += [ From 96f951bcc6d44be0bf76f6a034492696b1a266df Mon Sep 17 00:00:00 2001 From: Kyrylo Kireiev <90455454+KyryloKireiev@users.noreply.github.com> Date: Fri, 22 Mar 2024 14:01:30 +0200 Subject: [PATCH 02/18] feat: [AXM-47] Add course_status field to primary object (#2517) --- .../mobile_api/users/serializers.py | 87 +++++++++++++++++ lms/djangoapps/mobile_api/users/tests.py | 94 +++++++++++++++++-- lms/djangoapps/mobile_api/users/views.py | 24 ++++- 3 files changed, 196 insertions(+), 9 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index d7005e5f68e7..944f3c36defd 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -2,7 +2,10 @@ Serializer for user API """ +from typing import Dict, List, Optional, Tuple +from completion.exceptions import UnavailableCompletionData +from completion.utilities import get_key_to_last_completed_block from rest_framework import serializers from rest_framework.reverse import reverse @@ -11,7 +14,11 @@ from common.djangoapps.util.course import get_encoded_course_sharing_utm_params, get_link_for_about_page from lms.djangoapps.certificates.api import certificate_downloadable_status from lms.djangoapps.courseware.access import has_access +from lms.djangoapps.courseware.block_render import get_block_for_descriptor +from lms.djangoapps.courseware.courses import get_current_child +from lms.djangoapps.courseware.model_data import FieldDataCache from openedx.features.course_duration_limits.access import get_user_course_expiration_date +from xmodule.modulestore.django import modulestore class CourseOverviewField(serializers.RelatedField): # lint-amnesty, pylint: disable=abstract-method @@ -141,6 +148,86 @@ class Meta: lookup_field = 'username' +class CourseEnrollmentSerializerModifiedForPrimary(CourseEnrollmentSerializer): + """ + Serializes CourseEnrollment models for API v4. + + Adds `course_status` field into serializer data. + """ + course_status = serializers.SerializerMethodField() + + def get_course_status(self, model: CourseEnrollment) -> Optional[Dict[str, List[str]]]: + """ + Gets course status for the given user's enrollments. + """ + try: + block_id = str(get_key_to_last_completed_block(model.user, model.course.id)) + except UnavailableCompletionData: + block_id = "" + + if not block_id: + return None + + request = self.context.get('request') + path, unit_name = self._get_last_visited_block_path_and_unit_name(request, model) + path_ids = [str(block.location) for block in path] + + return { + 'last_visited_module_id': path_ids[0], + 'last_visited_module_path': path_ids, + 'last_visited_block_id': block_id, + 'last_visited_unit_display_name': unit_name, + } + + @staticmethod + def _get_last_visited_block_path_and_unit_name( + request: 'Request', # noqa: F821 + model: CourseEnrollment, + ) -> Tuple[List[Optional['XBlock']], Optional[str]]: # noqa: F821 + """ + Returns the path to the latest block and unit name visited by the current user. + + If there is no such visit, the first item deep enough down the course + tree is used. + """ + course = modulestore().get_course(model.course.id) + field_data_cache = FieldDataCache.cache_for_block_descendents( + course.id, model.user, course, depth=3) + + course_block = get_block_for_descriptor( + model.user, request, course, field_data_cache, course.id, course=course + ) + + unit_name = '' + path = [course_block] if course_block else [] + chapter = get_current_child(course_block, min_depth=3) + if chapter is not None: + path.append(chapter) + section = get_current_child(chapter, min_depth=2) + if section is not None: + path.append(section) + unit = get_current_child(section, min_depth=1) + if unit is not None: + unit_name = unit.display_name + + path.reverse() + return path, unit_name + + class Meta: + model = CourseEnrollment + fields = ( + 'audit_access_expires', + 'created', + 'mode', + 'is_active', + 'course', + 'certificate', + 'course_modes', + 'course_status', + ) + lookup_field = 'username' + + class UserSerializer(serializers.ModelSerializer): """ Serializes User models diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 08dc255426c2..8000576937bc 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -4,7 +4,7 @@ import datetime -from unittest.mock import patch +from unittest.mock import MagicMock, patch from urllib.parse import parse_qs import ddt @@ -492,8 +492,8 @@ def test_student_have_more_then_ten_enrollments(self): 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.assertEqual(response.data['enrollments']['num_pages'], 3) + self.assertEqual(len(response.data['enrollments']['results']), 5) self.assertIn('primary', response.data) self.assertEqual(response.data['primary']['course']['id'], str(latest_enrolment.id)) @@ -514,7 +514,7 @@ def test_student_have_progress_in_old_course_and_enroll_newest_course(self): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data['enrollments']['count'], 6) - self.assertEqual(len(response.data['enrollments']['results']), 6) + 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)) @@ -529,7 +529,7 @@ def test_student_have_progress_in_old_course_and_enroll_newest_course(self): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data['enrollments']['count'], 6) - self.assertEqual(len(response.data['enrollments']['results']), 6) + self.assertEqual(len(response.data['enrollments']['results']), 5) # 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)) @@ -542,7 +542,7 @@ def test_student_have_progress_in_old_course_and_enroll_newest_course(self): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data['enrollments']['count'], 7) - self.assertEqual(len(response.data['enrollments']['results']), 7) + self.assertEqual(len(response.data['enrollments']['results']), 5) # 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)) @@ -613,6 +613,88 @@ def test_do_progress_in_not_mobile_available_course(self): self.assertIn('primary', response.data) self.assertEqual(response.data['primary']['course']['id'], str(new_course.id)) + def test_pagination_for_user_enrollments_api_v4(self): + """ + Tests `UserCourseEnrollmentsV4Pagination`, api_version == v4. + """ + self.login() + courses = [CourseFactory.create(org="my_org", mobile_available=True) for _ in range(15)] + for course in courses: + self.enroll(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'], 14) + self.assertEqual(response.data['enrollments']['num_pages'], 3) + self.assertEqual(response.data['enrollments']['current_page'], 1) + self.assertEqual(len(response.data['enrollments']['results']), 5) + self.assertIn('next', response.data['enrollments']) + self.assertIn('previous', response.data['enrollments']) + self.assertIn('primary', response.data) + + def test_course_status_in_primary_obj_when_student_doesnt_have_progress(self): + """ + Testing modified `UserCourseEnrollmentsList` view with api_version == v4. + """ + self.login() + course = CourseFactory.create(org="edx", mobile_available=True) + self.enroll(course.id) + + response = self.api_response(api_version=API_V4) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['primary']['course_status'], None) + + @patch('lms.djangoapps.mobile_api.users.serializers.get_key_to_last_completed_block') + def test_course_status_in_primary_obj_when_student_have_progress( + self, + get_last_completed_block_mock: MagicMock, + ): + """ + Testing modified `UserCourseEnrollmentsList` view with api_version == v4. + """ + self.login() + # create test course structure + course = CourseFactory.create(org="edx", mobile_available=True) + section = BlockFactory.create( + parent=course, + category="chapter", + display_name="section", + ) + subsection = BlockFactory.create( + parent=section, + category="sequential", + display_name="subsection", + ) + vertical = BlockFactory.create( + parent=subsection, + category="vertical", + display_name="test unit", + ) + problem = BlockFactory.create( + parent=vertical, + category="problem", + display_name="problem", + ) + self.enroll(course.id) + get_last_completed_block_mock.return_value = problem.location + expected_course_status = { + 'last_visited_module_id': str(subsection.location), + 'last_visited_module_path': [ + str(subsection.location), + str(section.location), + str(course.location) + ], + 'last_visited_block_id': str(problem.location), + 'last_visited_unit_display_name': vertical.display_name, + } + + response = self.api_response(api_version=API_V4) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['primary']['course_status'], expected_course_status) + get_last_completed_block_mock.assert_called_once_with(self.user, 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 d6d77dc1edf6..acf8b7179590 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -40,7 +40,12 @@ from .. import errors from ..decorators import mobile_course_access, mobile_view -from .serializers import CourseEnrollmentSerializer, CourseEnrollmentSerializerv05, UserSerializer +from .serializers import ( + CourseEnrollmentSerializer, + CourseEnrollmentSerializerModifiedForPrimary, + CourseEnrollmentSerializerv05, + UserSerializer, +) log = logging.getLogger(__name__) @@ -400,7 +405,10 @@ def list(self, request, *args, **kwargs): 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) + serializer = CourseEnrollmentSerializerModifiedForPrimary( + primary_enrollment_obj, + context=self.get_serializer_context(), + ) enrollment_data.update({'primary': serializer.data}) return Response(enrollment_data) @@ -456,8 +464,10 @@ 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 in (API_V3, API_V4): + if self._paginator is None and api_version == API_V3: self._paginator = DefaultPagination() + if self._paginator is None and api_version == API_V4: + self._paginator = UserCourseEnrollmentsV4Pagination() return self._paginator @@ -472,3 +482,11 @@ def my_user_info(request, api_version): # updating it from the oauth2 related code is too complex user_logged_in.send(sender=User, user=request.user, request=request) return redirect("user-detail", api_version=api_version, username=request.user.username) + + +class UserCourseEnrollmentsV4Pagination(DefaultPagination): + """ + Pagination for `UserCourseEnrollments` API v4. + """ + page_size = 5 + max_page_size = 50 From 8644c2193087f6e0b3f2e3c2291fc0d9ffc78f24 Mon Sep 17 00:00:00 2001 From: NiedielnitsevIvan <81557788+NiedielnitsevIvan@users.noreply.github.com> Date: Fri, 22 Mar 2024 19:00:32 +0200 Subject: [PATCH 03/18] feat: [AXM-40] add courses progress to enrollment endpoint (#2519) * fix: workaround for staticcollection introduced in e40a01c * feat: [AXM-40] add courses progress to enrollment endpoint * refactor: [AXM-40] add caching to improve performance * refactor: [AXM-40] add progress only for primary course * refactor: [AXM-40] refactor enrollment caching optimization --------- Co-authored-by: Glib Glugovskiy --- .../mobile_api/users/serializers.py | 31 +++++++++++++++++++ lms/djangoapps/mobile_api/users/views.py | 16 ++++++---- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 944f3c36defd..64dffed1f045 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -4,6 +4,7 @@ from typing import Dict, List, Optional, Tuple +from django.core.cache import cache from completion.exceptions import UnavailableCompletionData from completion.utilities import get_key_to_last_completed_block from rest_framework import serializers @@ -17,6 +18,8 @@ from lms.djangoapps.courseware.block_render import get_block_for_descriptor from lms.djangoapps.courseware.courses import get_current_child from lms.djangoapps.courseware.model_data import FieldDataCache +from lms.djangoapps.grades.api import CourseGradeFactory +from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager from openedx.features.course_duration_limits.access import get_user_course_expiration_date from xmodule.modulestore.django import modulestore @@ -154,7 +157,11 @@ class CourseEnrollmentSerializerModifiedForPrimary(CourseEnrollmentSerializer): Adds `course_status` field into serializer data. """ + course_status = serializers.SerializerMethodField() + progress = serializers.SerializerMethodField() + + BLOCK_STRUCTURE_CACHE_TIMEOUT = 60 * 60 # 1 hour def get_course_status(self, model: CourseEnrollment) -> Optional[Dict[str, List[str]]]: """ @@ -213,6 +220,29 @@ def _get_last_visited_block_path_and_unit_name( path.reverse() return path, unit_name + def get_progress(self, model: CourseEnrollment) -> Dict[str, int]: + """ + Returns the progress of the user in the course. + """ + assert isinstance(model, CourseEnrollment), f'Expected CourseEnrollment, got {type(model)}' + is_staff = bool(has_access(model.user, 'staff', model.course.id)) + + cache_key = f'course_block_structure_{str(model.course.id)}_{model.user.id}' + collected_block_structure = cache.get(cache_key) + if not collected_block_structure: + collected_block_structure = get_block_structure_manager(model.course.id).get_collected() + cache.set(cache_key, collected_block_structure, self.BLOCK_STRUCTURE_CACHE_TIMEOUT) + + course_grade = CourseGradeFactory().read(model.user, collected_block_structure=collected_block_structure) + + # recalculate course grade from visible grades (stored grade was calculated over all grades, visible or not) + course_grade.update(visible_grades_only=True, has_staff_access=is_staff) + subsection_grades = list(course_grade.subsection_grades.values()) + return { + 'num_points_earned': sum(map(lambda x: x.graded_total.earned if x.graded else 0, subsection_grades)), + 'num_points_possible': sum(map(lambda x: x.graded_total.possible if x.graded else 0, subsection_grades)), + } + class Meta: model = CourseEnrollment fields = ( @@ -224,6 +254,7 @@ class Meta: 'certificate', 'course_modes', 'course_status', + 'progress', ) lookup_field = 'username' diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index acf8b7179590..2463ef963b9e 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -4,6 +4,7 @@ import logging +from functools import cached_property from typing import List, Optional from completion.exceptions import UnavailableCompletionData @@ -324,7 +325,7 @@ class UserCourseEnrollmentsList(generics.ListAPIView): certified). * url: URL to the downloadable version of the certificate, if exists. """ - queryset = CourseEnrollment.objects.all() + lookup_field = 'username' # In Django Rest Framework v3, there is a default pagination @@ -352,6 +353,13 @@ def get_serializer_class(self): return CourseEnrollmentSerializerv05 return CourseEnrollmentSerializer + @cached_property + def queryset(self): + return CourseEnrollment.objects.all().select_related('course', 'user').filter( + user__username=self.kwargs['username'], + is_active=True + ).order_by('created').reverse() + def get_queryset(self): api_version = self.kwargs.get('api_version') mobile_available = self.get_mobile_available_enrollments() @@ -377,14 +385,10 @@ 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 - ).order_by('created').reverse() org = self.request.query_params.get('org', None) same_org = ( - enrollment for enrollment in enrollments + enrollment for enrollment in self.queryset if enrollment.course_overview and self.is_org(org, enrollment.course_overview.org) ) mobile_available = ( From c4db96e00eaa125d9856071b758d6d60d8f1c5c4 Mon Sep 17 00:00:00 2001 From: NiedielnitsevIvan <81557788+NiedielnitsevIvan@users.noreply.github.com> Date: Tue, 2 Apr 2024 16:51:55 +0300 Subject: [PATCH 04/18] feat: [AXM-53] add assertions for primary course (#2522) * feat: [AXM-53] add assertions for primary course * test: [AXM-53] fix tests * style: [AXM-53] change future_assignment default value to None * refactor: [AXM-53] add some optimization for assignments collecting --- .../mobile_api/users/serializers.py | 45 ++++++++++++++++--- lms/djangoapps/mobile_api/users/tests.py | 25 ++++++++--- lms/djangoapps/mobile_api/users/views.py | 12 ++++- 3 files changed, 67 insertions(+), 15 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 64dffed1f045..0a17dbb5a82f 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -2,6 +2,7 @@ Serializer for user API """ +from datetime import datetime from typing import Dict, List, Optional, Tuple from django.core.cache import cache @@ -16,8 +17,10 @@ from lms.djangoapps.certificates.api import certificate_downloadable_status from lms.djangoapps.courseware.access import has_access from lms.djangoapps.courseware.block_render import get_block_for_descriptor -from lms.djangoapps.courseware.courses import get_current_child +from lms.djangoapps.courseware.context_processor import get_user_timezone_or_last_seen_timezone_or_utc +from lms.djangoapps.courseware.courses import get_course_assignment_date_blocks, get_current_child from lms.djangoapps.courseware.model_data import FieldDataCache +from lms.djangoapps.course_home_api.dates.serializers import DateSummarySerializer from lms.djangoapps.grades.api import CourseGradeFactory from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager from openedx.features.course_duration_limits.access import get_user_course_expiration_date @@ -160,9 +163,14 @@ class CourseEnrollmentSerializerModifiedForPrimary(CourseEnrollmentSerializer): course_status = serializers.SerializerMethodField() progress = serializers.SerializerMethodField() + course_assignments = serializers.SerializerMethodField() BLOCK_STRUCTURE_CACHE_TIMEOUT = 60 * 60 # 1 hour + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.course = modulestore().get_course(self.instance.course.id) + def get_course_status(self, model: CourseEnrollment) -> Optional[Dict[str, List[str]]]: """ Gets course status for the given user's enrollments. @@ -186,8 +194,8 @@ def get_course_status(self, model: CourseEnrollment) -> Optional[Dict[str, List[ 'last_visited_unit_display_name': unit_name, } - @staticmethod def _get_last_visited_block_path_and_unit_name( + self, request: 'Request', # noqa: F821 model: CourseEnrollment, ) -> Tuple[List[Optional['XBlock']], Optional[str]]: # noqa: F821 @@ -197,12 +205,10 @@ def _get_last_visited_block_path_and_unit_name( If there is no such visit, the first item deep enough down the course tree is used. """ - course = modulestore().get_course(model.course.id) - field_data_cache = FieldDataCache.cache_for_block_descendents( - course.id, model.user, course, depth=3) + field_data_cache = FieldDataCache.cache_for_block_descendents(self.course.id, model.user, self.course, depth=3) course_block = get_block_for_descriptor( - model.user, request, course, field_data_cache, course.id, course=course + model.user, request, self.course, field_data_cache, self.course.id, course=self.course ) unit_name = '' @@ -243,6 +249,32 @@ def get_progress(self, model: CourseEnrollment) -> Dict[str, int]: 'num_points_possible': sum(map(lambda x: x.graded_total.possible if x.graded else 0, subsection_grades)), } + def get_course_assignments(self, model: CourseEnrollment) -> Optional[Dict[str, List[Dict[str, str]]]]: + """ + Returns the future assignment data and past assignments data for the user in the course. + """ + assignments = get_course_assignment_date_blocks( + self.course, + model.user, + self.context.get('request'), + include_past_dates=True + ) + next_assignment = None + past_assignment = [] + + timezone = get_user_timezone_or_last_seen_timezone_or_utc(model.user) + for assignment in sorted(assignments, key=lambda x: x.date): + if assignment.date < datetime.now(timezone): + past_assignment.append(assignment) + else: + next_assignment = DateSummarySerializer(assignment).data + break + + return { + 'future_assignment': next_assignment, + 'past_assignments': DateSummarySerializer(past_assignment, many=True).data, + } + class Meta: model = CourseEnrollment fields = ( @@ -255,6 +287,7 @@ class Meta: 'course_modes', 'course_status', 'progress', + 'course_assignments', ) lookup_field = 'username' diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 8000576937bc..f1a9798c8108 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -417,6 +417,7 @@ def test_student_dont_have_enrollments(self): 'configs': { 'iap_configs': {} }, + 'user_timezone': 'UTC', 'enrollments': { 'next': None, 'previous': None, @@ -434,7 +435,8 @@ def test_student_dont_have_enrollments(self): self.assertDictEqual(expected_result, response.data) self.assertNotIn('primary', response.data) - def test_student_have_one_enrollment(self): + @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) + def test_student_have_one_enrollment(self, cache_mock: MagicMock): """ Testing modified `UserCourseEnrollmentsList` view with api_version == v4. """ @@ -458,7 +460,8 @@ def test_student_have_one_enrollment(self): self.assertIn('primary', response.data) self.assertEqual(str(course.id), response.data['primary']['course']['id']) - def test_student_have_two_enrollments(self): + @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) + def test_student_have_two_enrollments(self, cache_mock: MagicMock): """ Testing modified `UserCourseEnrollmentsList` view with api_version == v4. """ @@ -477,7 +480,8 @@ def test_student_have_two_enrollments(self): 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): + @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) + def test_student_have_more_then_ten_enrollments(self, cache_mock: MagicMock): """ Testing modified `UserCourseEnrollmentsList` view with api_version == v4. """ @@ -497,7 +501,8 @@ def test_student_have_more_then_ten_enrollments(self): 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): + @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) + def test_student_have_progress_in_old_course_and_enroll_newest_course(self, cache_mock: MagicMock): """ Testing modified `UserCourseEnrollmentsList` view with api_version == v4. """ @@ -559,6 +564,7 @@ def test_student_enrolled_only_not_mobile_available_courses(self): "configs": { "iap_configs": {} }, + "user_timezone": "UTC", "enrollments": { "next": None, "previous": None, @@ -576,7 +582,8 @@ def test_student_enrolled_only_not_mobile_available_courses(self): self.assertDictEqual(expected_result, response.data) self.assertNotIn('primary', response.data) - def test_do_progress_in_not_mobile_available_course(self): + @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) + def test_do_progress_in_not_mobile_available_course(self, cache_mock: MagicMock): """ Testing modified `UserCourseEnrollmentsList` view with api_version == v4. """ @@ -613,7 +620,8 @@ def test_do_progress_in_not_mobile_available_course(self): self.assertIn('primary', response.data) self.assertEqual(response.data['primary']['course']['id'], str(new_course.id)) - def test_pagination_for_user_enrollments_api_v4(self): + @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) + def test_pagination_for_user_enrollments_api_v4(self, cache_mock: MagicMock): """ Tests `UserCourseEnrollmentsV4Pagination`, api_version == v4. """ @@ -632,7 +640,8 @@ def test_pagination_for_user_enrollments_api_v4(self): self.assertIn('previous', response.data['enrollments']) self.assertIn('primary', response.data) - def test_course_status_in_primary_obj_when_student_doesnt_have_progress(self): + @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) + def test_course_status_in_primary_obj_when_student_doesnt_have_progress(self, cache_mock: MagicMock): """ Testing modified `UserCourseEnrollmentsList` view with api_version == v4. """ @@ -645,10 +654,12 @@ def test_course_status_in_primary_obj_when_student_doesnt_have_progress(self): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data['primary']['course_status'], None) + @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) @patch('lms.djangoapps.mobile_api.users.serializers.get_key_to_last_completed_block') def test_course_status_in_primary_obj_when_student_have_progress( self, get_last_completed_block_mock: MagicMock, + cache_mock: MagicMock ): """ Testing modified `UserCourseEnrollmentsList` view with api_version == v4. diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 2463ef963b9e..2c5e7736b288 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -12,7 +12,7 @@ from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.contrib.auth.signals import user_logged_in from django.db import transaction -from django.shortcuts import redirect +from django.shortcuts import get_object_or_404, redirect from django.utils import dateparse from django.utils.decorators import method_decorator from opaque_keys import InvalidKeyError @@ -28,6 +28,7 @@ from common.djangoapps.student.models import CourseEnrollment, User # lint-amnesty, pylint: disable=reimported from lms.djangoapps.courseware.access import is_mobile_available_for_user from lms.djangoapps.courseware.access_utils import ACCESS_GRANTED +from lms.djangoapps.courseware.context_processor import get_user_timezone_or_last_seen_timezone_or_utc 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 @@ -358,7 +359,7 @@ def queryset(self): return CourseEnrollment.objects.all().select_related('course', 'user').filter( user__username=self.kwargs['username'], is_active=True - ).order_by('created').reverse() + ).order_by('-created') def get_queryset(self): api_version = self.kwargs.get('api_version') @@ -404,6 +405,7 @@ def list(self, request, *args, **kwargs): if api_version in (API_V2, API_V3, API_V4): enrollment_data = { 'configs': MobileConfig.get_structured_configs(), + 'user_timezone': str(get_user_timezone_or_last_seen_timezone_or_utc(self.get_user())), 'enrollments': response.data } if api_version == API_V4: @@ -419,6 +421,12 @@ def list(self, request, *args, **kwargs): return response + def get_user(self) -> User: + """ + Get user object by username. + """ + return get_object_or_404(User, username=self.kwargs['username']) + 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. From b348cb5b2c8ccea80ee3499fdcb7c851e1f6b68a Mon Sep 17 00:00:00 2001 From: NiedielnitsevIvan <81557788+NiedielnitsevIvan@users.noreply.github.com> Date: Mon, 8 Apr 2024 15:21:33 +0300 Subject: [PATCH 05/18] feat: [AXM-33] create enrollments filtering by course completion statuses (#2532) * feat: [AXM-33] create enrollments filtering by course completion statuses * test: [AXM-33] add tests for filtrations * style: [AXM-33] fix pylint issues --- .../student/models/course_enrollment.py | 60 ++++++ lms/djangoapps/mobile_api/users/enums.py | 22 ++ .../mobile_api/users/serializers.py | 2 +- lms/djangoapps/mobile_api/users/tests.py | 196 ++++++++++++++++++ lms/djangoapps/mobile_api/users/views.py | 56 +++-- .../features/course_duration_limits/access.py | 4 +- 6 files changed, 325 insertions(+), 15 deletions(-) create mode 100644 lms/djangoapps/mobile_api/users/enums.py diff --git a/common/djangoapps/student/models/course_enrollment.py b/common/djangoapps/student/models/course_enrollment.py index 729e80eb6594..9fd53c1cd796 100644 --- a/common/djangoapps/student/models/course_enrollment.py +++ b/common/djangoapps/student/models/course_enrollment.py @@ -129,11 +129,71 @@ class UnenrollmentNotAllowed(CourseEnrollmentException): pass +class CourseEnrollmentQuerySet(models.QuerySet): + """ + Custom queryset for CourseEnrollment with Table-level filter methods. + """ + + def active(self): + """ + Returns a queryset of CourseEnrollment objects for courses that are currently active. + """ + return self.filter(is_active=True) + + def without_certificates(self, user_username): + """ + Returns a queryset of CourseEnrollment objects for courses that do not have a certificate. + """ + from lms.djangoapps.certificates.models import GeneratedCertificate # pylint: disable=import-outside-toplevel + course_ids_with_certificates = GeneratedCertificate.objects.filter( + user__username=user_username + ).values_list('course_id', flat=True) + return self.exclude(course_id__in=course_ids_with_certificates) + + def with_certificates(self, user_username): + """ + Returns a queryset of CourseEnrollment objects for courses that have a certificate. + """ + from lms.djangoapps.certificates.models import GeneratedCertificate # pylint: disable=import-outside-toplevel + course_ids_with_certificates = GeneratedCertificate.objects.filter( + user__username=user_username + ).values_list('course_id', flat=True) + return self.filter(course_id__in=course_ids_with_certificates) + + def in_progress(self, user_username, time_zone=UTC): + """ + Returns a queryset of CourseEnrollment objects for courses that are currently in progress. + """ + now = datetime.now(time_zone) + return self.active().without_certificates(user_username).filter( + Q(course__start__lte=now, course__end__gte=now) + | Q(course__start__isnull=True, course__end__isnull=True) + | Q(course__start__isnull=True, course__end__gte=now) + | Q(course__start__lte=now, course__end__isnull=True), + ) + + def completed(self, user_username): + """ + Returns a queryset of CourseEnrollment objects for courses that have been completed. + """ + return self.active().with_certificates(user_username) + + def expired(self, user_username, time_zone=UTC): + """ + Returns a queryset of CourseEnrollment objects for courses that have expired. + """ + now = datetime.now(time_zone) + return self.active().without_certificates(user_username).filter(course__end__lt=now) + + class CourseEnrollmentManager(models.Manager): """ Custom manager for CourseEnrollment with Table-level filter methods. """ + def get_queryset(self): + return CourseEnrollmentQuerySet(self.model, using=self._db) + def is_small_course(self, course_id): """ Returns false if the number of enrollments are one greater than 'max_enrollments' else true diff --git a/lms/djangoapps/mobile_api/users/enums.py b/lms/djangoapps/mobile_api/users/enums.py new file mode 100644 index 000000000000..2a072b082fff --- /dev/null +++ b/lms/djangoapps/mobile_api/users/enums.py @@ -0,0 +1,22 @@ +""" +Enums for mobile_api users app. +""" +from enum import Enum + + +class EnrollmentStatuses(Enum): + """ + Enum for enrollment statuses. + """ + + ALL = 'all' + IN_PROGRESS = 'in_progress' + COMPLETED = 'completed' + EXPIRED = 'expired' + + @classmethod + def values(cls): + """ + Returns string representation of all enum values. + """ + return [e.value for e in cls] diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 0a17dbb5a82f..82eb3edee278 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -110,7 +110,7 @@ def get_audit_access_expires(self, model): """ Returns expiration date for a course audit expiration, if any or null """ - return get_user_course_expiration_date(model.user, model.course) + return get_user_course_expiration_date(model.user, model.course, model) def get_certificate(self, model): """Returns the information about the user's certificate in the course.""" diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index f1a9798c8108..5b842851db0b 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -36,6 +36,7 @@ MobileAuthUserTestMixin, MobileCourseAccessTestMixin ) +from lms.djangoapps.mobile_api.users.enums import EnrollmentStatuses 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 @@ -706,6 +707,201 @@ def test_course_status_in_primary_obj_when_student_have_progress( self.assertEqual(response.data['primary']['course_status'], expected_course_status) get_last_completed_block_mock.assert_called_once_with(self.user, course.id) + @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) + def test_user_enrollment_api_v4_in_progress_status(self, cache_mock: MagicMock): + """ + Testing + """ + self.login() + old_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.THREE_YEARS_AGO, + end=self.LAST_WEEK + ) + actual_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.LAST_WEEK, + end=self.NEXT_WEEK + ) + infinite_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.LAST_WEEK, + end=None + ) + + self.enroll(old_course.id) + self.enroll(actual_course.id) + self.enroll(infinite_course.id) + + response = self.api_response(api_version=API_V4, data={'status': EnrollmentStatuses.IN_PROGRESS.value}) + enrollments = response.data['enrollments'] + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(enrollments['count'], 2) + self.assertEqual(enrollments['results'][1]['course']['id'], str(actual_course.id)) + self.assertEqual(enrollments['results'][0]['course']['id'], str(infinite_course.id)) + self.assertNotIn('primary', response.data) + + def test_user_enrollment_api_v4_completed_status(self): + """ + Testing + """ + self.login() + old_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.THREE_YEARS_AGO, + end=self.LAST_WEEK + ) + actual_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.LAST_WEEK, + end=self.NEXT_WEEK + ) + infinite_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.LAST_WEEK, + end=None + ) + GeneratedCertificateFactory.create( + user=self.user, + course_id=infinite_course.id, + status=CertificateStatuses.downloadable, + mode='verified', + ) + + self.enroll(old_course.id) + self.enroll(actual_course.id) + self.enroll(infinite_course.id) + + response = self.api_response(api_version=API_V4, data={'status': EnrollmentStatuses.COMPLETED.value}) + enrollments = response.data['enrollments'] + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(enrollments['count'], 1) + self.assertEqual(enrollments['results'][0]['course']['id'], str(infinite_course.id)) + self.assertNotIn('primary', response.data) + + def test_user_enrollment_api_v4_expired_status(self): + """ + Testing + """ + self.login() + old_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.THREE_YEARS_AGO, + end=self.LAST_WEEK + ) + actual_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.LAST_WEEK, + end=self.NEXT_WEEK + ) + infinite_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.LAST_WEEK, + end=None + ) + self.enroll(old_course.id) + self.enroll(actual_course.id) + self.enroll(infinite_course.id) + + response = self.api_response(api_version=API_V4, data={'status': EnrollmentStatuses.EXPIRED.value}) + enrollments = response.data['enrollments'] + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(enrollments['count'], 1) + self.assertEqual(enrollments['results'][0]['course']['id'], str(old_course.id)) + self.assertNotIn('primary', response.data) + + def test_user_enrollment_api_v4_expired_course_with_certificate(self): + """ + Testing that the API returns a course with + an expiration date in the past if the user has a certificate for this course. + """ + self.login() + expired_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.THREE_YEARS_AGO, + end=self.LAST_WEEK + ) + expired_course_with_cert = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.THREE_YEARS_AGO, + end=self.LAST_WEEK + ) + GeneratedCertificateFactory.create( + user=self.user, + course_id=expired_course_with_cert.id, + status=CertificateStatuses.downloadable, + mode='verified', + ) + + self.enroll(expired_course_with_cert.id) + self.enroll(expired_course.id) + + response = self.api_response(api_version=API_V4, data={'status': EnrollmentStatuses.COMPLETED.value}) + enrollments = response.data['enrollments'] + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(enrollments['count'], 1) + self.assertEqual(enrollments['results'][0]['course']['id'], str(expired_course_with_cert.id)) + self.assertNotIn('primary', response.data) + + def test_user_enrollment_api_v4_status_all(self): + """ + Testing + """ + self.login() + old_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.THREE_YEARS_AGO, + end=self.LAST_WEEK + ) + actual_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.LAST_WEEK, + end=self.NEXT_WEEK + ) + infinite_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.LAST_WEEK, + end=None + ) + GeneratedCertificateFactory.create( + user=self.user, + course_id=infinite_course.id, + status=CertificateStatuses.downloadable, + mode='verified', + ) + + self.enroll(old_course.id) + self.enroll(actual_course.id) + self.enroll(infinite_course.id) + + response = self.api_response(api_version=API_V4, data={'status': EnrollmentStatuses.ALL.value}) + enrollments = response.data['enrollments'] + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(enrollments['count'], 3) + self.assertEqual(enrollments['results'][0]['course']['id'], str(infinite_course.id)) + self.assertEqual(enrollments['results'][1]['course']['id'], str(actual_course.id)) + self.assertEqual(enrollments['results'][2]['course']['id'], str(old_course.id)) + self.assertNotIn('primary', response.data) + @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 2c5e7736b288..4d2ddf2aad7a 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -42,6 +42,7 @@ from .. import errors from ..decorators import mobile_course_access, mobile_view +from .enums import EnrollmentStatuses from .serializers import ( CourseEnrollmentSerializer, CourseEnrollmentSerializerModifiedForPrimary, @@ -356,13 +357,33 @@ def get_serializer_class(self): @cached_property def queryset(self): - return CourseEnrollment.objects.all().select_related('course', 'user').filter( - user__username=self.kwargs['username'], + """ + Find and return the list of course enrollments for the user. + + In v4 added filtering by statuses. + """ + api_version = self.kwargs.get('api_version') + status = self.request.GET.get('status') + username = self.kwargs['username'] + + queryset = CourseEnrollment.objects.all().select_related('course', 'user').filter( + user__username=username, is_active=True ).order_by('-created') + if api_version == API_V4 and status in EnrollmentStatuses.values(): + if status == EnrollmentStatuses.IN_PROGRESS.value: + queryset = queryset.in_progress(user_username=username, time_zone=self.user_timezone) + elif status == EnrollmentStatuses.COMPLETED.value: + queryset = queryset.completed(user_username=username) + elif status == EnrollmentStatuses.EXPIRED.value: + queryset = queryset.expired(user_username=username, time_zone=self.user_timezone) + + return queryset + def get_queryset(self): api_version = self.kwargs.get('api_version') + status = self.request.GET.get('status') mobile_available = self.get_mobile_available_enrollments() not_duration_limited = ( @@ -370,7 +391,7 @@ def get_queryset(self): if check_course_expired(self.request.user, enrollment.course) == ACCESS_GRANTED ) - if api_version == API_V4: + if api_version == API_V4 and status not in EnrollmentStatuses.values(): primary_enrollment_obj = self.get_primary_enrollment_by_latest_enrollment_or_progress() if primary_enrollment_obj: mobile_available.remove(primary_enrollment_obj) @@ -401,26 +422,37 @@ def get_mobile_available_enrollments(self) -> List[Optional[CourseEnrollment]]: def list(self, request, *args, **kwargs): response = super().list(request, *args, **kwargs) api_version = self.kwargs.get('api_version') + status = self.request.GET.get('status') if api_version in (API_V2, API_V3, API_V4): enrollment_data = { 'configs': MobileConfig.get_structured_configs(), - 'user_timezone': str(get_user_timezone_or_last_seen_timezone_or_utc(self.get_user())), + 'user_timezone': str(self.user_timezone), '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 = CourseEnrollmentSerializerModifiedForPrimary( - primary_enrollment_obj, - context=self.get_serializer_context(), - ) - enrollment_data.update({'primary': serializer.data}) + if api_version == API_V4 and status not in EnrollmentStatuses.values(): + if status in EnrollmentStatuses.values(): + enrollment_data.update({'primary': None}) + else: + primary_enrollment_obj = self.get_primary_enrollment_by_latest_enrollment_or_progress() + if primary_enrollment_obj: + serializer = CourseEnrollmentSerializerModifiedForPrimary( + primary_enrollment_obj, + context=self.get_serializer_context(), + ) + enrollment_data.update({'primary': serializer.data}) return Response(enrollment_data) return response + @cached_property + def user_timezone(self): + """ + Get the user's timezone. + """ + return get_user_timezone_or_last_seen_timezone_or_utc(self.get_user()) + def get_user(self) -> User: """ Get user object by username. diff --git a/openedx/features/course_duration_limits/access.py b/openedx/features/course_duration_limits/access.py index ff817a315054..08a94702a5e0 100644 --- a/openedx/features/course_duration_limits/access.py +++ b/openedx/features/course_duration_limits/access.py @@ -68,7 +68,7 @@ def get_user_course_duration(user, course): return get_expected_duration(course.id) -def get_user_course_expiration_date(user, course): +def get_user_course_expiration_date(user, course, enrollment=None): """ Return expiration date for given user course pair. Return None if the course does not expire. @@ -81,7 +81,7 @@ def get_user_course_expiration_date(user, course): if access_duration is None: return None - enrollment = CourseEnrollment.get_enrollment(user, course.id) + enrollment = CourseEnrollment.get_enrollment(user, course.id) if not enrollment else enrollment if enrollment is None or enrollment.mode != CourseMode.AUDIT: return None From b64a6907ee1a86f3caa8b24dd5a718f699acf370 Mon Sep 17 00:00:00 2001 From: NiedielnitsevIvan <81557788+NiedielnitsevIvan@users.noreply.github.com> Date: Wed, 10 Apr 2024 17:55:02 +0300 Subject: [PATCH 06/18] feat: [AXM-236] Add progress for other courses (#2536) --- .../mobile_api/users/serializers.py | 57 ++++++++++++------- lms/djangoapps/mobile_api/users/views.py | 3 + 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 82eb3edee278..991205cc3091 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -106,6 +106,8 @@ class CourseEnrollmentSerializer(serializers.ModelSerializer): audit_access_expires = serializers.SerializerMethodField() course_modes = serializers.SerializerMethodField() + BLOCK_STRUCTURE_CACHE_TIMEOUT = 60 * 60 # 1 hour + def get_audit_access_expires(self, model): """ Returns expiration date for a course audit expiration, if any or null @@ -137,6 +139,40 @@ def get_course_modes(self, obj): for mode in course_modes ] + def to_representation(self, instance): + """ + Override the to_representation method to add the course_status field to the serialized data. + """ + data = super().to_representation(instance) + if 'progress' in self.context.get('requested_fields', []): + data['progress'] = self.calculate_progress(instance) + + return data + + def calculate_progress(self, model: CourseEnrollment) -> Dict[str, int]: + """ + Calculate the progress of the user in the course. + :param model: + :return: + """ + is_staff = bool(has_access(model.user, 'staff', model.course.id)) + + cache_key = f'course_block_structure_{str(model.course.id)}_{model.user.id}' + collected_block_structure = cache.get(cache_key) + if not collected_block_structure: + collected_block_structure = get_block_structure_manager(model.course.id).get_collected() + cache.set(cache_key, collected_block_structure, self.BLOCK_STRUCTURE_CACHE_TIMEOUT) + + course_grade = CourseGradeFactory().read(model.user, collected_block_structure=collected_block_structure) + + # recalculate course grade from visible grades (stored grade was calculated over all grades, visible or not) + course_grade.update(visible_grades_only=True, has_staff_access=is_staff) + subsection_grades = list(course_grade.subsection_grades.values()) + return { + 'num_points_earned': sum(map(lambda x: x.graded_total.earned if x.graded else 0, subsection_grades)), + 'num_points_possible': sum(map(lambda x: x.graded_total.possible if x.graded else 0, subsection_grades)), + } + class Meta: model = CourseEnrollment fields = ('audit_access_expires', 'created', 'mode', 'is_active', 'course', 'certificate', 'course_modes') @@ -165,8 +201,6 @@ class CourseEnrollmentSerializerModifiedForPrimary(CourseEnrollmentSerializer): progress = serializers.SerializerMethodField() course_assignments = serializers.SerializerMethodField() - BLOCK_STRUCTURE_CACHE_TIMEOUT = 60 * 60 # 1 hour - def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.course = modulestore().get_course(self.instance.course.id) @@ -230,24 +264,7 @@ def get_progress(self, model: CourseEnrollment) -> Dict[str, int]: """ Returns the progress of the user in the course. """ - assert isinstance(model, CourseEnrollment), f'Expected CourseEnrollment, got {type(model)}' - is_staff = bool(has_access(model.user, 'staff', model.course.id)) - - cache_key = f'course_block_structure_{str(model.course.id)}_{model.user.id}' - collected_block_structure = cache.get(cache_key) - if not collected_block_structure: - collected_block_structure = get_block_structure_manager(model.course.id).get_collected() - cache.set(cache_key, collected_block_structure, self.BLOCK_STRUCTURE_CACHE_TIMEOUT) - - course_grade = CourseGradeFactory().read(model.user, collected_block_structure=collected_block_structure) - - # recalculate course grade from visible grades (stored grade was calculated over all grades, visible or not) - course_grade.update(visible_grades_only=True, has_staff_access=is_staff) - subsection_grades = list(course_grade.subsection_grades.values()) - return { - 'num_points_earned': sum(map(lambda x: x.graded_total.earned if x.graded else 0, subsection_grades)), - 'num_points_possible': sum(map(lambda x: x.graded_total.possible if x.graded else 0, subsection_grades)), - } + return self.calculate_progress(model) def get_course_assignments(self, model: CourseEnrollment) -> Optional[Dict[str, List[Dict[str, str]]]]: """ diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 4d2ddf2aad7a..b8949d269632 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -346,7 +346,10 @@ def is_org(self, check_org, course_org): def get_serializer_context(self): context = super().get_serializer_context() + requested_fields = self.request.GET.get('requested_fields', '') + context['api_version'] = self.kwargs.get('api_version') + context['requested_fields'] = requested_fields.split(',') return context def get_serializer_class(self): From aaa1b2d25d232d5b838a1972679452f599f8b0c3 Mon Sep 17 00:00:00 2001 From: Kyrylo Kireiev <90455454+KyryloKireiev@users.noreply.github.com> Date: Tue, 16 Apr 2024 09:57:19 +0300 Subject: [PATCH 07/18] fix: [AXM-277] Change _get_last_visited_block_path_and_unit_name method implementation (#2540) --- .../mobile_api/users/serializers.py | 53 ++++++++----------- 1 file changed, 22 insertions(+), 31 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 991205cc3091..76453dc4a6b4 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -3,11 +3,13 @@ """ from datetime import datetime -from typing import Dict, List, Optional, Tuple +from typing import Dict, List, Optional, Tuple, Union from django.core.cache import cache from completion.exceptions import UnavailableCompletionData from completion.utilities import get_key_to_last_completed_block +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import UsageKey from rest_framework import serializers from rest_framework.reverse import reverse @@ -16,15 +18,14 @@ from common.djangoapps.util.course import get_encoded_course_sharing_utm_params, get_link_for_about_page from lms.djangoapps.certificates.api import certificate_downloadable_status from lms.djangoapps.courseware.access import has_access -from lms.djangoapps.courseware.block_render import get_block_for_descriptor from lms.djangoapps.courseware.context_processor import get_user_timezone_or_last_seen_timezone_or_utc -from lms.djangoapps.courseware.courses import get_course_assignment_date_blocks, get_current_child -from lms.djangoapps.courseware.model_data import FieldDataCache +from lms.djangoapps.courseware.courses import get_course_assignment_date_blocks from lms.djangoapps.course_home_api.dates.serializers import DateSummarySerializer from lms.djangoapps.grades.api import CourseGradeFactory from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager from openedx.features.course_duration_limits.access import get_user_course_expiration_date from xmodule.modulestore.django import modulestore +from xmodule.modulestore.exceptions import ItemNotFoundError class CourseOverviewField(serializers.RelatedField): # lint-amnesty, pylint: disable=abstract-method @@ -217,8 +218,10 @@ def get_course_status(self, model: CourseEnrollment) -> Optional[Dict[str, List[ if not block_id: return None - request = self.context.get('request') - path, unit_name = self._get_last_visited_block_path_and_unit_name(request, model) + path, unit_name = self._get_last_visited_block_path_and_unit_name(block_id) + if not path and unit_name: + return None + path_ids = [str(block.location) for block in path] return { @@ -228,37 +231,25 @@ def get_course_status(self, model: CourseEnrollment) -> Optional[Dict[str, List[ 'last_visited_unit_display_name': unit_name, } + @staticmethod def _get_last_visited_block_path_and_unit_name( - self, - request: 'Request', # noqa: F821 - model: CourseEnrollment, - ) -> Tuple[List[Optional['XBlock']], Optional[str]]: # noqa: F821 + block_id: str + ) -> Union[Tuple[None, None], Tuple[List['XBlock'], str]]: # noqa: F821 """ Returns the path to the latest block and unit name visited by the current user. - - If there is no such visit, the first item deep enough down the course - tree is used. """ - field_data_cache = FieldDataCache.cache_for_block_descendents(self.course.id, model.user, self.course, depth=3) + try: + last_visited_block = modulestore().get_item(UsageKey.from_string(block_id)) + vertical = last_visited_block.get_parent() + sequential = vertical.get_parent() + chapter = sequential.get_parent() + course = chapter.get_parent() + except (ItemNotFoundError, InvalidKeyError, AttributeError): + return None, None - course_block = get_block_for_descriptor( - model.user, request, self.course, field_data_cache, self.course.id, course=self.course - ) + path = [sequential, chapter, course] - unit_name = '' - path = [course_block] if course_block else [] - chapter = get_current_child(course_block, min_depth=3) - if chapter is not None: - path.append(chapter) - section = get_current_child(chapter, min_depth=2) - if section is not None: - path.append(section) - unit = get_current_child(section, min_depth=1) - if unit is not None: - unit_name = unit.display_name - - path.reverse() - return path, unit_name + return path, vertical.display_name def get_progress(self, model: CourseEnrollment) -> Dict[str, int]: """ From 9a9ce41915e2c9d513f82f2c1ecb27ff26b4f585 Mon Sep 17 00:00:00 2001 From: Kyrylo Kireiev <90455454+KyryloKireiev@users.noreply.github.com> Date: Mon, 29 Apr 2024 13:28:28 +0300 Subject: [PATCH 08/18] feat: [AXM-288] Change response to represent Future assignments (#2550) * feat: [AXM-288] Change response to represent Future assignments the same way as past assignments * refactor: [AXM-288] Refactor get_course_assignments Co-authored-by: monteri <36768631+monteri@users.noreply.github.com> * refactor: [AXM-288] Refactor get_course_assignments method --------- Co-authored-by: monteri <36768631+monteri@users.noreply.github.com> --- .../mobile_api/users/serializers.py | 24 ++++++++++++------- lms/djangoapps/mobile_api/users/tests.py | 15 ++++++++++++ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 76453dc4a6b4..82389199ef72 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -257,7 +257,7 @@ def get_progress(self, model: CourseEnrollment) -> Dict[str, int]: """ return self.calculate_progress(model) - def get_course_assignments(self, model: CourseEnrollment) -> Optional[Dict[str, List[Dict[str, str]]]]: + def get_course_assignments(self, model: CourseEnrollment) -> Dict[str, Optional[List[Dict[str, str]]]]: """ Returns the future assignment data and past assignments data for the user in the course. """ @@ -267,20 +267,28 @@ def get_course_assignments(self, model: CourseEnrollment) -> Optional[Dict[str, self.context.get('request'), include_past_dates=True ) - next_assignment = None - past_assignment = [] + past_assignments = [] + future_assignments = [] timezone = get_user_timezone_or_last_seen_timezone_or_utc(model.user) for assignment in sorted(assignments, key=lambda x: x.date): if assignment.date < datetime.now(timezone): - past_assignment.append(assignment) + past_assignments.append(assignment) else: - next_assignment = DateSummarySerializer(assignment).data - break + if not assignment.complete: + future_assignments.append(assignment) + + if future_assignments: + future_assignment_date = future_assignments[0].date.date() + next_assignments = [ + assignment for assignment in future_assignments if assignment.date.date() == future_assignment_date + ] + else: + next_assignments = [] return { - 'future_assignment': next_assignment, - 'past_assignments': DateSummarySerializer(past_assignment, many=True).data, + 'future_assignments': DateSummarySerializer(next_assignments, many=True).data, + 'past_assignments': DateSummarySerializer(past_assignments, many=True).data, } class Meta: diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 5b842851db0b..05a75f30f5c3 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -902,6 +902,21 @@ def test_user_enrollment_api_v4_status_all(self): self.assertEqual(enrollments['results'][2]['course']['id'], str(old_course.id)) self.assertNotIn('primary', response.data) + @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) + def test_response_contains_primary_enrollment_assignments_info(self, cache_mock: MagicMock): + self.login() + course = CourseFactory.create(org='edx', mobile_available=True) + self.enroll(course.id) + + response = self.api_response(api_version=API_V4) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIn('course_assignments', response.data['primary']) + self.assertIn('past_assignments', response.data['primary']['course_assignments']) + self.assertIn('future_assignments', response.data['primary']['course_assignments']) + self.assertListEqual(response.data['primary']['course_assignments']['past_assignments'], []) + self.assertListEqual(response.data['primary']['course_assignments']['future_assignments'], []) + @override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) class TestUserEnrollmentCertificates(UrlResetMixin, MobileAPITestCase, MilestonesTestCaseMixin): From 8accb6d08925f6173e75927cc54ac6a16d2df13b Mon Sep 17 00:00:00 2001 From: Kyrylo Kireiev <90455454+KyryloKireiev@users.noreply.github.com> Date: Thu, 25 Apr 2024 15:29:34 +0300 Subject: [PATCH 09/18] feat: [AXM-297] Add progress to assignments in BlocksInfoInCourseView API (#2546) * feat: [AXM-297, AXM-310] Add progress to assignments and total course progress * feat: [AXM-297] Add progress to assignments * style: [AXM-297] Try to fix linters (add docstrings) * refactor: [AXM-297] Add typing, refactor methods --- .../mobile_api/course_info/constants.py | 5 +++ .../mobile_api/course_info/utils.py | 43 +++++++++++++++++++ .../mobile_api/course_info/views.py | 43 ++++++++++++++++++- .../tests/test_course_info_views.py | 28 ++++++++++++ 4 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 lms/djangoapps/mobile_api/course_info/constants.py create mode 100644 lms/djangoapps/mobile_api/course_info/utils.py diff --git a/lms/djangoapps/mobile_api/course_info/constants.py b/lms/djangoapps/mobile_api/course_info/constants.py new file mode 100644 index 000000000000..d62cb463951a --- /dev/null +++ b/lms/djangoapps/mobile_api/course_info/constants.py @@ -0,0 +1,5 @@ +""" +Common constants for the `course_info` API. +""" + +BLOCK_STRUCTURE_CACHE_TIMEOUT = 60 * 60 # 1 hour diff --git a/lms/djangoapps/mobile_api/course_info/utils.py b/lms/djangoapps/mobile_api/course_info/utils.py new file mode 100644 index 000000000000..141c32da4cf4 --- /dev/null +++ b/lms/djangoapps/mobile_api/course_info/utils.py @@ -0,0 +1,43 @@ +""" +Common utils for the `course_info` API. +""" + +import logging +from typing import List, Optional, Union + +from django.core.cache import cache + +from lms.djangoapps.courseware.access import has_access +from lms.djangoapps.grades.api import CourseGradeFactory +from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager + +log = logging.getLogger(__name__) + + +def calculate_progress( + user: 'User', # noqa: F821 + course_id: 'CourseLocator', # noqa: F821 + cache_timeout: int, +) -> Optional[List[Union['ReadSubsectionGrade', 'ZeroSubsectionGrade']]]: # noqa: F821 + """ + Calculate the progress of the user in the course. + """ + is_staff = bool(has_access(user, 'staff', course_id)) + + try: + cache_key = f'course_block_structure_{str(course_id)}_{user.id}' + collected_block_structure = cache.get(cache_key) + if not collected_block_structure: + collected_block_structure = get_block_structure_manager(course_id).get_collected() + cache.set(cache_key, collected_block_structure, cache_timeout) + + course_grade = CourseGradeFactory().read(user, collected_block_structure=collected_block_structure) + + # recalculate course grade from visible grades (stored grade was calculated over all grades, visible or not) + course_grade.update(visible_grades_only=True, has_staff_access=is_staff) + subsection_grades = list(course_grade.subsection_grades.values()) + except Exception as err: # pylint: disable=broad-except + log.warning(f'Could not get grades for the course: {course_id}, error: {err}') + return [] + + return subsection_grades diff --git a/lms/djangoapps/mobile_api/course_info/views.py b/lms/djangoapps/mobile_api/course_info/views.py index bd34336cc824..c6de108727d8 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -3,7 +3,7 @@ """ import logging -from typing import Optional, Union +from typing import Dict, Optional, Union import django from django.contrib.auth import get_user_model @@ -20,11 +20,13 @@ from lms.djangoapps.courseware.courses import get_course_info_section_block from lms.djangoapps.course_goals.models import UserActivity from lms.djangoapps.course_api.blocks.views import BlocksInCourseView +from lms.djangoapps.mobile_api.course_info.constants import BLOCK_STRUCTURE_CACHE_TIMEOUT from lms.djangoapps.mobile_api.course_info.serializers import ( CourseInfoOverviewSerializer, CourseAccessSerializer, MobileCourseEnrollmentSerializer ) +from lms.djangoapps.mobile_api.course_info.utils import calculate_progress from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib.api.view_utils import view_auth_classes from openedx.core.lib.xblock_utils import get_course_update_items @@ -357,6 +359,12 @@ def list(self, request, **kwargs): # pylint: disable=W0221 course_info_context = {} if requested_user := self.get_requested_user(request.user, requested_username): + self._extend_sequential_info_with_assignment_progress( + requested_user, + course_key, + response.data['blocks'], + ) + course_info_context = { 'user': requested_user } @@ -380,3 +388,36 @@ def list(self, request, **kwargs): # pylint: disable=W0221 response.data.update(course_data) return response + + @staticmethod + def _extend_sequential_info_with_assignment_progress( + requested_user: User, + course_id: CourseKey, + blocks_info_data: Dict[str, Dict], + ) -> None: + """ + Extends sequential xblock info with assignment's name and progress. + """ + subsection_grades = calculate_progress(requested_user, course_id, BLOCK_STRUCTURE_CACHE_TIMEOUT) + grades_with_locations = {str(grade.location): grade for grade in subsection_grades} + + for block_id, block_info in blocks_info_data.items(): + if block_info['type'] == 'sequential': + grade = grades_with_locations.get(block_id) + if grade: + graded_total = grade.graded_total if grade.graded else None + points_earned = graded_total.earned if graded_total else 0 + points_possible = graded_total.possible if graded_total else 0 + assignment_type = grade.format + else: + points_earned, points_possible, assignment_type = 0, 0, None + + block_info.update( + { + 'assignment_progress': { + 'assignment_type': assignment_type, + 'num_points_earned': points_earned, + 'num_points_possible': points_possible, + } + } + ) diff --git a/lms/djangoapps/mobile_api/tests/test_course_info_views.py b/lms/djangoapps/mobile_api/tests/test_course_info_views.py index 25fe08980379..ca4750d121bf 100644 --- a/lms/djangoapps/mobile_api/tests/test_course_info_views.py +++ b/lms/djangoapps/mobile_api/tests/test_course_info_views.py @@ -422,3 +422,31 @@ def test_course_modes(self): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertListEqual(response.data['course_modes'], expected_course_modes) + + def test_extend_sequential_info_with_assignment_progress_get_only_sequential(self) -> None: + response = self.verify_response(url=self.url, params={'block_types_filter': 'sequential'}) + + expected_results = ( + { + 'assignment_type': 'Lecture Sequence', + 'num_points_earned': 0.0, + 'num_points_possible': 0.0 + }, + { + 'assignment_type': None, + 'num_points_earned': 0.0, + 'num_points_possible': 0.0 + }, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + for sequential_info, assignment_progress in zip(response.data['blocks'].values(), expected_results): + self.assertDictEqual(sequential_info['assignment_progress'], assignment_progress) + + @ddt.data('chapter', 'vertical', 'problem', 'video', 'html') + def test_extend_sequential_info_with_assignment_progress_for_other_types(self, block_type: 'str') -> None: + response = self.verify_response(url=self.url, params={'block_types_filter': block_type}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + for block_info in response.data['blocks'].values(): + self.assertNotEqual('assignment_progress', block_info) From 88d31aed38fbd7d71a68e3758779a11ef49e8bf1 Mon Sep 17 00:00:00 2001 From: Kyrylo Kireiev <90455454+KyryloKireiev@users.noreply.github.com> Date: Mon, 13 May 2024 15:38:06 +0300 Subject: [PATCH 10/18] feat: [AXM-287,310,331] Change course progress calculation logic (#2553) * feat: [AXM-287,310,331] Change course progress calculation logic * style: [AXM-287,310,331] Remove commented code * fix: [AXM-287,310,331] Change course assignments gather logic --- .../course_api/blocks/tests/test_views.py | 8 +- lms/djangoapps/courseware/courses.py | 8 +- .../mobile_api/course_info/serializers.py | 27 +++- .../mobile_api/course_info/views.py | 7 +- .../tests/test_course_info_serializers.py | 37 ++++- .../mobile_api/users/serializers.py | 49 +++--- lms/djangoapps/mobile_api/users/tests.py | 144 +++++++++++++++--- lms/djangoapps/mobile_api/users/views.py | 4 + 8 files changed, 226 insertions(+), 58 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/tests/test_views.py b/lms/djangoapps/course_api/blocks/tests/test_views.py index c1f673652096..4b9823328114 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_views.py +++ b/lms/djangoapps/course_api/blocks/tests/test_views.py @@ -3,7 +3,7 @@ """ from datetime import datetime from unittest import mock -from unittest.mock import Mock +from unittest.mock import MagicMock, Mock from urllib.parse import urlencode, urlunparse import ddt @@ -209,8 +209,9 @@ def test_not_authenticated_public_course_with_all_blocks(self): self.query_params['all_blocks'] = True self.verify_response(403) + @mock.patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[]) @mock.patch("lms.djangoapps.course_api.blocks.forms.permissions.is_course_public", Mock(return_value=True)) - def test_not_authenticated_public_course_with_blank_username(self): + def test_not_authenticated_public_course_with_blank_username(self, get_course_assignment_mock: MagicMock) -> None: """ Verify behaviour when accessing course blocks of a public course for anonymous user anonymously. """ @@ -368,7 +369,8 @@ def test_extra_field_when_not_requested(self): block_data['type'] == 'course' ) - def test_data_researcher_access(self): + @mock.patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[]) + def test_data_researcher_access(self, get_course_assignment_mock: MagicMock) -> None: """ Test if data researcher has access to the api endpoint """ diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 2fc727623541..ee0d12ce1a52 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -5,7 +5,7 @@ import logging from collections import defaultdict, namedtuple -from datetime import datetime +from datetime import datetime, timedelta import six import pytz @@ -587,7 +587,7 @@ def get_course_blocks_completion_summary(course_key, user): @request_cached() -def get_course_assignments(course_key, user, include_access=False): # lint-amnesty, pylint: disable=too-many-statements +def get_course_assignments(course_key, user, include_access=False, include_without_due=False,): # lint-amnesty, pylint: disable=too-many-statements """ Returns a list of assignment (at the subsection/sequential level) due dates for the given course. @@ -607,6 +607,10 @@ def get_course_assignments(course_key, user, include_access=False): # lint-amne for subsection_key in block_data.get_children(section_key): due = block_data.get_xblock_field(subsection_key, 'due') graded = block_data.get_xblock_field(subsection_key, 'graded', False) + + if not due and include_without_due: + due = now + timedelta(days=1000) + if due and graded: first_component_block_id = get_first_component_of_block(subsection_key, block_data) contains_gated_content = include_access and block_data.get_xblock_field( diff --git a/lms/djangoapps/mobile_api/course_info/serializers.py b/lms/djangoapps/mobile_api/course_info/serializers.py index d7a9471088aa..d5ad89eb69a5 100644 --- a/lms/djangoapps/mobile_api/course_info/serializers.py +++ b/lms/djangoapps/mobile_api/course_info/serializers.py @@ -2,7 +2,7 @@ Course Info serializers """ from rest_framework import serializers -from typing import Union +from typing import Dict, Union from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.models import CourseEnrollment @@ -13,6 +13,7 @@ from lms.djangoapps.courseware.access import has_access from lms.djangoapps.courseware.access import administrative_accesses_to_course_for_user from lms.djangoapps.courseware.access_utils import check_course_open_for_learner +from lms.djangoapps.courseware.courses import get_course_assignments from lms.djangoapps.mobile_api.users.serializers import ModeSerializer from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.features.course_duration_limits.access import get_user_course_expiration_date @@ -31,6 +32,7 @@ class CourseInfoOverviewSerializer(serializers.ModelSerializer): course_sharing_utm_parameters = serializers.SerializerMethodField() course_about = serializers.SerializerMethodField('get_course_about_url') course_modes = serializers.SerializerMethodField() + course_progress = serializers.SerializerMethodField() class Meta: model = CourseOverview @@ -47,6 +49,7 @@ class Meta: 'course_sharing_utm_parameters', 'course_about', 'course_modes', + 'course_progress', ) @staticmethod @@ -75,6 +78,28 @@ def get_course_modes(self, course_overview): for mode in course_modes ] + def get_course_progress(self, obj: 'CourseOverview') -> Dict[str, int]: # noqa: F821 #here + """ + Gets course progress calculated by course assignments. + """ + course_assignments = get_course_assignments( + obj.id, + self.context.get('user'), + include_without_due=True, + ) + + total_assignments_count = 0 + assignments_completed = 0 + + if course_assignments: + total_assignments_count = len(course_assignments) + assignments_completed = len([assignment for assignment in course_assignments if assignment.complete]) + + return { + 'total_assignments_count': total_assignments_count, + 'assignments_completed': assignments_completed, + } + class MobileCourseEnrollmentSerializer(serializers.ModelSerializer): """ diff --git a/lms/djangoapps/mobile_api/course_info/views.py b/lms/djangoapps/mobile_api/course_info/views.py index c6de108727d8..40d586839680 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -271,6 +271,11 @@ class BlocksInfoInCourseView(BlocksInCourseView): course, chapter, sequential, vertical, html, problem, video, and discussion. display_name: (str) The display name of the block. + course_progress: (dict) Contains information about how many assignments are in the course + and how many assignments the student has completed. + Included here: + * total_assignments_count: (int) Total course's assignments count. + * assignments_completed: (int) Assignments witch the student has completed. **Returns** @@ -366,7 +371,7 @@ def list(self, request, **kwargs): # pylint: disable=W0221 ) course_info_context = { - 'user': requested_user + 'user': requested_user, } user_enrollment = CourseEnrollment.get_enrollment(user=requested_user, course_key=course_key) course_data.update({ diff --git a/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py b/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py index 51d9acba54cc..c18d22f0ae99 100644 --- a/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py +++ b/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py @@ -147,7 +147,8 @@ def setUp(self): self.user = UserFactory() self.course_overview = CourseOverviewFactory() - def test_get_media(self): + @patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[]) + def test_get_media(self, get_course_assignment_mock: MagicMock) -> None: output_data = CourseInfoOverviewSerializer(self.course_overview, context={'user': self.user}).data self.assertIn('media', output_data) @@ -156,16 +157,46 @@ def test_get_media(self): self.assertIn('small', output_data['media']['image']) self.assertIn('large', output_data['media']['image']) + @patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[]) @patch('lms.djangoapps.mobile_api.course_info.serializers.get_link_for_about_page', return_value='mock_about_link') - def test_get_course_sharing_utm_parameters(self, mock_get_link_for_about_page: MagicMock) -> None: + def test_get_course_sharing_utm_parameters( + self, + mock_get_link_for_about_page: MagicMock, + get_course_assignment_mock: MagicMock, + ) -> None: output_data = CourseInfoOverviewSerializer(self.course_overview, context={'user': self.user}).data self.assertEqual(output_data['course_about'], mock_get_link_for_about_page.return_value) mock_get_link_for_about_page.assert_called_once_with(self.course_overview) - def test_get_course_modes(self): + @patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[]) + def test_get_course_modes(self, get_course_assignment_mock: MagicMock) -> None: expected_course_modes = [{'slug': 'audit', 'sku': None, 'android_sku': None, 'ios_sku': None, 'min_price': 0}] output_data = CourseInfoOverviewSerializer(self.course_overview, context={'user': self.user}).data self.assertListEqual(output_data['course_modes'], expected_course_modes) + + @patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[]) + def test_get_course_progress_no_assignments(self, get_course_assignment_mock: MagicMock) -> None: + expected_course_progress = {'total_assignments_count': 0, 'assignments_completed': 0} + + output_data = CourseInfoOverviewSerializer(self.course_overview, context={'user': self.user}).data + + self.assertIn('course_progress', output_data) + self.assertDictEqual(output_data['course_progress'], expected_course_progress) + get_course_assignment_mock.assert_called_once_with(self.course_overview.id, self.user, include_without_due=True) + + @patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments') + def test_get_course_progress_with_assignments(self, get_course_assignment_mock: MagicMock) -> None: + assignments_mock = [ + Mock(complete=False), Mock(complete=False), Mock(complete=True), Mock(complete=True), Mock(complete=True) + ] + get_course_assignment_mock.return_value = assignments_mock + expected_course_progress = {'total_assignments_count': 5, 'assignments_completed': 3} + + output_data = CourseInfoOverviewSerializer(self.course_overview, context={'user': self.user}).data + + self.assertIn('course_progress', output_data) + self.assertDictEqual(output_data['course_progress'], expected_course_progress) + get_course_assignment_mock.assert_called_once_with(self.course_overview.id, self.user, include_without_due=True) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 82389199ef72..dd82d9f99e49 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -5,7 +5,6 @@ from datetime import datetime from typing import Dict, List, Optional, Tuple, Union -from django.core.cache import cache from completion.exceptions import UnavailableCompletionData from completion.utilities import get_key_to_last_completed_block from opaque_keys import InvalidKeyError @@ -19,10 +18,9 @@ from lms.djangoapps.certificates.api import certificate_downloadable_status from lms.djangoapps.courseware.access import has_access from lms.djangoapps.courseware.context_processor import get_user_timezone_or_last_seen_timezone_or_utc -from lms.djangoapps.courseware.courses import get_course_assignment_date_blocks +from lms.djangoapps.courseware.courses import get_course_assignment_date_blocks, get_course_assignments from lms.djangoapps.course_home_api.dates.serializers import DateSummarySerializer -from lms.djangoapps.grades.api import CourseGradeFactory -from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager +from lms.djangoapps.mobile_api.utils import API_V4 from openedx.features.course_duration_limits.access import get_user_course_expiration_date from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError @@ -107,8 +105,6 @@ class CourseEnrollmentSerializer(serializers.ModelSerializer): audit_access_expires = serializers.SerializerMethodField() course_modes = serializers.SerializerMethodField() - BLOCK_STRUCTURE_CACHE_TIMEOUT = 60 * 60 # 1 hour - def get_audit_access_expires(self, model): """ Returns expiration date for a course audit expiration, if any or null @@ -140,38 +136,37 @@ def get_course_modes(self, obj): for mode in course_modes ] - def to_representation(self, instance): + def to_representation(self, instance: CourseEnrollment) -> 'OrderedDict': # noqa: F821 """ Override the to_representation method to add the course_status field to the serialized data. """ data = super().to_representation(instance) - if 'progress' in self.context.get('requested_fields', []): - data['progress'] = self.calculate_progress(instance) + + if 'course_progress' in self.context.get('requested_fields', []) and self.context.get('api_version') == API_V4: + data['course_progress'] = self.calculate_progress(instance) return data def calculate_progress(self, model: CourseEnrollment) -> Dict[str, int]: """ Calculate the progress of the user in the course. - :param model: - :return: """ - is_staff = bool(has_access(model.user, 'staff', model.course.id)) + course_assignments = get_course_assignments( + model.course_id, + model.user, + include_without_due=True, + ) - cache_key = f'course_block_structure_{str(model.course.id)}_{model.user.id}' - collected_block_structure = cache.get(cache_key) - if not collected_block_structure: - collected_block_structure = get_block_structure_manager(model.course.id).get_collected() - cache.set(cache_key, collected_block_structure, self.BLOCK_STRUCTURE_CACHE_TIMEOUT) + total_assignments_count = 0 + assignments_completed = 0 - course_grade = CourseGradeFactory().read(model.user, collected_block_structure=collected_block_structure) + if course_assignments: + total_assignments_count = len(course_assignments) + assignments_completed = len([assignment for assignment in course_assignments if assignment.complete]) - # recalculate course grade from visible grades (stored grade was calculated over all grades, visible or not) - course_grade.update(visible_grades_only=True, has_staff_access=is_staff) - subsection_grades = list(course_grade.subsection_grades.values()) return { - 'num_points_earned': sum(map(lambda x: x.graded_total.earned if x.graded else 0, subsection_grades)), - 'num_points_possible': sum(map(lambda x: x.graded_total.possible if x.graded else 0, subsection_grades)), + 'total_assignments_count': total_assignments_count, + 'assignments_completed': assignments_completed, } class Meta: @@ -199,7 +194,7 @@ class CourseEnrollmentSerializerModifiedForPrimary(CourseEnrollmentSerializer): """ course_status = serializers.SerializerMethodField() - progress = serializers.SerializerMethodField() + course_progress = serializers.SerializerMethodField() course_assignments = serializers.SerializerMethodField() def __init__(self, *args, **kwargs): @@ -213,7 +208,7 @@ def get_course_status(self, model: CourseEnrollment) -> Optional[Dict[str, List[ try: block_id = str(get_key_to_last_completed_block(model.user, model.course.id)) except UnavailableCompletionData: - block_id = "" + block_id = '' if not block_id: return None @@ -251,7 +246,7 @@ def _get_last_visited_block_path_and_unit_name( return path, vertical.display_name - def get_progress(self, model: CourseEnrollment) -> Dict[str, int]: + def get_course_progress(self, model: CourseEnrollment) -> Dict[str, int]: """ Returns the progress of the user in the course. """ @@ -302,7 +297,7 @@ class Meta: 'certificate', 'course_modes', 'course_status', - 'progress', + 'course_progress', 'course_assignments', ) lookup_field = 'username' diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 05a75f30f5c3..71199db1b0fe 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -4,7 +4,7 @@ import datetime -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, Mock, patch from urllib.parse import parse_qs import ddt @@ -436,8 +436,7 @@ def test_student_dont_have_enrollments(self): self.assertDictEqual(expected_result, response.data) self.assertNotIn('primary', response.data) - @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) - def test_student_have_one_enrollment(self, cache_mock: MagicMock): + def test_student_have_one_enrollment(self): """ Testing modified `UserCourseEnrollmentsList` view with api_version == v4. """ @@ -461,8 +460,7 @@ def test_student_have_one_enrollment(self, cache_mock: MagicMock): self.assertIn('primary', response.data) self.assertEqual(str(course.id), response.data['primary']['course']['id']) - @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) - def test_student_have_two_enrollments(self, cache_mock: MagicMock): + def test_student_have_two_enrollments(self): """ Testing modified `UserCourseEnrollmentsList` view with api_version == v4. """ @@ -481,8 +479,7 @@ def test_student_have_two_enrollments(self, cache_mock: MagicMock): self.assertIn('primary', response.data) self.assertEqual(response.data['primary']['course']['id'], str(course_second.id)) - @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) - def test_student_have_more_then_ten_enrollments(self, cache_mock: MagicMock): + def test_student_have_more_then_ten_enrollments(self): """ Testing modified `UserCourseEnrollmentsList` view with api_version == v4. """ @@ -502,8 +499,7 @@ def test_student_have_more_then_ten_enrollments(self, cache_mock: MagicMock): self.assertIn('primary', response.data) self.assertEqual(response.data['primary']['course']['id'], str(latest_enrolment.id)) - @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) - def test_student_have_progress_in_old_course_and_enroll_newest_course(self, cache_mock: MagicMock): + def test_student_have_progress_in_old_course_and_enroll_newest_course(self): """ Testing modified `UserCourseEnrollmentsList` view with api_version == v4. """ @@ -583,8 +579,7 @@ def test_student_enrolled_only_not_mobile_available_courses(self): self.assertDictEqual(expected_result, response.data) self.assertNotIn('primary', response.data) - @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) - def test_do_progress_in_not_mobile_available_course(self, cache_mock: MagicMock): + def test_do_progress_in_not_mobile_available_course(self): """ Testing modified `UserCourseEnrollmentsList` view with api_version == v4. """ @@ -621,8 +616,7 @@ def test_do_progress_in_not_mobile_available_course(self, cache_mock: MagicMock) self.assertIn('primary', response.data) self.assertEqual(response.data['primary']['course']['id'], str(new_course.id)) - @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) - def test_pagination_for_user_enrollments_api_v4(self, cache_mock: MagicMock): + def test_pagination_for_user_enrollments_api_v4(self): """ Tests `UserCourseEnrollmentsV4Pagination`, api_version == v4. """ @@ -641,8 +635,7 @@ def test_pagination_for_user_enrollments_api_v4(self, cache_mock: MagicMock): self.assertIn('previous', response.data['enrollments']) self.assertIn('primary', response.data) - @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) - def test_course_status_in_primary_obj_when_student_doesnt_have_progress(self, cache_mock: MagicMock): + def test_course_status_in_primary_obj_when_student_doesnt_have_progress(self): """ Testing modified `UserCourseEnrollmentsList` view with api_version == v4. """ @@ -655,12 +648,10 @@ def test_course_status_in_primary_obj_when_student_doesnt_have_progress(self, ca self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data['primary']['course_status'], None) - @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) @patch('lms.djangoapps.mobile_api.users.serializers.get_key_to_last_completed_block') def test_course_status_in_primary_obj_when_student_have_progress( self, get_last_completed_block_mock: MagicMock, - cache_mock: MagicMock ): """ Testing modified `UserCourseEnrollmentsList` view with api_version == v4. @@ -707,8 +698,7 @@ def test_course_status_in_primary_obj_when_student_have_progress( self.assertEqual(response.data['primary']['course_status'], expected_course_status) get_last_completed_block_mock.assert_called_once_with(self.user, course.id) - @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) - def test_user_enrollment_api_v4_in_progress_status(self, cache_mock: MagicMock): + def test_user_enrollment_api_v4_in_progress_status(self): """ Testing """ @@ -902,8 +892,7 @@ def test_user_enrollment_api_v4_status_all(self): self.assertEqual(enrollments['results'][2]['course']['id'], str(old_course.id)) self.assertNotIn('primary', response.data) - @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) - def test_response_contains_primary_enrollment_assignments_info(self, cache_mock: MagicMock): + def test_response_contains_primary_enrollment_assignments_info(self): self.login() course = CourseFactory.create(org='edx', mobile_available=True) self.enroll(course.id) @@ -917,6 +906,119 @@ def test_response_contains_primary_enrollment_assignments_info(self, cache_mock: self.assertListEqual(response.data['primary']['course_assignments']['past_assignments'], []) self.assertListEqual(response.data['primary']['course_assignments']['future_assignments'], []) + @patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignments', return_value=[]) + def test_course_progress_in_primary_enrollment_with_no_assignments( + self, + get_course_assignment_mock: MagicMock, + ) -> None: + self.login() + course = CourseFactory.create(org='edx', mobile_available=True) + self.enroll(course.id) + expected_course_progress = {'total_assignments_count': 0, 'assignments_completed': 0} + + response = self.api_response(api_version=API_V4) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIn('course_progress', response.data['primary']) + self.assertDictEqual(response.data['primary']['course_progress'], expected_course_progress) + + @patch( + 'lms.djangoapps.mobile_api.users.serializers.CourseEnrollmentSerializerModifiedForPrimary' + '.get_course_assignments' + ) + @patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignments') + def test_course_progress_in_primary_enrollment_with_assignments( + self, + get_course_assignment_mock: MagicMock, + assignments_mock: MagicMock, + ) -> None: + self.login() + course = CourseFactory.create(org='edx', mobile_available=True) + self.enroll(course.id) + course_assignments_mock = [ + Mock(complete=False), Mock(complete=False), Mock(complete=True), Mock(complete=True), Mock(complete=True) + ] + get_course_assignment_mock.return_value = course_assignments_mock + student_assignments_mock = { + 'future_assignments': [], + 'past_assignments': [], + } + assignments_mock.return_value = student_assignments_mock + expected_course_progress = {'total_assignments_count': 5, 'assignments_completed': 3} + + response = self.api_response(api_version=API_V4) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIn('course_progress', response.data['primary']) + self.assertDictEqual(response.data['primary']['course_progress'], expected_course_progress) + + @patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignments') + def test_course_progress_for_secondary_enrollments_no_query_param( + self, + get_course_assignment_mock: MagicMock, + ) -> None: + self.login() + courses = [CourseFactory.create(org='edx', mobile_available=True) for _ in range(5)] + for course in courses: + self.enroll(course.id) + + response = self.api_response(api_version=API_V4) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + for enrollment in response.data['enrollments']['results']: + self.assertNotIn('course_progress', enrollment) + + @patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignments') + def test_course_progress_for_secondary_enrollments_with_query_param( + self, + get_course_assignment_mock: MagicMock, + ) -> None: + self.login() + courses = [CourseFactory.create(org='edx', mobile_available=True) for _ in range(5)] + for course in courses: + self.enroll(course.id) + expected_course_progress = {'total_assignments_count': 0, 'assignments_completed': 0} + + response = self.api_response(api_version=API_V4, data={'requested_fields': 'course_progress'}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + for enrollment in response.data['enrollments']['results']: + self.assertIn('course_progress', enrollment) + self.assertDictEqual(enrollment['course_progress'], expected_course_progress) + + @patch( + 'lms.djangoapps.mobile_api.users.serializers.CourseEnrollmentSerializerModifiedForPrimary' + '.get_course_assignments' + ) + @patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignments') + def test_course_progress_for_secondary_enrollments_with_query_param_and_assignments( + self, + get_course_assignment_mock: MagicMock, + assignments_mock: MagicMock, + ) -> None: + self.login() + courses = [CourseFactory.create(org='edx', mobile_available=True) for _ in range(2)] + for course in courses: + self.enroll(course.id) + course_assignments_mock = [ + Mock(complete=False), Mock(complete=False), Mock(complete=True), Mock(complete=True), Mock(complete=True) + ] + get_course_assignment_mock.return_value = course_assignments_mock + student_assignments_mock = { + 'future_assignments': [], + 'past_assignments': [], + } + assignments_mock.return_value = student_assignments_mock + expected_course_progress = {'total_assignments_count': 5, 'assignments_completed': 3} + + response = self.api_response(api_version=API_V4, data={'requested_fields': 'course_progress'}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIn('course_progress', response.data['primary']) + self.assertDictEqual(response.data['primary']['course_progress'], expected_course_progress) + self.assertIn('course_progress', response.data['enrollments']['results'][0]) + self.assertDictEqual(response.data['enrollments']['results'][0]['course_progress'], expected_course_progress) + @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 b8949d269632..c9e9f725e7db 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -326,6 +326,10 @@ class UserCourseEnrollmentsList(generics.ListAPIView): * mode: The type of certificate registration for this course (honor or certified). * url: URL to the downloadable version of the certificate, if exists. + * course_progress: Contains information about how many assignments are in the course + and how many assignments the student has completed. + * total_assignments_count: Total course's assignments count. + * assignments_completed: Assignments witch the student has completed. """ lookup_field = 'username' From faa1fc4fd58c2fd31f9a0c388ca9e7077b522889 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Fri, 21 Jun 2024 16:51:42 +0300 Subject: [PATCH 11/18] refactor: [AXM-506] Move progress logic to core apps --- .../course_api/blocks/tests/test_views.py | 4 +- lms/djangoapps/courseware/courses.py | 62 +++++++++++++++++++ .../mobile_api/course_info/serializers.py | 22 +------ .../tests/test_course_info_serializers.py | 25 +++++--- .../mobile_api/users/serializers.py | 56 ++--------------- lms/djangoapps/mobile_api/users/tests.py | 10 +-- 6 files changed, 93 insertions(+), 86 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/tests/test_views.py b/lms/djangoapps/course_api/blocks/tests/test_views.py index 4b9823328114..f577762ad70f 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_views.py +++ b/lms/djangoapps/course_api/blocks/tests/test_views.py @@ -209,7 +209,7 @@ def test_not_authenticated_public_course_with_all_blocks(self): self.query_params['all_blocks'] = True self.verify_response(403) - @mock.patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[]) + @mock.patch('lms.djangoapps.courseware.courses.get_course_assignments', return_value=[]) @mock.patch("lms.djangoapps.course_api.blocks.forms.permissions.is_course_public", Mock(return_value=True)) def test_not_authenticated_public_course_with_blank_username(self, get_course_assignment_mock: MagicMock) -> None: """ @@ -369,7 +369,7 @@ def test_extra_field_when_not_requested(self): block_data['type'] == 'course' ) - @mock.patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[]) + @mock.patch('lms.djangoapps.courseware.courses.get_course_assignments', return_value=[]) def test_data_researcher_access(self, get_course_assignment_mock: MagicMock) -> None: """ Test if data researcher has access to the api endpoint diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index ee0d12ce1a52..7664362ac8b0 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -35,6 +35,7 @@ ) from lms.djangoapps.courseware.access_utils import check_authentication, check_data_sharing_consent, check_enrollment, \ check_correct_active_enterprise_customer, is_priority_access_error +from lms.djangoapps.courseware.context_processor import get_user_timezone_or_last_seen_timezone_or_utc from lms.djangoapps.courseware.courseware_access_exception import CoursewareAccessException from lms.djangoapps.courseware.date_summary import ( CertificateAvailableDate, @@ -1023,3 +1024,64 @@ def get_course_chapter_ids(course_key): log.exception('Failed to retrieve course from modulestore.') return [] return [str(chapter_key) for chapter_key in chapter_keys if chapter_key.block_type == 'chapter'] + + +def get_past_and_future_course_assignments(request, user, course): + """ + Returns the future assignment data and past assignments data for given user and course. + + Arguments: + request (Request): The HTTP GET request. + user (User): The user for whom the assignments are received. + course (Course): Course object for whom the assignments are received. + Returns: + tuple (list, list): Tuple of `past_assignments` list and `next_assignments` list. + `next_assignments` list contains only uncompleted assignments. + """ + assignments = get_course_assignment_date_blocks(course, user, request, include_past_dates=True) + past_assignments = [] + future_assignments = [] + + timezone = get_user_timezone_or_last_seen_timezone_or_utc(user) + for assignment in sorted(assignments, key=lambda x: x.date): + if assignment.date < datetime.now(timezone): + past_assignments.append(assignment) + else: + if not assignment.complete: + future_assignments.append(assignment) + + if future_assignments: + future_assignment_date = future_assignments[0].date.date() + next_assignments = [ + assignment for assignment in future_assignments if assignment.date.date() == future_assignment_date + ] + else: + next_assignments = [] + + return next_assignments, past_assignments + + +def calculate_progress(course_key, user): + """ + Calculate the progress of the user in the course by assignments. + + Arguments: + course_key (CourseLocator): The Course for which course progress is requested. + user (User): The user for whom course progress is requested. + Returns: + dict (dict): Dictionary contains information about total assignments count + in the given course and how many assignments the user has completed. + """ + course_assignments = get_course_assignments(course_key, user, include_without_due=True) + + total_assignments_count = 0 + assignments_completed = 0 + + if course_assignments: + total_assignments_count = len(course_assignments) + assignments_completed = len([assignment for assignment in course_assignments if assignment.complete]) + + return { + 'total_assignments_count': total_assignments_count, + 'assignments_completed': assignments_completed, + } diff --git a/lms/djangoapps/mobile_api/course_info/serializers.py b/lms/djangoapps/mobile_api/course_info/serializers.py index d5ad89eb69a5..b5077c8f92cd 100644 --- a/lms/djangoapps/mobile_api/course_info/serializers.py +++ b/lms/djangoapps/mobile_api/course_info/serializers.py @@ -13,7 +13,7 @@ from lms.djangoapps.courseware.access import has_access from lms.djangoapps.courseware.access import administrative_accesses_to_course_for_user from lms.djangoapps.courseware.access_utils import check_course_open_for_learner -from lms.djangoapps.courseware.courses import get_course_assignments +from lms.djangoapps.courseware.courses import calculate_progress from lms.djangoapps.mobile_api.users.serializers import ModeSerializer from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.features.course_duration_limits.access import get_user_course_expiration_date @@ -78,27 +78,11 @@ def get_course_modes(self, course_overview): for mode in course_modes ] - def get_course_progress(self, obj: 'CourseOverview') -> Dict[str, int]: # noqa: F821 #here + def get_course_progress(self, obj: 'CourseOverview') -> Dict[str, int]: # noqa: F821 """ Gets course progress calculated by course assignments. """ - course_assignments = get_course_assignments( - obj.id, - self.context.get('user'), - include_without_due=True, - ) - - total_assignments_count = 0 - assignments_completed = 0 - - if course_assignments: - total_assignments_count = len(course_assignments) - assignments_completed = len([assignment for assignment in course_assignments if assignment.complete]) - - return { - 'total_assignments_count': total_assignments_count, - 'assignments_completed': assignments_completed, - } + return calculate_progress(obj.id, self.context.get('user')) class MobileCourseEnrollmentSerializer(serializers.ModelSerializer): diff --git a/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py b/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py index c18d22f0ae99..660f0c69318d 100644 --- a/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py +++ b/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py @@ -147,8 +147,8 @@ def setUp(self): self.user = UserFactory() self.course_overview = CourseOverviewFactory() - @patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[]) - def test_get_media(self, get_course_assignment_mock: MagicMock) -> None: + @patch('lms.djangoapps.mobile_api.course_info.serializers.calculate_progress') + def test_get_media(self, calculate_progress_mock: MagicMock) -> None: output_data = CourseInfoOverviewSerializer(self.course_overview, context={'user': self.user}).data self.assertIn('media', output_data) @@ -157,8 +157,11 @@ def test_get_media(self, get_course_assignment_mock: MagicMock) -> None: self.assertIn('small', output_data['media']['image']) self.assertIn('large', output_data['media']['image']) - @patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[]) - @patch('lms.djangoapps.mobile_api.course_info.serializers.get_link_for_about_page', return_value='mock_about_link') + @patch('lms.djangoapps.mobile_api.course_info.serializers.calculate_progress') + @patch( + 'lms.djangoapps.mobile_api.course_info.serializers.get_link_for_about_page', + return_value='mock_about_link' + ) def test_get_course_sharing_utm_parameters( self, mock_get_link_for_about_page: MagicMock, @@ -169,7 +172,7 @@ def test_get_course_sharing_utm_parameters( self.assertEqual(output_data['course_about'], mock_get_link_for_about_page.return_value) mock_get_link_for_about_page.assert_called_once_with(self.course_overview) - @patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[]) + @patch('lms.djangoapps.mobile_api.course_info.serializers.calculate_progress') def test_get_course_modes(self, get_course_assignment_mock: MagicMock) -> None: expected_course_modes = [{'slug': 'audit', 'sku': None, 'android_sku': None, 'ios_sku': None, 'min_price': 0}] @@ -177,7 +180,7 @@ def test_get_course_modes(self, get_course_assignment_mock: MagicMock) -> None: self.assertListEqual(output_data['course_modes'], expected_course_modes) - @patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments', return_value=[]) + @patch('lms.djangoapps.courseware.courses.get_course_assignments') def test_get_course_progress_no_assignments(self, get_course_assignment_mock: MagicMock) -> None: expected_course_progress = {'total_assignments_count': 0, 'assignments_completed': 0} @@ -185,9 +188,11 @@ def test_get_course_progress_no_assignments(self, get_course_assignment_mock: Ma self.assertIn('course_progress', output_data) self.assertDictEqual(output_data['course_progress'], expected_course_progress) - get_course_assignment_mock.assert_called_once_with(self.course_overview.id, self.user, include_without_due=True) + get_course_assignment_mock.assert_called_once_with( + self.course_overview.id, self.user, include_without_due=True + ) - @patch('lms.djangoapps.mobile_api.course_info.serializers.get_course_assignments') + @patch('lms.djangoapps.courseware.courses.get_course_assignments') def test_get_course_progress_with_assignments(self, get_course_assignment_mock: MagicMock) -> None: assignments_mock = [ Mock(complete=False), Mock(complete=False), Mock(complete=True), Mock(complete=True), Mock(complete=True) @@ -199,4 +204,6 @@ def test_get_course_progress_with_assignments(self, get_course_assignment_mock: self.assertIn('course_progress', output_data) self.assertDictEqual(output_data['course_progress'], expected_course_progress) - get_course_assignment_mock.assert_called_once_with(self.course_overview.id, self.user, include_without_due=True) + get_course_assignment_mock.assert_called_once_with( + self.course_overview.id, self.user, include_without_due=True + ) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index dd82d9f99e49..cb3cfb6cf0b0 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -2,7 +2,6 @@ Serializer for user API """ -from datetime import datetime from typing import Dict, List, Optional, Tuple, Union from completion.exceptions import UnavailableCompletionData @@ -17,8 +16,7 @@ from common.djangoapps.util.course import get_encoded_course_sharing_utm_params, get_link_for_about_page from lms.djangoapps.certificates.api import certificate_downloadable_status from lms.djangoapps.courseware.access import has_access -from lms.djangoapps.courseware.context_processor import get_user_timezone_or_last_seen_timezone_or_utc -from lms.djangoapps.courseware.courses import get_course_assignment_date_blocks, get_course_assignments +from lms.djangoapps.courseware.courses import get_past_and_future_course_assignments, calculate_progress from lms.djangoapps.course_home_api.dates.serializers import DateSummarySerializer from lms.djangoapps.mobile_api.utils import API_V4 from openedx.features.course_duration_limits.access import get_user_course_expiration_date @@ -143,32 +141,10 @@ def to_representation(self, instance: CourseEnrollment) -> 'OrderedDict': # noq data = super().to_representation(instance) if 'course_progress' in self.context.get('requested_fields', []) and self.context.get('api_version') == API_V4: - data['course_progress'] = self.calculate_progress(instance) + data['course_progress'] = calculate_progress(instance.course_id, instance.user) return data - def calculate_progress(self, model: CourseEnrollment) -> Dict[str, int]: - """ - Calculate the progress of the user in the course. - """ - course_assignments = get_course_assignments( - model.course_id, - model.user, - include_without_due=True, - ) - - total_assignments_count = 0 - assignments_completed = 0 - - if course_assignments: - total_assignments_count = len(course_assignments) - assignments_completed = len([assignment for assignment in course_assignments if assignment.complete]) - - return { - 'total_assignments_count': total_assignments_count, - 'assignments_completed': assignments_completed, - } - class Meta: model = CourseEnrollment fields = ('audit_access_expires', 'created', 'mode', 'is_active', 'course', 'certificate', 'course_modes') @@ -250,37 +226,15 @@ def get_course_progress(self, model: CourseEnrollment) -> Dict[str, int]: """ Returns the progress of the user in the course. """ - return self.calculate_progress(model) + return calculate_progress(model.course_id, model.user) def get_course_assignments(self, model: CourseEnrollment) -> Dict[str, Optional[List[Dict[str, str]]]]: """ Returns the future assignment data and past assignments data for the user in the course. """ - assignments = get_course_assignment_date_blocks( - self.course, - model.user, - self.context.get('request'), - include_past_dates=True + next_assignments, past_assignments = get_past_and_future_course_assignments( + self.context.get('request'), model.user, self.course ) - past_assignments = [] - future_assignments = [] - - timezone = get_user_timezone_or_last_seen_timezone_or_utc(model.user) - for assignment in sorted(assignments, key=lambda x: x.date): - if assignment.date < datetime.now(timezone): - past_assignments.append(assignment) - else: - if not assignment.complete: - future_assignments.append(assignment) - - if future_assignments: - future_assignment_date = future_assignments[0].date.date() - next_assignments = [ - assignment for assignment in future_assignments if assignment.date.date() == future_assignment_date - ] - else: - next_assignments = [] - return { 'future_assignments': DateSummarySerializer(next_assignments, many=True).data, 'past_assignments': DateSummarySerializer(past_assignments, many=True).data, diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 71199db1b0fe..9bc3fa542113 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -906,7 +906,7 @@ def test_response_contains_primary_enrollment_assignments_info(self): self.assertListEqual(response.data['primary']['course_assignments']['past_assignments'], []) self.assertListEqual(response.data['primary']['course_assignments']['future_assignments'], []) - @patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignments', return_value=[]) + @patch('lms.djangoapps.courseware.courses.get_course_assignments', return_value=[]) def test_course_progress_in_primary_enrollment_with_no_assignments( self, get_course_assignment_mock: MagicMock, @@ -926,7 +926,7 @@ def test_course_progress_in_primary_enrollment_with_no_assignments( 'lms.djangoapps.mobile_api.users.serializers.CourseEnrollmentSerializerModifiedForPrimary' '.get_course_assignments' ) - @patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignments') + @patch('lms.djangoapps.courseware.courses.get_course_assignments') def test_course_progress_in_primary_enrollment_with_assignments( self, get_course_assignment_mock: MagicMock, @@ -952,7 +952,7 @@ def test_course_progress_in_primary_enrollment_with_assignments( self.assertIn('course_progress', response.data['primary']) self.assertDictEqual(response.data['primary']['course_progress'], expected_course_progress) - @patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignments') + @patch('lms.djangoapps.courseware.courses.get_course_assignments') def test_course_progress_for_secondary_enrollments_no_query_param( self, get_course_assignment_mock: MagicMock, @@ -968,7 +968,7 @@ def test_course_progress_for_secondary_enrollments_no_query_param( for enrollment in response.data['enrollments']['results']: self.assertNotIn('course_progress', enrollment) - @patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignments') + @patch('lms.djangoapps.courseware.courses.get_course_assignments') def test_course_progress_for_secondary_enrollments_with_query_param( self, get_course_assignment_mock: MagicMock, @@ -990,7 +990,7 @@ def test_course_progress_for_secondary_enrollments_with_query_param( 'lms.djangoapps.mobile_api.users.serializers.CourseEnrollmentSerializerModifiedForPrimary' '.get_course_assignments' ) - @patch('lms.djangoapps.mobile_api.users.serializers.get_course_assignments') + @patch('lms.djangoapps.courseware.courses.get_course_assignments') def test_course_progress_for_secondary_enrollments_with_query_param_and_assignments( self, get_course_assignment_mock: MagicMock, From 0ff9f55b8c4b00368d936f95587fb52178699275 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Fri, 21 Jun 2024 19:35:40 +0300 Subject: [PATCH 12/18] style: [AXM-506] Replace noqa to pylint disable --- lms/djangoapps/mobile_api/course_info/serializers.py | 2 +- lms/djangoapps/mobile_api/course_info/utils.py | 6 +++--- lms/djangoapps/mobile_api/users/serializers.py | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/mobile_api/course_info/serializers.py b/lms/djangoapps/mobile_api/course_info/serializers.py index b5077c8f92cd..85ad1b58524d 100644 --- a/lms/djangoapps/mobile_api/course_info/serializers.py +++ b/lms/djangoapps/mobile_api/course_info/serializers.py @@ -78,7 +78,7 @@ def get_course_modes(self, course_overview): for mode in course_modes ] - def get_course_progress(self, obj: 'CourseOverview') -> Dict[str, int]: # noqa: F821 + def get_course_progress(self, obj: CourseOverview) -> Dict[str, int]: """ Gets course progress calculated by course assignments. """ diff --git a/lms/djangoapps/mobile_api/course_info/utils.py b/lms/djangoapps/mobile_api/course_info/utils.py index 141c32da4cf4..8f06bce79e29 100644 --- a/lms/djangoapps/mobile_api/course_info/utils.py +++ b/lms/djangoapps/mobile_api/course_info/utils.py @@ -15,10 +15,10 @@ def calculate_progress( - user: 'User', # noqa: F821 - course_id: 'CourseLocator', # noqa: F821 + user: 'User', # lint-amnesty, pylint: disable=unused-variable + course_id: 'CourseLocator', # lint-amnesty, pylint: disable=unused-variable cache_timeout: int, -) -> Optional[List[Union['ReadSubsectionGrade', 'ZeroSubsectionGrade']]]: # noqa: F821 +) -> Optional[List[Union['ReadSubsectionGrade', 'ZeroSubsectionGrade']]]: # lint-amnesty, pylint: disable=unused-variable, line-too-long """ Calculate the progress of the user in the course. """ diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index cb3cfb6cf0b0..45b3170e9281 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -134,7 +134,7 @@ def get_course_modes(self, obj): for mode in course_modes ] - def to_representation(self, instance: CourseEnrollment) -> 'OrderedDict': # noqa: F821 + def to_representation(self, instance: CourseEnrollment) -> 'OrderedDict': # lint-amnesty, pylint: disable=unused-variable, line-too-long """ Override the to_representation method to add the course_status field to the serialized data. """ @@ -205,7 +205,7 @@ def get_course_status(self, model: CourseEnrollment) -> Optional[Dict[str, List[ @staticmethod def _get_last_visited_block_path_and_unit_name( block_id: str - ) -> Union[Tuple[None, None], Tuple[List['XBlock'], str]]: # noqa: F821 + ) -> Union[Tuple[None, None], Tuple[List['XBlock'], str]]: # lint-amnesty, pylint: disable=unused-variable """ Returns the path to the latest block and unit name visited by the current user. """ From 8fa5ef39c06e63b8e620a707104100c8f9a5492d Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Tue, 25 Jun 2024 23:13:52 +0300 Subject: [PATCH 13/18] refactor: [AXM-506] Refactor get_course_assignments func --- lms/djangoapps/courseware/courses.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 7664362ac8b0..522499475e69 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -609,10 +609,7 @@ def get_course_assignments(course_key, user, include_access=False, include_witho due = block_data.get_xblock_field(subsection_key, 'due') graded = block_data.get_xblock_field(subsection_key, 'graded', False) - if not due and include_without_due: - due = now + timedelta(days=1000) - - if due and graded: + if (due or include_without_due) and graded: first_component_block_id = get_first_component_of_block(subsection_key, block_data) contains_gated_content = include_access and block_data.get_xblock_field( subsection_key, 'contains_gated_content', False) @@ -629,7 +626,11 @@ def get_course_assignments(course_key, user, include_access=False, include_witho else: complete = False - past_due = not complete and due < now + if due: + past_due = not complete and due < now + else: + past_due = False + due = None assignments.append(_Assignment( subsection_key, title, url, due, contains_gated_content, complete, past_due, assignment_type, None, first_component_block_id From 7aa27c29c5d8bcd2ff5f808f628f87c35f2be44e Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Wed, 26 Jun 2024 00:01:29 +0300 Subject: [PATCH 14/18] style: [AXM-506] Remove unused import --- lms/djangoapps/courseware/courses.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 522499475e69..d7cf05cde7f1 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -5,7 +5,7 @@ import logging from collections import defaultdict, namedtuple -from datetime import datetime, timedelta +from datetime import datetime import six import pytz From 7c53063b3406e1b44df99416caa4a5d859e26c3c Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Mon, 1 Jul 2024 17:28:01 +0300 Subject: [PATCH 15/18] refactor: [AXM-506] Fix code review issues --- .../mobile_api/users/serializers.py | 49 +++++-------------- lms/djangoapps/mobile_api/users/tests.py | 4 +- lms/djangoapps/mobile_api/users/views.py | 27 +++++----- .../features/course_duration_limits/access.py | 2 +- 4 files changed, 26 insertions(+), 56 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 45b3170e9281..174164d00a97 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -2,11 +2,10 @@ Serializer for user API """ -from typing import Dict, List, Optional, Tuple, Union +from typing import Dict, List, Optional from completion.exceptions import UnavailableCompletionData from completion.utilities import get_key_to_last_completed_block -from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import UsageKey from rest_framework import serializers from rest_framework.reverse import reverse @@ -21,7 +20,8 @@ from lms.djangoapps.mobile_api.utils import API_V4 from openedx.features.course_duration_limits.access import get_user_course_expiration_date from xmodule.modulestore.django import modulestore -from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem +from xmodule.modulestore.search import path_to_location class CourseOverviewField(serializers.RelatedField): # lint-amnesty, pylint: disable=abstract-method @@ -182,46 +182,21 @@ def get_course_status(self, model: CourseEnrollment) -> Optional[Dict[str, List[ Gets course status for the given user's enrollments. """ try: - block_id = str(get_key_to_last_completed_block(model.user, model.course.id)) - except UnavailableCompletionData: - block_id = '' - - if not block_id: - return None - - path, unit_name = self._get_last_visited_block_path_and_unit_name(block_id) - if not path and unit_name: + block_key = get_key_to_last_completed_block(model.user, model.course.id) + path = path_to_location(modulestore(), block_key, self.context['request'], full_path=True) + except (ItemNotFoundError, NoPathToItem, UnavailableCompletionData): return None - path_ids = [str(block.location) for block in path] + path_ids = [str(block) for block in path] + unit = modulestore().get_item(UsageKey.from_string(path_ids[3]), depth=0) return { - 'last_visited_module_id': path_ids[0], - 'last_visited_module_path': path_ids, - 'last_visited_block_id': block_id, - 'last_visited_unit_display_name': unit_name, + 'last_visited_module_id': path_ids[2], + 'last_visited_module_path': path_ids[:3], + 'last_visited_block_id': path_ids[-1], + 'last_visited_unit_display_name': unit.display_name, } - @staticmethod - def _get_last_visited_block_path_and_unit_name( - block_id: str - ) -> Union[Tuple[None, None], Tuple[List['XBlock'], str]]: # lint-amnesty, pylint: disable=unused-variable - """ - Returns the path to the latest block and unit name visited by the current user. - """ - try: - last_visited_block = modulestore().get_item(UsageKey.from_string(block_id)) - vertical = last_visited_block.get_parent() - sequential = vertical.get_parent() - chapter = sequential.get_parent() - course = chapter.get_parent() - except (ItemNotFoundError, InvalidKeyError, AttributeError): - return None, None - - path = [sequential, chapter, course] - - return path, vertical.display_name - def get_course_progress(self, model: CourseEnrollment) -> Dict[str, int]: """ Returns the progress of the user in the course. diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 9bc3fa542113..6cd5e3d29a4e 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -684,9 +684,9 @@ def test_course_status_in_primary_obj_when_student_have_progress( expected_course_status = { 'last_visited_module_id': str(subsection.location), 'last_visited_module_path': [ - str(subsection.location), + str(course.location), str(section.location), - str(course.location) + str(subsection.location), ], 'last_visited_block_id': str(problem.location), 'last_visited_unit_display_name': vertical.display_name, diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index c9e9f725e7db..aed654233cdb 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -391,7 +391,7 @@ def queryset(self): def get_queryset(self): api_version = self.kwargs.get('api_version') status = self.request.GET.get('status') - mobile_available = self.get_mobile_available_enrollments() + mobile_available = self.get_same_org_mobile_available_enrollments() not_duration_limited = ( enrollment for enrollment in mobile_available @@ -410,7 +410,7 @@ def get_queryset(self): # return all courses, with associated expiration return mobile_available - def get_mobile_available_enrollments(self) -> List[Optional[CourseEnrollment]]: + def get_same_org_mobile_available_enrollments(self) -> List[Optional[CourseEnrollment]]: """ Gets list with `CourseEnrollment` for mobile available courses. """ @@ -438,16 +438,13 @@ def list(self, request, *args, **kwargs): 'enrollments': response.data } if api_version == API_V4 and status not in EnrollmentStatuses.values(): - if status in EnrollmentStatuses.values(): - enrollment_data.update({'primary': None}) - else: - primary_enrollment_obj = self.get_primary_enrollment_by_latest_enrollment_or_progress() - if primary_enrollment_obj: - serializer = CourseEnrollmentSerializerModifiedForPrimary( - primary_enrollment_obj, - context=self.get_serializer_context(), - ) - enrollment_data.update({'primary': serializer.data}) + primary_enrollment_obj = self.get_primary_enrollment_by_latest_enrollment_or_progress() + if primary_enrollment_obj: + serializer = CourseEnrollmentSerializerModifiedForPrimary( + primary_enrollment_obj, + context=self.get_serializer_context(), + ) + enrollment_data.update({'primary': serializer.data}) return Response(enrollment_data) @@ -470,16 +467,14 @@ def get_primary_enrollment_by_latest_enrollment_or_progress(self) -> Optional[Co """ Gets primary enrollment obj by latest enrollment or latest progress on the course. """ - mobile_available = self.get_mobile_available_enrollments() + mobile_available = self.get_same_org_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, + course__id__in=mobile_available_course_ids ).order_by('-created').first() if not latest_enrollment: diff --git a/openedx/features/course_duration_limits/access.py b/openedx/features/course_duration_limits/access.py index 08a94702a5e0..8f73ae2266ea 100644 --- a/openedx/features/course_duration_limits/access.py +++ b/openedx/features/course_duration_limits/access.py @@ -81,7 +81,7 @@ def get_user_course_expiration_date(user, course, enrollment=None): if access_duration is None: return None - enrollment = CourseEnrollment.get_enrollment(user, course.id) if not enrollment else enrollment + enrollment = enrollment or CourseEnrollment.get_enrollment(user, course.id) if enrollment is None or enrollment.mode != CourseMode.AUDIT: return None From eee5e7b60d94488e5859ffd4da38ad5240d67aba Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Tue, 2 Jul 2024 13:38:26 +0300 Subject: [PATCH 16/18] refactor: [AXM-506] Rename progress calculation functions --- lms/djangoapps/courseware/courses.py | 37 +++++++++++++++- .../mobile_api/course_info/serializers.py | 4 +- .../mobile_api/course_info/utils.py | 43 ------------------- .../mobile_api/course_info/views.py | 5 +-- .../tests/test_course_info_serializers.py | 12 +++--- .../mobile_api/users/serializers.py | 6 +-- lms/djangoapps/mobile_api/users/views.py | 4 +- 7 files changed, 51 insertions(+), 60 deletions(-) delete mode 100644 lms/djangoapps/mobile_api/course_info/utils.py diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index d7cf05cde7f1..948f3bd5db44 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -12,6 +12,7 @@ from crum import get_current_request from dateutil.parser import parse as parse_date from django.conf import settings +from django.core.cache import cache from django.http import Http404, QueryDict from django.urls import reverse from django.utils.translation import gettext as _ @@ -51,7 +52,9 @@ from lms.djangoapps.courseware.masquerade import check_content_start_date_for_masquerade_user from lms.djangoapps.courseware.model_data import FieldDataCache from lms.djangoapps.courseware.block_render import get_block +from lms.djangoapps.grades.api import CourseGradeFactory from lms.djangoapps.survey.utils import SurveyRequiredAccessError, check_survey_required_and_unanswered +from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.enrollments.api import get_course_enrollment_details from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers @@ -770,6 +773,38 @@ def _ora_assessment_to_assignment( ) +def get_assignments_grades(user, course_id, cache_timeout): + """ + Calculate the progress of the assignment for the user in the course. + + Arguments: + user (User): Django User object. + course_id (CourseLocator): The course key. + cache_timeout (int): Cache timeout in seconds + Returns: + list (ReadSubsectionGrade, ZeroSubsectionGrade): The list with assignments grades. + """ + is_staff = bool(has_access(user, 'staff', course_id)) + + try: + cache_key = f'course_block_structure_{str(course_id)}_{user.id}' + collected_block_structure = cache.get(cache_key) + if not collected_block_structure: + collected_block_structure = get_block_structure_manager(course_id).get_collected() + cache.set(cache_key, collected_block_structure, cache_timeout) + + course_grade = CourseGradeFactory().read(user, collected_block_structure=collected_block_structure) + + # recalculate course grade from visible grades (stored grade was calculated over all grades, visible or not) + course_grade.update(visible_grades_only=True, has_staff_access=is_staff) + subsection_grades = list(course_grade.subsection_grades.values()) + except Exception as err: # pylint: disable=broad-except + log.warning(f'Could not get grades for the course: {course_id}, error: {err}') + return [] + + return subsection_grades + + def get_first_component_of_block(block_key, block_data): """ This function returns the first leaf block of a section(block_key) @@ -1062,7 +1097,7 @@ def get_past_and_future_course_assignments(request, user, course): return next_assignments, past_assignments -def calculate_progress(course_key, user): +def get_assignments_completions(course_key, user): """ Calculate the progress of the user in the course by assignments. diff --git a/lms/djangoapps/mobile_api/course_info/serializers.py b/lms/djangoapps/mobile_api/course_info/serializers.py index 85ad1b58524d..40b3cc8ae61a 100644 --- a/lms/djangoapps/mobile_api/course_info/serializers.py +++ b/lms/djangoapps/mobile_api/course_info/serializers.py @@ -13,7 +13,7 @@ from lms.djangoapps.courseware.access import has_access from lms.djangoapps.courseware.access import administrative_accesses_to_course_for_user from lms.djangoapps.courseware.access_utils import check_course_open_for_learner -from lms.djangoapps.courseware.courses import calculate_progress +from lms.djangoapps.courseware.courses import get_assignments_completions from lms.djangoapps.mobile_api.users.serializers import ModeSerializer from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.features.course_duration_limits.access import get_user_course_expiration_date @@ -82,7 +82,7 @@ def get_course_progress(self, obj: CourseOverview) -> Dict[str, int]: """ Gets course progress calculated by course assignments. """ - return calculate_progress(obj.id, self.context.get('user')) + return get_assignments_completions(obj.id, self.context.get('user')) class MobileCourseEnrollmentSerializer(serializers.ModelSerializer): diff --git a/lms/djangoapps/mobile_api/course_info/utils.py b/lms/djangoapps/mobile_api/course_info/utils.py deleted file mode 100644 index 8f06bce79e29..000000000000 --- a/lms/djangoapps/mobile_api/course_info/utils.py +++ /dev/null @@ -1,43 +0,0 @@ -""" -Common utils for the `course_info` API. -""" - -import logging -from typing import List, Optional, Union - -from django.core.cache import cache - -from lms.djangoapps.courseware.access import has_access -from lms.djangoapps.grades.api import CourseGradeFactory -from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager - -log = logging.getLogger(__name__) - - -def calculate_progress( - user: 'User', # lint-amnesty, pylint: disable=unused-variable - course_id: 'CourseLocator', # lint-amnesty, pylint: disable=unused-variable - cache_timeout: int, -) -> Optional[List[Union['ReadSubsectionGrade', 'ZeroSubsectionGrade']]]: # lint-amnesty, pylint: disable=unused-variable, line-too-long - """ - Calculate the progress of the user in the course. - """ - is_staff = bool(has_access(user, 'staff', course_id)) - - try: - cache_key = f'course_block_structure_{str(course_id)}_{user.id}' - collected_block_structure = cache.get(cache_key) - if not collected_block_structure: - collected_block_structure = get_block_structure_manager(course_id).get_collected() - cache.set(cache_key, collected_block_structure, cache_timeout) - - course_grade = CourseGradeFactory().read(user, collected_block_structure=collected_block_structure) - - # recalculate course grade from visible grades (stored grade was calculated over all grades, visible or not) - course_grade.update(visible_grades_only=True, has_staff_access=is_staff) - subsection_grades = list(course_grade.subsection_grades.values()) - except Exception as err: # pylint: disable=broad-except - log.warning(f'Could not get grades for the course: {course_id}, error: {err}') - return [] - - return subsection_grades diff --git a/lms/djangoapps/mobile_api/course_info/views.py b/lms/djangoapps/mobile_api/course_info/views.py index 40d586839680..0279c59cf55c 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -17,7 +17,7 @@ from common.djangoapps.student.models import CourseEnrollment, User as StudentUser from common.djangoapps.static_replace import make_static_urls_absolute from lms.djangoapps.certificates.api import certificate_downloadable_status -from lms.djangoapps.courseware.courses import get_course_info_section_block +from lms.djangoapps.courseware.courses import get_assignments_grades, get_course_info_section_block from lms.djangoapps.course_goals.models import UserActivity from lms.djangoapps.course_api.blocks.views import BlocksInCourseView from lms.djangoapps.mobile_api.course_info.constants import BLOCK_STRUCTURE_CACHE_TIMEOUT @@ -26,7 +26,6 @@ CourseAccessSerializer, MobileCourseEnrollmentSerializer ) -from lms.djangoapps.mobile_api.course_info.utils import calculate_progress from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib.api.view_utils import view_auth_classes from openedx.core.lib.xblock_utils import get_course_update_items @@ -403,7 +402,7 @@ def _extend_sequential_info_with_assignment_progress( """ Extends sequential xblock info with assignment's name and progress. """ - subsection_grades = calculate_progress(requested_user, course_id, BLOCK_STRUCTURE_CACHE_TIMEOUT) + subsection_grades = get_assignments_grades(requested_user, course_id, BLOCK_STRUCTURE_CACHE_TIMEOUT) grades_with_locations = {str(grade.location): grade for grade in subsection_grades} for block_id, block_info in blocks_info_data.items(): diff --git a/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py b/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py index 660f0c69318d..1ab1ba988e9c 100644 --- a/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py +++ b/lms/djangoapps/mobile_api/tests/test_course_info_serializers.py @@ -147,8 +147,8 @@ def setUp(self): self.user = UserFactory() self.course_overview = CourseOverviewFactory() - @patch('lms.djangoapps.mobile_api.course_info.serializers.calculate_progress') - def test_get_media(self, calculate_progress_mock: MagicMock) -> None: + @patch('lms.djangoapps.mobile_api.course_info.serializers.get_assignments_completions') + def test_get_media(self, get_assignments_completions_mock: MagicMock) -> None: output_data = CourseInfoOverviewSerializer(self.course_overview, context={'user': self.user}).data self.assertIn('media', output_data) @@ -157,7 +157,7 @@ def test_get_media(self, calculate_progress_mock: MagicMock) -> None: self.assertIn('small', output_data['media']['image']) self.assertIn('large', output_data['media']['image']) - @patch('lms.djangoapps.mobile_api.course_info.serializers.calculate_progress') + @patch('lms.djangoapps.mobile_api.course_info.serializers.get_assignments_completions') @patch( 'lms.djangoapps.mobile_api.course_info.serializers.get_link_for_about_page', return_value='mock_about_link' @@ -165,15 +165,15 @@ def test_get_media(self, calculate_progress_mock: MagicMock) -> None: def test_get_course_sharing_utm_parameters( self, mock_get_link_for_about_page: MagicMock, - get_course_assignment_mock: MagicMock, + get_assignments_completions_mock: MagicMock, ) -> None: output_data = CourseInfoOverviewSerializer(self.course_overview, context={'user': self.user}).data self.assertEqual(output_data['course_about'], mock_get_link_for_about_page.return_value) mock_get_link_for_about_page.assert_called_once_with(self.course_overview) - @patch('lms.djangoapps.mobile_api.course_info.serializers.calculate_progress') - def test_get_course_modes(self, get_course_assignment_mock: MagicMock) -> None: + @patch('lms.djangoapps.mobile_api.course_info.serializers.get_assignments_completions') + def test_get_course_modes(self, get_assignments_completions_mock: MagicMock) -> None: expected_course_modes = [{'slug': 'audit', 'sku': None, 'android_sku': None, 'ios_sku': None, 'min_price': 0}] output_data = CourseInfoOverviewSerializer(self.course_overview, context={'user': self.user}).data diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 174164d00a97..d8de11e50ff8 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -15,7 +15,7 @@ from common.djangoapps.util.course import get_encoded_course_sharing_utm_params, get_link_for_about_page from lms.djangoapps.certificates.api import certificate_downloadable_status from lms.djangoapps.courseware.access import has_access -from lms.djangoapps.courseware.courses import get_past_and_future_course_assignments, calculate_progress +from lms.djangoapps.courseware.courses import get_assignments_completions, get_past_and_future_course_assignments from lms.djangoapps.course_home_api.dates.serializers import DateSummarySerializer from lms.djangoapps.mobile_api.utils import API_V4 from openedx.features.course_duration_limits.access import get_user_course_expiration_date @@ -141,7 +141,7 @@ def to_representation(self, instance: CourseEnrollment) -> 'OrderedDict': # lin data = super().to_representation(instance) if 'course_progress' in self.context.get('requested_fields', []) and self.context.get('api_version') == API_V4: - data['course_progress'] = calculate_progress(instance.course_id, instance.user) + data['course_progress'] = get_assignments_completions(instance.course_id, instance.user) return data @@ -201,7 +201,7 @@ def get_course_progress(self, model: CourseEnrollment) -> Dict[str, int]: """ Returns the progress of the user in the course. """ - return calculate_progress(model.course_id, model.user) + return get_assignments_completions(model.course_id, model.user) def get_course_assignments(self, model: CourseEnrollment) -> Dict[str, Optional[List[Dict[str, str]]]]: """ diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index aed654233cdb..44900b53b03f 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -5,7 +5,7 @@ import logging from functools import cached_property -from typing import List, Optional +from typing import Optional from completion.exceptions import UnavailableCompletionData from completion.utilities import get_key_to_last_completed_block @@ -410,7 +410,7 @@ def get_queryset(self): # return all courses, with associated expiration return mobile_available - def get_same_org_mobile_available_enrollments(self) -> List[Optional[CourseEnrollment]]: + def get_same_org_mobile_available_enrollments(self) -> list[CourseEnrollment]: """ Gets list with `CourseEnrollment` for mobile available courses. """ From 12a6866eb57f148ca0ce259f2efb719a77b4b978 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Wed, 3 Jul 2024 17:29:19 +0300 Subject: [PATCH 17/18] refactor: [AXM-506] Refactor user's enrolments API --- .../student/models/course_enrollment.py | 38 ++++++++++--------- lms/djangoapps/courseware/courses.py | 3 +- lms/djangoapps/mobile_api/users/views.py | 14 +++---- 3 files changed, 29 insertions(+), 26 deletions(-) diff --git a/common/djangoapps/student/models/course_enrollment.py b/common/djangoapps/student/models/course_enrollment.py index 9fd53c1cd796..09862916e321 100644 --- a/common/djangoapps/student/models/course_enrollment.py +++ b/common/djangoapps/student/models/course_enrollment.py @@ -140,50 +140,52 @@ def active(self): """ return self.filter(is_active=True) - def without_certificates(self, user_username): + def without_certificates(self, username): """ Returns a queryset of CourseEnrollment objects for courses that do not have a certificate. """ - from lms.djangoapps.certificates.models import GeneratedCertificate # pylint: disable=import-outside-toplevel - course_ids_with_certificates = GeneratedCertificate.objects.filter( - user__username=user_username - ).values_list('course_id', flat=True) - return self.exclude(course_id__in=course_ids_with_certificates) + return self.exclude(course_id__in=self.get_user_course_ids_with_certificates(username)) - def with_certificates(self, user_username): + def with_certificates(self, username): """ Returns a queryset of CourseEnrollment objects for courses that have a certificate. """ - from lms.djangoapps.certificates.models import GeneratedCertificate # pylint: disable=import-outside-toplevel - course_ids_with_certificates = GeneratedCertificate.objects.filter( - user__username=user_username - ).values_list('course_id', flat=True) - return self.filter(course_id__in=course_ids_with_certificates) + return self.filter(course_id__in=self.get_user_course_ids_with_certificates(username)) - def in_progress(self, user_username, time_zone=UTC): + def in_progress(self, username, time_zone=UTC): """ Returns a queryset of CourseEnrollment objects for courses that are currently in progress. """ now = datetime.now(time_zone) - return self.active().without_certificates(user_username).filter( + return self.active().without_certificates(username).filter( Q(course__start__lte=now, course__end__gte=now) | Q(course__start__isnull=True, course__end__isnull=True) | Q(course__start__isnull=True, course__end__gte=now) | Q(course__start__lte=now, course__end__isnull=True), ) - def completed(self, user_username): + def completed(self, username): """ Returns a queryset of CourseEnrollment objects for courses that have been completed. """ - return self.active().with_certificates(user_username) + return self.active().with_certificates(username) - def expired(self, user_username, time_zone=UTC): + def expired(self, username, time_zone=UTC): """ Returns a queryset of CourseEnrollment objects for courses that have expired. """ now = datetime.now(time_zone) - return self.active().without_certificates(user_username).filter(course__end__lt=now) + return self.active().without_certificates(username).filter(course__end__lt=now) + + def get_user_course_ids_with_certificates(self, username): + """ + Gets user's course ids with certificates. + """ + from lms.djangoapps.certificates.models import GeneratedCertificate # pylint: disable=import-outside-toplevel + course_ids_with_certificates = GeneratedCertificate.objects.filter( + user__username=username + ).values_list('course_id', flat=True) + return course_ids_with_certificates class CourseEnrollmentManager(models.Manager): diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 948f3bd5db44..0ff6a11a627b 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -787,7 +787,8 @@ def get_assignments_grades(user, course_id, cache_timeout): is_staff = bool(has_access(user, 'staff', course_id)) try: - cache_key = f'course_block_structure_{str(course_id)}_{user.id}' + course = get_course_with_access(user, 'load', course_id) + cache_key = f'course_block_structure_{str(course_id)}_{str(course.course_version)}_{user.id}' collected_block_structure = cache.get(cache_key) if not collected_block_structure: collected_block_structure = get_block_structure_manager(course_id).get_collected() diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 44900b53b03f..d959e188b4ee 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -363,7 +363,7 @@ def get_serializer_class(self): return CourseEnrollmentSerializer @cached_property - def queryset(self): + def queryset_for_user(self): """ Find and return the list of course enrollments for the user. @@ -380,11 +380,11 @@ def queryset(self): if api_version == API_V4 and status in EnrollmentStatuses.values(): if status == EnrollmentStatuses.IN_PROGRESS.value: - queryset = queryset.in_progress(user_username=username, time_zone=self.user_timezone) + queryset = queryset.in_progress(username=username, time_zone=self.user_timezone) elif status == EnrollmentStatuses.COMPLETED.value: - queryset = queryset.completed(user_username=username) + queryset = queryset.completed(username=username) elif status == EnrollmentStatuses.EXPIRED.value: - queryset = queryset.expired(user_username=username, time_zone=self.user_timezone) + queryset = queryset.expired(username=username, time_zone=self.user_timezone) return queryset @@ -417,7 +417,7 @@ def get_same_org_mobile_available_enrollments(self) -> list[CourseEnrollment]: org = self.request.query_params.get('org', None) same_org = ( - enrollment for enrollment in self.queryset + enrollment for enrollment in self.queryset_for_user if enrollment.course_overview and self.is_org(org, enrollment.course_overview.org) ) mobile_available = ( @@ -473,7 +473,7 @@ def get_primary_enrollment_by_latest_enrollment_or_progress(self) -> Optional[Co mobile_available_course_ids = [enrollment.course_id for enrollment in mobile_available] - latest_enrollment = self.queryset.filter( + latest_enrollment = self.queryset_for_user.filter( course__id__in=mobile_available_course_ids ).order_by('-created').first() @@ -488,7 +488,7 @@ def get_primary_enrollment_by_latest_enrollment_or_progress(self) -> Optional[Co if not latest_progress: return latest_enrollment - enrollment_with_latest_progress = self.queryset.filter( + enrollment_with_latest_progress = self.queryset_for_user.filter( course_id=latest_progress.course_id, user__username=self.kwargs['username'], ).first() From cfa816c520e277b487a464211977e2305cd11b0b Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Thu, 4 Jul 2024 09:39:39 +0300 Subject: [PATCH 18/18] fix: [AXM-506] Try to fix pipelines --- lms/djangoapps/mobile_api/course_info/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/mobile_api/course_info/serializers.py b/lms/djangoapps/mobile_api/course_info/serializers.py index 40b3cc8ae61a..69bed47b03fc 100644 --- a/lms/djangoapps/mobile_api/course_info/serializers.py +++ b/lms/djangoapps/mobile_api/course_info/serializers.py @@ -80,7 +80,7 @@ def get_course_modes(self, course_overview): def get_course_progress(self, obj: CourseOverview) -> Dict[str, int]: """ - Gets course progress calculated by course assignments. + Gets course progress calculated by course completed assignments. """ return get_assignments_completions(obj.id, self.context.get('user'))