diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 7dd23b785f..ab22747b52 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -279,6 +279,74 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): datetime.now(UTC) < date ) + def _has_access_check(self, *args): + """ + Helper method created for the following method + gate_sequence_if_it_is_a_timed_exam_and_contains_content_type_gated_problems + Created to simplify the relevant unit tests + This way the has_access call can be patched without impacting other has_access calls made during the test + """ + # importing here to avoid a circular import + from lms.djangoapps.courseware.access import has_access + return has_access(*args) + + def gate_sequence_if_it_is_a_timed_exam_and_contains_content_type_gated_problems(self): + """ + Problem: + Content type gating for FBE (Feature Based Enrollments) previously only gated individual blocks. + This was an issue because audit learners could start a timed exam + and then be unable to complete it because the graded content would be gated. + Even if they later upgraded, they could still be unable to complete the exam + because the timer could have expired. + + Solution: + Gate the entire sequence when we think the above problem can occur. + + If: + 1. This sequence is a timed exam + 2. And this sequence contains problems which this user cannot load due to content type gating + Then: + We will gate access to the entire sequence. + Otherwise, learners would have the ability to start their timer for an exam, + but then not have the ability to complete it. + + We are displaying the gating fragment within the sequence, as is done for gating for prereqs, + rather than content type gating the entire sequence because that would remove the next/previous navigation. + + This functionality still needs to be replicated in the frontend-app-learning courseware MFE + The ticket to track this is https://openedx.atlassian.net/browse/REV-1220 + Note that this will break compatability with using sequences outside of edx-platform + but we are ok with this for now + """ + # importing here to avoid a circular import + from openedx.features.content_type_gating.models import ContentTypeGatingConfig + + if not self.is_time_limited: + return + + try: + user = User.objects.get(id=self.runtime.user_id) + if not ContentTypeGatingConfig.enabled_for_enrollment(user=user, course_key=self.runtime.course_id): + return + + for vertical in self.get_children(): + for block in vertical.get_children(): + problem_eligible_for_content_gating = (getattr(block, 'graded', False) and + block.has_score and + getattr(block, 'weight', 0) != 0) + if problem_eligible_for_content_gating: + access = self._has_access_check(user, 'load', block, self.course_id) + # If any block has been gated by content type gating inside the sequence + # and the sequence is a timed exam, then gate the entire sequence. + # In order to avoid scope creep, we are not handling other potential causes + # of access failures as part of this work. + if not access and access.error_code == 'incorrect_user_group': + self.gated_sequence_fragment = access.user_fragment + break + self.gated_sequence_fragment = None # Don't gate other cases + except User.DoesNotExist: + pass + def student_view(self, context): _ = self.runtime.service(self, "i18n").ugettext context = context or {} @@ -287,32 +355,8 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): prereq_met = True prereq_meta_info = {} - if TIMED_EXAM_GATING_WAFFLE_FLAG.is_enabled(): - # Content type gating for FBE previously only gated individual blocks - # This was an issue because audit learners could start a timed exam and then be unable to complete the exam - # even if they later upgrade because the timer would have expired. - # For this reason we check if content gating is enabled for the user - # and gate the entire sequence in that case - # This functionality still needs to be replicated in the frontend-app-learning courseware MFE - # The ticket to track this is https://openedx.atlassian.net/browse/REV-1220 - # Note that this will break compatability with using sequences outside of edx-platform - # but we are ok with this for now - if self.is_time_limited: - try: - user = User.objects.get(id=self.runtime.user_id) - # importing here to avoid a circular import - from openedx.features.content_type_gating.models import ContentTypeGatingConfig - from openedx.features.content_type_gating.helpers import CONTENT_GATING_PARTITION_ID - if ContentTypeGatingConfig.enabled_for_enrollment(user=user, course_key=self.runtime.course_id): - # Get the content type gating locked content fragment to render for this sequence - partition = self.descriptor._get_user_partition(CONTENT_GATING_PARTITION_ID) # pylint: disable=protected-access - user_group = partition.scheme.get_group_for_user(self.runtime.course_id, user, partition) - self.gated_sequence_fragment = partition.access_denied_fragment( - self.descriptor, user, user_group, [] - ) - except User.DoesNotExist: - pass + self.gate_sequence_if_it_is_a_timed_exam_and_contains_content_type_gated_problems() if self._required_prereq(): if self.runtime.user_is_staff: diff --git a/common/lib/xmodule/xmodule/tests/test_sequence.py b/common/lib/xmodule/xmodule/tests/test_sequence.py index 03919a5d4d..6ed038fff6 100644 --- a/common/lib/xmodule/xmodule/tests/test_sequence.py +++ b/common/lib/xmodule/xmodule/tests/test_sequence.py @@ -14,7 +14,9 @@ from django.utils.timezone import now from freezegun import freeze_time from mock import Mock, patch from six.moves import range +from web_fragments.fragment import Fragment +from lms.djangoapps.courseware.access_response import AccessResponse from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag from student.tests.factories import UserFactory from xmodule.seq_module import TIMED_EXAM_GATING_WAFFLE_FLAG, SequenceModule @@ -76,10 +78,12 @@ class SequenceBlockTestCase(XModuleXmlImportTest): for _ in range(3): xml.VerticalFactory.build(parent=sequence_3_1) - xml.SequenceFactory.build( + sequence_5_1 = xml.SequenceFactory.build( parent=chapter_5, is_time_limited=str(True) ) + vertical_5_1 = xml.VerticalFactory.build(parent=sequence_5_1) + xml.ProblemFactory.build(parent=vertical_5_1) return course @@ -181,6 +185,63 @@ class SequenceBlockTestCase(XModuleXmlImportTest): ) mocked_user.assert_called_once() + @override_waffle_flag(TIMED_EXAM_GATING_WAFFLE_FLAG, active=True) + @patch('xmodule.seq_module.User.objects.get', return_value=UserFactory.build()) + @patch('openedx.features.content_type_gating.models.ContentTypeGatingConfig.enabled_for_enrollment', + return_value=True) + def test_that_timed_sequence_gating_respects_access_configurations(self, mocked_user, mocked_config): # pylint: disable=unused-argument + """ + Verify that if a time limited sequence contains content type gated problems, we gate the sequence + Verify that if a time limited sequence contains gated problems, but not due to content type gating, + then sequence is not gated + Verify that if all problems in a time limited sequence can be accessed, the sequence is not gated + """ + # the one problem in this sequence needs to have graded set to true in order to test content type gating + self.sequence_5_1.get_children()[0].get_children()[0].graded = True + gated_fragment = Fragment('i_am_gated') + + # When a time limited sequence contains content type gated problems, the sequence itself is gated + content_gating_error = AccessResponse(False, error_code="incorrect_user_group", user_fragment=gated_fragment) + with patch.object(SequenceModule, '_has_access_check', return_value=content_gating_error): + view = self._get_rendered_view( + self.sequence_5_1, + extra_context=dict(next_url='NextSequential', prev_url='PrevSequential'), + view=STUDENT_VIEW + ) + self.assertIn('i_am_gated', view) + # check a few elements to ensure the correct page was loaded + self.assertIn("seq_module.html", view) + self.assertIn('NextSequential', view) + self.assertIn('PrevSequential', view) + + # When a time limited sequence contains inaccessible problems for reasons other than content type gating + # the sequence is not gated, because handling these cases was out of scope for this ticket + some_other_error = AccessResponse(False, error_code="some_other_error", user_fragment=gated_fragment) + with patch.object(SequenceModule, '_has_access_check', return_value=some_other_error): + view = self._get_rendered_view( + self.sequence_5_1, + extra_context=dict(next_url='NextSequential', prev_url='PrevSequential'), + view=STUDENT_VIEW + ) + self.assertNotIn('i_am_gated', view) + # check a few elements to ensure the correct page was loaded + self.assertIn("seq_module.html", view) + self.assertIn('NextSequential', view) + self.assertIn('PrevSequential', view) + + # When all problems inside a time limited sequence can be accessed, the sequence is not gated + with patch.object(SequenceModule, '_has_access_check', return_value=AccessResponse(True)): + view = self._get_rendered_view( + self.sequence_5_1, + extra_context=dict(next_url='NextSequential', prev_url='PrevSequential'), + view=STUDENT_VIEW + ) + self.assertNotIn('i_am_gated', view) + # check a few elements to ensure the correct page was loaded + self.assertIn("seq_module.html", view) + self.assertIn('NextSequential', view) + self.assertIn('PrevSequential', view) + @ddt.unpack @ddt.data( {'view': STUDENT_VIEW},