Merge pull request #23457 from open-craft/agrendalath/bb-2063-fix-marking-blocks-as-completed-on-course-outline-page-upstream

[BB-2063] Calculate completion for custom blocks on the Course Outline page

lms/djangoapps/course_api/blocks/serializers.py

@@ -79,11 +79,9 @@ SUPPORTED_FIELDS = [
         VisibilityTransformer,
         requested_field_name='visible_to_staff_only',
     ),
-    SupportedFieldType(
+    SupportedFieldType(BlockCompletionTransformer.COMPLETION, BlockCompletionTransformer),
-        BlockCompletionTransformer.COMPLETION,
+    SupportedFieldType(BlockCompletionTransformer.COMPLETE),
-        BlockCompletionTransformer,
+    SupportedFieldType(BlockCompletionTransformer.RESUME_BLOCK),
-        'completion'
-    ),
 
     *[SupportedFieldType(field_name) for field_name in ExtraFieldsTransformer.get_requested_extra_fields()],
 ]

lms/djangoapps/course_api/blocks/transformers/block_completion.py

@@ -45,6 +45,17 @@ class BlockCompletionTransformer(BlockStructureTransformer):
     def collect(cls, block_structure):
         block_structure.request_xblock_fields('completion_mode')
 
+    @staticmethod
+    def _is_block_excluded(block_structure, block_key):
+        """
+        Checks whether block's completion method is of `EXCLUDED` type.
+        """
+        completion_mode = block_structure.get_xblock_field(
+            block_key, 'completion_mode'
+        )
+
+        return completion_mode == CompletionMode.EXCLUDED
+
     def mark_complete(self, complete_course_blocks, latest_complete_block_key, block_key, block_structure):
         """
         Helper function to mark a block as 'complete' as dictated by
@@ -58,14 +69,13 @@ class BlockCompletionTransformer(BlockStructureTransformer):
         """
         if block_key in complete_course_blocks:
             block_structure.override_xblock_field(block_key, self.COMPLETE, True)
-            if block_key == latest_complete_block_key:
+            if str(block_key) == str(latest_complete_block_key):
                 block_structure.override_xblock_field(block_key, self.RESUME_BLOCK, True)
 
         children = block_structure.get_children(block_key)
-        non_discussion_children = (child_key for child_key in children
-                                   if block_structure.get_xblock_field(child_key, 'category') != 'discussion')
         all_children_complete = all(block_structure.get_xblock_field(child_key, self.COMPLETE)
-                                    for child_key in non_discussion_children)
+                                    for child_key in children
+                                    if not self._is_block_excluded(block_structure, child_key))
 
         if children and all_children_complete:
             block_structure.override_xblock_field(block_key, self.COMPLETE, True)

openedx/features/course_experience/tests/views/test_course_home.py

@@ -208,7 +208,7 @@ class TestCourseHomePage(CourseHomePageTestCase):  # lint-amnesty, pylint: disab
 
         # Fetch the view and verify the query counts
         # TODO: decrease query count as part of REVO-28
