From 2095e3504bee0709f39ee85e29ca0a9be18353a0 Mon Sep 17 00:00:00 2001 From: Agrendalath <piotr@surowiec.it> Date: Tue, 17 Dec 2024 23:43:21 +0100 Subject: [PATCH] fix: sidebar completion for completable XBlocks with children It is possible to create a completable XBlock with children. An example is the Library Content Block with the `MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW` feature toggle. The sidebar should use the same mechanism as the `BlockCompletionTransformer` and the `edx-completion` library. It means that we should treat: 1. An aggregator XBlock as completed only when all its children are completed. 2. A completable XBlock as completed when it is directly marked as completed (without checking the completion of its children). --- .../outline/tests/test_view.py | 40 +++++++++++++++++++ .../course_home_api/outline/views.py | 16 +++++++- 2 files changed, 55 insertions(+), 1 deletion(-) 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 76928846f080..8398832dcf06 100644 --- a/lms/djangoapps/course_home_api/outline/tests/test_view.py +++ b/lms/djangoapps/course_home_api/outline/tests/test_view.py @@ -13,6 +13,7 @@ from django.conf import settings # lint-amnesty, pylint: disable=wrong-import-order from django.test import override_settings from django.urls import reverse # lint-amnesty, pylint: disable=wrong-import-order +from django.test import override_settings from edx_toggles.toggles.testutils import override_waffle_flag # lint-amnesty, pylint: disable=wrong-import-order from cms.djangoapps.contentstore.outlines import update_outline_from_modulestore @@ -758,6 +759,45 @@ def test_blocks_complete_with_problem(self, problem_complete): assert sequence_data['complete'] == problem_complete assert vertical_data['complete'] == problem_complete + @ddt.data( + # In the following tests, the library is treated as an aggregate block. The library completion does not matter. + (False, False, False, False), # Nothing is completed. + (True, False, False, True), # Only the problem is completed. + (False, True, False, False), # Only the library is completed. + (True, True, False, True), # Both the library and the problem are completed. + # In the following tests, the library is treated as a completable block. The problem completion does not matter. + (False, False, True, False), # Nothing is completed. + (True, False, True, False), # Only the problem is completed. + (False, True, True, True), # Only the library is completed. + (True, True, True, True), # Both the library and the problem are completed. + ) + @ddt.unpack + def test_blocks_complete_with_library_content_block( + self, problem_complete, library_complete, library_complete_on_view, expected + ): + """ + Test that the API checks the children completion only when the XBlock's completion mode is `AGGREGATOR`. + + The completion of the `COMPLETABLE` XBlocks should not depend on the completion of their children. + """ + self.add_blocks_to_course() + library = BlockFactory.create(parent=self.vertical, category='library_content', graded=True, has_score=True) + problem = BlockFactory.create(parent=library, category='problem', graded=True, has_score=True) + CourseEnrollment.enroll(self.user, self.course.id) + self.create_completion(problem, int(problem_complete)) + self.create_completion(library, int(library_complete)) + + with override_settings( + FEATURES={**settings.FEATURES, 'MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW': library_complete_on_view} + ): + 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'] == expected + assert vertical_data['complete'] == expected + def test_blocks_completion_stat(self): """ Test that the API returns the correct completion statistics for the blocks. diff --git a/lms/djangoapps/course_home_api/outline/views.py b/lms/djangoapps/course_home_api/outline/views.py index e7ebb2eef28f..d822d980f90a 100644 --- a/lms/djangoapps/course_home_api/outline/views.py +++ b/lms/djangoapps/course_home_api/outline/views.py @@ -516,7 +516,7 @@ def mark_complete_recursive(self, block): if not block: return block - if 'children' in block: + if 'children' in block and block['type'] in self.aggregator_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) @@ -608,6 +608,20 @@ def completions_dict(self): for block_key, completion in completions } + @cached_property + def aggregator_block_types(self) -> set[str]: + """ + Return a set of block types that belong to XBlockCompletionMode.AGGREGATOR. + + We use this information to determine if the block completion should depend on the completion of its children: + 1. If the block is an aggregator, it should be marked as completed when all its children are completed. + 2. If the block is completable, it should be directly marked as completed - regardless of its children. + """ + return { + block_type for (block_type, block_cls) in XBlock.load_classes() + if XBlockCompletionMode.get_mode(block_cls) == XBlockCompletionMode.AGGREGATOR + } + @cached_property def completable_block_types(self): """