From 5a27b5df3930b56a40095cea0202e6882dc5f7e2 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Thu, 19 Mar 2020 19:27:25 +0100 Subject: [PATCH 1/2] Calculate completion for custom blocks on the Course Outline page This adds support for displaying completion on the course outline page, to remove the discrepancies between this view and the learning sequence. It also simplifies course outline page by reusing existing APIs for determining completion state and finding the "Resume block"'s target. --- .../course_api/blocks/serializers.py | 8 +- openedx/features/course_experience/utils.py | 119 +----------------- 2 files changed, 7 insertions(+), 120 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/serializers.py b/lms/djangoapps/course_api/blocks/serializers.py index a489fb2485..64bf73c8bb 100644 --- a/lms/djangoapps/course_api/blocks/serializers.py +++ b/lms/djangoapps/course_api/blocks/serializers.py @@ -79,11 +79,9 @@ SUPPORTED_FIELDS = [ VisibilityTransformer, requested_field_name='visible_to_staff_only', ), - SupportedFieldType( - BlockCompletionTransformer.COMPLETION, - BlockCompletionTransformer, - 'completion' - ), + SupportedFieldType(BlockCompletionTransformer.COMPLETION, BlockCompletionTransformer), + SupportedFieldType(BlockCompletionTransformer.COMPLETE), + SupportedFieldType(BlockCompletionTransformer.RESUME_BLOCK), *[SupportedFieldType(field_name) for field_name in ExtraFieldsTransformer.get_requested_extra_fields()], ] diff --git a/openedx/features/course_experience/utils.py b/openedx/features/course_experience/utils.py index 29104d595a..1968174c69 100644 --- a/openedx/features/course_experience/utils.py +++ b/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 From e571693455021bd63c3c25f8e249cfcb667e4166 Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Thu, 30 Jul 2020 20:01:56 +0200 Subject: [PATCH 2/2] Support EXCLUDED blocks in Block Completion Transformer For now only the discussion blocks were supported. If we had a custom XBlock that specified `completion_mode = XBlockCompletionMode.EXCLUDED`, then it could never be marked as completed on the course outline page, despite being marked as such inside the learning sequence. --- .../blocks/transformers/block_completion.py | 18 ++++++++++++++---- .../tests/views/test_course_home.py | 2 +- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/transformers/block_completion.py b/lms/djangoapps/course_api/blocks/transformers/block_completion.py index 3810825114..65bd3738e5 100644 --- a/lms/djangoapps/course_api/blocks/transformers/block_completion.py +++ b/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) diff --git a/openedx/features/course_experience/tests/views/test_course_home.py b/openedx/features/course_experience/tests/views/test_course_home.py index b5850b8cd2..3dd3247dae 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/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)