diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 10ed2f580e..0857fdd4ce 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -200,33 +200,27 @@ class XBlockVisibilityTestCase(TestCase): def test_private_unreleased_xblock(self): """Verifies that a private unreleased xblock is not visible""" - vertical = self._create_xblock_with_start_date('private_unreleased', self.future) - self.assertFalse(utils.is_xblock_visible_to_students(vertical)) + self._test_visible_to_students(False, 'private_unreleased', self.future) def test_private_released_xblock(self): """Verifies that a private released xblock is not visible""" - vertical = self._create_xblock_with_start_date('private_released', self.past) - self.assertFalse(utils.is_xblock_visible_to_students(vertical)) + self._test_visible_to_students(False, 'private_released', self.past) def test_public_unreleased_xblock(self): """Verifies that a public (published) unreleased xblock is not visible""" - vertical = self._create_xblock_with_start_date('public_unreleased', self.future, publish=True) - self.assertFalse(utils.is_xblock_visible_to_students(vertical)) + self._test_visible_to_students(False, 'public_unreleased', self.future, publish=True) def test_public_released_xblock(self): - """Verifies that public (published) released xblock is visible""" - vertical = self._create_xblock_with_start_date('public_released', self.past, publish=True) - self.assertTrue(utils.is_xblock_visible_to_students(vertical)) + """Verifies that public (published) released xblock is visible if staff lock is not enabled.""" + self._test_visible_to_students(True, 'public_released', self.past, publish=True) def test_private_no_start_xblock(self): """Verifies that a private xblock with no start date is not visible""" - vertical = self._create_xblock_with_start_date('private_no_start', None) - self.assertFalse(utils.is_xblock_visible_to_students(vertical)) + self._test_visible_to_students(False, 'private_no_start', None) def test_public_no_start_xblock(self): - """Verifies that a public (published) xblock with no start date is visible""" - vertical = self._create_xblock_with_start_date('public_no_start', None, publish=True) - self.assertTrue(utils.is_xblock_visible_to_students(vertical)) + """Verifies that a public (published) xblock with no start date is visible unless staff lock is enabled""" + self._test_visible_to_students(True, 'public_no_start', None, publish=True) def test_draft_released_xblock(self): """Verifies that a xblock with an unreleased draft and a released published version is visible""" @@ -238,12 +232,28 @@ class XBlockVisibilityTestCase(TestCase): self.assertTrue(utils.is_xblock_visible_to_students(vertical)) - def _create_xblock_with_start_date(self, name, start_date, publish=False): + def _test_visible_to_students(self, expected_visible_without_lock, name, start_date, publish=False): + """ + Helper method that checks that is_xblock_visible_to_students returns the correct value both + with and without visible_to_staff_only set. + """ + no_staff_lock = self._create_xblock_with_start_date(name, start_date, publish, visible_to_staff_only=False) + self.assertEqual(expected_visible_without_lock, utils.is_xblock_visible_to_students(no_staff_lock)) + + # any xblock with visible_to_staff_only set to True should not be visible to students. + staff_lock = self._create_xblock_with_start_date( + name + "_locked", start_date, publish, visible_to_staff_only=True + ) + self.assertFalse(utils.is_xblock_visible_to_students(staff_lock)) + + def _create_xblock_with_start_date(self, name, start_date, publish=False, visible_to_staff_only=False): """Helper to create an xblock with a start date, optionally publishing it""" location = Location('edX', 'visibility', '2012_Fall', 'vertical', name) vertical = modulestore().create_xmodule(location) vertical.start = start_date + if visible_to_staff_only: + vertical.visible_to_staff_only = visible_to_staff_only modulestore().update_item(vertical, self.dummy_user, allow_not_found=True) if publish: diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 399aaa7f5c..f1ccdbc717 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -139,6 +139,10 @@ def is_xblock_visible_to_students(xblock): except ItemNotFoundError: return False + # If visible_to_staff_only is True, this xblock is not visible to students regardless of start date. + if published.visible_to_staff_only: + return False + # Check start date if 'detached' not in published._class_tags and published.start is not None: return datetime.now(UTC) > published.start diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 0db837f68b..561feeef3a 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -26,6 +26,7 @@ class CourseMetadata(object): 'name', # from xblock 'tags', # from xblock 'video_speed_optimizations', + 'visible_to_staff_only' ] @classmethod diff --git a/common/lib/xmodule/xmodule/modulestore/inheritance.py b/common/lib/xmodule/xmodule/modulestore/inheritance.py index 8228b8525f..4316655a7d 100644 --- a/common/lib/xmodule/xmodule/modulestore/inheritance.py +++ b/common/lib/xmodule/xmodule/modulestore/inheritance.py @@ -51,6 +51,11 @@ class InheritanceMixin(XBlockMixin): default=None, scope=Scope.user_state, ) + visible_to_staff_only = Boolean( + help=_("If true, can be seen only by course staff, regardless of start date."), + default=False, + scope=Scope.settings, + ) course_edit_method = String( display_name=_("Course Editor"), help=_("Enter the method by which this course is edited (\"XML\" or \"Studio\")."), diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py index fb64031652..e0746ba15a 100644 --- a/common/lib/xmodule/xmodule/tests/test_xml_module.py +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -563,3 +563,4 @@ class TestXmlAttributes(XModuleXmlImportTest): def test_inheritable_attributes(self): self.check_inheritable_attribute('days_early_for_beta', 2) self.check_inheritable_attribute('max_attempts', 5) + self.check_inheritable_attribute('visible_to_staff_only', True) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index f55a7cd419..f081862576 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -41,6 +41,7 @@ def has_access(user, action, obj, course_key=None): Things this module understands: - start dates for modules + - visible_to_staff_only for modules - DISABLE_START_DATES - different access for instructor, staff, course staff, and students. @@ -247,6 +248,9 @@ def _has_access_descriptor(user, action, descriptor, course_key=None): students to see modules. If not, views should check the course, so we don't have to hit the enrollments table on every module load. """ + if descriptor.visible_to_staff_only and not _has_staff_access_to_descriptor(user, descriptor, course_key): + return False + # If start dates are off, can always load if settings.FEATURES['DISABLE_START_DATES'] and not is_masquerading_as_student(user): debug("Allow: DISABLE_START_DATES") diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index e353c5bea6..1868779b35 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -1,6 +1,7 @@ import courseware.access as access import datetime +import mock from mock import Mock from django.test import TestCase @@ -94,6 +95,50 @@ class AccessTestCase(TestCase): with self.assertRaises(ValueError): access._has_access_descriptor(user, 'not_load_or_staff', date) + @mock.patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) + def test__has_access_descriptor_staff_lock(self): + """ + Tests that "visible_to_staff_only" overrides start date. + """ + mock_unit = Mock() + mock_unit._class_tags = {} # Needed for detached check in _has_access_descriptor + + def verify_access(student_should_have_access): + """ Verify the expected result from _has_access_descriptor """ + self.assertEqual(student_should_have_access, access._has_access_descriptor( + self.anonymous_user, 'load', mock_unit, course_key=self.course.course_key) + ) + # staff always has access + self.assertTrue(access._has_access_descriptor( + self.course_staff, 'load', mock_unit, course_key=self.course.course_key) + ) + + # No start date, staff lock on + mock_unit.visible_to_staff_only = True + verify_access(False) + + # No start date, staff lock off. + mock_unit.visible_to_staff_only = False + verify_access(True) + + # Start date in the past, staff lock on. + mock_unit.start = datetime.datetime.now(pytz.utc) - datetime.timedelta(days=1) + mock_unit.visible_to_staff_only = True + verify_access(False) + + # Start date in the past, staff lock off. + mock_unit.visible_to_staff_only = False + verify_access(True) + + # Start date in the future, staff lock on. + mock_unit.start = datetime.datetime.now(pytz.utc) + datetime.timedelta(days=1) # release date in the future + mock_unit.visible_to_staff_only = True + verify_access(False) + + # Start date in the future, staff lock off. + mock_unit.visible_to_staff_only = False + verify_access(False) + def test__has_access_course_desc_can_enroll(self): user = Mock() yesterday = datetime.datetime.now(pytz.utc) - datetime.timedelta(days=1) diff --git a/lms/lib/xblock/mixin.py b/lms/lib/xblock/mixin.py index c8b1f3af43..ede951dcea 100644 --- a/lms/lib/xblock/mixin.py +++ b/lms/lib/xblock/mixin.py @@ -49,3 +49,8 @@ class LmsBlockMixin(XBlockMixin): scope=Scope.settings, deprecated=True ) + visible_to_staff_only = Boolean( + help=_("If true, can be seen only by course staff, regardless of start date."), + default=False, + scope=Scope.settings, + )