fix: AA-724: Updating the HiddenContentTransformer

We heard about a bug where learners granted extensions would still
lose access to content if it was marked as "hidden after due date".
This was caused by the HiddenContentTransformer using the due date
from collection time (publish time) rather than the user date returned
from the edx-when DateOverrideTransformer.

A small subtletly of these PRs is that Transformers with the
FilteringTransformerMixin are executed before those without it so
part of the fix was to make the HiddenContentTransformer not use the
FilteringTransformerMixin to ensure the DateOverrideTransformer had
run first.

Part 2/3 - Updating transform method + updating Read version

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)

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,
+        )