Only show drag handles in draft mode

cms/djangoapps/contentstore/views/item.py

@@ -35,6 +35,7 @@ from ..utils import get_modulestore
 from .access import has_course_access
 from .helpers import _xmodule_recurse
 from contentstore.utils import compute_publish_state, PublishState
+from xmodule.modulestore.draft import DIRECT_ONLY_CATEGORIES
 from contentstore.views.preview import get_preview_fragment
 from edxmako.shortcuts import render_to_string
 from models.settings.course_grading import CourseGradingModel
@@ -193,6 +194,7 @@ def xblock_view_handler(request, package_id, view_name, tag=None, branch=None, v
     if 'application/json' in accept_header:
         store = get_modulestore(old_location)
         component = store.get_item(old_location)
+        is_read_only = _xblock_is_read_only(component)
 
         # wrap the generated fragment in the xmodule_editor div so that the javascript
         # can bind to it correctly
@@ -212,12 +214,18 @@ def xblock_view_handler(request, package_id, view_name, tag=None, branch=None, v
             store.update_item(component, None)
 
         elif view_name == 'student_view' and component.has_children:
+            context = {
+                'runtime_type': 'studio',
+                'container_view': False,
+                'read_only': is_read_only,
+                'root_xblock': component,
+            }
             # For non-leaf xblocks on the unit page, show the special rendering
             # which links to the new container page.
             html = render_to_string('container_xblock_component.html', {
+                'xblock_context': context,
                 'xblock': component,
                 'locator': locator,
-                'reordering_enabled': True,
             })
             return JsonResponse({
                 'html': html,
@@ -225,8 +233,6 @@ def xblock_view_handler(request, package_id, view_name, tag=None, branch=None, v
             })
         elif view_name in ('student_view', 'container_preview'):
             is_container_view = (view_name == 'container_preview')
-            component_publish_state = compute_publish_state(component)
-            is_read_only_view = component_publish_state == PublishState.public
 
             # Only show the new style HTML for the container view, i.e. for non-verticals
             # Note: this special case logic can be removed once the unit page is replaced
@@ -234,7 +240,7 @@ def xblock_view_handler(request, package_id, view_name, tag=None, branch=None, v
             context = {
                 'runtime_type': 'studio',
                 'container_view': is_container_view,
-                'read_only': is_read_only_view,
+                'read_only': is_read_only,
                 'root_xblock': component,
             }
 
@@ -244,6 +250,7 @@ def xblock_view_handler(request, package_id, view_name, tag=None, branch=None, v
             # into the preview fragment, so we don't want to add another header here.
             if not is_container_view:
                 fragment.content = render_to_string('component.html', {
+                    'xblock_context': context,
                     'preview': fragment.content,
                     'label': component.display_name or component.scope_ids.block_type,
                 })
@@ -263,6 +270,17 @@ def xblock_view_handler(request, package_id, view_name, tag=None, branch=None, v
         return HttpResponse(status=406)
 
 
+def _xblock_is_read_only(xblock):
+    """
+    Returns true if the specified xblock is read-only, meaning that it cannot be edited.
+    """
+    # We allow direct editing of xblocks in DIRECT_ONLY_CATEGORIES (for example, static pages).
+    if xblock.category in DIRECT_ONLY_CATEGORIES:
+        return False
+    component_publish_state = compute_publish_state(xblock)
+    return component_publish_state == PublishState.public
+
+
 def _save_item(request, usage_loc, item_location, data=None, children=None, metadata=None, nullout=None,
                grader_type=None, publish=None):
     """

cms/djangoapps/contentstore/views/tests/test_container.py

@@ -2,10 +2,12 @@
 Unit tests for the container view.
 """
 
+import json
+
 from contentstore.tests.utils import CourseTestCase
 from contentstore.utils import compute_publish_state, PublishState
 from contentstore.views.helpers import xblock_studio_url
-from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.django import loc_mapper, modulestore
 from xmodule.modulestore.tests.factories import ItemFactory
 
 
@@ -56,7 +58,6 @@ class ContainerViewTestCase(CourseTestCase):
             parent_location=published_xblock_with_child.location,
             category="html", display_name="Child HTML"
         )
-        draft_xblock_with_child = modulestore('draft').convert_to_draft(published_xblock_with_child.location)
         branch_name = "MITx.999.Robot_Super_Course/branch/draft/block"
         self._test_html_content(
             published_xblock_with_child,
@@ -73,6 +74,11 @@ class ContainerViewTestCase(CourseTestCase):
                 r'<a href="#" class="navigation-link navigation-current">Wrapper</a>'
             ).format(branch_name=branch_name)
         )
+
+        # Now make the unit and its children into a draft and validate the container again
+        modulestore('draft').convert_to_draft(self.vertical.location)
+        modulestore('draft').convert_to_draft(self.child_vertical.location)
+        draft_xblock_with_child = modulestore('draft').convert_to_draft(published_xblock_with_child.location)
         self._test_html_content(
             draft_xblock_with_child,
             branch_name=branch_name,
@@ -112,3 +118,32 @@ class ContainerViewTestCase(CourseTestCase):
             branch_name=branch_name
         )
         self.assertIn(expected_unit_link, html)
+
+    def test_container_preview_html(self):
+        """
+        Verify that an xblock returns the expected HTML for a container preview
+        """
+        # First verify that the behavior is correct with a published container
+        self._test_preview_html(self.child_vertical)
+
+        # Now make the unit and its children into a draft and validate the preview again
+        modulestore('draft').convert_to_draft(self.vertical.location)
+        draft_container = modulestore('draft').convert_to_draft(self.child_vertical.location)
+        self._test_preview_html(draft_container)
+
+    def _test_preview_html(self, xblock):
+        locator = loc_mapper().translate_location(self.course.id, xblock.location, published=False)
+        publish_state = compute_publish_state(xblock)
+        preview_url = '/xblock/{locator}/container_preview'.format(locator=locator)
+
+        resp = self.client.get(preview_url, HTTP_ACCEPT='application/json')
+        self.assertEqual(resp.status_code, 200)
+        resp_content = json.loads(resp.content)
+        html = resp_content['html']
+
+        # Verify that there are no drag handles for public pages
+        drag_handle_html = '<span data-tooltip="Drag to reorder" class="drag-handle action"></span>'
+        if publish_state == PublishState.public:
+            self.assertNotIn(drag_handle_html, html)
+        else:
+            self.assertIn(drag_handle_html, html)

cms/static/js/spec/views/container_spec.js

@@ -8,7 +8,8 @@ define([ "jquery", "js/spec_helpers/create_sinon", "URI", "js/views/container",
             describe("Supports reordering components", function () {
 
                 var model, containerView, mockContainerHTML, respondWithMockXBlockFragment,
-                    init, dragHandle, verifyRequest, verifyNumReorderCalls, respondToRequest,
+                    init, dragHandleVertically, dragHandleOver, verifyRequest, verifyNumReorderCalls,
+                    respondToRequest,
 
                     rootLocator = 'testCourse/branch/draft/split_test/splitFFF',
                     containerTestUrl = '/xblock/' + rootLocator,
@@ -65,11 +66,18 @@ define([ "jquery", "js/spec_helpers/create_sinon", "URI", "js/views/container",
                     return requests;
                 };
 
-                dragHandle = function (index, dy) {
+                dragHandleVertically = function (index, dy) {
                     var handle = containerView.$(".drag-handle:eq(" + index + ")");
                     handle.simulate("drag", {dy: dy});
                 };
 
+                dragHandleOver = function (index, targetElement) {
+                    var handle = containerView.$(".drag-handle:eq(" + index + ")"),
+                        dy = handle.y - targetElement.y;
+
+                    handle.simulate("drag", {dy: dy});
+                };
+
                 verifyRequest = function (requests, reorderCallIndex, expectedURL, expectedChildren) {
                     var request, children, i;
                     // 0th call is the response to the initial render call to get HTML.
@@ -95,14 +103,14 @@ define([ "jquery", "js/spec_helpers/create_sinon", "URI", "js/views/container",
                 it('does nothing if item not moved far enough', function () {
                     var requests = init(this);
                     // Drag the first thing in Group A (text component) down very slightly, but not past second thing.
-                    dragHandle(2, 5);
+                    dragHandleVertically(2, 5);
                     verifyNumReorderCalls(requests, 0);
                 });
 
                 it('can reorder within a group', function () {
                     var requests = init(this);
                     // Drag the first component in Group A to the end
-                    dragHandle(2, 80);
+                    dragHandleVertically(2, 80);
                     respondToRequest(requests, 0, 200);
                     verifyNumReorderCalls(requests, 1);
                     verifyRequest(requests, 0, groupAUrl, [groupAComponent2, groupAComponent3, groupAComponent1]);
@@ -111,7 +119,7 @@ define([ "jquery", "js/spec_helpers/create_sinon", "URI", "js/views/container",
                 it('can drag from one group to another', function () {
                     var requests = init(this);
                     // Drag the first component in Group A into the second group.
-                    dragHandle(2, 300);
+                    dragHandleVertically(2, 300);
                     respondToRequest(requests, 0, 200);
                     respondToRequest(requests, 1, 200);
                     // Will get an event to move into Group B and an event to remove from Group A.
@@ -124,7 +132,7 @@ define([ "jquery", "js/spec_helpers/create_sinon", "URI", "js/views/container",
                 it('does not remove from old group if addition to new group fails', function () {
                     var requests = init(this);
                     // Drag the first component in Group A into the second group.
-                    dragHandle(2, 300);
+                    dragHandleVertically(2, 300);
                     respondToRequest(requests, 0, 500);
                     // Send failure for addition to new group-- no removal event should be received.
                     verifyNumReorderCalls(requests, 1);
@@ -135,7 +143,7 @@ define([ "jquery", "js/spec_helpers/create_sinon", "URI", "js/views/container",
                 it('can swap group A and group B', function () {
                     var requests = init(this);
                     // Drag Group B before group A.
-                    dragHandle(5, -300);
+                    dragHandleVertically(5, -300);
                     respondToRequest(requests, 0, 200);
                     verifyNumReorderCalls(requests, 1);
                     verifyRequest(requests, 0, containerTestUrl, [groupB, groupA]);
@@ -145,7 +153,7 @@ define([ "jquery", "js/spec_helpers/create_sinon", "URI", "js/views/container",
                 it('can drag a component to the top level, and nest one group in another', function () {
                     var requests = init(this);
                     // Drag text item in Group A to the top level (in first position).
-                    dragHandle(2, -40);
+                    dragHandleVertically(2, -40);
                     respondToRequest(requests, 0, 200);
                     respondToRequest(requests, 1, 200);
                     verifyNumReorderCalls(requests, 2);
@@ -153,7 +161,7 @@ define([ "jquery", "js/spec_helpers/create_sinon", "URI", "js/views/container",
                     verifyRequest(requests, 1, groupAUrl, [groupAComponent2, groupAComponent3]);
 
                     // Drag Group A into Group B.
-                    dragHandle(1, 150);
+                    dragHandleVertically(1, 150);
                     respondToRequest(requests, 2, 200);
                     respondToRequest(requests, 3, 200);
                     verifyNumReorderCalls(requests, 4);
@@ -175,7 +183,7 @@ define([ "jquery", "js/spec_helpers/create_sinon", "URI", "js/views/container",
                         requests = init(this);
 
                         // Drag the first component in Group A into the second group.
-                        dragHandle(2, 200);
+                        dragHandleVertically(2, 200);
 
                         expect(savingSpies.constructor).toHaveBeenCalled();
                         expect(savingSpies.show).toHaveBeenCalled();
@@ -194,7 +202,7 @@ define([ "jquery", "js/spec_helpers/create_sinon", "URI", "js/views/container",
                         var requests = init(this);
 
                         // Drag the first component in Group A into the second group.
-                        dragHandle(2, 200);
+                        dragHandleVertically(2, 200);
 
                         expect(savingSpies.constructor).toHaveBeenCalled();
                         expect(savingSpies.show).toHaveBeenCalled();

cms/templates/component.html

@@ -26,6 +26,7 @@
       </li>
     </ul>
 </div>
-<span data-tooltip="${_("Drag to reorder")}" class="drag-handle"></span>
+% if not xblock_context['read_only']:
+    <span data-tooltip="${_("Drag to reorder")}" class="drag-handle"></span>
+% endif
 ${preview}
-

cms/templates/container_xblock_component.html

@@ -21,8 +21,7 @@ from contentstore.views.helpers import xblock_studio_url
             </ul>
         </div>
     </header>
-    ## We currently support reordering only on the unit page.
-    % if reordering_enabled:
+    % if not xblock_context['read_only']:
         <span data-tooltip="${_("Drag to reorder")}" class="drag-handle"></span>
     % endif
 </section>

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

@@ -154,7 +154,7 @@ class DraftModuleStore(MongoModuleStore):
         self.refresh_cached_metadata_inheritance_tree(draft_location)
         self.fire_updated_modulestore_signal(get_course_id_no_run(draft_location), draft_location)
 
-        return self._load_items([original])[0]
+        return wrap_draft(self._load_items([original])[0])
 
     def update_item(self, xblock, user=None, allow_not_found=False):
         """

common/lib/xmodule/xmodule/vertical_module.py

@@ -46,7 +46,8 @@ class VerticalModule(VerticalFields, XModule):
             })
 
         fragment.add_content(self.system.render_template(template_name, {
-            'items': contents
+            'items': contents,
+            'xblock_context': context,
         }))
         return fragment
 

common/test/acceptance/tests/test_studio_container.py

+"""
+Acceptance tests for Studio related to the container page.
+"""
+from ..pages.studio.auto_auth import AutoAuthPage
+from ..pages.studio.overview import CourseOutlinePage
+from ..fixtures.course import CourseFixture, XBlockFixtureDesc
+
+from .helpers import UniqueCourseTest
+
+
+class ContainerBase(UniqueCourseTest):
+    """
+    Base class for tests that do operations on the container page.
+    """
+    __test__ = False
+
+    def setUp(self):
+        """
+        Create a unique identifier for the course used in this test.
+        """
+        # Ensure that the superclass sets up
+        super(ContainerBase, self).setUp()
+
+        self.auth_page = AutoAuthPage(self.browser, staff=True)
+        self.outline = CourseOutlinePage(
+            self.browser,
+            self.course_info['org'],
+            self.course_info['number'],
+            self.course_info['run']
+        )
+
+        self.setup_fixtures()
+
+        self.auth_page.visit()
+
+    def setup_fixtures(self):
+        course_fix = CourseFixture(
+            self.course_info['org'],
+            self.course_info['number'],
+            self.course_info['run'],
+            self.course_info['display_name']
+        )
+
+        course_fix.add_children(
+            XBlockFixtureDesc('chapter', 'Test Section').add_children(
+                XBlockFixtureDesc('sequential', 'Test Subsection').add_children(
+                    XBlockFixtureDesc('vertical', 'Test Unit').add_children(
+                        XBlockFixtureDesc('vertical', 'Test Container').add_children(
+                            XBlockFixtureDesc('vertical', 'Group A').add_children(
+                                XBlockFixtureDesc('html', 'Group A Item 1'),
+                                XBlockFixtureDesc('html', 'Group A Item 2')
+                            ),
+                            XBlockFixtureDesc('vertical', 'Group B').add_children(
+                                XBlockFixtureDesc('html', 'Group B Item 1'),
+                                XBlockFixtureDesc('html', 'Group B Item 2')
+                            )
+                        )
+                    )
+                )
+            )
+        ).install()
+
+    def go_to_container_page(self, make_draft=False):
+        self.outline.visit()
+        subsection = self.outline.section('Test Section').subsection('Test Subsection')
+        unit = subsection.toggle_expand().unit('Test Unit').go_to()
+        if make_draft:
+            unit.edit_draft()
+        container = unit.components[0].go_to_container()
+        return container
+
+
+class DragAndDropTest(ContainerBase):
+    """
+    Tests of reordering within the container page.
+    """
+    __test__ = True
+
+    def verify_ordering(self, container, expected_ordering):
+        xblocks = container.xblocks
+        for xblock in xblocks:
+            print xblock.name
+        # TODO: need to verify parenting structure on page. Just checking
+        # the order of the xblocks is not sufficient.
+
+
+    def test_reorder_in_group(self):
+        container = self.go_to_container_page(make_draft=True)
+        # Swap Group A Item 1 and Group A Item 2.
+        container.drag(1, 2)
+
+        expected_ordering = [{"Group A": ["Group A Item 2", "Group A Item 1"]},
+                             {"Group B": ["Group B Item 1", "Group B Item 2"]}]
+        self.verify_ordering(container, expected_ordering)
+
+        # Reload the page to see that the reordering was saved persisted.
+        container = self.go_to_container_page()
+        self.verify_ordering(container, expected_ordering)

lms/templates/vert_module_studio_view.html

@@ -6,7 +6,9 @@ from django.utils.translation import ugettext as _
         % for idx, item in enumerate(items):
         <li class="vertical-element is-draggable">
             <div class="vert vert-${idx}" data-id="${item['id']}">
+                % if not xblock_context['read_only']:
                 <span data-tooltip="${_('Drag to reorder')}" class="drag-handle action"></span>
+                % endif
                 ${item['content']}
             </div>
         </li>