feat: include units when calculating completion percentage (#34816)

This is an enhancement to the API used for the courseware navigation
sidebar.

lms/djangoapps/course_home_api/outline/serializers.py

@@ -66,6 +66,8 @@ def get_blocks(self, block):  # pylint: disable=missing-function-docstring
         }
         if 'special_exam_info' in self.context.get('extra_fields', []) and block.get('special_exam_info'):
             serialized[block_key]['special_exam_info'] = block.get('special_exam_info').get('short_description')
+        if 'completion_stat' in self.context.get('extra_fields', []):
+            serialized[block_key]['completion_stat'] = block.get('completion_stat', {})
 
         for child in children:
             serialized.update(self.get_blocks(child))

lms/djangoapps/course_home_api/outline/tests/test_view.py

@@ -9,6 +9,7 @@
 
 import ddt  # lint-amnesty, pylint: disable=wrong-import-order
 import json  # lint-amnesty, pylint: disable=wrong-import-order
+from completion.models import BlockCompletion
 from django.conf import settings  # lint-amnesty, pylint: disable=wrong-import-order
 from django.urls import reverse  # lint-amnesty, pylint: disable=wrong-import-order
 from edx_toggles.toggles.testutils import override_waffle_flag  # lint-amnesty, pylint: disable=wrong-import-order
@@ -489,7 +490,7 @@ def add_blocks_to_course(self):
                 parent_location=self.chapter.location
             )
             self.vertical = BlockFactory.create(
-                category='problem',
+                category='vertical',
                 graded=True,
                 has_score=True,
                 parent_location=self.sequential.location
@@ -500,11 +501,20 @@ def add_blocks_to_course(self):
                 parent_location=self.chapter.location
             )
             self.ungraded_vertical = BlockFactory.create(
-                category='problem',
+                category='vertical',
                 parent_location=self.ungraded_sequential.location
             )
         update_outline_from_modulestore(self.course.id)
 
