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 <[email protected]>
raccoongang · May 17, 2024 · d22f1c0 · d22f1c0
1 parent 3918e0d
commit d22f1c0
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 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
@@ -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
@@ -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,25 +535,32 @@ 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:
             completion = self.completions_dict.get(block['id'], 0)
 
         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
+            )
         }