From 12a5781725ae7bf78cfcb33c796107c864f12d81 Mon Sep 17 00:00:00 2001 From: Ben McMorran Date: Mon, 4 Aug 2014 17:16:19 -0400 Subject: [PATCH] Add ability to set staff lock from course outline --- .../contentstore/tests/test_utils.py | 108 +++++ cms/djangoapps/contentstore/utils.py | 38 ++ cms/djangoapps/contentstore/views/item.py | 55 ++- .../contentstore/views/tests/test_item.py | 182 +++++++- cms/static/js/models/xblock_info.js | 22 +- cms/static/js/spec/models/xblock_info_spec.js | 4 +- .../views/pages/container_subviews_spec.js | 109 ++++- .../spec/views/pages/course_outline_spec.js | 37 +- cms/static/js/views/course_outline.js | 3 +- .../js/views/modals/edit_outline_item.js | 89 +++- .../js/views/pages/container_subviews.js | 21 +- cms/static/js/views/utils/xblock_utils.js | 15 +- cms/static/js/views/xblock_outline.js | 26 +- cms/static/sass/elements/_forms.scss | 40 ++ cms/static/sass/elements/_modal-window.scss | 66 ++- cms/static/sass/views/_container.scss | 12 + cms/templates/js/course-outline.underscore | 2 +- .../js/edit-outline-item-modal.underscore | 80 +++- cms/templates/js/publish-xblock.underscore | 22 +- .../test/acceptance/pages/lms/courseware.py | 16 + .../test/acceptance/pages/studio/container.py | 11 +- .../test/acceptance/pages/studio/overview.py | 51 +++ .../acceptance/tests/test_studio_container.py | 58 ++- .../acceptance/tests/test_studio_outline.py | 387 +++++++++++++++++- 24 files changed, 1305 insertions(+), 149 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 8ec18168df..a7399b0abd 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -315,3 +315,111 @@ class ReleaseDateSourceTest(CourseTestCase): """Tests a sequential's release date being set by itself""" self._update_release_dates(self.date_one, self.date_two, self.date_two) self._verify_release_date_source(self.sequential, self.sequential) + + +class StaffLockTest(CourseTestCase): + """Base class for testing staff lock functions.""" + + def setUp(self): + super(StaffLockTest, self).setUp() + + self.chapter = ItemFactory.create(category='chapter', parent_location=self.course.location) + self.sequential = ItemFactory.create(category='sequential', parent_location=self.chapter.location) + self.vertical = ItemFactory.create(category='vertical', parent_location=self.sequential.location) + self.orphan = ItemFactory.create(category='vertical', parent_location=self.sequential.location) + + # Read again so that children lists are accurate + self.chapter = self.store.get_item(self.chapter.location) + self.sequential = self.store.get_item(self.sequential.location) + self.vertical = self.store.get_item(self.vertical.location) + + # Orphan the orphaned xblock + self.sequential.children = [self.vertical.location] + self.sequential = self.store.update_item(self.sequential, ModuleStoreEnum.UserID.test) + + def _set_staff_lock(self, xblock, is_locked): + """If is_locked is True, xblock is staff locked. Otherwise, the xblock staff lock field is removed.""" + field = xblock.fields['visible_to_staff_only'] + if is_locked: + field.write_to(xblock, True) + else: + field.delete_from(xblock) + return self.store.update_item(xblock, ModuleStoreEnum.UserID.test) + + def _update_staff_locks(self, chapter_locked, sequential_locked, vertical_locked): + """ + Sets the staff lock on the chapter, sequential, and vertical + If the corresponding argument is False, then the field is deleted from the xblock + """ + self.chapter = self._set_staff_lock(self.chapter, chapter_locked) + self.sequential = self._set_staff_lock(self.sequential, sequential_locked) + self.vertical = self._set_staff_lock(self.vertical, vertical_locked) + + +class StaffLockSourceTest(StaffLockTest): + """Tests for finding the source of an xblock's staff lock.""" + + def _verify_staff_lock_source(self, item, expected_source): + """Helper to verify that the staff lock source of a given item matches the expected source""" + source = utils.find_staff_lock_source(item) + self.assertEqual(source.location, expected_source.location) + self.assertTrue(source.visible_to_staff_only) + + def test_chapter_source_for_vertical(self): + """Tests a vertical's staff lock being set by its chapter""" + self._update_staff_locks(True, False, False) + self._verify_staff_lock_source(self.vertical, self.chapter) + + def test_sequential_source_for_vertical(self): + """Tests a vertical's staff lock being set by its sequential""" + self._update_staff_locks(True, True, False) + self._verify_staff_lock_source(self.vertical, self.sequential) + self._update_staff_locks(False, True, False) + self._verify_staff_lock_source(self.vertical, self.sequential) + + def test_vertical_source_for_vertical(self): + """Tests a vertical's staff lock being set by itself""" + self._update_staff_locks(True, True, True) + self._verify_staff_lock_source(self.vertical, self.vertical) + self._update_staff_locks(False, True, True) + self._verify_staff_lock_source(self.vertical, self.vertical) + self._update_staff_locks(False, False, True) + self._verify_staff_lock_source(self.vertical, self.vertical) + + def test_orphan_has_no_source(self): + """Tests that a orphaned xblock has no staff lock source""" + self.assertIsNone(utils.find_staff_lock_source(self.orphan)) + + def test_no_source_for_vertical(self): + """Tests a vertical with no staff lock set anywhere""" + self._update_staff_locks(False, False, False) + self.assertIsNone(utils.find_staff_lock_source(self.vertical)) + + +class InheritedStaffLockTest(StaffLockTest): + """Tests for determining if an xblock inherits a staff lock.""" + + def test_no_inheritance(self): + """Tests that a locked or unlocked vertical with no locked ancestors does not have an inherited lock""" + self._update_staff_locks(False, False, False) + self.assertFalse(utils.ancestor_has_staff_lock(self.vertical)) + self._update_staff_locks(False, False, True) + self.assertFalse(utils.ancestor_has_staff_lock(self.vertical)) + + def test_inheritance_in_locked_section(self): + """Tests that a locked or unlocked vertical in a locked section has an inherited lock""" + self._update_staff_locks(True, False, False) + self.assertTrue(utils.ancestor_has_staff_lock(self.vertical)) + self._update_staff_locks(True, False, True) + self.assertTrue(utils.ancestor_has_staff_lock(self.vertical)) + + def test_inheritance_in_locked_subsection(self): + """Tests that a locked or unlocked vertical in a locked subsection has an inherited lock""" + self._update_staff_locks(False, True, False) + self.assertTrue(utils.ancestor_has_staff_lock(self.vertical)) + self._update_staff_locks(False, True, True) + self.assertTrue(utils.ancestor_has_staff_lock(self.vertical)) + + def test_no_inheritance_for_orphan(self): + """Tests that an orphaned xblock does not inherit staff lock""" + self.assertFalse(utils.ancestor_has_staff_lock(self.orphan)) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 21540614e3..b3b9ed3730 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -197,6 +197,44 @@ def find_release_date_source(xblock): return find_release_date_source(parent) +def find_staff_lock_source(xblock): + """ + Returns the xblock responsible for setting this xblock's staff lock, or None if the xblock is not staff locked. + If this xblock is explicitly locked, return it, otherwise find the ancestor which sets this xblock's staff lock. + """ + + # Stop searching if this xblock has explicitly set its own staff lock + if xblock.fields['visible_to_staff_only'].is_set_on(xblock): + return xblock + + # Stop searching at the section level + if xblock.category == 'chapter': + return None + + parent_location = modulestore().get_parent_location(xblock.location, + revision=ModuleStoreEnum.RevisionOption.draft_preferred) + # Orphaned xblocks set their own staff lock + if not parent_location: + return None + + parent = modulestore().get_item(parent_location) + return find_staff_lock_source(parent) + + +def ancestor_has_staff_lock(xblock, parent_xblock=None): + """ + Returns True iff one of xblock's ancestors has staff lock. + Can avoid mongo query by passing in parent_xblock. + """ + if parent_xblock is None: + parent_location = modulestore().get_parent_location(xblock.location, + revision=ModuleStoreEnum.RevisionOption.draft_preferred) + if not parent_location: + return False + parent_xblock = modulestore().get_item(parent_location) + return parent_xblock.visible_to_staff_only + + def add_extra_panel_tab(tab_type, course): """ Used to add the panel tab to a course if it does not exist. diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index bcbb105ba6..a246ec28a9 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -28,17 +28,18 @@ from xmodule.modulestore import ModuleStoreEnum, PublishState from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError from xmodule.modulestore.inheritance import own_metadata +from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES from xmodule.x_module import PREVIEW_VIEWS, STUDIO_VIEW, STUDENT_VIEW from xmodule.course_module import DEFAULT_START_DATE -from contentstore.utils import find_release_date_source from django.contrib.auth.models import User from util.date_utils import get_default_time_display from util.json_request import expect_json, JsonResponse from .access import has_course_access -from contentstore.utils import is_currently_visible_to_students +from contentstore.utils import find_release_date_source, find_staff_lock_source, is_currently_visible_to_students, \ + ancestor_has_staff_lock from contentstore.views.helpers import is_unit, xblock_studio_url, xblock_primary_child_category, \ xblock_type_display_name, get_parent_xblock from contentstore.views.preview import get_preview_fragment @@ -381,10 +382,10 @@ def _save_xblock(user, xblock, data=None, children=None, metadata=None, nullout= if grader_type is not None: result.update(CourseGradingModel.update_section_grader_type(xblock, grader_type, user)) - # If publish is set to 'republish' and this item has previously been published, then this - # new item should be republished. This is used by staff locking to ensure that changing the draft - # value of the staff lock will also update the published version. - if publish == 'republish': + # If publish is set to 'republish' and this item is not in direct only categories and has previously been published, + # then this item should be republished. This is used by staff locking to ensure that changing the draft + # value of the staff lock will also update the published version, but only at the unit level. + if publish == 'republish' and xblock.category not in DIRECT_ONLY_CATEGORIES: published = modulestore().compute_publish_state(xblock) != PublishState.private if published: publish = 'make_public' @@ -653,6 +654,7 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F # Treat DEFAULT_START_DATE as a magic number that means the release date has not been set release_date = get_default_time_display(xblock.start) if xblock.start != DEFAULT_START_DATE else None + visibility_state = _compute_visibility_state(xblock, child_info, is_unit_with_changes) if not xblock.category == 'course' else None published = modulestore().compute_publish_state(xblock) != PublishState.private xblock_info = { @@ -665,7 +667,8 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F 'studio_url': xblock_studio_url(xblock, parent_xblock), "released_to_students": datetime.now(UTC) > xblock.start, "release_date": release_date, - "visibility_state": _compute_visibility_state(xblock, child_info, is_unit_with_changes) if not xblock.category == 'course' else None, + "visibility_state": visibility_state, + "has_explicit_staff_lock": xblock.fields['visible_to_staff_only'].is_set_on(xblock), "start": xblock.fields['start'].to_json(xblock.start), "graded": xblock.graded, "due_date": get_default_time_display(xblock.due), @@ -681,6 +684,11 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F xblock_info['ancestor_info'] = _create_xblock_ancestor_info(xblock, course_outline) if child_info: xblock_info['child_info'] = child_info + if visibility_state == VisibilityState.staff_only: + xblock_info["ancestor_has_staff_lock"] = ancestor_has_staff_lock(xblock, parent_xblock) + else: + xblock_info["ancestor_has_staff_lock"] = False + # Currently, 'edited_by', 'published_by', and 'release_date_from', and 'has_changes' are only used by the # container page when rendering a unit. Since they are expensive to compute, only include them for units # that are not being rendered on the course outline. @@ -691,6 +699,17 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F xblock_info['has_changes'] = is_unit_with_changes if release_date: xblock_info["release_date_from"] = _get_release_date_from(xblock) + if visibility_state == VisibilityState.staff_only: + xblock_info["staff_lock_from"] = _get_staff_lock_from(xblock) + else: + xblock_info["staff_lock_from"] = None + if course_outline: + if xblock_info["has_explicit_staff_lock"]: + xblock_info["staff_only_message"] = True + elif child_info and child_info["children"]: + xblock_info["staff_only_message"] = all([child["staff_only_message"] for child in child_info["children"]]) + else: + xblock_info["staff_only_message"] = False return xblock_info @@ -818,9 +837,21 @@ def _get_release_date_from(xblock): """ Returns a string representation of the section or subsection that sets the xblock's release date """ - source = find_release_date_source(xblock) - # Translators: this will be a part of the release date message. - # For example, 'Released: Jul 02, 2014 at 4:00 UTC with Section "Week 1"' + return _xblock_type_and_display_name(find_release_date_source(xblock)) + + +def _get_staff_lock_from(xblock): + """ + Returns a string representation of the section or subsection that sets the xblock's release date + """ + source = find_staff_lock_source(xblock) + return _xblock_type_and_display_name(source) if source else None + + +def _xblock_type_and_display_name(xblock): + """ + Returns a string representation of the xblock's type and display name + """ return _('{section_or_subsection} "{display_name}"').format( - section_or_subsection=xblock_type_display_name(source), - display_name=source.display_name_with_default) + section_or_subsection=xblock_type_display_name(xblock), + display_name=xblock.display_name_with_default) diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 12aefeef47..ef2bba6d08 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -20,7 +20,7 @@ from contentstore.views.component import ( component_handler, get_component_templates ) -from contentstore.views.item import create_xblock_info, ALWAYS, VisibilityState +from contentstore.views.item import create_xblock_info, ALWAYS, VisibilityState, _xblock_type_and_display_name from contentstore.tests.utils import CourseTestCase from student.tests.factories import UserFactory from xmodule.capa_module import CapaDescriptor @@ -308,7 +308,8 @@ class TestCreateItem(ItemTest): # Check that its name is not None new_tab = self.get_item_from_modulestore(usage_key) - self.assertEquals(new_tab.display_name, 'Empty') + self.assertEquals(new_tab.display_name, 'Empty') + class TestDuplicateItem(ItemTest): """ @@ -669,6 +670,20 @@ class TestEditItem(ItemTest): ) self.assertEqual(published.display_name, new_display_name_2) + def test_direct_only_categories_not_republished(self): + """Verify that republish is ignored for items in DIRECT_ONLY_CATEGORIES""" + # Create a vertical child with published and unpublished versions. + # If the parent sequential is not re-published, then the child problem should also not be re-published. + resp = self.create_xblock(parent_usage_key=self.seq_usage_key, display_name='vertical', category='vertical') + vertical_usage_key = self.response_usage_key(resp) + vertical_update_url = reverse_usage_url('xblock_handler', vertical_usage_key) + self.client.ajax_post(vertical_update_url, data={'publish': 'make_public'}) + self.client.ajax_post(vertical_update_url, data={'metadata': {'display_name': 'New Display Name'}}) + + self._verify_published_with_draft(self.seq_usage_key) + self.client.ajax_post(self.seq_update_url, data={'publish': 'republish'}) + self._verify_published_with_draft(self.seq_usage_key) + def _make_draft_content_different_from_published(self): """ Helper method to create different draft and published versions of a problem. @@ -1323,10 +1338,13 @@ class TestXBlockPublishingInfo(ItemTest): """ Creates a child xblock for the given parent. """ - return ItemFactory.create( + child = ItemFactory.create( parent_location=parent.location, category=category, display_name=display_name, - user_id=self.user.id, publish_item=publish_item, visible_to_staff_only=staff_only + user_id=self.user.id, publish_item=publish_item ) + if staff_only: + self._enable_staff_only(child.location) + return child def _get_child_xblock_info(self, xblock_info, index): """ @@ -1346,6 +1364,17 @@ class TestXBlockPublishingInfo(ItemTest): include_children_predicate=ALWAYS, ) + def _get_xblock_outline_info(self, location): + """ + Returns the xblock info for the specified location as neeeded for the course outline page. + """ + return create_xblock_info( + modulestore().get_item(location), + include_child_info=True, + include_children_predicate=ALWAYS, + course_outline=True + ) + def _set_release_date(self, location, start): """ Sets the release date for the specified xblock. @@ -1354,12 +1383,12 @@ class TestXBlockPublishingInfo(ItemTest): xblock.start = start self.store.update_item(xblock, self.user.id) - def _set_staff_only(self, location, staff_only): + def _enable_staff_only(self, location): """ - Sets staff only for the specified xblock. + Enables staff only for the specified xblock. """ xblock = modulestore().get_item(location) - xblock.visible_to_staff_only = staff_only + xblock.visible_to_staff_only = True self.store.update_item(xblock, self.user.id) def _set_display_name(self, location, display_name): @@ -1370,22 +1399,50 @@ class TestXBlockPublishingInfo(ItemTest): xblock.display_name = display_name self.store.update_item(xblock, self.user.id) - def _verify_visibility_state(self, xblock_info, expected_state, path=None): + def _verify_xblock_info_state(self, xblock_info, xblock_info_field, expected_state, path=None, should_equal=True): """ - Verify the publish state of an item in the xblock_info. If no path is provided - then the root item will be verified. + Verify the state of an xblock_info field. If no path is provided then the root item will be verified. + If should_equal is True, assert that the current state matches the expected state, otherwise assert that they + do not match. """ if path: direct_child_xblock_info = self._get_child_xblock_info(xblock_info, path[0]) remaining_path = path[1:] if len(path) > 1 else None - self._verify_visibility_state(direct_child_xblock_info, expected_state, remaining_path) + self._verify_xblock_info_state(direct_child_xblock_info, xblock_info_field, expected_state, remaining_path, should_equal) else: - self.assertEqual(xblock_info['visibility_state'], expected_state) + if should_equal: + self.assertEqual(xblock_info[xblock_info_field], expected_state) + else: + self.assertNotEqual(xblock_info[xblock_info_field], expected_state) + + def _verify_has_staff_only_message(self, xblock_info, expected_state, path=None): + """ + Verify the staff_only_message field of xblock_info. + """ + self._verify_xblock_info_state(xblock_info, 'staff_only_message', expected_state, path) + + def _verify_visibility_state(self, xblock_info, expected_state, path=None, should_equal=True): + """ + Verify the publish state of an item in the xblock_info. + """ + self._verify_xblock_info_state(xblock_info, 'visibility_state', expected_state, path, should_equal) + + def _verify_explicit_staff_lock_state(self, xblock_info, expected_state, path=None, should_equal=True): + """ + Verify the explicit staff lock state of an item in the xblock_info. + """ + self._verify_xblock_info_state(xblock_info, 'has_explicit_staff_lock', expected_state, path, should_equal) + + def _verify_staff_lock_from_state(self, xblock_info, expected_state, path=None, should_equal=True): + """ + Verify the staff_lock_from state of an item in the xblock_info. + """ + self._verify_xblock_info_state(xblock_info, 'staff_lock_from', expected_state, path, should_equal) def test_empty_chapter(self): empty_chapter = self._create_child(self.course, 'chapter', "Empty Chapter") xblock_info = self._get_xblock_info(empty_chapter.location) - self.assertEqual(xblock_info['visibility_state'], VisibilityState.unscheduled) + self._verify_visibility_state(xblock_info, VisibilityState.unscheduled) def test_empty_sequential(self): chapter = self._create_child(self.course, 'chapter', "Test Chapter") @@ -1465,16 +1522,83 @@ class TestXBlockPublishingInfo(ItemTest): # Finally verify the state of the chapter self._verify_visibility_state(xblock_info, VisibilityState.ready) - def test_staff_only(self): - chapter = self._create_child(self.course, 'chapter', "Test Chapter") + def test_staff_only_section(self): + """ + Tests that an explicitly staff-locked section and all of its children are visible to staff only. + """ + chapter = self._create_child(self.course, 'chapter', "Test Chapter", staff_only=True) sequential = self._create_child(chapter, 'sequential', "Test Sequential") - unit = self._create_child(sequential, 'vertical', "Published Unit") - self._set_staff_only(unit.location, True) + self._create_child(sequential, 'vertical', "Unit") xblock_info = self._get_xblock_info(chapter.location) self._verify_visibility_state(xblock_info, VisibilityState.staff_only) self._verify_visibility_state(xblock_info, VisibilityState.staff_only, path=self.FIRST_SUBSECTION_PATH) self._verify_visibility_state(xblock_info, VisibilityState.staff_only, path=self.FIRST_UNIT_PATH) + self._verify_explicit_staff_lock_state(xblock_info, True) + self._verify_explicit_staff_lock_state(xblock_info, False, path=self.FIRST_SUBSECTION_PATH) + self._verify_explicit_staff_lock_state(xblock_info, False, path=self.FIRST_UNIT_PATH) + + self._verify_staff_lock_from_state(xblock_info, _xblock_type_and_display_name(chapter), path=self.FIRST_UNIT_PATH) + + def test_no_staff_only_section(self): + """ + Tests that a section with a staff-locked subsection and a visible subsection is not staff locked itself. + """ + chapter = self._create_child(self.course, 'chapter', "Test Chapter") + self._create_child(chapter, 'sequential', "Test Visible Sequential") + self._create_child(chapter, 'sequential', "Test Staff Locked Sequential", staff_only=True) + xblock_info = self._get_xblock_info(chapter.location) + self._verify_visibility_state(xblock_info, VisibilityState.staff_only, should_equal=False) + self._verify_visibility_state(xblock_info, VisibilityState.staff_only, path=[0], should_equal=False) + self._verify_visibility_state(xblock_info, VisibilityState.staff_only, path=[1]) + + def test_staff_only_subsection(self): + """ + Tests that an explicitly staff-locked subsection and all of its children are visible to staff only. + In this case the parent section is also visible to staff only because all of its children are staff only. + """ + chapter = self._create_child(self.course, 'chapter', "Test Chapter") + sequential = self._create_child(chapter, 'sequential', "Test Sequential", staff_only=True) + self._create_child(sequential, 'vertical', "Unit") + xblock_info = self._get_xblock_info(chapter.location) + self._verify_visibility_state(xblock_info, VisibilityState.staff_only) + self._verify_visibility_state(xblock_info, VisibilityState.staff_only, path=self.FIRST_SUBSECTION_PATH) + self._verify_visibility_state(xblock_info, VisibilityState.staff_only, path=self.FIRST_UNIT_PATH) + + self._verify_explicit_staff_lock_state(xblock_info, False) + self._verify_explicit_staff_lock_state(xblock_info, True, path=self.FIRST_SUBSECTION_PATH) + self._verify_explicit_staff_lock_state(xblock_info, False, path=self.FIRST_UNIT_PATH) + + self._verify_staff_lock_from_state(xblock_info, _xblock_type_and_display_name(sequential), path=self.FIRST_UNIT_PATH) + + def test_no_staff_only_subsection(self): + """ + Tests that a subsection with a staff-locked unit and a visible unit is not staff locked itself. + """ + chapter = self._create_child(self.course, 'chapter', "Test Chapter") + sequential = self._create_child(chapter, 'sequential', "Test Sequential") + self._create_child(sequential, 'vertical', "Unit") + self._create_child(sequential, 'vertical', "Locked Unit", staff_only=True) + xblock_info = self._get_xblock_info(chapter.location) + self._verify_visibility_state(xblock_info, VisibilityState.staff_only, self.FIRST_SUBSECTION_PATH, should_equal=False) + self._verify_visibility_state(xblock_info, VisibilityState.staff_only, self.FIRST_UNIT_PATH, should_equal=False) + self._verify_visibility_state(xblock_info, VisibilityState.staff_only, self.SECOND_UNIT_PATH) + + def test_staff_only_unit(self): + chapter = self._create_child(self.course, 'chapter', "Test Chapter") + sequential = self._create_child(chapter, 'sequential', "Test Sequential") + unit = self._create_child(sequential, 'vertical', "Unit", staff_only=True) + xblock_info = self._get_xblock_info(chapter.location) + self._verify_visibility_state(xblock_info, VisibilityState.staff_only) + self._verify_visibility_state(xblock_info, VisibilityState.staff_only, path=self.FIRST_SUBSECTION_PATH) + self._verify_visibility_state(xblock_info, VisibilityState.staff_only, path=self.FIRST_UNIT_PATH) + + self._verify_explicit_staff_lock_state(xblock_info, False) + self._verify_explicit_staff_lock_state(xblock_info, False, path=self.FIRST_SUBSECTION_PATH) + self._verify_explicit_staff_lock_state(xblock_info, True, path=self.FIRST_UNIT_PATH) + + self._verify_staff_lock_from_state(xblock_info, _xblock_type_and_display_name(unit), path=self.FIRST_UNIT_PATH) + def test_unscheduled_section_with_live_subsection(self): chapter = self._create_child(self.course, 'chapter', "Test Chapter") sequential = self._create_child(chapter, 'sequential', "Test Sequential") @@ -1499,3 +1623,27 @@ class TestXBlockPublishingInfo(ItemTest): self._verify_visibility_state(xblock_info, VisibilityState.live, path=self.FIRST_SUBSECTION_PATH) self._verify_visibility_state(xblock_info, VisibilityState.live, path=self.FIRST_UNIT_PATH) self._verify_visibility_state(xblock_info, VisibilityState.staff_only, path=self.SECOND_UNIT_PATH) + + def test_locked_section_staff_only_message(self): + """ + Tests that a locked section has a staff only message and its descendants do not. + """ + chapter = self._create_child(self.course, 'chapter', "Test Chapter", staff_only=True) + sequential = self._create_child(chapter, 'sequential', "Test Sequential") + self._create_child(sequential, 'vertical', "Unit") + xblock_info = self._get_xblock_outline_info(chapter.location) + self._verify_has_staff_only_message(xblock_info, True) + self._verify_has_staff_only_message(xblock_info, False, path=self.FIRST_SUBSECTION_PATH) + self._verify_has_staff_only_message(xblock_info, False, path=self.FIRST_UNIT_PATH) + + def test_locked_unit_staff_only_message(self): + """ + Tests that a lone locked unit has a staff only message along with its ancestors. + """ + chapter = self._create_child(self.course, 'chapter', "Test Chapter") + sequential = self._create_child(chapter, 'sequential', "Test Sequential") + self._create_child(sequential, 'vertical', "Unit", staff_only=True) + xblock_info = self._get_xblock_outline_info(chapter.location) + self._verify_has_staff_only_message(xblock_info, True) + self._verify_has_staff_only_message(xblock_info, True, path=self.FIRST_SUBSECTION_PATH) + self._verify_has_staff_only_message(xblock_info, True, path=self.FIRST_UNIT_PATH) diff --git a/cms/static/js/models/xblock_info.js b/cms/static/js/models/xblock_info.js index 2bb2cd49e7..3004fd1b4a 100644 --- a/cms/static/js/models/xblock_info.js +++ b/cms/static/js/models/xblock_info.js @@ -102,7 +102,25 @@ function(Backbone, _, str, ModuleUtils) { /** * The same as `due_date` but as an ISO-formatted date string. */ - 'due': null + 'due': null, + /** + * True iff this xblock is explicitly staff locked. + */ + 'has_explicit_staff_lock': null, + /** + * True iff this any of this xblock's ancestors are staff locked. + */ + 'ancestor_has_staff_lock': null, + /** + * The xblock which is determining the staff lock value. For instance, for a unit, + * this will either be the parent subsection or the grandparent section. + * This can be null if the xblock has no inherited staff lock. + */ + 'staff_lock_from': null, + /** + * True iff this xblock should display a "Contains staff only content" message. + */ + 'staff_only_message': null }, initialize: function () { @@ -157,7 +175,7 @@ function(Backbone, _, str, ModuleUtils) { * @return {Boolean} */ isEditableOnCourseOutline: function() { - return this.isSequential() || this.isChapter(); + return this.isSequential() || this.isChapter() || this.isVertical(); } }); return XBlockInfo; diff --git a/cms/static/js/spec/models/xblock_info_spec.js b/cms/static/js/spec/models/xblock_info_spec.js index 3930d5d5df..5074407478 100644 --- a/cms/static/js/spec/models/xblock_info_spec.js +++ b/cms/static/js/spec/models/xblock_info_spec.js @@ -1,11 +1,11 @@ define(['backbone', 'js/models/xblock_info'], - function(Backbone, XBlockInfo) { + function(Backbone, XBlockInfo) { describe('XblockInfo isEditableOnCourseOutline', function() { it('works correct', function() { expect(new XBlockInfo({'category': 'chapter'}).isEditableOnCourseOutline()).toBe(true); expect(new XBlockInfo({'category': 'course'}).isEditableOnCourseOutline()).toBe(false); expect(new XBlockInfo({'category': 'sequential'}).isEditableOnCourseOutline()).toBe(true); - expect(new XBlockInfo({'category': 'vertical'}).isEditableOnCourseOutline()).toBe(false); + expect(new XBlockInfo({'category': 'vertical'}).isEditableOnCourseOutline()).toBe(true); }); }); } diff --git a/cms/static/js/spec/views/pages/container_subviews_spec.js b/cms/static/js/spec/views/pages/container_subviews_spec.js index fc3dca0e89..fca5939de0 100644 --- a/cms/static/js/spec/views/pages/container_subviews_spec.js +++ b/cms/static/js/spec/views/pages/container_subviews_spec.js @@ -124,6 +124,8 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin lastDraftCss = ".wrapper-last-draft", releaseDateTitleCss = ".wrapper-release .title", releaseDateContentCss = ".wrapper-release .copy", + releaseDateDateCss = ".wrapper-release .copy .release-date", + releaseDateWithCss = ".wrapper-release .copy .release-with", promptSpies, sendDiscardChangesToServer, verifyPublishingBitUnscheduled; sendDiscardChangesToServer = function() { @@ -342,8 +344,8 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin release_date: "Jul 02, 2014 at 14:20 UTC", release_date_from: 'Section "Week 1"' }); expect(containerPage.$(releaseDateTitleCss).text()).toContain("Scheduled:"); - expect(containerPage.$(releaseDateContentCss).text()). - toContain('Jul 02, 2014 at 14:20 UTC with Section "Week 1"'); + expect(containerPage.$(releaseDateDateCss).text()).toContain('Jul 02, 2014 at 14:20 UTC'); + expect(containerPage.$(releaseDateWithCss).text()).toContain('with Section "Week 1"'); }); it('renders correctly when released', function () { @@ -353,8 +355,8 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin release_date: "Jul 02, 2014 at 14:20 UTC", release_date_from: 'Section "Week 1"' }); expect(containerPage.$(releaseDateTitleCss).text()).toContain("Released:"); - expect(containerPage.$(releaseDateContentCss).text()). - toContain('Jul 02, 2014 at 14:20 UTC with Section "Week 1"'); + expect(containerPage.$(releaseDateDateCss).text()).toContain('Jul 02, 2014 at 14:20 UTC'); + expect(containerPage.$(releaseDateWithCss).text()).toContain('with Section "Week 1"'); }); it('renders correctly when the release date is not set', function () { @@ -375,20 +377,22 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin }); containerPage.xblockPublisher.render(); expect(containerPage.$(releaseDateTitleCss).text()).toContain("Release:"); - expect(containerPage.$(releaseDateContentCss).text()). - toContain('Jul 02, 2014 at 14:20 UTC with Section "Week 1"'); + expect(containerPage.$(releaseDateDateCss).text()).toContain('Jul 02, 2014 at 14:20 UTC'); + expect(containerPage.$(releaseDateWithCss).text()).toContain('with Section "Week 1"'); }); }); describe("Content Visibility", function () { - var requestStaffOnly, verifyStaffOnly, promptSpy, + var requestStaffOnly, verifyStaffOnly, verifyExplicitStaffOnly, verifyImplicitStaffOnly, promptSpy, visibilityTitleCss = '.wrapper-visibility .title'; requestStaffOnly = function(isStaffOnly) { + var newVisibilityState; + containerPage.$('.action-staff-lock').click(); - // If removing the staff lock, click 'Yes' to confirm - if (!isStaffOnly) { + // If removing explicit staff lock with no implicit staff lock, click 'Yes' to confirm + if (!isStaffOnly && !containerPage.model.get('ancestor_has_staff_lock')) { edit_helpers.confirmPrompt(promptSpy); } @@ -403,24 +407,46 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin visible_to_staff_only: isStaffOnly ? true : null } }); + create_sinon.expectJsonRequest(requests, 'GET', '/xblock/locator-container'); + if (isStaffOnly || containerPage.model.get('ancestor_has_staff_lock')) { + newVisibilityState = VisibilityState.staffOnly; + } else { + newVisibilityState = VisibilityState.live; + } create_sinon.respondWithJson(requests, createXBlockInfo({ published: containerPage.model.get('published'), - visibility_state: isStaffOnly ? VisibilityState.staffOnly : VisibilityState.live, + has_explicit_staff_lock: isStaffOnly, + visibility_state: newVisibilityState, release_date: "Jul 02, 2000 at 14:20 UTC" })); }; verifyStaffOnly = function(isStaffOnly) { if (isStaffOnly) { - expect(containerPage.$('.action-staff-lock i')).toHaveClass('icon-check'); - expect(containerPage.$('.wrapper-visibility .copy').text()).toBe('Staff Only'); + expect(containerPage.$('.wrapper-visibility .copy').text()).toContain('Staff Only'); expect(containerPage.$(bitPublishingCss)).toHaveClass(staffOnlyClass); - expect(containerPage.$(bitPublishingCss)).toHaveClass(scheduledClass); + } else { + expect(containerPage.$('.wrapper-visibility .copy').text().trim()).toBe('Staff and Students'); + expect(containerPage.$(bitPublishingCss)).not.toHaveClass(staffOnlyClass); + verifyExplicitStaffOnly(false); + verifyImplicitStaffOnly(false); + } + }; + + verifyExplicitStaffOnly = function(isStaffOnly) { + if (isStaffOnly) { + expect(containerPage.$('.action-staff-lock i')).toHaveClass('icon-check'); } else { expect(containerPage.$('.action-staff-lock i')).toHaveClass('icon-check-empty'); - expect(containerPage.$('.wrapper-visibility .copy').text()).toBe('Staff and Students'); - expect(containerPage.$(bitPublishingCss)).not.toHaveClass(staffOnlyClass); + } + }; + + verifyImplicitStaffOnly = function(isStaffOnly) { + if (isStaffOnly) { + expect(containerPage.$('.wrapper-visibility .inherited-from')).toExist(); + } else { + expect(containerPage.$('.wrapper-visibility .inherited-from')).not.toExist(); } }; @@ -444,36 +470,79 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin published: true, has_changes: true }); - expect(containerPage.$(visibilityTitleCss).text()).toContain('Will Be Visible To') + expect(containerPage.$(visibilityTitleCss).text()).toContain('Will Be Visible To'); }); - it("can be set to staff only", function() { + it("can be explicitly set to staff only", function() { renderContainerPage(this, mockContainerXBlockHtml); requestStaffOnly(true); + verifyExplicitStaffOnly(true); + verifyImplicitStaffOnly(false); verifyStaffOnly(true); }); - it("can remove staff only setting", function() { + it("can be implicitly set to staff only", function() { + renderContainerPage(this, mockContainerXBlockHtml, { + visibility_state: VisibilityState.staffOnly, + ancestor_has_staff_lock: true, + staff_lock_from: "Section Foo" + }); + verifyImplicitStaffOnly(true); + verifyExplicitStaffOnly(false); + verifyStaffOnly(true); + }); + + it("can be explicitly and implicitly set to staff only", function() { + renderContainerPage(this, mockContainerXBlockHtml, { + visibility_state: VisibilityState.staffOnly, + ancestor_has_staff_lock: true, + staff_lock_from: "Section Foo" + }); + requestStaffOnly(true); + // explicit staff lock overrides the display of implicit staff lock + verifyImplicitStaffOnly(false); + verifyExplicitStaffOnly(true); + verifyStaffOnly(true); + }); + + it("can remove explicit staff only setting without having implicit staff only", function() { promptSpy = edit_helpers.createPromptSpy(); renderContainerPage(this, mockContainerXBlockHtml, { visibility_state: VisibilityState.staffOnly, - release_date: "Jul 02, 2000 at 14:20 UTC" + has_explicit_staff_lock: true, + ancestor_has_staff_lock: false }); requestStaffOnly(false); verifyStaffOnly(false); }); + it("can remove explicit staff only setting while having implicit staff only", function() { + promptSpy = edit_helpers.createPromptSpy(); + renderContainerPage(this, mockContainerXBlockHtml, { + visibility_state: VisibilityState.staffOnly, + ancestor_has_staff_lock: true, + has_explicit_staff_lock: true, + staff_lock_from: "Section Foo" + }); + requestStaffOnly(false); + verifyExplicitStaffOnly(false); + verifyImplicitStaffOnly(true); + verifyStaffOnly(true); + }); + it("does not refresh if removing staff only is canceled", function() { var requestCount; promptSpy = edit_helpers.createPromptSpy(); renderContainerPage(this, mockContainerXBlockHtml, { visibility_state: VisibilityState.staffOnly, - release_date: "Jul 02, 2000 at 14:20 UTC" + has_explicit_staff_lock: true, + ancestor_has_staff_lock: false }); requestCount = requests.length; containerPage.$('.action-staff-lock').click(); edit_helpers.confirmPrompt(promptSpy, true); // Click 'No' to cancel expect(requests.length).toBe(requestCount); + verifyExplicitStaffOnly(true); verifyStaffOnly(true); }); diff --git a/cms/static/js/spec/views/pages/course_outline_spec.js b/cms/static/js/spec/views/pages/course_outline_spec.js index fc55c788a3..41f57cec85 100644 --- a/cms/static/js/spec/views/pages/course_outline_spec.js +++ b/cms/static/js/spec/views/pages/course_outline_spec.js @@ -20,6 +20,7 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" published: true, edited_on: 'Jul 02, 2014 at 20:56 UTC', edited_by: 'MockUser', + has_explicit_staff_lock: false, child_info: { display_name: 'Section', category: 'chapter', @@ -38,6 +39,7 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" published: true, edited_on: 'Jul 02, 2014 at 20:56 UTC', edited_by: 'MockUser', + has_explicit_staff_lock: false, child_info: { category: 'sequential', display_name: 'Subsection', @@ -57,6 +59,7 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" edited_on: 'Jul 02, 2014 at 20:56 UTC', edited_by: 'MockUser', course_graders: '["Lab", "Howework"]', + has_explicit_staff_lock: false, child_info: { category: 'vertical', display_name: 'Unit', @@ -359,18 +362,21 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" expect($("due_date")).not.toExist(); expect($("grading_format")).not.toExist(); + // Staff lock controls are always visible + expect($("#staff_lock")).toExist(); + $(".edit-outline-item-modal .action-save").click(); create_sinon.expectJsonRequest(requests, 'POST', '/xblock/mock-section', { "metadata":{ - "start":"2015-01-02T00:00:00.000Z", + "start":"2015-01-02T00:00:00.000Z" } }); expect(requests[0].requestHeaders['X-HTTP-Method-Override']).toBe('PATCH'); // This is the response for the change operation. create_sinon.respondWithJson(requests, {}); - var mockResponseSectionJSON = $.extend(true, {}, + var mockResponseSectionJSON = $.extend(true, {}, createMockSectionJSON('mock-section', 'Mock Section', [ createMockSubsectionJSON('mock-subsection', 'Mock Subsection', [{ id: 'mock-unit', @@ -386,7 +392,7 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" ]) ]), { - release_date: 'Jan 02, 2015 at 00:00 UTC', + release_date: 'Jan 02, 2015 at 00:00 UTC', } ); create_sinon.expectJsonRequest(requests, 'GET', '/xblock/outline/mock-section') @@ -405,14 +411,15 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" return getItemHeaders('subsection').find('.wrapper-xblock-field'); }; - setEditModalValues = function (start_date, due_date, grading_type) { + setEditModalValues = function (start_date, due_date, grading_type, is_locked) { $("#start_date").val(start_date); $("#due_date").val(due_date); $("#grading_type").val(grading_type); - } + $("#staff_lock").prop('checked', is_locked); + }; // Contains hard-coded dates because dates are presented in different formats. - var mockServerValuesJson = $.extend(true, {}, + mockServerValuesJson = $.extend(true, {}, createMockSectionJSON('mock-section', 'Mock Section', [ createMockSubsectionJSON('mock-subsection', 'Mock Subsection', [{ id: 'mock-unit', @@ -436,7 +443,9 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" release_date: 'Jul 09, 2014 at 00:00 UTC', start: "2014-07-09T00:00:00Z", format: "Lab", - due: "2014-07-10T00:00:00Z" + due: "2014-07-10T00:00:00Z", + has_explicit_staff_lock: true, + staff_only_message: true }] } } @@ -504,11 +513,13 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" it('can be edited', function() { createCourseOutlinePage(this, mockCourseJSON, false); outlinePage.$('.outline-subsection .configure-button').click(); - setEditModalValues("7/9/2014", "7/10/2014", "Lab"); + setEditModalValues("7/9/2014", "7/10/2014", "Lab", true); $(".edit-outline-item-modal .action-save").click(); create_sinon.expectJsonRequest(requests, 'POST', '/xblock/mock-subsection', { "graderType":"Lab", + "publish": "republish", "metadata":{ + "visible_to_staff_only": true, "start":"2014-07-09T00:00:00.000Z", "due":"2014-07-10T00:00:00.000Z" } @@ -525,18 +536,20 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" expect($(".outline-subsection .status-release-value")).toContainText("Jul 09, 2014 at 00:00 UTC"); expect($(".outline-subsection .status-grading-date")).toContainText("Due: Jul 10, 2014 at 00:00 UTC"); expect($(".outline-subsection .status-grading-value")).toContainText("Lab"); + expect($(".outline-subsection .status-message-copy")).toContainText("Contains staff only content"); expect($(".outline-item .outline-subsection .status-grading-value")).toContainText("Lab"); outlinePage.$('.outline-item .outline-subsection .configure-button').click(); expect($("#start_date").val()).toBe('7/9/2014'); expect($("#due_date").val()).toBe('7/10/2014'); expect($("#grading_type").val()).toBe('Lab'); + expect($("#staff_lock").is(":checked")).toBe(true); }); - it('release date, due date and grading type can be cleared.', function() { + it('release date, due date, grading type, and staff lock can be cleared.', function() { createCourseOutlinePage(this, mockCourseJSON, false); outlinePage.$('.outline-item .outline-subsection .configure-button').click(); - setEditModalValues("7/9/2014", "7/10/2014", "Lab"); + setEditModalValues("7/9/2014", "7/10/2014", "Lab", true); $(".edit-outline-item-modal .action-save").click(); // This is the response for the change operation. @@ -547,11 +560,13 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" expect($(".outline-subsection .status-release-value")).toContainText("Jul 09, 2014 at 00:00 UTC"); expect($(".outline-subsection .status-grading-date")).toContainText("Due: Jul 10, 2014 at 00:00 UTC"); expect($(".outline-subsection .status-grading-value")).toContainText("Lab"); + expect($(".outline-subsection .status-message-copy")).toContainText("Contains staff only content"); outlinePage.$('.outline-subsection .configure-button').click(); expect($("#start_date").val()).toBe('7/9/2014'); expect($("#due_date").val()).toBe('7/10/2014'); expect($("#grading_type").val()).toBe('Lab'); + expect($("#staff_lock").is(":checked")).toBe(true); $(".edit-outline-item-modal .scheduled-date-input .action-clear").click(); $(".edit-outline-item-modal .due-date-input .action-clear").click(); @@ -559,6 +574,7 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" expect($("#due_date").val()).toBe(''); $("#grading_type").val('notgraded'); + $("#staff_lock").prop('checked', false); $(".edit-outline-item-modal .action-save").click(); @@ -573,6 +589,7 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" expect($(".outline-subsection .status-release-value")).not.toContainText("Jul 09, 2014 at 00:00 UTC"); expect($(".outline-subsection .status-grading-date")).not.toExist(); expect($(".outline-subsection .status-grading-value")).not.toExist(); + expect($(".outline-subsection .status-message-copy")).not.toContainText("Contains staff only content"); }); }); diff --git a/cms/static/js/views/course_outline.js b/cms/static/js/views/course_outline.js index 0faf546ec6..a2e5306eac 100644 --- a/cms/static/js/views/course_outline.js +++ b/cms/static/js/views/course_outline.js @@ -146,7 +146,8 @@ define(["jquery", "underscore", "js/views/xblock_outline", "js/views/utils/view_ editXBlock: function() { var modal = new EditSectionXBlockModal({ model: this.model, - onSave: this.refresh.bind(this) + onSave: this.refresh.bind(this), + parentInfo: this.parentInfo }); modal.show(); diff --git a/cms/static/js/views/modals/edit_outline_item.js b/cms/static/js/views/modals/edit_outline_item.js index 99e76c9491..2be69b3831 100644 --- a/cms/static/js/views/modals/edit_outline_item.js +++ b/cms/static/js/views/modals/edit_outline_item.js @@ -1,19 +1,19 @@ /** * The EditSectionXBlockModal is a Backbone view that shows an editor in a modal window. - * It has nested views: for release date, due date and grading format. + * It has nested views: for release date, due date, grading format, and staff lock. * It is invoked using the editXBlock method and uses xblock_info as a model, * and upon save parent invokes refresh function that fetches updated model and * re-renders edited course outline. */ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/modals/base_modal', - 'date', 'js/views/utils/xblock_utils', 'js/utils/date_utils' + 'date', 'js/views/utils/xblock_utils', 'js/utils/date_utils', 'js/views/utils/view_utils' ], function( - $, Backbone, _, gettext, BaseModal, date, XBlockViewUtils, DateUtils + $, Backbone, _, gettext, BaseModal, date, XBlockViewUtils, DateUtils, ViewUtils ) { 'use strict'; var EditSectionXBlockModal, BaseDateView, ReleaseDateView, DueDateView, - GradingView; + GradingView, StaffLockView; EditSectionXBlockModal = BaseModal.extend({ events : { @@ -38,13 +38,10 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/modals/base_mod }, getTitle: function () { - if (this.model.isChapter() || this.model.isSequential()) { - return _.template( - gettext('<%= sectionName %> Settings'), - {sectionName: this.model.get('display_name')}); - } else { - return ''; - } + return _.template( + gettext('<%= sectionName %> Settings'), + { sectionName: this.model.get('display_name') } + ); }, getContentHtml: function() { @@ -61,9 +58,12 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/modals/base_mod var requestData = _.extend({}, this.getRequestData(), { metadata: this.getMetadata() }); - XBlockViewUtils.updateXBlockFields(this.model, requestData, { - success: this.options.onSave - }); + // Only update if something changed to prevent items from erroneously entering draft state + if (!_.isEqual(requestData, { metadata: {} })) { + XBlockViewUtils.updateXBlockFields(this.model, requestData, { + success: this.options.onSave + }); + } this.hide(); }, @@ -89,7 +89,8 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/modals/base_mod */ getContext: function () { return _.extend({ - xblockInfo: this.model + xblockInfo: this.model, + xblockType: XBlockViewUtils.getXBlockType(this.model.get('category'), this.parentInfo, true) }, this.invokeComponentMethod('getContext')); }, @@ -115,13 +116,23 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/modals/base_mod initializeComponents: function () { this.components = []; this.components.push( - new ReleaseDateView({ - selector: '.scheduled-date-input', + new StaffLockView({ + selector: '.edit-staff-lock', parentView: this, model: this.model }) ); + if (this.model.isChapter() || this.model.isSequential()) { + this.components.push( + new ReleaseDateView({ + selector: '.scheduled-date-input', + parentView: this, + model: this.model + }) + ); + } + if (this.model.isSequential()) { this.components.push( new DueDateView({ @@ -240,5 +251,49 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/modals/base_mod } }); + StaffLockView = Backbone.View.extend({ + isModelLocked: function() { + return this.model.get('has_explicit_staff_lock'); + }, + + isAncestorLocked: function() { + return this.model.get('ancestor_has_staff_lock'); + }, + + afterRender: function () { + this.setElement(this.options.parentView.$(this.options.selector).get(0)); + this.setLock(this.isModelLocked()); + }, + + setLock: function(value) { + this.$('#staff_lock').prop('checked', value); + }, + + isLocked: function() { + return this.$('#staff_lock').is(':checked'); + }, + + hasChanges: function() { + return this.isModelLocked() != this.isLocked(); + }, + + getRequestData: function() { + return this.hasChanges() ? { publish: 'republish' } : {}; + }, + + getMetadata: function() { + // Setting visible_to_staff_only to null when disabled will delete the field from this + // xblock, allowing it to inherit the value of its ancestors. + return this.hasChanges() ? { visible_to_staff_only: this.isLocked() ? true : null } : {}; + }, + + getContext: function () { + return { + hasExplicitStaffLock: this.isModelLocked(), + ancestorLocked: this.isAncestorLocked() + } + } + }); + return EditSectionXBlockModal; }); diff --git a/cms/static/js/views/pages/container_subviews.js b/cms/static/js/views/pages/container_subviews.js index 5127c755dd..1957633f3b 100644 --- a/cms/static/js/views/pages/container_subviews.js +++ b/cms/static/js/views/pages/container_subviews.js @@ -100,7 +100,7 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/ onSync: function(model) { if (ViewUtils.hasChangedAttributes(model, [ - 'has_changes', 'published', 'edited_on', 'edited_by', 'visibility_state' + 'has_changes', 'published', 'edited_on', 'edited_by', 'visibility_state', 'has_explicit_staff_lock' ])) { this.render(); } @@ -118,7 +118,9 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/ publishedBy: this.model.get('published_by'), released: this.model.get('released_to_students'), releaseDate: this.model.get('release_date'), - releaseDateFrom: this.model.get('release_date_from') + releaseDateFrom: this.model.get('release_date_from'), + hasExplicitStaffLock: this.model.get('has_explicit_staff_lock'), + staffLockFrom: this.model.get('staff_lock_from') })); return this; @@ -161,12 +163,13 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/ }, toggleStaffLock: function (e) { - var xblockInfo = this.model, self=this, enableStaffLock, + var xblockInfo = this.model, self=this, enableStaffLock, hasInheritedStaffLock, saveAndPublishStaffLock, revertCheckBox; if (e && e.preventDefault) { e.preventDefault(); } - enableStaffLock = xblockInfo.get('visibility_state') !== VisibilityState.staffOnly; + enableStaffLock = !xblockInfo.get('has_explicit_staff_lock'); + hasInheritedStaffLock = xblockInfo.get('ancestor_has_staff_lock'); revertCheckBox = function() { self.checkStaffLock(!enableStaffLock); @@ -189,8 +192,14 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/ }; this.checkStaffLock(enableStaffLock); - if (enableStaffLock) { - ViewUtils.runOperationShowingMessage(gettext('Hiding Unit from Students…'), + if (enableStaffLock && !hasInheritedStaffLock) { + ViewUtils.runOperationShowingMessage(gettext('Hiding from Students…'), + _.bind(saveAndPublishStaffLock, self)); + } else if (enableStaffLock && hasInheritedStaffLock) { + ViewUtils.runOperationShowingMessage(gettext('Explicitly Hiding from Students…'), + _.bind(saveAndPublishStaffLock, self)); + } else if (!enableStaffLock && hasInheritedStaffLock) { + ViewUtils.runOperationShowingMessage(gettext('Inheriting Student Visibility…'), _.bind(saveAndPublishStaffLock, self)); } else { ViewUtils.confirmThenRunOperation(gettext("Make Visible to Students"), diff --git a/cms/static/js/views/utils/xblock_utils.js b/cms/static/js/views/utils/xblock_utils.js index fe1bb8971a..3fe68f90bd 100644 --- a/cms/static/js/views/utils/xblock_utils.js +++ b/cms/static/js/views/utils/xblock_utils.js @@ -165,6 +165,18 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils", "js/util return listType; }; + getXBlockType = function(category, parentInfo, translate) { + var xblockType = category; + if (category === 'chapter') { + xblockType = translate ? gettext('section') : 'section'; + } else if (category === 'sequential') { + xblockType = translate ? gettext('subsection') : 'subsection'; + } else if (category === 'vertical' && (!parentInfo || parentInfo.get('category') === 'sequential')) { + xblockType = translate ? gettext('unit') : 'unit'; + } + return xblockType; + }; + return { 'VisibilityState': VisibilityState, 'addXBlock': addXBlock, @@ -172,6 +184,7 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils", "js/util 'updateXBlockField': updateXBlockField, 'getXBlockVisibilityClass': getXBlockVisibilityClass, 'getXBlockListTypeClass': getXBlockListTypeClass, - 'updateXBlockFields': updateXBlockFields + 'updateXBlockFields': updateXBlockFields, + 'getXBlockType': getXBlockType }; }); diff --git a/cms/static/js/views/xblock_outline.js b/cms/static/js/views/xblock_outline.js index 15fafffb86..aef1d28fdb 100644 --- a/cms/static/js/views/xblock_outline.js +++ b/cms/static/js/views/xblock_outline.js @@ -57,9 +57,9 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/ var xblockInfo = this.model, childInfo = xblockInfo.get('child_info'), parentInfo = this.parentInfo, - xblockType = this.getXBlockType(this.model.get('category'), this.parentInfo), - xblockTypeDisplayName = this.getXBlockType(this.model.get('category'), this.parentInfo, true), - parentType = parentInfo ? this.getXBlockType(parentInfo.get('category')) : null, + xblockType = XBlockViewUtils.getXBlockType(this.model.get('category'), this.parentInfo), + xblockTypeDisplayName = XBlockViewUtils.getXBlockType(this.model.get('category'), this.parentInfo, true), + parentType = parentInfo ? XBlockViewUtils.getXBlockType(parentInfo.get('category')) : null, addChildName = null, defaultNewChildName = null, html, @@ -78,12 +78,14 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/ xblockType: xblockType, xblockTypeDisplayName: xblockTypeDisplayName, parentType: parentType, - childType: childInfo ? this.getXBlockType(childInfo.category, xblockInfo) : null, + childType: childInfo ? XBlockViewUtils.getXBlockType(childInfo.category, xblockInfo) : null, childCategory: childInfo ? childInfo.category : null, addChildLabel: addChildName, defaultNewChildName: defaultNewChildName, isCollapsed: isCollapsed, - includesChildren: this.shouldRenderChildren() + includesChildren: this.shouldRenderChildren(), + hasExplicitStaffLock: this.model.get('has_explicit_staff_lock'), + staffOnlyMessage: this.model.get('staff_only_message') }); if (this.parentInfo) { this.setElement($(html)); @@ -184,18 +186,6 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/ }); }, - getXBlockType: function(category, parentInfo, translate) { - var xblockType = category; - if (category === 'chapter') { - xblockType = translate ? gettext('section') : 'section'; - } else if (category === 'sequential') { - xblockType = translate ? gettext('subsection') : 'subsection'; - } else if (category === 'vertical' && (!parentInfo || parentInfo.get('category') === 'sequential')) { - xblockType = translate ? gettext('unit') : 'unit'; - } - return xblockType; - }, - onSync: function(event) { if (ViewUtils.hasChangedAttributes(this.model, ['visibility_state', 'child_info', 'display_name'])) { this.onXBlockChange(); @@ -266,7 +256,7 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/ var self = this, parentView = this.parentView; event.preventDefault(); - var xblockType = this.getXBlockType(this.model.get('category'), parentView.model, true); + var xblockType = XBlockViewUtils.getXBlockType(this.model.get('category'), parentView.model, true); XBlockViewUtils.deleteXBlock(this.model, xblockType).done(function() { if (parentView) { parentView.onChildDeleted(self, event); diff --git a/cms/static/sass/elements/_forms.scss b/cms/static/sass/elements/_forms.scss index b9d486451b..ea0e144ec8 100644 --- a/cms/static/sass/elements/_forms.scss +++ b/cms/static/sass/elements/_forms.scss @@ -59,6 +59,46 @@ textarea.text { // forms - additional UI form { + // CASE: cosmetic checkbox input + .checkbox-cosmetic { + + .input-checkbox-checked, .input-checkbox-unchecked, .label { + display: inline-block; + vertical-align: middle; + } + + .input-checkbox-checked, .input-checkbox-unchecked { + width: $baseline; + } + + .input-checkbox { + @extend %cont-text-sr; + + // CASE: unchecked + ~ label .input-checkbox-checked { + display: none; + } + + ~ label .input-checkbox-unchecked { + display: inline-block; + } + + // CASE: checked + &:checked { + + ~ label .input-checkbox-checked { + display: inline-block; + } + + ~ label .input-checkbox-unchecked { + display: none; + } + } + + } + } + + // CASE: file input input[type=file] { @extend %t-copy-sub1; } diff --git a/cms/static/sass/elements/_modal-window.scss b/cms/static/sass/elements/_modal-window.scss index 543218c5d8..907ba57cfc 100644 --- a/cms/static/sass/elements/_modal-window.scss +++ b/cms/static/sass/elements/_modal-window.scss @@ -54,7 +54,7 @@ // sections within a modal .modal-section { - margin-bottom: ($baseline/2); + margin-bottom: ($baseline*0.75); &:last-child { margin-bottom: 0; @@ -245,12 +245,6 @@ margin-right: ($baseline/2); margin-bottom: ($baseline/4); - - // TODO: refactor the _forms.scss partial to allow for this area to inherit from it - label, input, textarea { - display: block; - } - label { @extend %t-copy-sub1; @extend %t-strong; @@ -288,6 +282,27 @@ .due-time { width: ($baseline*7); } + + .tip { + @extend %t-copy-sub1; + @include transition(color, 0.15s, ease-in-out); + display: block; + margin-top: ($baseline/4); + color: $gray-l2; + } + + .tip-warning { + color: $gray-d2; + } + } + + // CASE: type-based input + .field-text { + + // TODO: refactor the _forms.scss partial to allow for this area to inherit from it + label, input, textarea { + display: block; + } } // CASE: select input @@ -305,15 +320,52 @@ .input { width: 100%; } + + // CASE: checkbox input + .field-checkbox { + + .label, label { + margin-bottom: 0; + } + } } } + + + // UI: grading section .edit-settings-grading { .grading-type { margin-bottom: $baseline; } } + + // UI: staff lock section + .edit-staff-lock { + + .checkbox-cosmetic .input-checkbox { + @extend %cont-text-sr; + + // CASE: unchecked + ~ .tip-warning { + display: block; + } + + // CASE: checked + &:checked { + + ~ .tip-warning { + display: none; + } + } + } + + // needed to override poorly scoped margin-bottom on any label element in a view (from _forms.scss) + .checkbox-cosmetic .label { + margin-bottom: 0; + } + } } // xblock custom actions diff --git a/cms/static/sass/views/_container.scss b/cms/static/sass/views/_container.scss index 4064a34e0f..f068f51d95 100644 --- a/cms/static/sass/views/_container.scss +++ b/cms/static/sass/views/_container.scss @@ -157,6 +157,11 @@ .release-date { @extend %t-strong; } + + .release-with { + @extend %t-title8; + display: block; + } } .wrapper-visibility { @@ -170,6 +175,13 @@ margin-left: ($baseline/4); color: $gray-d1; } + + .inherited-from { + @extend %t-title8; + display: block; + } + + } .wrapper-pub-actions { diff --git a/cms/templates/js/course-outline.underscore b/cms/templates/js/course-outline.underscore index 32a2ef6061..f1ba753da7 100644 --- a/cms/templates/js/course-outline.underscore +++ b/cms/templates/js/course-outline.underscore @@ -6,7 +6,7 @@ var published = xblockInfo.get('published'); var statusMessage = null; var statusType = null; -if (visibilityState === 'staff_only') { +if (staffOnlyMessage) { statusType = 'staff-only'; statusMessage = gettext('Contains staff only content'); } else if (visibilityState === 'needs_attention') { diff --git a/cms/templates/js/edit-outline-item-modal.underscore b/cms/templates/js/edit-outline-item-modal.underscore index 6d3c90cf34..a7811dfc74 100644 --- a/cms/templates/js/edit-outline-item-modal.underscore +++ b/cms/templates/js/edit-outline-item-modal.underscore @@ -1,30 +1,29 @@
- <% if (xblockInfo.isChapter() || xblockInfo.isSequential()) { %> -

- <%= interpolate(gettext("Change the settings for %(display_name)s"), {display_name: xblockInfo.get('display_name')}, true) %> -

- <% } %> +