+    def create_completion(self, problem, completion):
+        return BlockCompletion.objects.create(
+            user=self.user,
+            context_key=problem.context_key,
+            block_type='problem',
+            block_key=problem.location,
+            completion=completion,
+        )
+
     @ddt.data(CourseMode.AUDIT, CourseMode.VERIFIED)
     def test_get_authenticated_enrolled_user(self, enrollment_mode):
         """
@@ -594,6 +604,18 @@ def test_proctored_exam(self, mock_summary):
             hide_after_due=False,
             is_onboarding_exam=False,
         )
+        vertical = BlockFactory.create(
+            parent=sequence,
+            category='vertical',
+            graded=True,
+            has_score=True,
+        )
+        BlockFactory.create(
+            parent=vertical,
+            category='problem',
+            graded=True,
+            has_score=True,
+        )
         sequence.is_proctored_exam = True
         update_outline_from_modulestore(course.id)
         CourseEnrollment.enroll(self.user, course.id)
@@ -608,7 +630,7 @@ def test_proctored_exam(self, mock_summary):
 
         exam_data = response.data['blocks'][str(sequence.location)]
         assert not exam_data['complete']
-        assert exam_data['display_name'] == 'Test Proctored Exam'
+        assert exam_data['display_name'] == 'Test Proctored Exam (1 Question)'
         assert exam_data['special_exam_info'] == 'My Exam'
         assert exam_data['due'] is not None
 
@@ -623,8 +645,8 @@ def test_assignment(self):
         assert response.status_code == 200
 
         exam_data = response.data['blocks'][str(self.sequential.location)]
-        assert exam_data['display_name'] == 'Test (1 Question)'
-        assert exam_data['icon'] == 'fa-pencil-square-o'
+        assert exam_data['display_name'] == 'Test'
+        assert exam_data['icon'] is None
         assert str(self.vertical.location) in exam_data['children']
 
         ungraded_data = response.data['blocks'][str(self.ungraded_sequential.location)]
@@ -686,3 +708,91 @@ def test_hide_learning_sequences(self):
         replace_course_outline(new_learning_seq_outline)
         blocks = self.client.get(self.url).data['blocks']
         assert seq_block_id not in blocks
+
+    def test_empty_blocks_complete(self):
+        """
+        Test that the API returns the correct complete state for empty blocks.
+        """
+        self.add_blocks_to_course()
+        CourseEnrollment.enroll(self.user, self.course.id)
+        url = reverse('course-home:course-navigation', args=[self.course.id])
+        response = self.client.get(url)
+        assert response.status_code == 200
+
+        sequence_data = response.data['blocks'][str(self.sequential.location)]
+        vertical_data = response.data['blocks'][str(self.vertical.location)]
+        assert sequence_data['complete']
+        assert vertical_data['complete']
+
+    @ddt.data(True, False)
+    def test_blocks_complete_with_problem(self, problem_complete):
+        self.add_blocks_to_course()
+        problem = BlockFactory.create(parent=self.vertical, category='problem', graded=True, has_score=True)
+        CourseEnrollment.enroll(self.user, self.course.id)
+        self.create_completion(problem, int(problem_complete))
+
+        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)]
+
+        assert sequence_data['complete'] == problem_complete
+        assert vertical_data['complete'] == problem_complete
+
+    def test_blocks_completion_stat(self):
+        """
+        Test that the API returns the correct completion statistics for the blocks.
+        """
+        self.add_blocks_to_course()
+        completed_problem = BlockFactory.create(parent=self.vertical, category='problem', graded=True, has_score=True)
+        uncompleted_problem = BlockFactory.create(parent=self.vertical, category='problem', graded=True, has_score=True)
+        update_outline_from_modulestore(self.course.id)
+        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-navigation', args=[self.course.id]))
+
+        expected_sequence_completion_stat = {
+            'completion': 0,
+            'completable_children': 1,
+        }
+        expected_vertical_completion_stat = {
+            'completion': 1,
+            'completable_children': 2,
+        }
+        sequence_data = response.data['blocks'][str(self.sequential.location)]
+        vertical_data = response.data['blocks'][str(self.vertical.location)]
+
+        assert not sequence_data['complete']
+        assert not vertical_data['complete']
+        assert sequence_data['completion_stat'] == expected_sequence_completion_stat
+        assert vertical_data['completion_stat'] == expected_vertical_completion_stat
+
+    def test_blocks_completion_stat_all_problem_completed(self):
+        """
+        Test that the API returns the correct completion statistics for the blocks when all problems are completed.
+        """
+        self.add_blocks_to_course()
+        problem1 = BlockFactory.create(parent=self.vertical, category='problem', graded=True, has_score=True)
+        problem2 = BlockFactory.create(parent=self.vertical, category='problem', graded=True, has_score=True)
+        update_outline_from_modulestore(self.course.id)
+        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-navigation', args=[self.course.id]))
+
+        expected_sequence_completion_stat = {
+            'completion': 1,
+            'completable_children': 1,
+        }
+        expected_vertical_completion_stat = {
+            'completion': 2,
+            'completable_children': 2,
+        }
+        sequence_data = response.data['blocks'][str(self.sequential.location)]
+        vertical_data = response.data['blocks'][str(self.vertical.location)]
+
+        assert sequence_data['complete']
+        assert vertical_data['complete']
+        assert sequence_data['completion_stat'] == expected_sequence_completion_stat
+        assert vertical_data['completion_stat'] == expected_vertical_completion_stat

lms/djangoapps/course_home_api/outline/views.py

@@ -462,11 +462,9 @@ def get(self, request, *args, **kwargs):
             is_masquerading=user_is_masquerading,
         )
         if navigation_sidebar_caching_is_disabled := courseware_disable_navigation_sidebar_blocks_caching():
-            cached = False
             course_blocks = None
         else:
             course_blocks = cache.get(cache_key)
-            cached = course_blocks is not None
 
         if not course_blocks:
             if getattr(enrollment, 'is_active', False) or bool(staff_access):
@@ -478,17 +476,12 @@ def get(self, request, *args, **kwargs):
                 cache.set(cache_key, course_blocks, self.COURSE_BLOCKS_CACHE_TIMEOUT)
 
         course_blocks = self.filter_inaccessible_blocks(course_blocks, course_key)
-
-        if cached:
-            # Note: The course_blocks received from get_course_outline_block_tree already has completion data,
-            # but since the course_blocks can be cached, and this status can change quite often,
-            # we need to update it every time if the data has not been cached.
-            course_blocks = self.mark_complete_recursive(course_blocks)
+        course_blocks = self.mark_complete_recursive(course_blocks)
 
         context = self.get_serializer_context()
         context.update({
             'include_vertical': True,
-            'extra_fields': ['special_exam_info'],
+            'extra_fields': ['special_exam_info', 'completion_stat'],
             'enable_prerequisite_block_type': True,
         })
 
@@ -508,7 +501,7 @@ def filter_inaccessible_blocks(self, course_blocks, course_key):
             for section_data in course_sections:
                 section_data['children'] = self.get_accessible_sequences(
                     user_course_outline,
-                    section_data.get('children', [])
+                    section_data.get('children', ['completion'])
                 )
                 accessible_sequence_ids = {str(usage_key) for usage_key in user_course_outline.accessible_sequences}
                 for sequence_data in section_data['children']:
@@ -520,15 +513,60 @@ 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['complete'] = all(
-                child['complete'] for child in block['children'] if child['type'] in self.completable_block_types
-            )
+            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 completable_children)
+            block['completion_stat'] = self.get_block_completion_stat(block, completable_children)
         else:
-            block['complete'] = self.completions_dict.get(block['id'], False)
+            # If the block is a course, chapter, sequential, or vertical, without children,
+            # it should be completed by default.
+            completion = self.completions_dict.get(block['id'], 0)
+            block['complete'] = bool(completion) or block['type'] in ['course', 'chapter', 'sequential', 'vertical']
+            block['completion_stat'] = self.get_block_completion_stat(block, completable_children=[])
+
         return block
 
+    def get_block_completion_stat(self, block, completable_children):
+        """
+        Get the completion status of a block.
+
+        Returns dictionary with the completion status and the number
+        of completable children of a block.
+        Completion is the value from 0 to 1 meaning the percentage of completion for lower-level blocks,
+        and sum of the completion status of the completable children.
+        """
+        block_type = block['type']
+        completable_children_num = len(completable_children)
+
+        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': completable_children_num,
+        }
+
+    def get_completable_children(self, block):
+        """
+        Get the completable children of a block.
+        """
+        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):
         """
@@ -573,11 +611,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
+            )
         }