Skip to content

Commit

Permalink
fix: sidebar completion for completable XBlocks with children
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
Agrendalath committed Dec 17, 2024
1 parent 98214a5 commit 2095e35
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 1 deletion.
40 changes: 40 additions & 0 deletions lms/djangoapps/course_home_api/outline/tests/test_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
16 changes: 15 additions & 1 deletion lms/djangoapps/course_home_api/outline/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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):
"""
Expand Down

0 comments on commit 2095e35

Please sign in to comment.