+ <%= interpolate(gettext("Change the settings for %(display_name)s"), {display_name: xblockInfo.get('display_name')}, true) %> +

-
-

<%= gettext('Release Date and Time') %>

-
-
    -
  • - - -
    • -
  • - - -
    • -
+ <% if (xblockInfo.isChapter() || xblockInfo.isSequential()) { %> +
+

<%= gettext('Release Date and Time') %>

+
+
    +
  • + + +
    • +
  • + + +
    • +
<% if (xblockInfo.isSequential()) { %>
    @@ -38,6 +37,7 @@ <% } %>
+ <% } %> <% if (xblockInfo.isSequential()) { %>
@@ -85,5 +85,41 @@
<% } %> + +
+

<%= gettext('Student Visibility') %>

+
+
    +
  • + + + + <% if (hasExplicitStaffLock && !ancestorLocked) { %> +

    + <% if (xblockInfo.isVertical()) { %> + <%= gettext('If the unit was previously published and released to students, any changes you made to the unit when it was hidden will now be visible to students.') %> + <% } else { %> + <% var message = gettext('If you make this %(xblockType)s visible to students, students will be able to see its content after the release date has passed and you have published the unit(s).'); %> + <%= interpolate(message, { xblockType: xblockType }, true) %> + <% } %> +

    + +

    + <% if (xblockInfo.isChapter()) { %> + <%= gettext('Any subsections or units that are explicitly hidden from students will remain hidden after you clear this option for the section.') %> + <% } %> + <% if (xblockInfo.isSequential()) { %> + <%= gettext('Any units that are explicitly hidden from students will remain hidden after you clear this option for the subsection.') %> + <% } %> +

    + <% } %> +
    • +
