Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: sidebar completion for completable XBlocks with children #36043

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 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 @@ -758,6 +758,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
Loading