From 159ee6375dbb946c84f731640243adaba0c785c9 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Tue, 23 Apr 2024 23:15:46 +0300 Subject: [PATCH 1/4] feat: [AXM-297, AXM-310] Add progress to assignments and total course progress --- .../mobile_api/course_info/constants.py | 2 + .../mobile_api/course_info/serializers.py | 15 ++++++ .../mobile_api/course_info/utils.py | 25 ++++++++++ .../mobile_api/course_info/views.py | 50 +++++++++++++++++++ 4 files changed, 92 insertions(+) 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..ede7d020ba4b --- /dev/null +++ b/lms/djangoapps/mobile_api/course_info/constants.py @@ -0,0 +1,2 @@ +# BLOCK_STRUCTURE_CACHE_TIMEOUT = 60 * 60 # 1 hour +BLOCK_STRUCTURE_CACHE_TIMEOUT = 10 # 10 second diff --git a/lms/djangoapps/mobile_api/course_info/serializers.py b/lms/djangoapps/mobile_api/course_info/serializers.py index b2bb0ce24701..4337ff209913 100644 --- a/lms/djangoapps/mobile_api/course_info/serializers.py +++ b/lms/djangoapps/mobile_api/course_info/serializers.py @@ -14,6 +14,8 @@ 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.mobile_api.users.serializers import ModeSerializer +from lms.djangoapps.mobile_api.course_info.constants import BLOCK_STRUCTURE_CACHE_TIMEOUT +from lms.djangoapps.mobile_api.course_info.utils import calculate_progress 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 +33,7 @@ class CourseInfoOverviewSerializer(serializers.ModelSerializer): course_sharing_utm_parameters = serializers.SerializerMethodField() course_about = serializers.SerializerMethodField('get_course_about_url') course_modes = serializers.SerializerMethodField() + total_course_progress = serializers.SerializerMethodField() class Meta: model = CourseOverview @@ -47,6 +50,7 @@ class Meta: 'course_sharing_utm_parameters', 'course_about', 'course_modes', + 'total_course_progress', ) @staticmethod @@ -75,6 +79,17 @@ def get_course_modes(self, course_overview): for mode in course_modes ] + def get_total_course_progress(self, obj): + """ + + """ + course_progress = calculate_progress(self.context.get('user'), obj.id, BLOCK_STRUCTURE_CACHE_TIMEOUT) + + return { + 'num_points_earned': sum(map(lambda x: x.graded_total.earned if x.graded else 0, course_progress)), + 'num_points_possible': sum(map(lambda x: x.graded_total.possible if x.graded else 0, course_progress)), + } + class MobileCourseEnrollmentSerializer(serializers.ModelSerializer): """ 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..5af34c459d2b --- /dev/null +++ b/lms/djangoapps/mobile_api/course_info/utils.py @@ -0,0 +1,25 @@ +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 + + +def calculate_progress(user, course_id, cache_timeout): + """ + Calculate the progress of the user in the course. + """ + is_staff = bool(has_access(user, 'staff', course_id)) + + 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()) + 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..77e2423ec27b 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -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 @@ -378,5 +380,53 @@ def list(self, request, **kwargs): # pylint: disable=W0221 course_data.update(CourseInfoOverviewSerializer(course_overview, context=course_info_context).data) + self._extend_sequential_info_with_assignment_progress( + requested_user, + course_id, + response.data['blocks'], + ) + response.data.update(course_data) return response + + def _extend_sequential_info_with_assignment_progress( + self, + requested_user, + course_id, + blocks_info_data, + ): + """ + + """ + + 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: + points_earned = ( + grade.graded_total.earned + if grades_with_locations[block_id].graded + else 0 + ) + points_possible = ( + grade.graded_total.possible + if grades_with_locations[block_id].graded + else 0 + ) + assignment_type = grade.format + else: + points_earned, points_possible = 0, 0 + assignment_type = None + + block_info.update( + { + 'assignment_progress': { + 'assignment_type': assignment_type, + 'num_points_earned': points_earned, + 'num_points_possible': points_possible, + } + } + ) From 3d70e8c008afd1d11209534f11132374cf2b8d0c Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Wed, 24 Apr 2024 16:12:53 +0300 Subject: [PATCH 2/4] feat: [AXM-297] Add progress to assignments --- .../mobile_api/course_info/constants.py | 3 +- .../mobile_api/course_info/serializers.py | 15 ---------- .../mobile_api/course_info/views.py | 27 +++++++++--------- .../tests/test_course_info_views.py | 28 +++++++++++++++++++ 4 files changed, 42 insertions(+), 31 deletions(-) diff --git a/lms/djangoapps/mobile_api/course_info/constants.py b/lms/djangoapps/mobile_api/course_info/constants.py index ede7d020ba4b..67f26ca78ff1 100644 --- a/lms/djangoapps/mobile_api/course_info/constants.py +++ b/lms/djangoapps/mobile_api/course_info/constants.py @@ -1,2 +1 @@ -# BLOCK_STRUCTURE_CACHE_TIMEOUT = 60 * 60 # 1 hour -BLOCK_STRUCTURE_CACHE_TIMEOUT = 10 # 10 second +BLOCK_STRUCTURE_CACHE_TIMEOUT = 60 * 60 # 1 hour diff --git a/lms/djangoapps/mobile_api/course_info/serializers.py b/lms/djangoapps/mobile_api/course_info/serializers.py index 4337ff209913..b2bb0ce24701 100644 --- a/lms/djangoapps/mobile_api/course_info/serializers.py +++ b/lms/djangoapps/mobile_api/course_info/serializers.py @@ -14,8 +14,6 @@ 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.mobile_api.users.serializers import ModeSerializer -from lms.djangoapps.mobile_api.course_info.constants import BLOCK_STRUCTURE_CACHE_TIMEOUT -from lms.djangoapps.mobile_api.course_info.utils import calculate_progress from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.features.course_duration_limits.access import get_user_course_expiration_date @@ -33,7 +31,6 @@ class CourseInfoOverviewSerializer(serializers.ModelSerializer): course_sharing_utm_parameters = serializers.SerializerMethodField() course_about = serializers.SerializerMethodField('get_course_about_url') course_modes = serializers.SerializerMethodField() - total_course_progress = serializers.SerializerMethodField() class Meta: model = CourseOverview @@ -50,7 +47,6 @@ class Meta: 'course_sharing_utm_parameters', 'course_about', 'course_modes', - 'total_course_progress', ) @staticmethod @@ -79,17 +75,6 @@ def get_course_modes(self, course_overview): for mode in course_modes ] - def get_total_course_progress(self, obj): - """ - - """ - course_progress = calculate_progress(self.context.get('user'), obj.id, BLOCK_STRUCTURE_CACHE_TIMEOUT) - - return { - 'num_points_earned': sum(map(lambda x: x.graded_total.earned if x.graded else 0, course_progress)), - 'num_points_possible': sum(map(lambda x: x.graded_total.possible if x.graded else 0, course_progress)), - } - 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 77e2423ec27b..352efca9b4aa 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 @@ -359,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,25 +386,18 @@ def list(self, request, **kwargs): # pylint: disable=W0221 course_data.update(CourseInfoOverviewSerializer(course_overview, context=course_info_context).data) - self._extend_sequential_info_with_assignment_progress( - requested_user, - course_id, - response.data['blocks'], - ) - response.data.update(course_data) return response + @staticmethod def _extend_sequential_info_with_assignment_progress( - self, - requested_user, - course_id, - blocks_info_data, - ): + 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} 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 67d2c79f9017..a5175626a29e 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 fa6349cc8382a6d5b11b424be578bbc3679158a9 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Wed, 24 Apr 2024 17:44:56 +0300 Subject: [PATCH 3/4] style: [AXM-297] Try to fix linters (add docstrings) --- lms/djangoapps/mobile_api/course_info/constants.py | 4 ++++ lms/djangoapps/mobile_api/course_info/utils.py | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/lms/djangoapps/mobile_api/course_info/constants.py b/lms/djangoapps/mobile_api/course_info/constants.py index 67f26ca78ff1..d62cb463951a 100644 --- a/lms/djangoapps/mobile_api/course_info/constants.py +++ b/lms/djangoapps/mobile_api/course_info/constants.py @@ -1 +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 index 5af34c459d2b..b86361223275 100644 --- a/lms/djangoapps/mobile_api/course_info/utils.py +++ b/lms/djangoapps/mobile_api/course_info/utils.py @@ -1,3 +1,7 @@ +""" +Common utils for the `course_info` API. +""" + from django.core.cache import cache from lms.djangoapps.courseware.access import has_access From 2f9e155c504c182f729ab35f8051c7d31effba91 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Thu, 25 Apr 2024 10:43:05 +0300 Subject: [PATCH 4/4] refactor: [AXM-297] Add typing, refactor methods --- .../mobile_api/course_info/utils.py | 34 +++++++++++++------ .../mobile_api/course_info/views.py | 16 +++------ 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/lms/djangoapps/mobile_api/course_info/utils.py b/lms/djangoapps/mobile_api/course_info/utils.py index b86361223275..141c32da4cf4 100644 --- a/lms/djangoapps/mobile_api/course_info/utils.py +++ b/lms/djangoapps/mobile_api/course_info/utils.py @@ -2,28 +2,42 @@ 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, course_id, cache_timeout): +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)) - 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) + 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) - 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 [] - # 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 subsection_grades diff --git a/lms/djangoapps/mobile_api/course_info/views.py b/lms/djangoapps/mobile_api/course_info/views.py index 352efca9b4aa..c6de108727d8 100644 --- a/lms/djangoapps/mobile_api/course_info/views.py +++ b/lms/djangoapps/mobile_api/course_info/views.py @@ -405,20 +405,12 @@ def _extend_sequential_info_with_assignment_progress( if block_info['type'] == 'sequential': grade = grades_with_locations.get(block_id) if grade: - points_earned = ( - grade.graded_total.earned - if grades_with_locations[block_id].graded - else 0 - ) - points_possible = ( - grade.graded_total.possible - if grades_with_locations[block_id].graded - else 0 - ) + 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 = 0, 0 - assignment_type = None + points_earned, points_possible, assignment_type = 0, 0, None block_info.update( {