+
+
diff --git a/cms/templates/js/publish-xblock.underscore b/cms/templates/js/publish-xblock.underscore index 4133c9f1b0..b830f095c7 100644 --- a/cms/templates/js/publish-xblock.underscore +++ b/cms/templates/js/publish-xblock.underscore @@ -46,10 +46,11 @@ var visibleToStaffOnly = visibilityState === 'staff_only';
<%= releaseLabel %>

<% if (releaseDate) { %> - <% var message = gettext("%(release_date)s with %(section_or_subsection)s") %> - <%= interpolate(message, { - release_date: '' + releaseDate + '', - section_or_subsection: '' + releaseDateFrom + '' }, true) %> + <%= releaseDate %> + + <%= interpolate(gettext('with %(release_date_from)s'), { release_date_from: releaseDateFrom }, true) %> + + <% } else { %> <%= gettext("Unscheduled") %> <% } %> @@ -65,13 +66,20 @@ var visibleToStaffOnly = visibilityState === 'staff_only'; <% } %> <% if (visibleToStaffOnly) { %> -

<%= gettext("Staff Only") %>

+

+ <%= gettext("Staff Only") %> + <% if (!hasExplicitStaffLock) { %> + + <%= interpolate(gettext("with %(section_or_subsection)s"),{ section_or_subsection: staffLockFrom }, true) %> + + <% } %> +

<% } else { %>

<%= gettext("Staff and Students") %>

<% } %>

