diff --git a/lms/djangoapps/course_blocks/transformers/hidden_content.py b/lms/djangoapps/course_blocks/transformers/hidden_content.py index 9870667cec..2a331323d4 100644 --- a/lms/djangoapps/course_blocks/transformers/hidden_content.py +++ b/lms/djangoapps/course_blocks/transformers/hidden_content.py @@ -7,10 +7,7 @@ from datetime import datetime from pytz import utc -from openedx.core.djangoapps.content.block_structure.transformer import ( - BlockStructureTransformer, - FilteringTransformerMixin -) +from openedx.core.djangoapps.content.block_structure.transformer import BlockStructureTransformer from xmodule.seq_module import SequenceBlock from .utils import collect_merged_boolean_field, collect_merged_date_field @@ -18,21 +15,24 @@ from .utils import collect_merged_boolean_field, collect_merged_date_field MAXIMUM_DATE = utc.localize(datetime.max) -class HiddenContentTransformer(FilteringTransformerMixin, BlockStructureTransformer): +class HiddenContentTransformer(BlockStructureTransformer): """ A transformer that enforces the hide_after_due field on blocks by removing children blocks from the block structure for - which the user does not have access. The due and hide_after_due - fields on a block is percolated down to its descendants, so that + which the user does not have access. The hide_after_due + field on a block is percolated down to its descendants, so that all blocks enforce the hidden content settings from their ancestors. For a block with multiple parents, access is denied only if access is denied from all its parents. Staff users are exempted from hidden content rules. + + IMPORTANT: Must be run _after_ the DateOverrideTransformer from edx-when + in case the 'due' date on a block has been shifted for a user. """ WRITE_VERSION = 3 - READ_VERSION = 2 + READ_VERSION = 3 MERGED_DUE_DATE = 'merged_due_date' MERGED_HIDE_AFTER_DUE = 'merged_hide_after_due' @@ -55,16 +55,6 @@ class HiddenContentTransformer(FilteringTransformerMixin, BlockStructureTransfor block_key, cls, cls.MERGED_HIDE_AFTER_DUE, False ) - @classmethod - def _get_merged_due_date(cls, block_structure, block_key): - """ - Returns the merged value for the start 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_DUE_DATE, False - ) - @classmethod def collect(cls, block_structure): """ @@ -90,16 +80,12 @@ class HiddenContentTransformer(FilteringTransformerMixin, BlockStructureTransfor block_structure.request_xblock_fields('self_paced', 'end', 'due') - def transform_block_filters(self, usage_info, block_structure): + def transform(self, usage_info, block_structure): # Users with staff access bypass the Visibility check. if usage_info.has_staff_access: return [block_structure.create_universal_filter()] - return [ - block_structure.create_removal_filter( - lambda block_key: self._is_block_hidden(block_structure, block_key), - ), - ] + block_structure.remove_block_traversal(lambda block_key: self._is_block_hidden(block_structure, block_key)) def _is_block_hidden(self, block_structure, block_key): """ @@ -111,5 +97,9 @@ class HiddenContentTransformer(FilteringTransformerMixin, BlockStructureTransfor if self_paced: hidden_date = block_structure[block_structure.root_block_usage_key].end else: - hidden_date = self._get_merged_due_date(block_structure, block_key) + # Important Note: + # A small subtlety of grabbing the due date here is that this transformer relies on the + # DateOverrideTransformer (located in edx-when repo) to first set any overrides (one + # example is a user receiving an extension on an assignment). + hidden_date = block_structure.get_xblock_field(block_key, 'due', None) or MAXIMUM_DATE return not SequenceBlock.verify_current_content_visibility(hidden_date, hide_after_due) 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 4a041f7b6d..20812058c2 100644 --- a/lms/djangoapps/course_blocks/transformers/tests/test_hidden_content.py +++ b/lms/djangoapps/course_blocks/transformers/tests/test_hidden_content.py @@ -8,14 +8,20 @@ from datetime import timedelta import ddt from django.utils.timezone import now -from ..hidden_content import HiddenContentTransformer -from .helpers import BlockParentsMapTestCase, update_block +from edx_when.api import get_dates_for_course, set_date_for_block +from edx_when.field_data import DateOverrideTransformer + +from common.djangoapps.student.tests.factories import UserFactory +from lms.djangoapps.course_blocks.transformers.hidden_content import HiddenContentTransformer +from lms.djangoapps.course_blocks.transformers.tests.helpers import BlockParentsMapTestCase, update_block +from openedx.core.djangoapps.content.block_structure.tests.helpers import mock_registered_transformers +from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers @ddt.ddt class HiddenContentTransformerTestCase(BlockParentsMapTestCase): """ - VisibilityTransformer Test + HiddenContentTransformer Test """ TRANSFORMER_CLASS_TO_TEST = HiddenContentTransformer ALL_BLOCKS = {0, 1, 2, 3, 4, 5, 6} @@ -70,6 +76,7 @@ class HiddenContentTransformerTestCase(BlockParentsMapTestCase): hide_due_values, 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) @@ -82,3 +89,52 @@ class HiddenContentTransformerTestCase(BlockParentsMapTestCase): blocks_with_differing_access=None, transformers=self.transformers, ) + + def test_hidden_content_with_transformer_override(self): + """ + Tests content is hidden if the date changes after collection and + during the transform phase (for example, by the DateOverrideTransformer). + """ + with mock_registered_transformers([DateOverrideTransformer, self.TRANSFORMER_CLASS_TO_TEST]): + transformers = BlockStructureTransformers( + [DateOverrideTransformer(self.student), self.TRANSFORMER_CLASS_TO_TEST()] + ) + + 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) + + # Due date is in the past so some blocks are hidden + self.assert_transform_results( + self.student, + self.ALL_BLOCKS - {1, 3, 4}, + blocks_with_differing_access=None, + transformers=transformers, + ) + + # 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) + # 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) + + # Now all blocks are returned for the student + self.assert_transform_results( + self.student, + self.ALL_BLOCKS, + blocks_with_differing_access=None, + transformers=transformers, + ) + + # But not for a different user + different_user = UserFactory() + with mock_registered_transformers([DateOverrideTransformer, self.TRANSFORMER_CLASS_TO_TEST]): + transformers = BlockStructureTransformers( + [DateOverrideTransformer(different_user), self.TRANSFORMER_CLASS_TO_TEST()] + ) + self.assert_transform_results( + different_user, + self.ALL_BLOCKS - {1, 3, 4}, + blocks_with_differing_access=None, + transformers=transformers, + )