From ef581e1146824ac476bb22efecb51725ff588b5c Mon Sep 17 00:00:00 2001 From: Andy Armstrong Date: Fri, 25 Jul 2014 17:10:43 -0400 Subject: [PATCH] Apply code review comments and fix tests --- cms/djangoapps/contentstore/tests/utils.py | 28 +-- cms/djangoapps/contentstore/utils.py | 14 -- .../contentstore/views/component.py | 12 +- cms/djangoapps/contentstore/views/item.py | 66 ++++--- .../views/tests/test_course_index.py | 5 +- .../contentstore/views/tests/test_item.py | 186 +++++++++++------- cms/static/js/models/xblock_info.js | 15 +- .../views/pages/container_subviews_spec.js | 80 ++++---- .../spec/views/pages/course_outline_spec.js | 6 +- cms/static/js/spec/views/unit_outline_spec.js | 12 +- cms/static/js/views/baseview.js | 3 + .../js/views/pages/container_subviews.js | 30 +-- cms/static/js/views/unit_outline.js | 1 - cms/static/js/views/utils/xblock_utils.js | 54 ++++- cms/static/js/views/xblock_outline.js | 7 +- cms/templates/course_outline.html | 2 +- cms/templates/js/course-outline.underscore | 19 +- cms/templates/js/publish-xblock.underscore | 35 ++-- cms/templates/js/unit-outline.underscore | 14 +- cms/templates/widgets/units.html | 3 +- .../xmodule/xmodule/modulestore/__init__.py | 12 +- .../lib/xmodule/xmodule/modulestore/mixed.py | 6 +- .../xmodule/modulestore/mongo/draft.py | 16 +- .../modulestore/split_mongo/split_draft.py | 14 +- .../tests/test_mixed_modulestore.py | 14 +- .../test/acceptance/tests/base_studio_test.py | 3 +- .../acceptance/tests/test_studio_general.py | 2 +- .../acceptance/tests/test_studio_outline.py | 2 +- .../tests/test_studio_split_test.py | 2 +- 29 files changed, 366 insertions(+), 297 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index b1660f5804..4efe09d6de 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -9,7 +9,7 @@ from django.test.client import Client from django.contrib.auth.models import User from xmodule.contentstore.django import contentstore -from xmodule.modulestore import LegacyPublishState, ModuleStoreEnum, mongo +from xmodule.modulestore import PublishState, ModuleStoreEnum, mongo from xmodule.modulestore.inheritance import own_metadata from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -151,16 +151,16 @@ class CourseTestCase(ModuleStoreTestCase): # create a Draft vertical vertical = self.store.get_item(course_id.make_usage_key('vertical', self.TEST_VERTICAL), depth=1) draft_vertical = self.store.convert_to_draft(vertical.location, self.user.id) - self.assertEqual(self.store.compute_publish_state(draft_vertical), LegacyPublishState.draft) + self.assertEqual(self.store.compute_publish_state(draft_vertical), PublishState.draft) # create a Private (draft only) vertical private_vertical = self.store.create_item(self.user.id, course_id, 'vertical', self.PRIVATE_VERTICAL) - self.assertEqual(self.store.compute_publish_state(private_vertical), LegacyPublishState.private) + self.assertEqual(self.store.compute_publish_state(private_vertical), PublishState.private) # create a Published (no draft) vertical public_vertical = self.store.create_item(self.user.id, course_id, 'vertical', self.PUBLISHED_VERTICAL) public_vertical = self.store.publish(public_vertical.location, self.user.id) - self.assertEqual(self.store.compute_publish_state(public_vertical), LegacyPublishState.public) + self.assertEqual(self.store.compute_publish_state(public_vertical), PublishState.public) # add the new private and new public as children of the sequential sequential = self.store.get_item(course_id.make_usage_key('sequential', self.SEQUENTIAL)) @@ -197,7 +197,7 @@ class CourseTestCase(ModuleStoreTestCase): def verify_item_publish_state(item, publish_state): """Verifies the publish state of the item is as expected.""" - if publish_state in (LegacyPublishState.private, LegacyPublishState.draft): + if publish_state in (PublishState.private, PublishState.draft): self.assertTrue(getattr(item, 'is_draft', False)) else: self.assertFalse(getattr(item, 'is_draft', False)) @@ -210,18 +210,18 @@ class CourseTestCase(ModuleStoreTestCase): return item # verify that the draft vertical is draft - vertical = get_and_verify_publish_state('vertical', self.TEST_VERTICAL, LegacyPublishState.draft) + vertical = get_and_verify_publish_state('vertical', self.TEST_VERTICAL, PublishState.draft) for child in vertical.get_children(): - verify_item_publish_state(child, LegacyPublishState.draft) + verify_item_publish_state(child, PublishState.draft) # make sure that we don't have a sequential that is not in draft mode - sequential = get_and_verify_publish_state('sequential', self.SEQUENTIAL, LegacyPublishState.public) + sequential = get_and_verify_publish_state('sequential', self.SEQUENTIAL, PublishState.public) # verify that we have the private vertical - private_vertical = get_and_verify_publish_state('vertical', self.PRIVATE_VERTICAL, LegacyPublishState.private) + private_vertical = get_and_verify_publish_state('vertical', self.PRIVATE_VERTICAL, PublishState.private) # verify that we have the public vertical - public_vertical = get_and_verify_publish_state('vertical', self.PUBLISHED_VERTICAL, LegacyPublishState.public) + public_vertical = get_and_verify_publish_state('vertical', self.PUBLISHED_VERTICAL, PublishState.public) # verify verticals are children of sequential for vert in [vertical, private_vertical, public_vertical]: @@ -332,7 +332,7 @@ class CourseTestCase(ModuleStoreTestCase): it'll return public in that case """ supposed_state = self.store.compute_publish_state(item) - if supposed_state == LegacyPublishState.draft and isinstance(item.runtime.modulestore, DraftModuleStore): + if supposed_state == PublishState.draft and isinstance(item.runtime.modulestore, DraftModuleStore): # see if the draft differs from the published published = self.store.get_item(item.location, revision=ModuleStoreEnum.RevisionOption.published_only) if item.get_explicitly_set_fields_by_scope() != published.get_explicitly_set_fields_by_scope(): @@ -345,13 +345,13 @@ class CourseTestCase(ModuleStoreTestCase): # checking children: if published differs from item, return draft return supposed_state # published == item in all respects, so return public - return LegacyPublishState.public - elif supposed_state == LegacyPublishState.public and item.location.category in DIRECT_ONLY_CATEGORIES: + return PublishState.public + elif supposed_state == PublishState.public and item.location.category in DIRECT_ONLY_CATEGORIES: if not all([ self.store.has_item(child_loc, revision=ModuleStoreEnum.RevisionOption.draft_only) for child_loc in item.children ]): - return LegacyPublishState.draft + return PublishState.draft else: return supposed_state else: diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 7749dde550..0f8d4f1e2e 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -150,20 +150,6 @@ def course_image_url(course): return path -def compute_publish_state(xblock): - """ - Returns whether this xblock is draft, public, or private. - - Returns: - LegacyPublishState.draft - content is in the process of being edited, but still has a previous - version deployed to LMS - LegacyPublishState.public - content is locked and deployed to LMS - LegacyPublishState.private - content is editable and not deployed to LMS - """ - - return modulestore().compute_publish_state(xblock) - - def is_currently_visible_to_students(xblock): """ Returns true if there is a published version of the xblock that is currently visible to students. diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 4933d196a6..bbba49f575 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -11,8 +11,8 @@ from django.conf import settings from xmodule.modulestore.exceptions import ItemNotFoundError from edxmako.shortcuts import render_to_response +from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore -from xmodule.modulestore import LegacyPublishState from xblock.core import XBlock from xblock.django.request import webob_to_django_response, django_to_webob_request @@ -21,7 +21,7 @@ from xblock.fields import Scope from xblock.plugin import PluginMissingError from xblock.runtime import Mixologist -from contentstore.utils import get_lms_link_for_item, compute_publish_state +from contentstore.utils import get_lms_link_for_item from contentstore.views.helpers import get_parent_xblock, is_unit, xblock_type_display_name from contentstore.views.item import create_xblock_info @@ -100,8 +100,8 @@ def subsection_handler(request, usage_key_string): can_view_live = False subsection_units = item.get_children() for unit in subsection_units: - state = compute_publish_state(unit) - if state in (LegacyPublishState.public, LegacyPublishState.draft): + has_published = modulestore().has_item(unit.location, revision=ModuleStoreEnum.RevisionOption.published_only) + if has_published: can_view_live = True break @@ -178,6 +178,10 @@ def container_handler(request, usage_key_string): # about the block's ancestors and siblings for use by the Unit Outline. xblock_info = create_xblock_info(xblock, include_ancestor_info=is_unit_page) + # On the unit page only, add 'has_changes' to indicate when there are changes that can be discarded. + if is_unit_page: + xblock_info['has_changes'] = modulestore().has_changes(xblock.location) + # Create the link for preview. preview_lms_base = settings.FEATURES.get('PREVIEW_LMS_BASE') # need to figure out where this item is in the list of children as the diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 8c867639d2..d22494f0d9 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -636,9 +636,9 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F "id": unicode(xblock.location), "display_name": xblock.display_name_with_default, "category": xblock.category, - "published": published, "edited_on": get_default_time_display(xblock.subtree_edited_on) if xblock.subtree_edited_on else None, "edited_by": safe_get_username(xblock.subtree_edited_by), + "published": published, "published_on": get_default_time_display(xblock.published_date) if xblock.published_date else None, "published_by": safe_get_username(xblock.published_by), 'studio_url': xblock_studio_url(xblock), @@ -646,7 +646,7 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F "release_date": release_date, "release_date_from": _get_release_date_from(xblock) if release_date else None, "currently_visible_to_students": currently_visible_to_students, - "publish_state": _compute_publish_state(xblock, child_info) if not xblock.category == 'course' else None + "visibility_state": _compute_visibility_state(xblock, child_info) if not xblock.category == 'course' else None } if data is not None: xblock_info["data"] = data @@ -659,63 +659,71 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F return xblock_info -class PublishState(object): +class VisibilityState(object): """ - Represents the possible publish states for an xblock: - live - the block and all of its children are live to students (except for staff only items) - ready - the block and all of its children are ready to go live in the future - unscheduled - the block and all of its children are unscheduled - has_unpublished_content - the block or its children have unpublished content that is not staff only + Represents the possible visibility states for an xblock: + + live - the block and all of its descendants are live to students (excluding staff only items) + Note: Live means both published and released. + + ready - the block is ready to go live and all of its descendants are live or ready (excluding staff only items) + Note: content is ready when it is published and scheduled with a release date in the future. + + unscheduled - the block and all of its descendants have no release date (excluding staff only items) + Note: it is valid for items to be published with no release date in which case they are still unscheduled. + + needs_attention - the block or its descendants are not fully live, ready or unscheduled (excluding staff only items) + For example: one subsection has draft content, or there's both unreleased and released content in one section. + staff_only - all of the block's content is to be shown to staff only + Note: staff only items do not affect their parent's state. """ live = 'live' ready = 'ready' - unscheduled = 'unscheduled' - has_unpublished_content = 'has_unpublished_content' + unscheduled = 'unscheduled' # unscheduled + needs_attention = 'needs_attention' staff_only = 'staff_only' -def _compute_publish_state(xblock, child_info): +def _compute_visibility_state(xblock, child_info): """ Returns the current publish state for the specified xblock and its children """ if xblock.visible_to_staff_only: - return PublishState.staff_only + return VisibilityState.staff_only elif is_unit(xblock) and modulestore().has_changes(xblock.location): - return PublishState.has_unpublished_content + return VisibilityState.needs_attention is_unscheduled = xblock.start == DEFAULT_START_DATE + is_live = datetime.now(UTC) > xblock.start children = child_info and child_info['children'] if children and len(children) > 0: all_staff_only = True all_unscheduled = True all_live = True for child in child_info['children']: - child_state = child['publish_state'] - if child_state == PublishState.has_unpublished_content: + child_state = child['visibility_state'] + if child_state == VisibilityState.needs_attention: return child_state - elif not child_state == PublishState.staff_only: + elif not child_state == VisibilityState.staff_only: all_staff_only = False - if not child_state == PublishState.unscheduled: + if not child_state == VisibilityState.unscheduled: all_unscheduled = False - if not child_state == PublishState.live: + if not child_state == VisibilityState.live: all_live = False if all_staff_only: - return PublishState.staff_only + return VisibilityState.staff_only elif all_unscheduled: - if not is_unscheduled: - return PublishState.has_unpublished_content - else: - return PublishState.unscheduled + return VisibilityState.unscheduled if is_unscheduled else VisibilityState.needs_attention elif all_live: - return PublishState.live + return VisibilityState.live if is_live else VisibilityState.needs_attention else: - return PublishState.ready + return VisibilityState.ready if not is_unscheduled else VisibilityState.needs_attention if is_unscheduled: - return PublishState.unscheduled - elif datetime.now(UTC) > xblock.start: - return PublishState.live + return VisibilityState.unscheduled + elif is_live: + return VisibilityState.live else: - return PublishState.ready + return VisibilityState.ready def _create_xblock_ancestor_info(xblock): diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index 8bad464fbf..4123349450 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -8,6 +8,7 @@ from contentstore.tests.utils import CourseTestCase from contentstore.utils import reverse_course_url, add_instructor from contentstore.views.access import has_course_access from contentstore.views.course import course_outline_initial_state +from contentstore.views.item import create_xblock_info, VisibilityState from course_action_state.models import CourseRerunState from contentstore.views.item import create_xblock_info from contentstore.views.item import create_xblock_info, PublishState @@ -230,7 +231,7 @@ class TestCourseOutline(CourseTestCase): self.assertEqual(json_response['category'], 'course') self.assertEqual(json_response['id'], 'i4x://MITx/999/course/Robot_Super_Course') self.assertEqual(json_response['display_name'], 'Robot Super Course') - self.assertIsNone(json_response['publish_state']) + self.assertIsNone(json_response['visibility_state']) # Now verify the first child children = json_response['child_info']['children'] @@ -239,7 +240,7 @@ class TestCourseOutline(CourseTestCase): self.assertEqual(first_child_response['category'], 'chapter') self.assertEqual(first_child_response['id'], 'i4x://MITx/999/chapter/Week_1') self.assertEqual(first_child_response['display_name'], 'Week 1') - self.assertEqual(first_child_response['publish_state'], PublishState.unscheduled) + self.assertEqual(first_child_response['visibility_state'], VisibilityState.unscheduled) self.assertTrue(len(first_child_response['child_info']['children']) > 0) # Finally, validate the entire response for consistency diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 80396c497b..ed80c26205 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -19,11 +19,11 @@ from contentstore.views.component import ( component_handler, get_component_templates ) -from contentstore.views.item import create_xblock_info, ALWAYS, PublishState +from contentstore.views.item import create_xblock_info, ALWAYS, VisibilityState from contentstore.tests.utils import CourseTestCase from student.tests.factories import UserFactory from xmodule.capa_module import CapaDescriptor -from xmodule.modulestore import LegacyPublishState, ModuleStoreEnum +from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.factories import ItemFactory from xmodule.x_module import STUDIO_VIEW, STUDENT_VIEW @@ -425,19 +425,6 @@ class TestEditItem(ItemTest): self.course_update_url = reverse_usage_url("xblock_handler", self.usage_key) - def verify_publish_state(self, usage_key, expected_publish_state): - """ - Helper method that gets the item from the module store and verifies that the publish state is as expected. - Returns the item corresponding to the given usage_key. - """ - item = self.get_item_from_modulestore( - usage_key, - (expected_publish_state == LegacyPublishState.private) or - (expected_publish_state == LegacyPublishState.draft) - ) - self.assertEqual(expected_publish_state, self.store.compute_publish_state(item)) - return item - def test_delete_field(self): """ Sending null in for a field 'deletes' it @@ -543,15 +530,18 @@ class TestEditItem(ItemTest): self.assertEqual(unit1_usage_key, children[2]) self.assertEqual(unit2_usage_key, children[1]) + def _is_location_published(self, location): + return modulestore().has_item(location, revision=ModuleStoreEnum.RevisionOption.published_only) + def test_make_public(self): """ Test making a private problem public (publishing it). """ # When the problem is first created, it is only in draft (because of its category). - self.verify_publish_state(self.problem_usage_key, LegacyPublishState.private) + self.assertFalse(self._is_location_published(self.problem_usage_key)) self.client.ajax_post( self.problem_update_url, data={'publish': 'make_public'} ) - self.verify_publish_state(self.problem_usage_key, LegacyPublishState.public) + self.assertTrue(self._is_location_published(self.problem_usage_key)) def test_make_draft(self): """ Test creating a draft version of a public problem. """ @@ -564,7 +554,8 @@ class TestEditItem(ItemTest): self.problem_update_url, data={'publish': 'discard_changes'} ) - published = self.verify_publish_state(self.problem_usage_key, LegacyPublishState.public) + self.assertTrue(self._is_location_published(self.problem_usage_key)) + published = modulestore().get_item(self.problem_usage_key, revision=ModuleStoreEnum.RevisionOption.published_only) self.assertIsNone(published.due) def test_republish(self): @@ -576,7 +567,7 @@ class TestEditItem(ItemTest): } # When the problem is first created, it is only in draft (because of its category). - self.verify_publish_state(self.problem_usage_key, LegacyPublishState.private) + self.assertFalse(self._is_location_published(self.problem_usage_key)) # Republishing when only in draft will update the draft but not cause a public item to be created. self.client.ajax_post( @@ -588,7 +579,7 @@ class TestEditItem(ItemTest): } } ) - self.verify_publish_state(self.problem_usage_key, LegacyPublishState.private) + self.assertFalse(self._is_location_published(self.problem_usage_key)) draft = self.get_item_from_modulestore(self.problem_usage_key, verify_is_draft=True) self.assertEqual(draft.display_name, new_display_name) @@ -609,7 +600,7 @@ class TestEditItem(ItemTest): } } ) - self.verify_publish_state(self.problem_usage_key, LegacyPublishState.public) + self.assertTrue(self._is_location_published(self.problem_usage_key)) published = modulestore().get_item( self.problem_usage_key, revision=ModuleStoreEnum.RevisionOption.published_only @@ -625,7 +616,8 @@ class TestEditItem(ItemTest): self.problem_update_url, data={'publish': 'make_public'} ) - published = self.verify_publish_state(self.problem_usage_key, LegacyPublishState.public) + self.assertTrue(self._is_location_published(self.problem_usage_key)) + published = modulestore().get_item(self.problem_usage_key, revision=ModuleStoreEnum.RevisionOption.published_only) # Update the draft version and check that published is different. self.client.ajax_post( @@ -659,7 +651,8 @@ class TestEditItem(ItemTest): self.problem_update_url, data={'publish': 'make_public'} ) - published = self.verify_publish_state(self.problem_usage_key, LegacyPublishState.public) + self.assertTrue(self._is_location_published(self.problem_usage_key)) + published = modulestore().get_item(self.problem_usage_key, revision=ModuleStoreEnum.RevisionOption.published_only) # Now make a draft self.client.ajax_post( @@ -704,8 +697,8 @@ class TestEditItem(ItemTest): # The unit and its children should be private initially unit_update_url = reverse_usage_url('xblock_handler', unit_usage_key) - self.verify_publish_state(unit_usage_key, LegacyPublishState.private) - self.verify_publish_state(html_usage_key, LegacyPublishState.private) + self.assertFalse(self._is_location_published(unit_usage_key)) + self.assertFalse(self._is_location_published(html_usage_key)) # Make the unit public and verify that the problem is also made public resp = self.client.ajax_post( @@ -713,8 +706,8 @@ class TestEditItem(ItemTest): data={'publish': 'make_public'} ) self.assertEqual(resp.status_code, 200) - self.verify_publish_state(unit_usage_key, LegacyPublishState.public) - self.verify_publish_state(html_usage_key, LegacyPublishState.public) + self.assertTrue(self._is_location_published(unit_usage_key)) + self.assertTrue(self._is_location_published(html_usage_key)) # Make a draft for the unit and verify that the problem also has a draft resp = self.client.ajax_post( @@ -725,8 +718,10 @@ class TestEditItem(ItemTest): } ) self.assertEqual(resp.status_code, 200) - self.verify_publish_state(unit_usage_key, LegacyPublishState.draft) - self.verify_publish_state(html_usage_key, LegacyPublishState.draft) + self.assertTrue(self._is_location_published(unit_usage_key)) + self.assertTrue(self._is_location_published(html_usage_key)) + self.assertTrue(modulestore().get_item(unit_usage_key).has_changes()) + self.assertTrue(modulestore().get_item(html_usage_key).has_changes()) class TestEditSplitModule(ItemTest): @@ -1243,10 +1238,14 @@ class TestXBlockPublishingInfo(ItemTest): """ Unit tests for XBlock's outline handling. """ - def _create_child(self, parent, category, display_name, publish_item=False): + FIRST_SUBSECTION_PATH = [0] + FIRST_UNIT_PATH = [0, 0] + SECOND_UNIT_PATH = [0, 1] + + def _create_child(self, parent, category, display_name, publish_item=False, staff_only=False): return ItemFactory.create( parent_location=parent.location, category=category, display_name=display_name, - user_id=self.user.id, publish_item=publish_item + user_id=self.user.id, publish_item=publish_item, visible_to_staff_only=staff_only ) def _get_child(self, xblock_info, index): @@ -1291,87 +1290,122 @@ class TestXBlockPublishingInfo(ItemTest): xblock.display_name = display_name self.store.update_item(xblock, self.user.id) - def test_empty_chapter_publishing_info(self): + def _verify_visibility_state(self, xblock_info, expected_state, path=None): + """ + Verify the publish state of an item in the xblock_info. If no path is provided + then the root item is verified. + """ + if path: + direct_child = self._get_child(xblock_info, path[0]) + remaining_path = path[1:] if len(path) > 1 else None + self._verify_visibility_state(direct_child, expected_state, remaining_path) + else: + self.assertEqual(xblock_info['visibility_state'], expected_state) + + 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['publish_state'], PublishState.unscheduled) + self.assertEqual(xblock_info['visibility_state'], VisibilityState.unscheduled) - def test_empty_section_publishing_info(self): + def test_empty_sequential(self): chapter = self._create_child(self.course, 'chapter', "Test Chapter") self._create_child(chapter, 'sequential', "Empty Sequential") xblock_info = self._get_xblock_info(chapter.location) - self.assertEqual(xblock_info['publish_state'], PublishState.unscheduled) - self.assertEqual(self._get_child(xblock_info, 0)['publish_state'], PublishState.unscheduled) + self.assertEqual(xblock_info['visibility_state'], VisibilityState.unscheduled) + self.assertEqual(self._get_child(xblock_info, 0)['visibility_state'], VisibilityState.unscheduled) - def test_published_unit_publishing_info(self): + def test_published_unit(self): chapter = self._create_child(self.course, 'chapter', "Test Chapter") sequential = self._create_child(chapter, 'sequential', "Test Sequential") self._create_child(sequential, 'vertical', "Published Unit", publish_item=True) + self._create_child(sequential, 'vertical', "Staff Only Unit", staff_only=True) self._set_release_date(chapter.location, datetime.now(UTC) + timedelta(days=1)) xblock_info = self._get_xblock_info(chapter.location) - self.assertEqual(xblock_info['publish_state'], PublishState.ready) - sequential_child_info = self._get_child(xblock_info, 0) - self.assertEqual(sequential_child_info['publish_state'], PublishState.ready) - unit_child_info = self._get_child(sequential_child_info, 0) - self.assertEqual(unit_child_info['publish_state'], PublishState.ready) + self._verify_visibility_state(xblock_info, VisibilityState.ready) + self._verify_visibility_state(xblock_info, VisibilityState.ready, path=self.FIRST_SUBSECTION_PATH) + self._verify_visibility_state(xblock_info, VisibilityState.ready, path=self.FIRST_UNIT_PATH) + self._verify_visibility_state(xblock_info, VisibilityState.staff_only, path=self.SECOND_UNIT_PATH) - def test_released_unit_publishing_info(self): + def test_released_unit(self): chapter = self._create_child(self.course, 'chapter', "Test Chapter") sequential = self._create_child(chapter, 'sequential', "Test Sequential") self._create_child(sequential, 'vertical', "Published Unit", publish_item=True) + self._create_child(sequential, 'vertical', "Staff Only Unit", staff_only=True) self._set_release_date(chapter.location, datetime.now(UTC) - timedelta(days=1)) xblock_info = self._get_xblock_info(chapter.location) - self.assertEqual(xblock_info['publish_state'], PublishState.live) - sequential_child_info = self._get_child(xblock_info, 0) - self.assertEqual(sequential_child_info['publish_state'], PublishState.live) - unit_child_info = self._get_child(sequential_child_info, 0) - self.assertEqual(unit_child_info['publish_state'], PublishState.live) + self._verify_visibility_state(xblock_info, VisibilityState.live) + 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_partially_released_section_publishing_info(self): + def test_unpublished_changes(self): + chapter = self._create_child(self.course, 'chapter', "Test Chapter") + sequential = self._create_child(chapter, 'sequential', "Test Sequential") + unit = self._create_child(sequential, 'vertical', "Published Unit", publish_item=True) + self._create_child(sequential, 'vertical', "Staff Only Unit", staff_only=True) + self._set_display_name(unit.location, 'Updated Unit') + xblock_info = self._get_xblock_info(chapter.location) + self._verify_visibility_state(xblock_info, VisibilityState.needs_attention) + self._verify_visibility_state(xblock_info, VisibilityState.needs_attention, path=self.FIRST_SUBSECTION_PATH) + self._verify_visibility_state(xblock_info, VisibilityState.needs_attention, path=self.FIRST_UNIT_PATH) + self._verify_visibility_state(xblock_info, VisibilityState.staff_only, path=self.SECOND_UNIT_PATH) + + def test_partially_released_section(self): chapter = self._create_child(self.course, 'chapter', "Test Chapter") released_sequential = self._create_child(chapter, 'sequential', "Released Sequential") self._create_child(released_sequential, 'vertical', "Released Unit", publish_item=True) + self._create_child(released_sequential, 'vertical', "Staff Only Unit", staff_only=True) self._set_release_date(chapter.location, datetime.now(UTC) - timedelta(days=1)) published_sequential = self._create_child(chapter, 'sequential', "Published Sequential") self._create_child(published_sequential, 'vertical', "Published Unit", publish_item=True) + self._create_child(published_sequential, 'vertical', "Staff Only Unit", staff_only=True) self._set_release_date(published_sequential.location, datetime.now(UTC) + timedelta(days=1)) xblock_info = self._get_xblock_info(chapter.location) # Verify the state of the released sequential - released_sequential_child_info = self._get_child(xblock_info, 0) - released_unit_child_info = self._get_child(released_sequential_child_info, 0) - self.assertEqual(released_unit_child_info['publish_state'], PublishState.live) - self.assertEqual(released_sequential_child_info['publish_state'], PublishState.live) + self._verify_visibility_state(xblock_info, VisibilityState.live, path=[0]) + self._verify_visibility_state(xblock_info, VisibilityState.live, path=[0, 0]) + self._verify_visibility_state(xblock_info, VisibilityState.staff_only, path=[0, 1]) # Verify the state of the published sequential - public_sequential_child_info = self._get_child(xblock_info, 1) - public_unit_child_info = self._get_child(public_sequential_child_info, 0) - self.assertEqual(public_sequential_child_info['publish_state'], PublishState.ready) - self.assertEqual(public_unit_child_info['publish_state'], PublishState.ready) + self._verify_visibility_state(xblock_info, VisibilityState.ready, path=[1]) + self._verify_visibility_state(xblock_info, VisibilityState.ready, path=[1, 0]) + self._verify_visibility_state(xblock_info, VisibilityState.staff_only, path=[1, 1]) # Finally verify the state of the chapter - self.assertEqual(xblock_info['publish_state'], PublishState.ready) + self._verify_visibility_state(xblock_info, VisibilityState.ready) - def test_unpublished_changes_publishing_info(self): - chapter = self._create_child(self.course, 'chapter', "Test Chapter") - sequential = self._create_child(chapter, 'sequential', "Test Sequential") - unit = self._create_child(sequential, 'vertical', "Published Unit", publish_item=True) - self._set_display_name(unit.location, 'Updated Unit') - xblock_info = self._get_xblock_info(chapter.location) - self.assertEqual(xblock_info['publish_state'], PublishState.has_unpublished_content) - sequential_child_info = self._get_child(xblock_info, 0) - self.assertEqual(sequential_child_info['publish_state'], PublishState.has_unpublished_content) - unit_child_info = self._get_child(sequential_child_info, 0) - self.assertEqual(unit_child_info['publish_state'], PublishState.has_unpublished_content) - - def test_staff_only_publishing_info(self): + def test_staff_only(self): chapter = self._create_child(self.course, 'chapter', "Test Chapter") sequential = self._create_child(chapter, 'sequential', "Test Sequential") unit = self._create_child(sequential, 'vertical', "Published Unit") self._set_staff_only(unit.location, True) xblock_info = self._get_xblock_info(chapter.location) - self.assertEqual(xblock_info['publish_state'], PublishState.staff_only) - sequential_child_info = self._get_child(xblock_info, 0) - self.assertEqual(sequential_child_info['publish_state'], PublishState.staff_only) - unit_child_info = self._get_child(sequential_child_info, 0) - self.assertEqual(unit_child_info['publish_state'], PublishState.staff_only) + 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) + + 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") + self._create_child(sequential, 'vertical', "Published Unit", publish_item=True) + self._create_child(sequential, 'vertical', "Staff Only Unit", staff_only=True) + self._set_release_date(sequential.location, datetime.now(UTC) - timedelta(days=1)) + xblock_info = self._get_xblock_info(chapter.location) + self._verify_visibility_state(xblock_info, VisibilityState.needs_attention) + 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_unreleased_section_with_live_subsection(self): + chapter = self._create_child(self.course, 'chapter', "Test Chapter") + sequential = self._create_child(chapter, 'sequential', "Test Sequential") + self._create_child(sequential, 'vertical', "Published Unit", publish_item=True) + self._create_child(sequential, 'vertical', "Staff Only Unit", staff_only=True) + self._set_release_date(chapter.location, datetime.now(UTC) + timedelta(days=1)) + self._set_release_date(sequential.location, datetime.now(UTC) - timedelta(days=1)) + xblock_info = self._get_xblock_info(chapter.location) + self._verify_visibility_state(xblock_info, VisibilityState.needs_attention) + 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) diff --git a/cms/static/js/models/xblock_info.js b/cms/static/js/models/xblock_info.js index 02b07a4533..d05a2476c8 100644 --- a/cms/static/js/models/xblock_info.js +++ b/cms/static/js/models/xblock_info.js @@ -45,14 +45,15 @@ define(["backbone", "underscore", "js/utils/module"], function(Backbone, _, Modu */ "published_by": null, /** - * Represents the possible publish states for an xblock: - * is_live - the block and all of its children are live to students (except for staff only items) - * is_ready - the block and all of its children are ready to go live in the future - * unscheduled - the block and all of its children are unscheduled - * has_unpublished_content - the block or its children have unpublished content that is not staff only - * is_staff_only - all of the block's content is to be shown to staff only + * True if the xblock has changes. + * Note: this is not always provided as a performance optimization. */ - "publish_state": null, + "has_changes": null, + /** + * Represents the possible publish states for an xblock. See the documentation + * for XBlockVisibility to see a comprehensive enumeration of the states. + */ + "visibility_state": null, /** * True iff the release date of the xblock is in the past. */ 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 e247e3078f..e9155f029d 100644 --- a/cms/static/js/spec/views/pages/container_subviews_spec.js +++ b/cms/static/js/spec/views/pages/container_subviews_spec.js @@ -1,7 +1,9 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers", "js/views/feedback_prompt", "js/views/pages/container", "js/views/pages/container_subviews", - "js/models/xblock_info"], - function ($, _, str, create_sinon, edit_helpers, Prompt, ContainerPage, ContainerSubviews, XBlockInfo) { + "js/models/xblock_info", "js/views/utils/xblock_utils"], + function ($, _, str, create_sinon, edit_helpers, Prompt, ContainerPage, ContainerSubviews, + XBlockInfo, XBlockUtils) { + var VisibilityState = XBlockUtils.VisibilityState; describe("Container Subviews", function() { var model, containerPage, requests, createContainerPage, renderContainerPage, @@ -23,7 +25,9 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin id: 'locator-container', display_name: 'Test Container', category: 'vertical', - publish_state: 'unscheduled', + published: false, + has_changes: false, + visibility_state: 'unscheduled', edited_on: "Jul 02, 2014 at 14:20 UTC", edited_by: "joe", published_on: "Jul 01, 2014 at 12:45 UTC", published_by: "amako", currently_visible_to_students: false @@ -85,22 +89,23 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin it('updates when publish state changes', function () { renderContainerPage(this, mockContainerXBlockHtml); - fetch({publish_state: 'ready'}); + fetch({published: true}); expect(containerPage.$(viewPublishedCss)).not.toHaveClass(disabledCss); - fetch({publish_state: 'unscheduled'}); + fetch({published: false}); expect(containerPage.$(viewPublishedCss)).toHaveClass(disabledCss); }); it('updates when has_changes attribute changes', function () { renderContainerPage(this, mockContainerXBlockHtml); - fetch({publish_state: 'has-unpublished-changes'}); + fetch({has_changes: true}); expect(containerPage.$(previewCss)).not.toHaveClass(disabledCss); - fetch({publish_state: 'ready'}); + fetch({published: true, has_changes: false}); expect(containerPage.$(previewCss)).toHaveClass(disabledCss); - fetch({publish_state: 'unscheduled'}); + // If published is false, preview is always enabled. + fetch({published: false, has_changes: false}); expect(containerPage.$(previewCss)).not.toHaveClass(disabledCss); }); }); @@ -123,7 +128,7 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin // Helper function to do the discard operation, up until the server response. containerPage.render(); respondWithHtml(mockContainerXBlockHtml); - fetch({publish_state: 'has_unpublished_content'}); + fetch({published: true, has_changes: true, visibility_state: VisibilityState.needsAttention}); expect(containerPage.$(discardChangesButtonCss)).not.toHaveClass('is-disabled'); expect(containerPage.$(bitPublishingCss)).toHaveClass(hasWarningsClass); // Click discard changes @@ -151,26 +156,29 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin expect(containerPage.$(bitPublishingCss)).not.toHaveClass(readyClass); }; renderContainerPage(this, mockContainerXBlockHtml); - fetch({publishState: 'unscheduled'}); + fetch({published: false, has_changes: false}); + verifyPrivateState(); + + fetch({published: false, has_changes: true}); verifyPrivateState(); }); it('renders correctly with published content', function () { renderContainerPage(this, mockContainerXBlockHtml); - fetch({publish_state: 'ready'}); + fetch({published: true, has_changes: false, visibility_state: VisibilityState.ready}); expect(containerPage.$(headerCss).text()).toContain('Published'); expect(containerPage.$(publishButtonCss)).toHaveClass(disabledCss); expect(containerPage.$(discardChangesButtonCss)).toHaveClass(disabledCss); expect(containerPage.$(bitPublishingCss)).toHaveClass(readyClass); - fetch({publish_state: 'has_unpublished_content'}); + fetch({published: true, has_changes: true, visibility_state: VisibilityState.needsAttention}); expect(containerPage.$(headerCss).text()).toContain('Draft (Unpublished changes)'); expect(containerPage.$(publishButtonCss)).not.toHaveClass(disabledCss); expect(containerPage.$(discardChangesButtonCss)).not.toHaveClass(disabledCss); expect(containerPage.$(bitPublishingCss)).toHaveClass(hasWarningsClass); - fetch({publish_state: 'live'}); - expect(containerPage.$(headerCss).text()).toContain('Published'); + fetch({published: true, has_changes: false, visibility_state: VisibilityState.live}); + expect(containerPage.$(headerCss).text()).toContain('Published and Live'); expect(containerPage.$(publishButtonCss)).toHaveClass(disabledCss); expect(containerPage.$(discardChangesButtonCss)).toHaveClass(disabledCss); expect(containerPage.$(bitPublishingCss)).toHaveClass(liveClass); @@ -179,7 +187,9 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin it('can publish private content', function () { var notificationSpy = edit_helpers.createNotificationSpy(); renderContainerPage(this, mockContainerXBlockHtml); + expect(containerPage.$(bitPublishingCss)).not.toHaveClass(hasWarningsClass); expect(containerPage.$(bitPublishingCss)).not.toHaveClass(readyClass); + expect(containerPage.$(bitPublishingCss)).not.toHaveClass(liveClass); // Click publish containerPage.$(publishButtonCss).click(); @@ -195,15 +205,17 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin create_sinon.expectJsonRequest(requests, "GET", "/xblock/locator-container"); // Response to fetch - respondWithJson(createXBlockInfo({publish_state: 'ready'})); + respondWithJson(createXBlockInfo({ + published: true, has_changes: false, visibility_state: VisibilityState.ready + })); // Verify updates displayed expect(containerPage.$(bitPublishingCss)).toHaveClass(readyClass); - // Verify that the "publish_state" value has been updated - expect(containerPage.model.get("publish_state")).toBe('ready'); + // Verify that the "published" value has been cleared out of the model. + expect(containerPage.model.get("publish")).toBeNull(); }); - it('can does not fetch if publish fails', function () { + it('does not refresh if publish fails', function () { renderContainerPage(this, mockContainerXBlockHtml); expect(containerPage.$(bitPublishingCss)).not.toHaveClass(readyClass); @@ -218,8 +230,8 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin // Verify still in draft state. expect(containerPage.$(bitPublishingCss)).not.toHaveClass(readyClass); - // Verify that the "publish_state" value has been updated - expect(containerPage.model.get("publish_state")).toBe('unscheduled'); + // Verify that the "published" value has been cleared out of the model. + expect(containerPage.model.get("publish")).toBeNull(); }); it('can discard changes', function () { @@ -260,7 +272,8 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin }); it('does not discard changes on cancel', function () { - renderContainerPage(this, mockContainerXBlockHtml, { publish_state: 'has_unpublished_content' }); + renderContainerPage(this, mockContainerXBlockHtml); + fetch({published: true, has_changes: true}); var numRequests = requests.length; // Click discard changes @@ -284,10 +297,7 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin it('renders the last saved date and user when there are changes', function () { renderContainerPage(this, mockContainerXBlockHtml); - fetch({ - publish_state: 'has_unpublished_content', - edited_on: "Jul 02, 2014 at 14:20 UTC", edited_by: "joe" - }); + fetch({has_changes: true, edited_on: "Jul 02, 2014 at 14:20 UTC", edited_by: "joe"}); expect(containerPage.$(lastDraftCss).text()). toContain("Draft saved on Jul 02, 2014 at 14:20 UTC by joe"); }); @@ -296,7 +306,7 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin it('renders correctly when unreleased', function () { renderContainerPage(this, mockContainerXBlockHtml); fetch({ - publish_state: 'ready', released_to_students: false, + visibility_state: VisibilityState.ready, released_to_students: false, release_date: "Jul 02, 2014 at 14:20 UTC", release_date_from: 'Section "Week 1"' }); expect(containerPage.$(releaseDateTitleCss).text()).toContain("Scheduled:"); @@ -307,7 +317,7 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin it('renders correctly when released', function () { renderContainerPage(this, mockContainerXBlockHtml); fetch({ - publish_state: 'live', released_to_students: true, + visibility_state: VisibilityState.live, released_to_students: true, release_date: "Jul 02, 2014 at 14:20 UTC", release_date_from: 'Section "Week 1"' }); expect(containerPage.$(releaseDateTitleCss).text()).toContain("Released:"); @@ -318,7 +328,7 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin it('renders correctly when the release date is not set', function () { renderContainerPage(this, mockContainerXBlockHtml); fetch({ - publish_state: 'unscheduled', "released_to_students": false, + visibility_state: VisibilityState.unscheduled, "released_to_students": false, release_date: null, release_date_from: null }); expect(containerPage.$(releaseDateTitleCss).text()).toContain("Release:"); @@ -328,7 +338,7 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin it('renders correctly when the unit is not published', function () { renderContainerPage(this, mockContainerXBlockHtml); fetch({ - publish_state: 'has_unpublished_content', released_to_students: true, + visibility_state: VisibilityState.needsAttention, released_to_students: true, release_date: "Jul 02, 2014 at 14:20 UTC", release_date_from: 'Section "Week 1"' }); containerPage.xblockPublisher.render(); @@ -362,7 +372,8 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin }); create_sinon.expectJsonRequest(requests, 'GET', '/xblock/locator-container'); create_sinon.respondWithJson(requests, createXBlockInfo({ - publish_state: isStaffOnly ? 'staff_only' : 'unscheduled' + published: containerPage.model.get('published'), + visibility_state: isStaffOnly ? VisibilityState.staffOnly : VisibilityState.live })); }; @@ -385,14 +396,15 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin it("can be set to staff only", function() { renderContainerPage(this, mockContainerXBlockHtml); - containerPage.$('.action-staff-lock').click(); requestStaffOnly(true); verifyStaffOnly(true); }); it("can remove staff only setting", function() { promptSpy = edit_helpers.createPromptSpy(); - renderContainerPage(this, mockContainerXBlockHtml, { publish_state: 'staff_only' }); + renderContainerPage(this, mockContainerXBlockHtml, { + visibility_state: VisibilityState.staffOnly + }); requestStaffOnly(false); verifyStaffOnly(false); expect(containerPage.$(bitPublishingCss)).not.toHaveClass(readyClass); @@ -401,7 +413,9 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin it("does not refresh if removing staff only is canceled", function() { var requestCount; promptSpy = edit_helpers.createPromptSpy(); - renderContainerPage(this, mockContainerXBlockHtml, { publish_state: 'staff_only' }); + renderContainerPage(this, mockContainerXBlockHtml, { + visibility_state: VisibilityState.staffOnly + }); requestCount = requests.length; containerPage.$('.action-staff-lock').click(); edit_helpers.confirmPrompt(promptSpy, true); // Click 'No' to cancel 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 15ab173e9f..187b55a1ea 100644 --- a/cms/static/js/spec/views/pages/course_outline_spec.js +++ b/cms/static/js/spec/views/pages/course_outline_spec.js @@ -118,7 +118,7 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" category: 'vertical', studio_url: '/container/mock-unit', is_container: true, - publish_state: 'unscheduled', + visibility_state: 'unscheduled', edited_on: 'Jul 02, 2014 at 20:56 UTC', edited_by: 'MockUser' }]) @@ -432,9 +432,5 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" expect(unitAnchor.attr('href')).toBe('/container/mock-unit'); }); }); - - describe("Publishing State", function() { - // TODO: implement this!!!! - }); }); }); diff --git a/cms/static/js/spec/views/unit_outline_spec.js b/cms/static/js/spec/views/unit_outline_spec.js index 63aab30c93..117964751a 100644 --- a/cms/static/js/spec/views/unit_outline_spec.js +++ b/cms/static/js/spec/views/unit_outline_spec.js @@ -25,14 +25,14 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" category: 'vertical', display_name: displayName, studio_url: '/container/mock-unit', - publish_state: 'unscheduled', + visibility_state: 'unscheduled', ancestor_info: { ancestors: [{ id: 'mock-subsection', category: 'sequential', display_name: 'Mock Subsection', studio_url: '/course/mock-course?show=mock-subsection', - publish_state: 'unscheduled', + visibility_state: 'unscheduled', child_info: { category: 'vertical', display_name: 'Unit', @@ -41,13 +41,13 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" category: 'vertical', display_name: displayName, studio_url: '/container/mock-unit', - publish_state: 'unscheduled' + visibility_state: 'unscheduled' }, { id: 'mock-unit-2', category: 'vertical', display_name: 'Mock Unit 2', studio_url: '/container/mock-unit-2', - publish_state: 'unscheduled' + visibility_state: 'unscheduled' }] } }, { @@ -55,13 +55,13 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers" category: 'chapter', display_name: 'Section', studio_url: '/course/slashes:mock-course?show=mock-section', - publish_state: 'unscheduled' + visibility_state: 'unscheduled' }, { id: 'mock-course', category: 'course', display_name: 'Mock Course', studio_url: '/course/mock-course', - publish_state: 'unscheduled' + visibility_state: 'unscheduled' }] }, metadata: { diff --git a/cms/static/js/views/baseview.js b/cms/static/js/views/baseview.js index b89591c987..32ffd0171b 100644 --- a/cms/static/js/views/baseview.js +++ b/cms/static/js/views/baseview.js @@ -17,6 +17,9 @@ define(["jquery", "underscore", "backbone", "gettext", "js/utils/handle_iframe_b }, options: { + // UX is moving towards using 'is-collapsed' in preference over 'collapsed', + // but use the old scheme as the default so that existing code doesn't need + // to be rewritten. collapsedClass: 'collapsed' }, diff --git a/cms/static/js/views/pages/container_subviews.js b/cms/static/js/views/pages/container_subviews.js index 37bf7a2513..3c752a2a8c 100644 --- a/cms/static/js/views/pages/container_subviews.js +++ b/cms/static/js/views/pages/container_subviews.js @@ -1,10 +1,11 @@ /** * Subviews (usually small side panels) for XBlockContainerPage. */ -define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/view_utils"], - function ($, _, gettext, BaseView, ViewUtils) { - - var disabledCss = "is-disabled"; +define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/view_utils", + "js/views/utils/xblock_utils"], + function ($, _, gettext, BaseView, ViewUtils, XBlockViewUtils) { + var VisibilityState = XBlockViewUtils.VisibilityState, + disabledCss = "is-disabled"; /** * A view that refreshes the view when certain values in the XBlockInfo have changed @@ -53,20 +54,19 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/ */ var PreviewActionController = ContainerStateListenerView.extend({ shouldRefresh: function(model) { - return ViewUtils.hasChangedAttributes(model, ['edited_on', 'published_on', 'publish_state']); + return ViewUtils.hasChangedAttributes(model, ['has_changes', 'published']); }, render: function() { var previewAction = this.$el.find('.button-preview'), - viewLiveAction = this.$el.find('.button-view'), - publishState = this.model.get('publish_state'); - if (publishState !== 'unscheduled') { + viewLiveAction = this.$el.find('.button-view'); + if (this.model.get('published')) { viewLiveAction.removeClass(disabledCss); } else { viewLiveAction.addClass(disabledCss); } - if (publishState !== 'live' && publishState !== 'ready') { + if (this.model.get('has_changes') || !this.model.get('published')) { previewAction.removeClass(disabledCss); } else { @@ -99,14 +99,18 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/ }, onSync: function(model) { - if (ViewUtils.hasChangedAttributes(model, [ 'edited_on', 'published_on', 'publish_state' ])) { + if (ViewUtils.hasChangedAttributes(model, [ + 'has_changes', 'published', 'edited_on', 'edited_by', 'visibility_state' + ])) { this.render(); } }, render: function () { this.$el.html(this.template({ - publishState: this.model.get('publish_state'), + visibilityState: this.model.get('visibility_state'), + visibilityClass: XBlockViewUtils.getXBlockVisibilityClass(this.model.get('visibility_state')), + hasChanges: this.model.get('has_changes'), editedOn: this.model.get('edited_on'), editedBy: this.model.get('edited_by'), published: this.model.get('published'), @@ -162,7 +166,7 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/ if (e && e.preventDefault) { e.preventDefault(); } - enableStaffLock = xblockInfo.get('publish_state') !== 'staff_only'; + enableStaffLock = xblockInfo.get('visibility_state') !== VisibilityState.staffOnly; revertCheckBox = function() { self.checkStaffLock(!enableStaffLock); @@ -221,7 +225,7 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/ }, onSync: function(model) { - if (ViewUtils.hasChangedAttributes(model, ['published_on'])) { + if (ViewUtils.hasChangedAttributes(model, ['published', 'published_on', 'published_by'])) { this.render(); } }, diff --git a/cms/static/js/views/unit_outline.js b/cms/static/js/views/unit_outline.js index 867e6843f6..cea6b91b68 100644 --- a/cms/static/js/views/unit_outline.js +++ b/cms/static/js/views/unit_outline.js @@ -10,7 +10,6 @@ define(['js/views/xblock_outline'], // takes XBlockInfo as a model templateName: 'unit-outline', - className: 'group-configurations-list', render: function() { XBlockOutlineView.prototype.render.call(this); diff --git a/cms/static/js/views/utils/xblock_utils.js b/cms/static/js/views/utils/xblock_utils.js index bb16af32a6..5115467f10 100644 --- a/cms/static/js/views/utils/xblock_utils.js +++ b/cms/static/js/views/utils/xblock_utils.js @@ -3,7 +3,36 @@ */ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils", "js/utils/module"], function($, _, gettext, ViewUtils, ModuleUtils) { - var addXBlock, deleteXBlock, createUpdateRequestData, updateXBlockField; + var addXBlock, deleteXBlock, createUpdateRequestData, updateXBlockField, VisibilityState, + getXBlockVisibilityClass; + + /** + * Represents the possible visibility states for an xblock: + * + * live - the block and all of its descendants are live to students (excluding staff only) + * Note: Live means both published and released. + * + * ready - the block is ready to go live and all of its descendants are live or ready (excluding staff only) + * Note: content is ready when it is published and scheduled with a release date in the future. + * + * unscheduled - the block and all of its descendants have no release date (excluding staff only) + * Note: it is valid for items to be published with no release date in which case they are unscheduled. + * + * needsAttention - the block or its descendants need attention + * i.e. there is some content that is not fully live, ready, unscheduled or staff only. + * For example: one subsection has draft content, or there's both unreleased and released content + * in one section. + * + * staffOnly - all of the block's content is to be shown to staff only + * Note: staff only items do not affect their parent's state. + */ + VisibilityState = { + live: 'live', + ready: 'ready', + unscheduled: 'unscheduled', + needsAttention: 'needs_attention', + staffOnly: 'staff_only' + }; /** * Adds an xblock based upon the data attributes of the specified add button. A promise @@ -83,9 +112,30 @@ define(["jquery", "underscore", "gettext", "js/views/utils/view_utils", "js/util }); }; + /** + * Returns the CSS class to represent the specified xblock visibility state. + */ + getXBlockVisibilityClass = function(visibilityState) { + if (visibilityState === VisibilityState.staffOnly) { + return 'is-staff-only'; + } + if (visibilityState === VisibilityState.live) { + return 'is-live'; + } + if (visibilityState === VisibilityState.ready) { + return 'is-ready'; + } + if (visibilityState === VisibilityState.needsAttention) { + return 'has-warnings'; + } + return ''; + }; + return { + 'VisibilityState': VisibilityState, 'addXBlock': addXBlock, 'deleteXBlock': deleteXBlock, - 'updateXBlockField': updateXBlockField + 'updateXBlockField': updateXBlockField, + 'getXBlockVisibilityClass': getXBlockVisibilityClass }; }); diff --git a/cms/static/js/views/xblock_outline.js b/cms/static/js/views/xblock_outline.js index e3bbce1ed7..c4ee845c92 100644 --- a/cms/static/js/views/xblock_outline.js +++ b/cms/static/js/views/xblock_outline.js @@ -68,6 +68,7 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/ } html = this.template({ xblockInfo: xblockInfo, + visibilityClass: XBlockViewUtils.getXBlockVisibilityClass(xblockInfo.get('visibility_state')), parentInfo: this.parentInfo, xblockType: xblockType, parentType: parentType, @@ -200,7 +201,11 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/ } else { locatorElement = this.$('.outline-item[data-locator="' + locatorToShow + '"]'); } - ViewUtils.setScrollOffset(locatorElement, scrollOffset); + if (locatorElement.length > 0) { + ViewUtils.setScrollOffset(locatorElement, scrollOffset); + } else { + console.error("Failed to show item with locator " + locatorToShow + ""); + } if (editDisplayName) { locatorElement.find('> div[class$="header"] .xblock-field-value-edit').click(); } diff --git a/cms/templates/course_outline.html b/cms/templates/course_outline.html index 29753df804..5be3164336 100644 --- a/cms/templates/course_outline.html +++ b/cms/templates/course_outline.html @@ -72,7 +72,7 @@ from contentstore.utils import reverse_usage_url <% course_locator = context_course.location %> -
+
diff --git a/cms/templates/js/course-outline.underscore b/cms/templates/js/course-outline.underscore index 379029d71a..2bcf4a36b8 100644 --- a/cms/templates/js/course-outline.underscore +++ b/cms/templates/js/course-outline.underscore @@ -1,18 +1,7 @@ <% var category = xblockInfo.get('category'); var releasedToStudents = xblockInfo.get('released_to_students'); -var publishState = xblockInfo.get('publish_state'); - -var publishClass = ''; -if (publishState === 'staff_only') { - publishClass = 'is-staff-only'; -} else if (publishState === 'live') { - publishClass = 'is-live'; -} else if (publishState === 'ready') { - publishClass = 'is-ready'; -} else if (publishState === 'has_unpublished_content') { - publishClass = 'has-warnings'; -} +var visibilityState = xblockInfo.get('visibility_state'); var listType = 'list-unknown'; if (xblockType === 'course') { @@ -25,10 +14,10 @@ if (xblockType === 'course') { var statusMessage = null; var statusType = null; -if (publishState === 'staff_only') { +if (visibilityState === 'staff_only') { statusType = 'staff-only'; statusMessage = 'Contains staff only content'; -} else if (publishState === 'has_unpublished_content') { +} else if (visibilityState === 'needs_attention') { if (category === 'vertical') { statusType = 'warning'; if (releasedToStudents) { @@ -49,7 +38,7 @@ if (statusType === 'warning') { } %> <% if (parentInfo) { %> -
  • diff --git a/cms/templates/js/publish-xblock.underscore b/cms/templates/js/publish-xblock.underscore index 76d5238c68..09a5a51653 100644 --- a/cms/templates/js/publish-xblock.underscore +++ b/cms/templates/js/publish-xblock.underscore @@ -1,45 +1,32 @@ <% -var publishClass = ''; -if (publishState === 'staff_only') { - publishClass = 'is-staff-only'; -} else if (publishState === 'live') { - publishClass = 'is-live'; -} else if (publishState === 'ready') { - publishClass = 'is-ready'; -} else if (publishState === 'has_unpublished_content') { - publishClass = 'has-warnings is-draft'; -} - var title = gettext("Draft (Never published)"); -if (publishState === 'staff_only') { +if (visibilityState === 'staff_only') { title = gettext("Unpublished (Staff only)"); -} else if (publishState === 'live') { +} else if (visibilityState === 'live') { title = gettext("Published and Live"); -} else if (publishState === 'ready') { +} else if (visibilityState === 'ready') { title = gettext("Published"); -} else if (publishState === 'has_unpublished_content') { +} else if (visibilityState === 'needs_attention') { title = gettext("Draft (Unpublished changes)"); } var releaseLabel = gettext("Release:"); -if (publishState === 'live') { +if (visibilityState === 'live') { releaseLabel = gettext("Released:"); -} else if (publishState === 'ready') { +} else if (visibilityState === 'ready') { releaseLabel = gettext("Scheduled:"); } -var canPublish = publishState !== 'ready' && publishState !== 'live'; -var canDiscardChanges = publishState === 'has_unpublished_content'; -var visibleToStaffOnly = publishState === 'staff_only'; +var visibleToStaffOnly = visibilityState === 'staff_only'; %> -
    +

    <%= gettext("Publishing Status") %> <%= title %>

    - <% if (publishState === 'has_unpublished_content' && editedOn && editedBy) { + <% if (hasChanges && editedOn && editedBy) { var message = gettext("Draft saved on %(last_saved_date)s by %(edit_username)s") %> <%= interpolate(message, { last_saved_date: '' + editedOn + '', @@ -91,12 +78,12 @@ var visibleToStaffOnly = publishState === 'staff_only';

    • - <%= gettext("Publish") %>
    • - <%= gettext("Discard Changes") %>
      • diff --git a/cms/templates/js/unit-outline.underscore b/cms/templates/js/unit-outline.underscore index ae30d8d565..10eeb4de9f 100644 --- a/cms/templates/js/unit-outline.underscore +++ b/cms/templates/js/unit-outline.underscore @@ -1,16 +1,4 @@ <% -var publishState = xblockInfo.get('publish_state'); -var publishClass = ''; -if (publishState === 'staff_only') { - publishClass = 'is-staff-only'; -} else if (publishState === 'live') { - publishClass = 'is-live'; -} else if (publishState === 'ready') { - publishClass = 'is-ready'; -} else if (publishState === 'has_unpublished_content') { - publishClass = 'has_warnings'; -} - var listType = 'list-for-' + xblockType; if (xblockType === 'course') { listType = 'list-sections'; @@ -21,7 +9,7 @@ if (xblockType === 'course') { } %> <% if (parentInfo) { %> -

    • diff --git a/cms/templates/widgets/units.html b/cms/templates/widgets/units.html index 04777a21d3..b874034a06 100644 --- a/cms/templates/widgets/units.html +++ b/cms/templates/widgets/units.html @@ -1,5 +1,4 @@ <%! from django.utils.translation import ugettext as _ %> -<%! from contentstore.utils import compute_publish_state %> <%! from contentstore.views.helpers import xblock_studio_url %>