Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Merge pull request #27622 from edx/ddumesnil/update-completion-aa-802

Browse Source 
feat: AA-802: Update the BlockCompletionTransformer
This commit is contained in:
Dillon Dumesnil
2021-05-17 06:05:29 -07:00
committed by GitHub
parent ba69657b29 6f773d1eca
commit e885733881
4 changed files with 94 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
lms/djangoapps/course_api/blocks/api.py
View File

@@ -88,6 +88,13 @@ def get_blocks(
HiddenContentTransformer()
]
# Note: A change to the BlockCompletionTransformer (https://github.com/edx/edx-platform/pull/27622/)
# will be introducing a bug if hide_access_denials is True. I'm accepting this risk because in
# the AccessDeniedMessageFilterTransformer, there is note about deleting it and I believe it is
# technically deprecated functionality. The only use case where hide_access_denials is True
# (outside of explicitly setting the temporary waffle flag) is in lms/djangoapps/course_api/blocks/urls.py
# for a v1 api that I also believe should have been deprecated and removed. When this code is removed,
# please also remove this comment. Thanks!
if hide_access_denials:
transformers += [AccessDeniedMessageFilterTransformer()]

18
lms/djangoapps/course_api/blocks/transformers/block_completion.py
View File

@@ -71,17 +71,17 @@ class BlockCompletionTransformer(BlockStructureTransformer):
block_structure.override_xblock_field(block_key, self.COMPLETE, True)
if str(block_key) == str(latest_complete_block_key):
block_structure.override_xblock_field(block_key, self.RESUME_BLOCK, True)
elif block_structure.get_xblock_field(block_key, 'completion_mode') == CompletionMode.AGGREGATOR:
children = block_structure.get_children(block_key)
all_children_complete = all(block_structure.get_xblock_field(child_key, self.COMPLETE)
for child_key in children
if not self._is_block_excluded(block_structure, child_key))
children = block_structure.get_children(block_key)
all_children_complete = all(block_structure.get_xblock_field(child_key, self.COMPLETE)
for child_key in children
if not self._is_block_excluded(block_structure, child_key))
if all_children_complete:
block_structure.override_xblock_field(block_key, self.COMPLETE, True)
if children and all_children_complete:
block_structure.override_xblock_field(block_key, self.COMPLETE, True)
if any(block_structure.get_xblock_field(child_key, self.RESUME_BLOCK) for child_key in children):
block_structure.override_xblock_field(block_key, self.RESUME_BLOCK, True)
if any(block_structure.get_xblock_field(child_key, self.RESUME_BLOCK) for child_key in children):
block_structure.override_xblock_field(block_key, self.RESUME_BLOCK, True)
def transform(self, usage_info, block_structure):
"""

88
lms/djangoapps/course_api/blocks/transformers/tests/test_block_completion.py
View File

@@ -21,6 +21,7 @@ class StubAggregatorXBlock(XBlock):
when transforming aggregator XBlock.
"""
completion_mode = XBlockCompletionMode.AGGREGATOR
has_children = True
class StubExcludedXBlock(XBlock):
@@ -36,7 +37,6 @@ class StubCompletableXBlock(XBlock, CompletableXBlockMixin):
XBlock to test behaviour of BlockCompletionTransformer
when transforming completable XBlock.
"""
pass # lint-amnesty, pylint: disable=unnecessary-pass
class BlockCompletionTransformerTestCase(TransformerRegistryTestMixin, CompletionWaffleTestMixin, ModuleStoreTestCase):
@@ -44,7 +44,9 @@ class BlockCompletionTransformerTestCase(TransformerRegistryTestMixin, Completio
Tests behaviour of BlockCompletionTransformer
"""
TRANSFORMER_CLASS_TO_TEST = BlockCompletionTransformer
COMPLETION_TEST_VALUE = 0.4
# Has to be 1.0 for 'complete' to function properly. The Completion api only uses 0.0 and 1.0 right now
# so this better reflects reality anyway. Should be updated if Completion api ever supports more.
COMPLETION_TEST_VALUE = 1.0
def setUp(self):
super().setUp()
@@ -53,31 +55,54 @@ class BlockCompletionTransformerTestCase(TransformerRegistryTestMixin, Completio
self.override_waffle_switch(True)
@XBlock.register_temp_plugin(StubAggregatorXBlock, identifier='aggregator')
def test_transform_gives_none_for_aggregator(self):
@XBlock.register_temp_plugin(StubCompletableXBlock, identifier='comp')
def test_transform_aggregators(self):
"""
If there is an aggregator (such as a unit) that contains no children for some reason
(likely a mistake by the course team), it should be marked 'complete'. 'completion' should
still be None.
We mark 'complete' so learners are still able to have their subsections/sections/course
marked as complete and are not blocked by this one empty aggregator.
"""
course = CourseFactory.create()
block = ItemFactory.create(category='aggregator', parent=course)
block_structure = get_course_blocks(
self.user, course.location, self.transformers
# Have to have at least one complete block to trigger entering the marking 'complete' flow
filled_aggregator = ItemFactory.create(category='aggregator', parent=course)
block = ItemFactory.create(category='comp', parent=filled_aggregator)
BlockCompletion.objects.submit_completion(
user=self.user,
block_key=block.location,
completion=self.COMPLETION_TEST_VALUE,
)
empty_aggregator = ItemFactory.create(category='aggregator', parent=course)
block_structure = get_course_blocks(self.user, course.location, self.transformers)
self._assert_block_has_proper_completion_value(
block_structure, block.location, None
self._assert_block_has_proper_completion_values(
block_structure, block.location, self.COMPLETION_TEST_VALUE, True
)
self._assert_block_has_proper_completion_values(
block_structure, filled_aggregator.location, None, True
)
self._assert_block_has_proper_completion_values(
block_structure, empty_aggregator.location, None, True
)
@XBlock.register_temp_plugin(StubExcludedXBlock, identifier='excluded')
def test_transform_gives_none_for_excluded(self):
"""
Excluded blocks always receive None for 'completion' and False for 'complete'
"""
course = CourseFactory.create()
block = ItemFactory.create(category='excluded', parent=course)
block_structure = get_course_blocks(
self.user, course.location, self.transformers
)
block_structure = get_course_blocks(self.user, course.location, self.transformers)
self._assert_block_has_proper_completion_value(
block_structure, block.location, None
)
self._assert_block_has_proper_completion_values(block_structure, block.location, None, False)
@XBlock.register_temp_plugin(StubCompletableXBlock, identifier='comp')
def test_transform_gives_value_for_completable(self):
"""
If a block is actually complete, make sure that value shows up in the transformed fields.
'completion' should have the value and 'complete' should be True in these cases.
"""
course = CourseFactory.create()
block = ItemFactory.create(category='comp', parent=course)
BlockCompletion.objects.submit_completion(
@@ -85,34 +110,33 @@ class BlockCompletionTransformerTestCase(TransformerRegistryTestMixin, Completio
block_key=block.location,
completion=self.COMPLETION_TEST_VALUE,
)
block_structure = get_course_blocks(
self.user, course.location, self.transformers
)
block_structure = get_course_blocks(self.user, course.location, self.transformers)
self._assert_block_has_proper_completion_value(
block_structure, block.location, self.COMPLETION_TEST_VALUE
self._assert_block_has_proper_completion_values(
block_structure, block.location, self.COMPLETION_TEST_VALUE, True
)
def test_transform_gives_zero_for_ordinary_block(self):
"""
I _think_ this is testing 'completion' is 0.0 before an ordinary block is viewed.
'html' blocks end up receiving a 'completion' of 1.0 after being viewed.
"""
course = CourseFactory.create()
block = ItemFactory.create(category='html', parent=course)
block_structure = get_course_blocks(
self.user, course.location, self.transformers
)
block_structure = get_course_blocks(self.user, course.location, self.transformers)
self._assert_block_has_proper_completion_value(
block_structure, block.location, 0.0
)
self._assert_block_has_proper_completion_values(block_structure, block.location, 0.0, False)
def _assert_block_has_proper_completion_value(
self, block_structure, block_key, expected_value
def _assert_block_has_proper_completion_values(
self, block_structure, block_key, expected_completion, expected_complete
):
"""
Checks whether block's completion has expected value.
Checks whether block's completion and complete have expected values.
"""
block_data = block_structure.get_transformer_block_data(
block_key, self.TRANSFORMER_CLASS_TO_TEST
)
block_data = block_structure.get_transformer_block_data(block_key, self.TRANSFORMER_CLASS_TO_TEST)
completion_value = block_data.fields['completion']
# complete isn't saved as a transformer field, but just a regular xblock field /shrug
complete_value = block_structure.get_xblock_field(block_key, 'complete', False)
assert completion_value == expected_value
assert completion_value == expected_completion
assert complete_value == expected_complete

38
openedx/features/course_experience/tests/views/test_course_outline.py
View File

@@ -462,6 +462,9 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT
vertical2 = ItemFactory.create(category='vertical', parent_location=sequential2.location)
vertical3 = ItemFactory.create(category='vertical', parent_location=sequential3.location)
vertical4 = ItemFactory.create(category='vertical', parent_location=sequential4.location)
problem = ItemFactory.create(category='problem', parent_location=vertical.location)
problem2 = ItemFactory.create(category='problem', parent_location=vertical2.location)
problem3 = ItemFactory.create(category='problem', parent_location=vertical3.location)
course.children = [chapter, chapter2]
chapter.children = [sequential, sequential2]
chapter2.children = [sequential3, sequential4]
@@ -469,6 +472,9 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT
sequential2.children = [vertical2]
sequential3.children = [vertical3]
sequential4.children = [vertical4]
vertical.children = [problem]
vertical2.children = [problem2]
vertical3.children = [problem3]
if hasattr(cls, 'user'):
CourseEnrollment.enroll(cls.user, course.id)
return course
@@ -545,8 +551,8 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT
response = self.client.get(course_home_url(course))
content = pq(response.content)
# Subsection should be checked
assert len(content('.fa-check')) == 1
# Subsection should be checked. Subsection 4 is also checked because it contains a vertical with no content
assert len(content('.fa-check')) == 2
def test_start_course(self):
"""
@@ -561,8 +567,8 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT
response = self.visit_course_home(course, start_count=1, resume_count=0)
content = pq(response.content)
vertical = course.children[0].children[0].children[0]
assert content('.action-resume-course').attr('href').endswith('/vertical/' + vertical.url_name)
problem = course.children[0].children[0].children[0].children[0]
assert content('.action-resume-course').attr('href').endswith('/problem/' + problem.url_name)
@override_settings(LMS_BASE='test_url:9999')
def test_resume_course_with_completion_api(self):
@@ -573,33 +579,33 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT
# Course tree
course = self.course
vertical1 = course.children[0].children[0].children[0]
vertical2 = course.children[0].children[1].children[0]
problem1 = course.children[0].children[0].children[0].children[0]
problem2 = course.children[0].children[1].children[0].children[0]
self.complete_sequential(self.course, vertical1)
self.complete_sequential(self.course, problem1)
# Test for 'resume' link
response = self.visit_course_home(course, resume_count=1)
# Test for 'resume' link URL - should be vertical 1
# Test for 'resume' link URL - should be problem 1
content = pq(response.content)
assert content('.action-resume-course').attr('href').endswith('/vertical/' + vertical1.url_name)
assert content('.action-resume-course').attr('href').endswith('/problem/' + problem1.url_name)
self.complete_sequential(self.course, vertical2)
self.complete_sequential(self.course, problem2)
# Test for 'resume' link
response = self.visit_course_home(course, resume_count=1)
# Test for 'resume' link URL - should be vertical 2
# Test for 'resume' link URL - should be problem 2
content = pq(response.content)
assert content('.action-resume-course').attr('href').endswith('/vertical/' + vertical2.url_name)
assert content('.action-resume-course').attr('href').endswith('/problem/' + problem2.url_name)
# visit sequential 1, make sure 'Resume Course' URL is robust against 'Last Visited'
# (even though I visited seq1/vert1, 'Resume Course' still points to seq2/vert2)
self.visit_sequential(course, course.children[0], course.children[0].children[0])
# Test for 'resume' link URL - should be vertical 2 (last completed block, NOT last visited)
# Test for 'resume' link URL - should be problem 2 (last completed block, NOT last visited)
response = self.visit_course_home(course, resume_count=1)
content = pq(response.content)
assert content('.action-resume-course').attr('href').endswith('/vertical/' + vertical2.url_name)
assert content('.action-resume-course').attr('href').endswith('/problem/' + problem2.url_name)
def test_resume_course_deleted_sequential(self):
"""
@@ -662,8 +668,8 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT
CourseEnrollment.get_enrollment(self.user, course.id).delete()
response = self.visit_course_home(course, start_count=1, resume_count=0)
content = pq(response.content)
vertical = course.children[0].children[0].children[0]
assert content('.action-resume-course').attr('href').endswith('/vertical/' + vertical.url_name)
problem = course.children[0].children[0].children[0].children[0]
assert content('.action-resume-course').attr('href').endswith('/problem/' + problem.url_name)
@override_waffle_switch(ENABLE_COMPLETION_TRACKING_SWITCH, active=True)
def test_course_outline_auto_open(self):