From 631aa9b660854c7950b2d6a28c1ae20fabe311ee Mon Sep 17 00:00:00 2001 From: Glib Glugovskiy Date: Wed, 20 Mar 2024 11:18:50 +0000 Subject: [PATCH 1/5] fix: workaround for staticcollection introduced in e40a01c --- openedx/core/djangoapps/theming/storage.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/theming/storage.py b/openedx/core/djangoapps/theming/storage.py index 4a48cbb6e0a5..9bb4938014ab 100644 --- a/openedx/core/djangoapps/theming/storage.py +++ b/openedx/core/djangoapps/theming/storage.py @@ -192,9 +192,11 @@ def converter(matchobj): This requires figuring out which files the matched URL resolves to and calling the url() method of the storage. """ - matches = matchobj.groupdict() - matched = matches["matched"] - url = matches["url"] + matched, url = matchobj.groups() + #matches = matchobj.groupdict() + # matched = matches["matched"] + # url = matches["url"] + # Ignore absolute/protocol-relative and data-uri URLs. if re.match(r"^[a-z]+:", url): From d7652e9612bf1f2f92d8792fc909e41996e1f596 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, 21 Mar 2024 15:24:56 +0200 Subject: [PATCH 2/5] feat: [AXM-40] add courses progress to enrollment endpoint --- .../mobile_api/users/serializers.py | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 944f3c36defd..6128822e4965 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 @@ -91,7 +94,47 @@ def to_representation(self, course_overview): # lint-amnesty, pylint: disable=a } -class CourseEnrollmentSerializer(serializers.ModelSerializer): +class CourseEnrollmentProgressMixin(serializers.Serializer): + """ + Mixin for CourseEnrollment serializers to add progress information. + """ + + progress = serializers.SerializerMethodField() + + BLOCK_STRUCTURE_CACHE_TIMEOUT = 60 * 60 # 1 hour + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + if self.context.get('api_version') == 'v4': + self.progress = serializers.SerializerMethodField() + self.Meta.fields = list(self.Meta.fields) + self.Meta.fields.append('progress') + + 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 CourseEnrollmentSerializer(serializers.ModelSerializer, CourseEnrollmentProgressMixin): """ Serializes CourseEnrollment models """ From 2c7aa3702710cf9fafc16d0152bf93daa8f872ae 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, 21 Mar 2024 15:57:33 +0200 Subject: [PATCH 3/5] refactor: [AXM-40] add caching to improve performance --- lms/djangoapps/mobile_api/users/views.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index acf8b7179590..52bb44973bc3 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() + queryset = CourseEnrollment.objects.all().select_related('course', 'user') lookup_field = 'username' # In Django Rest Framework v3, there is a default pagination @@ -354,7 +355,7 @@ def get_serializer_class(self): def get_queryset(self): api_version = self.kwargs.get('api_version') - mobile_available = self.get_mobile_available_enrollments() + mobile_available = self.mobile_available_enrollments not_duration_limited = ( enrollment for enrollment in mobile_available @@ -373,7 +374,8 @@ def get_queryset(self): # return all courses, with associated expiration return mobile_available - def get_mobile_available_enrollments(self) -> List[Optional[CourseEnrollment]]: + @cached_property + def mobile_available_enrollments(self) -> List[Optional[CourseEnrollment]]: """ Gets list with `CourseEnrollment` for mobile available courses. """ @@ -419,7 +421,7 @@ 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.mobile_available_enrollments if not mobile_available: return None From b943e608058af1adffdaef2beab3c193ad68fa8f 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, 21 Mar 2024 16:32:57 +0200 Subject: [PATCH 4/5] refactor: [AXM-40] add progress only for primary course --- .../mobile_api/users/serializers.py | 70 ++++++++----------- 1 file changed, 29 insertions(+), 41 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 6128822e4965..64dffed1f045 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -94,47 +94,7 @@ def to_representation(self, course_overview): # lint-amnesty, pylint: disable=a } -class CourseEnrollmentProgressMixin(serializers.Serializer): - """ - Mixin for CourseEnrollment serializers to add progress information. - """ - - progress = serializers.SerializerMethodField() - - BLOCK_STRUCTURE_CACHE_TIMEOUT = 60 * 60 # 1 hour - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - if self.context.get('api_version') == 'v4': - self.progress = serializers.SerializerMethodField() - self.Meta.fields = list(self.Meta.fields) - self.Meta.fields.append('progress') - - 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 CourseEnrollmentSerializer(serializers.ModelSerializer, CourseEnrollmentProgressMixin): +class CourseEnrollmentSerializer(serializers.ModelSerializer): """ Serializes CourseEnrollment models """ @@ -197,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]]]: """ @@ -256,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 = ( @@ -267,6 +254,7 @@ class Meta: 'certificate', 'course_modes', 'course_status', + 'progress', ) lookup_field = 'username' From 22982dfec9a8204f384eb4d661d938968e185a65 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: Fri, 22 Mar 2024 13:04:17 +0200 Subject: [PATCH 5/5] refactor: [AXM-40] refactor enrollment caching optimization --- lms/djangoapps/mobile_api/users/views.py | 22 ++++++++++++---------- openedx/core/djangoapps/theming/storage.py | 8 +++----- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 52bb44973bc3..2463ef963b9e 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -325,7 +325,7 @@ class UserCourseEnrollmentsList(generics.ListAPIView): certified). * url: URL to the downloadable version of the certificate, if exists. """ - queryset = CourseEnrollment.objects.all().select_related('course', 'user') + lookup_field = 'username' # In Django Rest Framework v3, there is a default pagination @@ -353,9 +353,16 @@ 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.mobile_available_enrollments + mobile_available = self.get_mobile_available_enrollments() not_duration_limited = ( enrollment for enrollment in mobile_available @@ -374,19 +381,14 @@ def get_queryset(self): # return all courses, with associated expiration return mobile_available - @cached_property - def mobile_available_enrollments(self) -> List[Optional[CourseEnrollment]]: + 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 = ( @@ -421,7 +423,7 @@ 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.mobile_available_enrollments + mobile_available = self.get_mobile_available_enrollments() if not mobile_available: return None diff --git a/openedx/core/djangoapps/theming/storage.py b/openedx/core/djangoapps/theming/storage.py index 9bb4938014ab..4a48cbb6e0a5 100644 --- a/openedx/core/djangoapps/theming/storage.py +++ b/openedx/core/djangoapps/theming/storage.py @@ -192,11 +192,9 @@ def converter(matchobj): This requires figuring out which files the matched URL resolves to and calling the url() method of the storage. """ - matched, url = matchobj.groups() - #matches = matchobj.groupdict() - # matched = matches["matched"] - # url = matches["url"] - + matches = matchobj.groupdict() + matched = matches["matched"] + url = matches["url"] # Ignore absolute/protocol-relative and data-uri URLs. if re.match(r"^[a-z]+:", url):