- - <% if (visibleToStaffOnly) { %> + + <% if (hasExplicitStaffLock) { %> <% } else { %> diff --git a/common/test/acceptance/pages/lms/courseware.py b/common/test/acceptance/pages/lms/courseware.py index 38cf30badf..202691be8a 100644 --- a/common/test/acceptance/pages/lms/courseware.py +++ b/common/test/acceptance/pages/lms/courseware.py @@ -12,10 +12,26 @@ class CoursewarePage(CoursePage): url_path = "courseware/" xblock_component_selector = '.vert .xblock' + section_selector = '.chapter' + subsection_selector = '.chapter ul li' def is_browser_on_page(self): return self.q(css='body.courseware').present + @property + def num_sections(self): + """ + Return the number of sections in the sidebar on the page + """ + return len(self.q(css=self.section_selector)) + + @property + def num_subsections(self): + """ + Return the number of subsections in the sidebar on the page, including in collapsed sections + """ + return len(self.q(css=self.subsection_selector)) + @property def num_xblock_components(self): """ diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index bcc65107bd..f9b66da633 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -133,6 +133,12 @@ class ContainerPage(PageObject): warning_text = warnings.first.text[0] return warning_text == "Caution: The last published version of this unit is live. By publishing changes you will change the student experience." + def shows_inherited_staff_lock(self, parent_type=None, parent_name=None): + """ + Returns True if the unit inherits staff lock from a section or subsection. + """ + return self.q(css='.bit-publishing .wrapper-visibility .copy .inherited-from').visible + @property def publish_action(self): """ @@ -153,7 +159,7 @@ class ContainerPage(PageObject): """ Returns True if staff lock is currently enabled, False otherwise """ return 'icon-check' in self.q(css='a.action-staff-lock>i').attrs('class') - def toggle_staff_lock(self): + def toggle_staff_lock(self, inherits_staff_lock=False): """ Toggles "hide from students" which enables or disables a staff-only lock. @@ -164,7 +170,8 @@ class ContainerPage(PageObject): self.q(css='a.action-staff-lock').first.click() else: click_css(self, 'a.action-staff-lock', 0, require_notification=False) - confirm_prompt(self) + if not inherits_staff_lock: + confirm_prompt(self) self.wait_for_ajax() return not was_locked_initially diff --git a/common/test/acceptance/pages/studio/overview.py b/common/test/acceptance/pages/studio/overview.py index 0407692530..2250fd7d03 100644 --- a/common/test/acceptance/pages/studio/overview.py +++ b/common/test/acceptance/pages/studio/overview.py @@ -75,6 +75,16 @@ class CourseOutlineItem(object): """ return self.q(css=self._bounded_selector(self.STATUS_MESSAGE_SELECTOR)).text[0] + @property + def has_staff_lock_warning(self): + """ Returns True iff the 'Contains staff only content' message is visible """ + return self.status_message == 'Contains staff only content' if self.has_status_message else False + + @property + def is_staff_only(self): + """ Returns True if the visiblity state of this item is staff only (has a black sidebar) """ + return "is-staff-only" in self.q(css=self._bounded_selector(''))[0].get_attribute("class") + def edit_name(self): """ Puts the item's name into editable form. @@ -102,6 +112,14 @@ class CourseOutlineItem(object): self.q(css=self._bounded_selector(self.NAME_INPUT_SELECTOR)).results[0].send_keys(Keys.ENTER) self.wait_for_ajax() + def set_staff_lock(self, is_locked): + """ + Sets the explicit staff lock of item on the container page to is_locked. + """ + modal = self.edit() + modal.is_explicitly_locked = is_locked + modal.save() + def in_editable_form(self): """ Return whether this outline item's display name is in its editable form. @@ -452,6 +470,17 @@ class CourseOutlinePage(CoursePage, CourseOutlineContainer): else: return ExpandCollapseLinkState.EXPAND + def expand_all_subsections(self): + """ + Expands all the subsections in this course. + """ + for section in self.sections(): + if section.is_collapsed: + section.toggle_expand() + for subsection in section.subsections(): + if subsection.is_collapsed: + subsection.toggle_expand() + class CourseOutlineModal(object): MODAL_SELECTOR = ".edit-outline-item-modal" @@ -554,6 +583,28 @@ class CourseOutlineModal(object): "Grading label is updated.", ).fulfill() + @property + def is_explicitly_locked(self): + """ + Returns true if the explict staff lock checkbox is checked, false otherwise. + """ + return self.find_css('#staff_lock')[0].is_selected() + + @is_explicitly_locked.setter + def is_explicitly_locked(self, value): + """ + Checks the explicit staff lock box if value is true, otherwise unchecks the box. + """ + if value != self.is_explicitly_locked: + self.find_css('label[for="staff_lock"]').click() + EmptyPromise(lambda: value == self.is_explicitly_locked, "Explicit staff lock is updated").fulfill() + + def shows_staff_lock_warning(self): + """ + Returns true iff the staff lock warning is visible. + """ + return self.find_css('.staff-lock .tip-warning').visible + def get_selected_option_text(self, element): """ Returns the text of the first selected option for the element. diff --git a/common/test/acceptance/tests/test_studio_container.py b/common/test/acceptance/tests/test_studio_container.py index 03fd208232..0d5c48ae4a 100644 --- a/common/test/acceptance/tests/test_studio_container.py +++ b/common/test/acceptance/tests/test_studio_container.py @@ -455,7 +455,7 @@ class UnitPublishingTest(ContainerBase): self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) # Start date set in course fixture to 1970. self._verify_release_date_info( - unit, self.RELEASE_TITLE_RELEASED, 'Jan 01, 1970 at 00:00 UTC with Section "Test Section"' + unit, self.RELEASE_TITLE_RELEASED, 'Jan 01, 1970 at 00:00 UTC\nwith Section "Test Section"' ) self._verify_last_published_and_saved(unit, self.LAST_PUBLISHED, self.LAST_PUBLISHED) # Should not be able to click on Publish action -- but I don't know how to test that it is not clickable. @@ -548,7 +548,7 @@ class UnitPublishingTest(ContainerBase): self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) self.assertTrue(unit.currently_visible_to_students) self._verify_release_date_info( - unit, self.RELEASE_TITLE_RELEASED, self.past_start_date_text + ' with Section "Unlocked Section"' + unit, self.RELEASE_TITLE_RELEASED, self.past_start_date_text + '\n' + 'with Section "Unlocked Section"' ) self._view_published_version(unit) self._verify_student_view_visible(['problem']) @@ -560,6 +560,7 @@ class UnitPublishingTest(ContainerBase): When I go to the unit page in Studio And when I select "Hide from students" Then the unit does not have a warning that it is visible to students + And the unit does not display inherited staff lock And when I click on the View Live Button Then I see the content in the unit when logged in as staff And when I view the course as a student @@ -569,6 +570,7 @@ class UnitPublishingTest(ContainerBase): checked = unit.toggle_staff_lock() self.assertTrue(checked) self.assertFalse(unit.currently_visible_to_students) + self.assertFalse(unit.shows_inherited_staff_lock()) self._verify_publish_title(unit, self.LOCKED_STATUS) self._view_published_version(unit) # Will initially be in staff view, locked component should be visible. @@ -592,7 +594,7 @@ class UnitPublishingTest(ContainerBase): self.assertFalse(unit.currently_visible_to_students) self._verify_release_date_info( unit, self.RELEASE_TITLE_RELEASE, - self.past_start_date_text + ' with Subsection "Subsection With Locked Unit"' + self.past_start_date_text + '\n' + 'with Subsection "Subsection With Locked Unit"' ) self._view_published_version(unit) self._verify_student_view_locked() @@ -620,6 +622,46 @@ class UnitPublishingTest(ContainerBase): # Switch to student view and verify visible. self._verify_student_view_visible(['discussion']) + def test_explicit_lock_overrides_implicit_subsection_lock_information(self): + """ + Scenario: A unit's explicit staff lock hides its inherited subsection staff lock information + Given I have a course with sections, subsections, and units + And I have enabled explicit staff lock on a subsection + When I visit the unit page + Then the unit page shows its inherited staff lock + And I enable explicit staff locking + Then the unit page does not show its inherited staff lock + And when I disable explicit staff locking + Then the unit page now shows its inherited staff lock + """ + self.outline.visit() + self.outline.expand_all_subsections() + subsection = self.outline.section_at(0).subsection_at(0) + unit = subsection.unit_at(0) + subsection.set_staff_lock(True) + unit_page = unit.go_to() + self._verify_explicit_lock_overrides_implicit_lock_information(unit_page) + + def test_explicit_lock_overrides_implicit_section_lock_information(self): + """ + Scenario: A unit's explicit staff lock hides its inherited subsection staff lock information + Given I have a course with sections, subsections, and units + And I have enabled explicit staff lock on a section + When I visit the unit page + Then the unit page shows its inherited staff lock + And I enable explicit staff locking + Then the unit page does not show its inherited staff lock + And when I disable explicit staff locking + Then the unit page now shows its inherited staff lock + """ + self.outline.visit() + self.outline.expand_all_subsections() + section = self.outline.section_at(0) + unit = section.subsection_at(0).unit_at(0) + section.set_staff_lock(True) + unit_page = unit.go_to() + self._verify_explicit_lock_overrides_implicit_lock_information(unit_page) + def test_published_unit_with_draft_child(self): """ Scenario: A published unit with a draft child can be published @@ -748,6 +790,16 @@ class UnitPublishingTest(ContainerBase): self.assertTrue(expected_published_prefix in unit.last_published_text) self.assertTrue(expected_saved_prefix in unit.last_saved_text) + def _verify_explicit_lock_overrides_implicit_lock_information(self, unit_page): + """ + Verifies that a unit with inherited staff lock does not display inherited information when explicitly locked. + """ + self.assertTrue(unit_page.shows_inherited_staff_lock()) + unit_page.toggle_staff_lock(inherits_staff_lock=True) + self.assertFalse(unit_page.shows_inherited_staff_lock()) + unit_page.toggle_staff_lock(inherits_staff_lock=True) + self.assertTrue(unit_page.shows_inherited_staff_lock()) + # TODO: need to work with Jay/Christine to get testing of "Preview" working. # def test_preview(self): # unit = self.go_to_unit_page() diff --git a/common/test/acceptance/tests/test_studio_outline.py b/common/test/acceptance/tests/test_studio_outline.py index 23c49da0e6..6d6a6cc7fd 100644 --- a/common/test/acceptance/tests/test_studio_outline.py +++ b/common/test/acceptance/tests/test_studio_outline.py @@ -11,6 +11,7 @@ from bok_choy.promise import EmptyPromise from ..pages.studio.overview import CourseOutlinePage, ContainerPage, ExpandCollapseLinkState from ..pages.studio.utils import add_discussion from ..pages.lms.courseware import CoursewarePage +from ..pages.lms.staff_view import StaffPage from ..fixtures.course import XBlockFixtureDesc from .base_studio_test import StudioCourseTest @@ -119,7 +120,9 @@ class WarningMessagesTest(CourseOutlineTest): return XBlockFixtureDesc('chapter', name).add_children( subsection if unit_state.publish_state == self.PublishState.NEVER_PUBLISHED else subsection.add_children( - XBlockFixtureDesc('vertical', name, metadata={'visible_to_staff_only': unit_state.is_locked}) + XBlockFixtureDesc('vertical', name, metadata={ + 'visible_to_staff_only': True if unit_state.is_locked else None + }) ) ) @@ -403,6 +406,388 @@ class EditingSectionsTest(CourseOutlineTest): self.assertIn(release_text, self.course_outline_page.section_at(0).subsection_at(0).release_date) +@attr('shard_2') +class StaffLockTest(CourseOutlineTest): + """ + Feature: Sections, subsections, and units can be locked and unlocked from the course outline. + """ + + __test__ = True + + def populate_course_fixture(self, course_fixture): + """ Create a course with one section, two subsections, and four units """ + course_fixture.add_children( + XBlockFixtureDesc('chapter', '1').add_children( + XBlockFixtureDesc('sequential', '1.1').add_children( + XBlockFixtureDesc('vertical', '1.1.1'), + XBlockFixtureDesc('vertical', '1.1.2') + ), + XBlockFixtureDesc('sequential', '1.2').add_children( + XBlockFixtureDesc('vertical', '1.2.1'), + XBlockFixtureDesc('vertical', '1.2.2') + ) + ) + ) + + def _verify_descendants_are_staff_only(self, item): + """Verifies that all the descendants of item are staff only""" + self.assertTrue(item.is_staff_only) + if hasattr(item, 'children'): + for child in item.children(): + self._verify_descendants_are_staff_only(child) + + def _remove_staff_lock_and_verify_warning(self, outline_item, expect_warning): + """Removes staff lock from a course outline item and checks whether or not a warning appears.""" + modal = outline_item.edit() + modal.is_explicitly_locked = False + if expect_warning: + self.assertTrue(modal.shows_staff_lock_warning()) + else: + self.assertFalse(modal.shows_staff_lock_warning()) + modal.save() + + def _toggle_lock_on_unlocked_item(self, outline_item): + """Toggles outline_item's staff lock on and then off, verifying the staff lock warning""" + self.assertFalse(outline_item.has_staff_lock_warning) + outline_item.set_staff_lock(True) + self.assertTrue(outline_item.has_staff_lock_warning) + self._verify_descendants_are_staff_only(outline_item) + outline_item.set_staff_lock(False) + self.assertFalse(outline_item.has_staff_lock_warning) + + def _verify_explicit_staff_lock_remains_after_unlocking_parent(self, child_item, parent_item): + """Verifies that child_item's explicit staff lock remains after removing parent_item's staff lock""" + child_item.set_staff_lock(True) + parent_item.set_staff_lock(True) + self.assertTrue(parent_item.has_staff_lock_warning) + self.assertTrue(child_item.has_staff_lock_warning) + parent_item.set_staff_lock(False) + self.assertFalse(parent_item.has_staff_lock_warning) + self.assertTrue(child_item.has_staff_lock_warning) + + def test_units_can_be_locked(self): + """ + Scenario: Units can be locked and unlocked from the course outline page + Given I have a course with a unit + When I click on the configuration icon + And I enable explicit staff locking + And I click save + Then the unit shows a staff lock warning + And when I click on the configuration icon + And I disable explicit staff locking + And I click save + Then the unit does not show a staff lock warning + """ + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + unit = self.course_outline_page.section_at(0).subsection_at(0).unit_at(0) + self._toggle_lock_on_unlocked_item(unit) + + def test_subsections_can_be_locked(self): + """ + Scenario: Subsections can be locked and unlocked from the course outline page + Given I have a course with a subsection + When I click on the subsection's configuration icon + And I enable explicit staff locking + And I click save + Then the subsection shows a staff lock warning + And all its descendants are staff locked + And when I click on the subsection's configuration icon + And I disable explicit staff locking + And I click save + Then the the subsection does not show a staff lock warning + """ + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + subsection = self.course_outline_page.section_at(0).subsection_at(0) + self._toggle_lock_on_unlocked_item(subsection) + + def test_sections_can_be_locked(self): + """ + Scenario: Sections can be locked and unlocked from the course outline page + Given I have a course with a section + When I click on the section's configuration icon + And I enable explicit staff locking + And I click save + Then the section shows a staff lock warning + And all its descendants are staff locked + And when I click on the section's configuration icon + And I disable explicit staff locking + And I click save + Then the section does not show a staff lock warning + """ + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + section = self.course_outline_page.section_at(0) + self._toggle_lock_on_unlocked_item(section) + + def test_explicit_staff_lock_remains_after_unlocking_section(self): + """ + Scenario: An explicitly locked unit is still locked after removing an inherited lock from a section + Given I have a course with sections, subsections, and units + And I have enabled explicit staff lock on a section and one of its units + When I click on the section's configuration icon + And I disable explicit staff locking + And I click save + Then the unit still shows a staff lock warning + """ + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + section = self.course_outline_page.section_at(0) + unit = section.subsection_at(0).unit_at(0) + self._verify_explicit_staff_lock_remains_after_unlocking_parent(unit, section) + + def test_explicit_staff_lock_remains_after_unlocking_subsection(self): + """ + Scenario: An explicitly locked unit is still locked after removing an inherited lock from a subsection + Given I have a course with sections, subsections, and units + And I have enabled explicit staff lock on a subsection and one of its units + When I click on the subsection's configuration icon + And I disable explicit staff locking + And I click save + Then the unit still shows a staff lock warning + """ + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + subsection = self.course_outline_page.section_at(0).subsection_at(0) + unit = subsection.unit_at(0) + self._verify_explicit_staff_lock_remains_after_unlocking_parent(unit, subsection) + + def test_section_displays_lock_when_all_subsections_locked(self): + """ + Scenario: All subsections in section are explicitly locked, section should display staff only warning + Given I have a course one section and two subsections + When I enable explicit staff lock on all the subsections + Then the section shows a staff lock warning + """ + self.course_outline_page.visit() + section = self.course_outline_page.section_at(0) + section.subsection_at(0).set_staff_lock(True) + section.subsection_at(1).set_staff_lock(True) + self.assertTrue(section.has_staff_lock_warning) + + def test_section_displays_lock_when_all_units_locked(self): + """ + Scenario: All units in a section are explicitly locked, section should display staff only warning + Given I have a course with one section, two subsections, and four units + When I enable explicit staff lock on all the units + Then the section shows a staff lock warning + """ + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + section = self.course_outline_page.section_at(0) + section.subsection_at(0).unit_at(0).set_staff_lock(True) + section.subsection_at(0).unit_at(1).set_staff_lock(True) + section.subsection_at(1).unit_at(0).set_staff_lock(True) + section.subsection_at(1).unit_at(1).set_staff_lock(True) + self.assertTrue(section.has_staff_lock_warning) + + def test_subsection_displays_lock_when_all_units_locked(self): + """ + Scenario: All units in subsection are explicitly locked, subsection should display staff only warning + Given I have a course with one subsection and two units + When I enable explicit staff lock on all the units + Then the subsection shows a staff lock warning + """ + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + subsection = self.course_outline_page.section_at(0).subsection_at(0) + subsection.unit_at(0).set_staff_lock(True) + subsection.unit_at(1).set_staff_lock(True) + self.assertTrue(subsection.has_staff_lock_warning) + + def test_section_does_not_display_lock_when_some_subsections_locked(self): + """ + Scenario: Only some subsections in section are explicitly locked, section should NOT display staff only warning + Given I have a course with one section and two subsections + When I enable explicit staff lock on one subsection + Then the section does not show a staff lock warning + """ + self.course_outline_page.visit() + section = self.course_outline_page.section_at(0) + section.subsection_at(0).set_staff_lock(True) + self.assertFalse(section.has_staff_lock_warning) + + def test_section_does_not_display_lock_when_some_units_locked(self): + """ + Scenario: Only some units in section are explicitly locked, section should NOT display staff only warning + Given I have a course with one section, two subsections, and four units + When I enable explicit staff lock on three units + Then the section does not show a staff lock warning + """ + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + section = self.course_outline_page.section_at(0) + section.subsection_at(0).unit_at(0).set_staff_lock(True) + section.subsection_at(0).unit_at(1).set_staff_lock(True) + section.subsection_at(1).unit_at(1).set_staff_lock(True) + self.assertFalse(section.has_staff_lock_warning) + + def test_subsection_does_not_display_lock_when_some_units_locked(self): + """ + Scenario: Only some units in subsection are explicitly locked, subsection should NOT display staff only warning + Given I have a course with one subsection and two units + When I enable explicit staff lock on one unit + Then the subsection does not show a staff lock warning + """ + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + subsection = self.course_outline_page.section_at(0).subsection_at(0) + subsection.unit_at(0).set_staff_lock(True) + self.assertFalse(subsection.has_staff_lock_warning) + + def test_locked_sections_do_not_appear_in_lms(self): + """ + Scenario: A locked section is not visible to students in the LMS + Given I have a course with two sections + When I enable explicit staff lock on one section + And I click the View Live button to switch to staff view + Then I see two sections in the sidebar + And when I click to toggle to student view + Then I see one section in the sidebar + """ + self.course_outline_page.visit() + self.course_outline_page.add_section_from_top_button() + self.course_outline_page.section_at(1).set_staff_lock(True) + self.course_outline_page.view_live() + courseware = CoursewarePage(self.browser, self.course_id) + courseware.wait_for_page() + self.assertEqual(courseware.num_sections, 2) + StaffPage(self.browser).toggle_staff_view() + self.assertEqual(courseware.num_sections, 1) + + def test_locked_subsections_do_not_appear_in_lms(self): + """ + Scenario: A locked subsection is not visible to students in the LMS + Given I have a course with two subsections + When I enable explicit staff lock on one subsection + And I click the View Live button to switch to staff view + Then I see two subsections in the sidebar + And when I click to toggle to student view + Then I see one section in the sidebar + """ + self.course_outline_page.visit() + self.course_outline_page.section_at(0).subsection_at(1).set_staff_lock(True) + self.course_outline_page.view_live() + courseware = CoursewarePage(self.browser, self.course_id) + courseware.wait_for_page() + self.assertEqual(courseware.num_subsections, 2) + StaffPage(self.browser).toggle_staff_view() + self.assertEqual(courseware.num_subsections, 1) + + def test_toggling_staff_lock_on_section_does_not_publish_draft_units(self): + """ + Scenario: Locking and unlocking a section will not publish its draft units + Given I have a course with a section and unit + And the unit has a draft and published version + When I enable explicit staff lock on the section + And I disable explicit staff lock on the section + And I click the View Live button to switch to staff view + Then I see the published version of the unit + """ + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + unit = self.course_outline_page.section_at(0).subsection_at(0).unit_at(0).go_to() + add_discussion(unit) + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + section = self.course_outline_page.section_at(0) + section.set_staff_lock(True) + section.set_staff_lock(False) + unit = section.subsection_at(0).unit_at(0).go_to() + unit.view_published_version() + courseware = CoursewarePage(self.browser, self.course_id) + courseware.wait_for_page() + self.assertEqual(courseware.num_xblock_components, 0) + + def test_toggling_staff_lock_on_subsection_does_not_publish_draft_units(self): + """ + Scenario: Locking and unlocking a subsection will not publish its draft units + Given I have a course with a subsection and unit + And the unit has a draft and published version + When I enable explicit staff lock on the subsection + And I disable explicit staff lock on the subsection + And I click the View Live button to switch to staff view + Then I see the published version of the unit + """ + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + unit = self.course_outline_page.section_at(0).subsection_at(0).unit_at(0).go_to() + add_discussion(unit) + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + subsection = self.course_outline_page.section_at(0).subsection_at(0) + subsection.set_staff_lock(True) + subsection.set_staff_lock(False) + unit = subsection.unit_at(0).go_to() + unit.view_published_version() + courseware = CoursewarePage(self.browser, self.course_id) + courseware.wait_for_page() + self.assertEqual(courseware.num_xblock_components, 0) + + def test_removing_staff_lock_from_unit_without_inherited_lock_shows_warning(self): + """ + Scenario: Removing explicit staff lock from a unit which does not inherit staff lock displays a warning. + Given I have a course with a subsection and unit + When I enable explicit staff lock on the unit + And I disable explicit staff lock on the unit + Then I see a modal warning. + """ + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + unit = self.course_outline_page.section_at(0).subsection_at(0).unit_at(0) + unit.set_staff_lock(True) + self._remove_staff_lock_and_verify_warning(unit, True) + + def test_removing_staff_lock_from_subsection_without_inherited_lock_shows_warning(self): + """ + Scenario: Removing explicit staff lock from a subsection which does not inherit staff lock displays a warning. + Given I have a course with a section and subsection + When I enable explicit staff lock on the subsection + And I disable explicit staff lock on the subsection + Then I see a modal warning. + """ + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + subsection = self.course_outline_page.section_at(0).subsection_at(0) + subsection.set_staff_lock(True) + self._remove_staff_lock_and_verify_warning(subsection, True) + + def test_removing_staff_lock_from_unit_with_inherited_lock_shows_no_warning(self): + """ + Scenario: Removing explicit staff lock from a unit which also inherits staff lock displays no warning. + Given I have a course with a subsection and unit + When I enable explicit staff lock on the subsection + And I enable explicit staff lock on the unit + When I disable explicit staff lock on the unit + Then I do not see a modal warning. + """ + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + subsection = self.course_outline_page.section_at(0).subsection_at(0) + unit = subsection.unit_at(0) + subsection.set_staff_lock(True) + unit.set_staff_lock(True) + self._remove_staff_lock_and_verify_warning(unit, False) + + def test_removing_staff_lock_from_subsection_with_inherited_lock_shows_no_warning(self): + """ + Scenario: Removing explicit staff lock from a subsection which also inherits staff lock displays no warning. + Given I have a course with a section and subsection + When I enable explicit staff lock on the section + And I enable explicit staff lock on the subsection + When I disable explicit staff lock on the subsection + Then I do not see a modal warning. + """ + self.course_outline_page.visit() + self.course_outline_page.expand_all_subsections() + section = self.course_outline_page.section_at(0) + subsection = section.subsection_at(0) + section.set_staff_lock(True) + subsection.set_staff_lock(True) + self._remove_staff_lock_and_verify_warning(subsection, False) + + @attr('shard_2') class EditNamesTest(CourseOutlineTest): """