diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py
index 8e0e5fc14a..6916120caf 100644
--- a/cms/djangoapps/contentstore/views/item.py
+++ b/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):
"""
diff --git a/cms/djangoapps/contentstore/views/tests/test_container.py b/cms/djangoapps/contentstore/views/tests/test_container.py
index 3c19efc896..a18e0552a8 100644
--- a/cms/djangoapps/contentstore/views/tests/test_container.py
+++ b/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'Wrapper'
).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 = ''
+ if publish_state == PublishState.public:
+ self.assertNotIn(drag_handle_html, html)
+ else:
+ self.assertIn(drag_handle_html, html)
diff --git a/cms/static/js/spec/views/container_spec.js b/cms/static/js/spec/views/container_spec.js
index dd13ef46a5..45e76ed74a 100644
--- a/cms/static/js/spec/views/container_spec.js
+++ b/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();
diff --git a/cms/templates/component.html b/cms/templates/component.html
index 88e3227892..6b22971200 100644
--- a/cms/templates/component.html
+++ b/cms/templates/component.html
@@ -26,6 +26,7 @@
-
+% if not xblock_context['read_only']:
+
+% endif
${preview}
-
diff --git a/cms/templates/container_xblock_component.html b/cms/templates/container_xblock_component.html
index cd91f8b50f..da2b98c820 100644
--- a/cms/templates/container_xblock_component.html
+++ b/cms/templates/container_xblock_component.html
@@ -21,8 +21,7 @@ from contentstore.views.helpers import xblock_studio_url
- ## We currently support reordering only on the unit page.
- % if reordering_enabled:
+ % if not xblock_context['read_only']:
% endif
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
index c8ce76e1ec..c50cd31c4a 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
+++ b/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):
"""
diff --git a/common/lib/xmodule/xmodule/vertical_module.py b/common/lib/xmodule/xmodule/vertical_module.py
index f18153c066..d8e8e57c07 100644
--- a/common/lib/xmodule/xmodule/vertical_module.py
+++ b/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
diff --git a/common/test/acceptance/tests/test_studio_container.py b/common/test/acceptance/tests/test_studio_container.py
new file mode 100644
index 0000000000..9f1a9d17a8
--- /dev/null
+++ b/common/test/acceptance/tests/test_studio_container.py
@@ -0,0 +1,98 @@
+"""
+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)
diff --git a/lms/templates/vert_module_studio_view.html b/lms/templates/vert_module_studio_view.html
index b8bab17950..d12a1ca896 100644
--- a/lms/templates/vert_module_studio_view.html
+++ b/lms/templates/vert_module_studio_view.html
@@ -6,7 +6,9 @@ from django.utils.translation import ugettext as _
% for idx, item in enumerate(items):
+ % if not xblock_context['read_only']:
+ % endif
${item['content']}