Apply code review comments and fix tests

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:

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.

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

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):

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

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.
              */

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!!!!
-            });
         });
     });

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: {

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'
             },
 

cms/static/js/views/pages/container_subviews.js

 /**
  * 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();
                 }
             },

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);

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
         };
     });

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 + '"]');
                     }
+                    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();
                     }

cms/templates/course_outline.html

@@ -72,7 +72,7 @@ from contentstore.utils import reverse_usage_url
                 <%
                 course_locator = context_course.location
                 %>
-                <article class="outline" data-locator="${course_locator}" data-course-key="${course_locator.course_key}">
+                <article class="outline outline-course" data-locator="${course_locator}" data-course-key="${course_locator.course_key}">
                 </article>
             </div>
             <div class="ui-loading">

cms/templates/js/course-outline.underscore

 <%
 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) { %>
-<li class="outline-item outline-<%= xblockType %> <%= publishClass %> is-draggable <%= includesChildren ? 'is-collapsible' : '' %> <%= isCollapsed ? 'is-collapsed' : '' %>"
+<li class="outline-item outline-<%= xblockType %> <%= visibilityClass %> is-draggable <%= includesChildren ? 'is-collapsible' : '' %> <%= isCollapsed ? 'is-collapsed' : '' %>"
     data-parent="<%= parentInfo.get('id') %>" data-locator="<%= xblockInfo.get('id') %>">
 
     <div class="<%= xblockType %>-header">

cms/templates/js/publish-xblock.underscore

 <%
-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';
 %>
-<div class="bit-publishing <%= publishClass %>">
+<div class="bit-publishing <%= visibilityClass %>">
     <h3 class="bar-mod-title pub-status"><span class="sr"><%= gettext("Publishing Status") %></span>
         <%= title %>
     </h3>
 
     <div class="wrapper-last-draft bar-mod-content">
         <p class="copy meta">
-            <% 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: '<span class="date">' + editedOn + '</span>',
@@ -91,12 +78,12 @@ var visibleToStaffOnly = publishState === 'staff_only';
     <div class="wrapper-pub-actions bar-mod-actions">
         <ul class="action-list">
             <li class="action-item">
-                <a class="action-publish action-primary <% if (!canPublish) { %>is-disabled<% } %>"
+                <a class="action-publish action-primary <% if (published && !hasChanges) { %>is-disabled<% } %>"
                    href=""><%= gettext("Publish") %>
                 </a>
             </li>
             <li class="action-item">
-                <a class="action-discard action-secondary <% if (!canDiscardChanges) { %>is-disabled<% } %>"
+                <a class="action-discard action-secondary <% if (!published || !hasChanges) { %>is-disabled<% } %>"
                    href=""><%= gettext("Discard Changes") %>
                 </a>
             </li>

cms/templates/js/unit-outline.underscore

 <%
-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) { %>
-    <li class="outline-item outline-<%= xblockType %> <%= publishClass %>"
+    <li class="outline-item outline-<%= xblockType %> <%= visibilityClass %>"
         data-parent="<%= parentInfo.get('id') %>" data-locator="<%= xblockInfo.get('id') %>">
         <div class="<%= xblockType %>-header">
             <h3 class="<%= xblockType %>-header-details">

cms/templates/widgets/units.html

 <%! from django.utils.translation import ugettext as _ %>
-<%! from contentstore.utils import compute_publish_state %>
 <%! from contentstore.views.helpers import xblock_studio_url %>
 
 <!--
@@ -18,7 +17,7 @@ This def will enumerate through a passed in subsection and list all of the units
     <%include file="_ui-dnd-indicator-before.html" />
 
     <%
-      unit_state = compute_publish_state(unit)
+      unit_state = 'draft'   # Note: this is a hack since this HTML file is being removed in bulk-publishing
       if unit.location == selected:
         selected_class = 'editing'
       else:

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -90,10 +90,10 @@ class ModuleStoreEnum(object):
         test = -3
 
 
-class LegacyPublishState(object):
+class PublishState(object):
     """
     The legacy publish state for a given xblock-- either 'draft', 'private', or 'public'. These states
-    are no longer used in Studio directly, but are still referenced in a few places.
+    are no longer used in Studio, but they are still referenced in a few places in LMS.
     """
     draft = 'draft'
     private = 'private'
@@ -301,10 +301,10 @@ class ModuleStoreRead(object):
         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
+            PublishState.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
+            PublishState.public - content is locked and deployed to LMS
+            PublishState.private - content is editable and not deployed to LMS
         """
         pass
 
@@ -522,7 +522,7 @@ class ModuleStoreReadBase(ModuleStoreRead):
         """
         Returns PublishState.public since this is a read-only store.
         """
-        return LegacyPublishState.public
+        return PublishState.public
 
     def heartbeat(self):
         """

common/lib/xmodule/xmodule/modulestore/mixed.py

@@ -439,10 +439,10 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
         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
+            PublishState.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
+            PublishState.public - content is locked and deployed to LMS
+            PublishState.private - content is editable and not deployed to LMS
         """
         course_id = xblock.scope_ids.usage_id.course_key
         store = self._get_modulestore_for_courseid(course_id)

common/lib/xmodule/xmodule/modulestore/mongo/draft.py

@@ -11,7 +11,7 @@ import logging
 
 from opaque_keys.edx.locations import Location
 from xmodule.exceptions import InvalidVersionError
-from xmodule.modulestore import LegacyPublishState, ModuleStoreEnum
+from xmodule.modulestore import PublishState, ModuleStoreEnum
 from xmodule.modulestore.exceptions import (
     ItemNotFoundError, DuplicateItemError, InvalidBranchSetting, DuplicateCourseError
 )
@@ -613,7 +613,7 @@ class DraftModuleStore(MongoModuleStore):
             return False
 
         # don't check children if this block has changes (is not public)
-        if self.compute_publish_state(item) != LegacyPublishState.public:
+        if self.compute_publish_state(item) != PublishState.public:
             return True
         # if this block doesn't have changes, then check its children
         elif item.has_children:
@@ -792,10 +792,10 @@ class DraftModuleStore(MongoModuleStore):
         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
+            PublishState.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
+            PublishState.public - content is locked and deployed to LMS
+            PublishState.private - content is editable and not deployed to LMS
         """
         if getattr(xblock, 'is_draft', False):
             published_xblock_location = as_published(xblock.location)
@@ -803,11 +803,11 @@ class DraftModuleStore(MongoModuleStore):
                 {'_id': published_xblock_location.to_deprecated_son()}
             )
             if published_item is None:
-                return LegacyPublishState.private
+                return PublishState.private
             else:
-                return LegacyPublishState.draft
+                return PublishState.draft
         else:
-            return LegacyPublishState.public
+            return PublishState.public
 
     def _verify_branch_setting(self, expected_branch_setting):
         """

common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py

@@ -4,7 +4,7 @@ Module for the dual-branch fall-back Draft->Published Versioning ModuleStore
 
 from ..exceptions import ItemNotFoundError
 from split import SplitMongoModuleStore, EXCLUDE_ALL
-from xmodule.modulestore import ModuleStoreEnum, LegacyPublishState
+from xmodule.modulestore import ModuleStoreEnum, PublishState
 from xmodule.modulestore.exceptions import InsufficientSpecificationError
 from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished, DIRECT_ONLY_CATEGORIES, UnsupportedRevisionError
 
@@ -251,9 +251,9 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
         Returns whether this xblock is draft, public, or private.
 
         Returns:
-            LegacyPublishState.draft - published exists and is different from draft
-            LegacyPublishState.public - published exists and is the same as draft
-            LegacyPublishState.private - no published version exists
+            PublishState.draft - published exists and is different from draft
+            PublishState.public - published exists and is the same as draft
+            PublishState.private - no published version exists
         """
         # TODO figure out what to say if xblock is not from the HEAD of its branch
         def get_head(branch):
@@ -272,13 +272,13 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
 
         if not published_head:
             # published version does not exist
-            return LegacyPublishState.private
+            return PublishState.private
         elif get_version(draft_head) == get_version(published_head):
             # published and draft versions are equal
-            return LegacyPublishState.public
+            return PublishState.public
         else:
             # published and draft versions differ
-            return LegacyPublishState.draft
+            return PublishState.draft
 
     def convert_to_draft(self, location, user_id):
         """

common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py

@@ -9,7 +9,7 @@ from pytz import UTC
 
 from xmodule.tests import DATA_DIR
 from opaque_keys.edx.locations import Location
-from xmodule.modulestore import ModuleStoreEnum, LegacyPublishState
+from xmodule.modulestore import ModuleStoreEnum, PublishState
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.exceptions import InvalidVersionError
 
@@ -991,22 +991,22 @@ class TestMixedModuleStore(unittest.TestCase):
         item_location = item.location.version_agnostic()
         mongo_store = self.store._get_modulestore_for_courseid(self._course_key_from_string(self.MONGO_COURSEID))
         with check_mongo_calls(mongo_store, max_find, max_send):
-            self.assertEquals(self.store.compute_publish_state(item), LegacyPublishState.private)
+            self.assertEquals(self.store.compute_publish_state(item), PublishState.private)
 
         # Private -> Public
         self.store.publish(item_location, self.user_id)
         item = self.store.get_item(item_location)
-        self.assertEquals(self.store.compute_publish_state(item), LegacyPublishState.public)
+        self.assertEquals(self.store.compute_publish_state(item), PublishState.public)
 
         # Public -> Private
         self.store.unpublish(item_location, self.user_id)
         item = self.store.get_item(item_location)
-        self.assertEquals(self.store.compute_publish_state(item), LegacyPublishState.private)
+        self.assertEquals(self.store.compute_publish_state(item), PublishState.private)
 
         # Private -> Public
         self.store.publish(item_location, self.user_id)
         item = self.store.get_item(item_location)
-        self.assertEquals(self.store.compute_publish_state(item), LegacyPublishState.public)
+        self.assertEquals(self.store.compute_publish_state(item), PublishState.public)
 
         # Public -> Draft with NO changes
         # Note: This is where Split and Mongo differ
@@ -1014,14 +1014,14 @@ class TestMixedModuleStore(unittest.TestCase):
         item = self.store.get_item(item_location)
         self.assertEquals(
             self.store.compute_publish_state(item),
-            LegacyPublishState.draft if default_ms == 'draft' else LegacyPublishState.public
+            PublishState.draft if default_ms == 'draft' else PublishState.public
         )
 
         # Draft WITH changes
         item.display_name = 'new name'
         item = self.store.update_item(item, self.user_id)
         self.assertTrue(self.store.has_changes(item.location))
-        self.assertEquals(self.store.compute_publish_state(item), LegacyPublishState.draft)
+        self.assertEquals(self.store.compute_publish_state(item), PublishState.draft)
 
     @ddt.data('draft', 'split')
     def test_auto_publish(self, default_ms):

common/test/acceptance/tests/base_studio_test.py

 from ..pages.studio.auto_auth import AutoAuthPage
 from ..fixtures.course import CourseFixture
-from acceptance.tests.helpers import UniqueCourseTest
+from .helpers import UniqueCourseTest
+
 
 
 class StudioCourseTest(UniqueCourseTest):

common/test/acceptance/tests/test_studio_general.py

@@ -25,7 +25,7 @@ from ..pages.studio.signup import SignupPage
 from ..pages.studio.textbooks import TextbooksPage
 from ..fixtures.course import XBlockFixtureDesc
 
-from acceptance.tests.base_studio_test import StudioCourseTest
+from .base_studio_test import StudioCourseTest
 
 
 @attr('shard_1')

common/test/acceptance/tests/test_studio_outline.py

@@ -8,7 +8,7 @@ from ..pages.studio.overview import CourseOutlinePage, ContainerPage, ExpandColl
 from ..pages.lms.courseware import CoursewarePage
 from ..fixtures.course import XBlockFixtureDesc
 
-from acceptance.tests.base_studio_test import StudioCourseTest
+from .base_studio_test import StudioCourseTest
 
 
 class CourseOutlineTest(StudioCourseTest):

common/test/acceptance/tests/test_studio_split_test.py

@@ -20,7 +20,7 @@ from ..pages.studio.utils import add_advanced_component
 from ..pages.studio.unit import UnitPage
 from ..pages.xblock.utils import wait_for_xblock_initialization
 
-from acceptance.tests.base_studio_test import StudioCourseTest
+from .base_studio_test import StudioCourseTest
 
 from test_studio_container import ContainerBase