From 89acefeb1c23db8faff72147da15e431b8ad498a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 28 Mar 2024 10:14:45 +0200 Subject: [PATCH 1/4] feat: [AXM-53] add assertions for primary course --- .../mobile_api/users/serializers.py | 46 ++++++++++++++++--- lms/djangoapps/mobile_api/users/views.py | 12 ++++- 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 64dffed1f045..f082f0fe8591 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,15 @@ 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) + # import pdb; pdb.set_trace() + 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 +195,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 +206,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 +250,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 = {} + 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: + if not next_assignment: + next_assignment = DateSummarySerializer(assignment).data + + return { + 'future_assignment': next_assignment, + 'past_assignments': DateSummarySerializer(past_assignment, many=True).data, + } + class Meta: model = CourseEnrollment fields = ( @@ -255,6 +288,7 @@ class Meta: 'course_modes', 'course_status', 'progress', + 'course_assignments', ) lookup_field = 'username' 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 bc2403e278df0a7364aebd5b4484b748d35802e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Thu, 28 Mar 2024 20:01:49 +0200 Subject: [PATCH 2/4] test: [AXM-53] fix tests --- .../mobile_api/users/serializers.py | 1 - lms/djangoapps/mobile_api/users/tests.py | 25 +++++++++++++------ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index f082f0fe8591..851516e9ef6c 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -169,7 +169,6 @@ class CourseEnrollmentSerializerModifiedForPrimary(CourseEnrollmentSerializer): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - # import pdb; pdb.set_trace() self.course = modulestore().get_course(self.instance.course.id) def get_course_status(self, model: CourseEnrollment) -> Optional[Dict[str, List[str]]]: 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. From 7eb1ad7fcf7882c46c4b566fb68664858ee8b2dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Tue, 2 Apr 2024 14:45:12 +0300 Subject: [PATCH 3/4] style: [AXM-53] change future_assignment default value to None --- lms/djangoapps/mobile_api/users/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 851516e9ef6c..1a37148532cf 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -259,7 +259,7 @@ def get_course_assignments(self, model: CourseEnrollment) -> Optional[Dict[str, self.context.get('request'), include_past_dates=True ) - next_assignment = {} + next_assignment = None past_assignment = [] timezone = get_user_timezone_or_last_seen_timezone_or_utc(model.user) From 9655f59fd5d7d71e28e9b0500d99b3ce9cdd9e2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Tue, 2 Apr 2024 15:25:41 +0300 Subject: [PATCH 4/4] refactor: [AXM-53] add some optimization for assignments collecting --- lms/djangoapps/mobile_api/users/serializers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 1a37148532cf..0a17dbb5a82f 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -267,8 +267,8 @@ def get_course_assignments(self, model: CourseEnrollment) -> Optional[Dict[str, if assignment.date < datetime.now(timezone): past_assignment.append(assignment) else: - if not next_assignment: - next_assignment = DateSummarySerializer(assignment).data + next_assignment = DateSummarySerializer(assignment).data + break return { 'future_assignment': next_assignment,