From 400296ba788d80614426865a4bb95499778ebb68 Mon Sep 17 00:00:00 2001 From: "zia.fazal@arbisoft.com" Date: Wed, 15 Jun 2022 14:04:36 +0500 Subject: [PATCH] fix: Fixed broken HiddenContentTransformer when course pacing is self_paced. see commit 1ebe0e6 too --- .../transformers/hidden_content.py | 16 +++- .../transformers/tests/test_hidden_content.py | 88 +++++++++++++++---- 2 files changed, 82 insertions(+), 22 deletions(-) diff --git a/lms/djangoapps/course_blocks/transformers/hidden_content.py b/lms/djangoapps/course_blocks/transformers/hidden_content.py index 41ac25df6f..b0bbd7f137 100644 --- a/lms/djangoapps/course_blocks/transformers/hidden_content.py +++ b/lms/djangoapps/course_blocks/transformers/hidden_content.py @@ -32,7 +32,7 @@ class HiddenContentTransformer(BlockStructureTransformer): in case the 'due' date on a block has been shifted for a user. """ WRITE_VERSION = 4 - READ_VERSION = 3 + READ_VERSION = 4 MERGED_HIDE_AFTER_DUE = 'merged_hide_after_due' MERGED_END_DATE = 'merged_end_date' @@ -55,6 +55,16 @@ class HiddenContentTransformer(BlockStructureTransformer): block_key, cls, cls.MERGED_HIDE_AFTER_DUE, False ) + @classmethod + def _get_merged_end_date(cls, block_structure, block_key): + """ + Returns the merged value for the end date for the block with + the given block_key in the given block_structure. + """ + return block_structure.get_transformer_block_field( + block_key, cls, cls.MERGED_END_DATE + ) + @classmethod def collect(cls, block_structure): """ @@ -75,7 +85,7 @@ class HiddenContentTransformer(BlockStructureTransformer): default_date=MAXIMUM_DATE ) - block_structure.request_xblock_fields('self_paced', 'end', 'due') + block_structure.request_xblock_fields('self_paced', 'due') def transform(self, usage_info, block_structure): # Users with staff access bypass the Visibility check. @@ -92,7 +102,7 @@ class HiddenContentTransformer(BlockStructureTransformer): hide_after_due = self._get_merged_hide_after_due(block_structure, block_key) self_paced = block_structure[block_structure.root_block_usage_key].self_paced if self_paced: - hidden_date = block_structure[block_structure.root_block_usage_key].end + hidden_date = self._get_merged_end_date(block_structure, block_key) else: # Important Note: # A small subtlety of grabbing the due date here is that this transformer relies on the diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_hidden_content.py b/lms/djangoapps/course_blocks/transformers/tests/test_hidden_content.py index 20812058c2..f2039a7b1d 100644 --- a/lms/djangoapps/course_blocks/transformers/tests/test_hidden_content.py +++ b/lms/djangoapps/course_blocks/transformers/tests/test_hidden_content.py @@ -26,7 +26,7 @@ class HiddenContentTransformerTestCase(BlockParentsMapTestCase): TRANSFORMER_CLASS_TO_TEST = HiddenContentTransformer ALL_BLOCKS = {0, 1, 2, 3, 4, 5, 6} - class DueDateType: + class DateType: """ Use constant enum types for deterministic ddt test method names (rather than dynamically generated timestamps) """ @@ -39,7 +39,7 @@ class HiddenContentTransformerTestCase(BlockParentsMapTestCase): FUTURE_DATE = TODAY + timedelta(days=30) @classmethod - def due(cls, enum_value): + def get_date(cls, enum_value): """ Returns a start date for the given enum value """ @@ -50,25 +50,32 @@ class HiddenContentTransformerTestCase(BlockParentsMapTestCase): else: return None - # Following test cases are based on BlockParentsMapTestCase.parents_map + # Following test cases are based on BlockParentsMapTestCase.parents_map: + # 0 + # / \ + # 1 2 + # / \ / \ + # 3 4 / 5 + # \ / + # 6 @ddt.data( ({}, ALL_BLOCKS), - ({0: DueDateType.none}, ALL_BLOCKS), - ({0: DueDateType.future}, ALL_BLOCKS), - ({1: DueDateType.none}, ALL_BLOCKS), - ({1: DueDateType.future}, ALL_BLOCKS), - ({4: DueDateType.none}, ALL_BLOCKS), - ({4: DueDateType.future}, ALL_BLOCKS), + ({0: DateType.none}, ALL_BLOCKS), + ({0: DateType.future}, ALL_BLOCKS), + ({1: DateType.none}, ALL_BLOCKS), + ({1: DateType.future}, ALL_BLOCKS), + ({4: DateType.none}, ALL_BLOCKS), + ({4: DateType.future}, ALL_BLOCKS), - ({0: DueDateType.past}, {}), - ({1: DueDateType.past}, ALL_BLOCKS - {1, 3, 4}), - ({2: DueDateType.past}, ALL_BLOCKS - {2, 5}), - ({4: DueDateType.past}, ALL_BLOCKS - {4}), + ({0: DateType.past}, {}), + ({1: DateType.past}, ALL_BLOCKS - {1, 3, 4}), + ({2: DateType.past}, ALL_BLOCKS - {2, 5}), + ({4: DateType.past}, ALL_BLOCKS - {4}), - ({1: DueDateType.past, 2: DueDateType.past}, {0}), - ({1: DueDateType.none, 2: DueDateType.past}, ALL_BLOCKS - {2, 5}), - ({1: DueDateType.past, 2: DueDateType.none}, ALL_BLOCKS - {1, 3, 4}), + ({1: DateType.past, 2: DateType.past}, {0}), + ({1: DateType.none, 2: DateType.past}, ALL_BLOCKS - {2, 5}), + ({1: DateType.past, 2: DateType.none}, ALL_BLOCKS - {1, 3, 4}), ) @ddt.unpack def test_hidden_content( @@ -77,9 +84,52 @@ class HiddenContentTransformerTestCase(BlockParentsMapTestCase): expected_visible_blocks, ): """ Tests content is hidden if due date is in the past """ + for idx, due_date_type in hide_due_values.items(): block = self.get_block(idx) - block.due = self.DueDateType.due(due_date_type) + block.due = self.DateType.get_date(due_date_type) + block.hide_after_due = True + update_block(block) + + self.assert_transform_results( + self.student, + expected_visible_blocks, + blocks_with_differing_access=None, + transformers=self.transformers, + ) + + @ddt.data( + (DateType.none, {}, ALL_BLOCKS), + + (DateType.none, {0}, ALL_BLOCKS), + (DateType.future, {0, 2}, ALL_BLOCKS), + (DateType.none, {1}, ALL_BLOCKS), + (DateType.future, {1}, ALL_BLOCKS), + (DateType.none, {4}, ALL_BLOCKS), + (DateType.future, {4}, ALL_BLOCKS), + + (DateType.past, {0}, {}), + (DateType.past, {1}, ALL_BLOCKS - {1, 3, 4}), + (DateType.past, {2}, ALL_BLOCKS - {2, 5}), + (DateType.past, {4}, ALL_BLOCKS - {4}), + (DateType.past, {1, 2}, {0}), + (DateType.past, {2, 4}, ALL_BLOCKS - {2, 4, 5, 6}), + ) + @ddt.unpack + def test_hidden_content_self_paced_course( + self, + course_end_date_type, + hide_after_due_blocks, + expected_visible_blocks, + ): + """ Tests content is hidden if end date is in the past and course is self paced """ + course = self.get_block(0) + course.self_paced = True + course.end = self.DateType.get_date(course_end_date_type) + update_block(course) + + for block_idx in hide_after_due_blocks: + block = self.get_block(block_idx) block.hide_after_due = True update_block(block) @@ -103,7 +153,7 @@ class HiddenContentTransformerTestCase(BlockParentsMapTestCase): block = self.get_block(1) block.hide_after_due = True update_block(block) - set_date_for_block(self.course.id, block.location, 'due', self.DueDateType.PAST_DATE) + set_date_for_block(self.course.id, block.location, 'due', self.DateType.PAST_DATE) # Due date is in the past so some blocks are hidden self.assert_transform_results( @@ -114,7 +164,7 @@ class HiddenContentTransformerTestCase(BlockParentsMapTestCase): ) # Set an override for the due date to be in the future - set_date_for_block(self.course.id, block.location, 'due', self.DueDateType.FUTURE_DATE, user=self.student) + set_date_for_block(self.course.id, block.location, 'due', self.DateType.FUTURE_DATE, user=self.student) # this line is just to bust the cache for the user so it returns the updated date. get_dates_for_course(self.course.id, user=self.student, use_cached=False)