-        with self.assertNumQueries(78, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
+        with self.assertNumQueries(79, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
             with check_mongo_calls(4):
                 url = course_home_url(self.course)
                 self.client.get(url)

openedx/features/course_experience/utils.py

@@ -3,18 +3,11 @@ Common utilities for the course experience, including course outline.
 """
 
 
-from datetime import timedelta  # lint-amnesty, pylint: disable=unused-import
-
-from completion.models import BlockCompletion
-from django.db.models import Q  # lint-amnesty, pylint: disable=unused-import
 from django.utils import timezone
 from opaque_keys.edx.keys import CourseKey
-from six.moves import range
 
 from lms.djangoapps.course_api.blocks.api import get_blocks
 from lms.djangoapps.course_blocks.api import get_course_blocks
-from lms.djangoapps.course_blocks.utils import get_student_module_as_dict
-from lms.djangoapps.courseware.access import has_access  # lint-amnesty, pylint: disable=unused-import
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from openedx.core.lib.cache_utils import request_cached
 from openedx.features.course_experience import RELATIVE_DATES_FLAG
@@ -52,88 +45,6 @@ def get_course_outline_block_tree(request, course_id, user=None, allow_start_dat
 
         return block
 
-    def set_last_accessed_default(block):
-        """
-        Set default of False for resume_block on all blocks.
-        """
-        block['resume_block'] = False
-        block['complete'] = False
-        for child in block.get('children', []):
-            set_last_accessed_default(child)
-
-    def mark_blocks_completed(block, user, course_key):
-        """
-        Walk course tree, marking block completion.
-        Mark 'most recent completed block as 'resume_block'
-
-        """
-        last_completed_child_position = BlockCompletion.get_latest_block_completed(user, course_key)
-
-        if last_completed_child_position:
-            # Mutex w/ NOT 'course_block_completions'
-            recurse_mark_complete(
-                course_block_completions=BlockCompletion.get_learning_context_completions(user, course_key),
-                latest_completion=last_completed_child_position,
-                block=block
-            )
-
-    def recurse_mark_complete(course_block_completions, latest_completion, block):
-        """
-        Helper function to walk course tree dict,
-        marking blocks as 'complete' and 'last_complete'
-
-        If all blocks are complete, mark parent block complete
-        mark parent blocks of 'last_complete' as 'last_complete'
-
-        :param course_block_completions: dict[course_completion_object] =  completion_value
-        :param latest_completion: course_completion_object
-        :param block: course_outline_root_block block object or child block
-
-        :return:
-            block: course_outline_root_block block object or child block
-        """
-        block_key = block.serializer.instance
-
-        if course_block_completions.get(block_key):
-            block['complete'] = True
-            if block_key == latest_completion.full_block_key:
-                block['resume_block'] = True
-
-        if block.get('children'):
-            for idx in range(len(block['children'])):
-                recurse_mark_complete(
-                    course_block_completions,
-                    latest_completion,
-                    block=block['children'][idx]
-                )
-                if block['children'][idx].get('resume_block') is True:
-                    block['resume_block'] = True
-
-            completable_blocks = [child for child in block['children']
-                                  if child.get('type') != 'discussion']
-            if all(child.get('complete') for child in completable_blocks):
-                block['complete'] = True
-
-    def mark_last_accessed(user, course_key, block):
-        """
-        Recursively marks the branch to the last accessed block.
-        """
-        block_key = block.serializer.instance
-        student_module_dict = get_student_module_as_dict(user, course_key, block_key)
-
-        last_accessed_child_position = student_module_dict.get('position')
-        if last_accessed_child_position and block.get('children'):
-            block['resume_block'] = True
-            if last_accessed_child_position <= len(block['children']):
-                last_accessed_child_block = block['children'][last_accessed_child_position - 1]
-                last_accessed_child_block['resume_block'] = True
-                mark_last_accessed(user, course_key, last_accessed_child_block)
-            else:
-                # We should be using an id in place of position for last accessed.
-                # However, while using position, if the child block is no longer accessible
-                # we'll use the last child.
-                block['children'][-1]['resume_block'] = True
-
     def recurse_mark_scored(block):
         """
         Mark this block as 'scored' if any of its descendents are 'scored' (that is, 'has_score' and 'weight' > 0).
@@ -181,23 +92,6 @@ def get_course_outline_block_tree(request, course_id, user=None, allow_start_dat
     course_key = CourseKey.from_string(course_id)
     course_usage_key = modulestore().make_course_usage_key(course_key)
 
-    # Deeper query for course tree traversing/marking complete
-    # and last completed block
-    block_types_filter = [
-        'course',
-        'chapter',
-        'sequential',
-        'vertical',
-        'html',
-        'problem',
-        'video',
-        'discussion',
-        'drag-and-drop-v2',
-        'poll',
-        'word_cloud',
-        'lti',
-        'lti_consumer',
-    ]
     all_blocks = get_blocks(
         request,
         course_usage_key,
@@ -218,8 +112,10 @@ def get_course_outline_block_tree(request, course_id, user=None, allow_start_dat
             'start',
             'type',
             'weight',
+            'completion',
+            'complete',
+            'resume_block',
         ],
-        block_types_filter=block_types_filter,
         allow_start_dates_in_future=allow_start_dates_in_future,
     )
 
@@ -229,13 +125,6 @@ def get_course_outline_block_tree(request, course_id, user=None, allow_start_dat
         recurse_mark_scored(course_outline_root_block)
         recurse_num_graded_problems(course_outline_root_block)
         recurse_mark_auth_denial(course_outline_root_block)
-        if user:
-            set_last_accessed_default(course_outline_root_block)
-            mark_blocks_completed(
-                block=course_outline_root_block,
-                user=user,
-                course_key=course_key
-            )
     return course_outline_root_block
 
 
@@ -244,7 +133,7 @@ def get_resume_block(block):
     Gets the deepest block marked as 'resume_block'.
 
     """
-    if block.get('authorization_denial_reason') or not block['resume_block']:
+    if block.get('authorization_denial_reason') or not block.get('resume_block'):
         return None
     if not block.get('children'):
         return block