From d22f1c06d8efee71ac0462147dcdd475cc216d4f Mon Sep 17 00:00:00 2001 From: Ivan Niedielnitsev <81557788+NiedielnitsevIvan@users.noreply.github.com> Date: Tue, 14 May 2024 16:00:34 +0300 Subject: [PATCH] feat: [AXIMST-819] Section progress calculates by units progress (#2555) * fix: correct mistake after rebase * feat: [AXIMST-819] Section progress calculates by units progress * fix: [AXIMST-819] fix tests --------- Co-authored-by: Glib Glugovskiy --- .../outline/tests/test_view.py | 8 ++-- .../course_home_api/outline/views.py | 43 ++++++++++++------- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/lms/djangoapps/course_home_api/outline/tests/test_view.py b/lms/djangoapps/course_home_api/outline/tests/test_view.py index 12d57aa35bce..4f40d9a80192 100644 --- a/lms/djangoapps/course_home_api/outline/tests/test_view.py +++ b/lms/djangoapps/course_home_api/outline/tests/test_view.py @@ -715,7 +715,7 @@ def test_empty_blocks_complete(self): """ self.add_blocks_to_course() CourseEnrollment.enroll(self.user, self.course.id) - url = reverse('course-home:course-sidebar-blocks', args=[self.course.id]) + url = reverse('course-home:course-navigation', args=[self.course.id]) response = self.client.get(url) assert response.status_code == 200 @@ -731,7 +731,7 @@ def test_blocks_complete_with_problem(self, problem_complete): CourseEnrollment.enroll(self.user, self.course.id) self.create_completion(problem, int(problem_complete)) - response = self.client.get(reverse('course-home:course-sidebar-blocks', args=[self.course.id])) + response = self.client.get(reverse('course-home:course-navigation', args=[self.course.id])) sequence_data = response.data['blocks'][str(self.sequential.location)] vertical_data = response.data['blocks'][str(self.vertical.location)] @@ -750,7 +750,7 @@ def test_blocks_completion_stat(self): CourseEnrollment.enroll(self.user, self.course.id) self.create_completion(completed_problem, 1) self.create_completion(uncompleted_problem, 0) - response = self.client.get(reverse('course-home:course-sidebar-blocks', args=[self.course.id])) + response = self.client.get(reverse('course-home:course-navigation', args=[self.course.id])) expected_sequence_completion_stat = { 'completion': 0, @@ -779,7 +779,7 @@ def test_blocks_completion_stat_all_problem_completed(self): CourseEnrollment.enroll(self.user, self.course.id) self.create_completion(problem1, 1) self.create_completion(problem2, 1) - response = self.client.get(reverse('course-home:course-sidebar-blocks', args=[self.course.id])) + response = self.client.get(reverse('course-home:course-navigation', args=[self.course.id])) expected_sequence_completion_stat = { 'completion': 1, diff --git a/lms/djangoapps/course_home_api/outline/views.py b/lms/djangoapps/course_home_api/outline/views.py index c8ff7691cc82..21b982088c6b 100644 --- a/lms/djangoapps/course_home_api/outline/views.py +++ b/lms/djangoapps/course_home_api/outline/views.py @@ -463,9 +463,7 @@ def get(self, request, *args, **kwargs): ) if navigation_sidebar_caching_is_disabled := courseware_disable_navigation_sidebar_blocks_caching(): course_blocks = cache.get(cache_key) - cached = course_blocks is not None else: - cached = False course_blocks = None if not course_blocks: @@ -477,9 +475,8 @@ def get(self, request, *args, **kwargs): if not navigation_sidebar_caching_is_disabled: cache.set(cache_key, course_blocks, self.COURSE_BLOCKS_CACHE_TIMEOUT) - course_blocks = self.filter_unavailable_blocks(course_blocks, course_key) - if course_blocks and cached: - course_blocks = self.mark_complete_recursive(course_blocks) + course_blocks = self.filter_inaccessible_blocks(course_blocks, course_key) + course_blocks = self.mark_complete_recursive(course_blocks) context = self.get_serializer_context() context.update({ @@ -516,12 +513,13 @@ def mark_complete_recursive(self, block): """ Mark blocks as complete or not complete based on the completions_dict. """ + if not block: + return block + if 'children' in block: - block['children'] = [self.mark_complete_recursive(child) for child in block['children']] + block['children'] = [self.mark_complete_recursive(child) for child in block['children'] if child] completable_children = self.get_completable_children(block) - block['complete'] = all( - child['complete'] for child in block['children'] if child['type'] in self.completable_block_types - ) + block['complete'] = all(child['complete'] for child in completable_children) block['completion_stat'] = self.get_block_completion_stat(block, completable_children) else: # If the block is a course, chapter, sequential, or vertical, without children, @@ -537,9 +535,17 @@ def get_block_completion_stat(self, block, completable_children): Get the completion status of a block. """ block_type = block['type'] + completable_children_num = len(completable_children) - if block_type in ['course', 'chapter', 'sequential']: + if block_type in ['course', 'sequential']: completion = sum(child['complete'] for child in completable_children) + elif block_type == 'chapter': + # For sections, we have to count the status on the number of completed units + # and, accordingly, the number of units in it. + completion = sum(child['completion_stat']['completion'] for child in completable_children) + completable_children_num = sum( + child['completion_stat']['completable_children'] for child in completable_children + ) elif block_type == 'vertical': completion = sum(child['completion_stat']['completion'] for child in completable_children) else: @@ -547,15 +553,14 @@ def get_block_completion_stat(self, block, completable_children): return { 'completion': completion, - 'completable_children': len(completable_children), + 'completable_children': completable_children_num, } - @staticmethod - def get_completable_children(block): + def get_completable_children(self, block): """ Get the completable children of a block. """ - return [child for child in block.get('children', []) if child['type'] != 'discussion'] + return [child for child in block.get('children', []) if child['type'] in self.completable_block_types] @staticmethod def get_accessible_sections(user_course_outline, course_sections): @@ -601,11 +606,17 @@ def completions_dict(self): @cached_property def completable_block_types(self): """ - Return a set of block types that are completable. + Returns a set of block types that can be marked as completed. + + In addition to the lower-level x-blocks, it also includes blocks + that belong to XBlockCompletionMode.AGGREGATOR, because they can also be marked as complete. """ return { block_type for (block_type, block_cls) in XBlock.load_classes() - if XBlockCompletionMode.get_mode(block_cls) == XBlockCompletionMode.COMPLETABLE + if XBlockCompletionMode.get_mode(block_cls) in ( + XBlockCompletionMode.COMPLETABLE, + XBlockCompletionMode.AGGREGATOR + ) }