diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b3a6331f7f..79de8c09f4 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,8 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. +Studio: Add drag-and-drop support to the container page. STUD-1309. + Common: Add extensible third-party auth module. Blades: Handle situation if no response were sent from XQueue to LMS in Matlab 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..0310a9544b 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,37 @@ 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.vertical) + self._test_preview_html(self.child_vertical) + + # Now make the unit and its children into a draft and validate the preview again + draft_unit = modulestore('draft').convert_to_draft(self.vertical.location) + draft_container = modulestore('draft').convert_to_draft(self.child_vertical.location) + self._test_preview_html(draft_unit) + self._test_preview_html(draft_container) + + def _test_preview_html(self, xblock): + """ + Verify that the specified xblock has the expected HTML elements for container preview + """ + 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/djangoapps/contentstore/views/tests/test_tabs.py b/cms/djangoapps/contentstore/views/tests/test_tabs.py index fd04089bc6..886ed2668b 100644 --- a/cms/djangoapps/contentstore/views/tests/test_tabs.py +++ b/cms/djangoapps/contentstore/views/tests/test_tabs.py @@ -4,6 +4,7 @@ import json from contentstore.views import tabs from contentstore.tests.utils import CourseTestCase from django.test import TestCase +from xmodule.modulestore.django import loc_mapper from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from courseware.courses import get_course_by_id from xmodule.tabs import CourseTabList, WikiTab @@ -22,7 +23,7 @@ class TabsPageTests(CourseTestCase): self.url = self.course_locator.url_reverse('tabs') # add a static tab to the course, for code coverage - ItemFactory.create( + self.test_tab = ItemFactory.create( parent_location=self.course_location, category="static_tab", display_name="Static_1" @@ -172,6 +173,25 @@ class TabsPageTests(CourseTestCase): ) self.check_invalid_tab_id_response(resp) + def test_tab_preview_html(self): + """ + Verify that the static tab renders itself with the correct HTML + """ + locator = loc_mapper().translate_location(self.course.id, self.test_tab.location) + preview_url = '/xblock/{locator}/student_view'.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 the HTML contains the expected elements + self.assertIn('Edit', html) + self.assertIn('Duplicate this component', html) + self.assertIn('Delete this component', html) + self.assertIn('', html) + + class PrimitiveTabEdit(TestCase): """Tests for the primitive tab edit data manipulations""" diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index d8126c77e8..0dd3e24ed9 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -18,6 +18,7 @@ requirejs.config({ "jquery.iframe-transport": "xmodule_js/common_static/js/vendor/jQuery-File-Upload/js/jquery.iframe-transport", "jquery.inputnumber": "xmodule_js/common_static/js/vendor/html5-input-polyfills/number-polyfill", "jquery.immediateDescendents": "xmodule_js/common_static/coffee/src/jquery.immediateDescendents", + "jquery.simulate": "xmodule_js/common_static/js/vendor/jquery.simulate", "datepair": "xmodule_js/common_static/js/vendor/timepicker/datepair", "date": "xmodule_js/common_static/js/vendor/date", "underscore": "xmodule_js/common_static/js/vendor/underscore-min", @@ -100,6 +101,10 @@ requirejs.config({ deps: ["jquery"], exports: "jQuery.fn.inputNumber" }, + "jquery.simulate": { + deps: ["jquery"], + exports: "jQuery.fn.simulate" + }, "jquery.tinymce": { deps: ["jquery", "tinymce"], exports: "jQuery.fn.tinymce" @@ -216,6 +221,7 @@ define([ "js/spec/views/baseview_spec", "js/spec/views/paging_spec", + "js/spec/views/container_spec", "js/spec/views/unit_spec", "js/spec/views/xblock_spec", "js/spec/views/xblock_editor_spec", diff --git a/cms/static/js/spec/views/container_spec.js b/cms/static/js/spec/views/container_spec.js new file mode 100644 index 0000000000..c27bc634cf --- /dev/null +++ b/cms/static/js/spec/views/container_spec.js @@ -0,0 +1,215 @@ +define([ "jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/view_helpers", + "js/views/container", "js/models/xblock_info", "js/views/feedback_notification", "jquery.simulate", + "xmodule", "coffee/src/main", "xblock/cms.runtime.v1"], + function ($, create_sinon, view_helpers, ContainerView, XBlockInfo, Notification) { + + describe("Container View", function () { + + describe("Supports reordering components", function () { + + var model, containerView, mockContainerHTML, respondWithMockXBlockFragment, init, getComponent, + getDragHandle, dragComponentVertically, dragComponentAbove, + verifyRequest, verifyNumReorderCalls, respondToRequest, + + rootLocator = 'testCourse/branch/draft/split_test/splitFFF', + containerTestUrl = '/xblock/' + rootLocator, + + groupAUrl = "/xblock/locator-group-A", + groupA = "locator-group-A", + groupAComponent1 = "locator-component-A1", + groupAComponent2 = "locator-component-A2", + groupAComponent3 = "locator-component-A3", + + groupBUrl = "/xblock/locator-group-B", + groupB = "locator-group-B", + groupBComponent1 = "locator-component-B1", + groupBComponent2 = "locator-component-B2", + groupBComponent3 = "locator-component-B3"; + + mockContainerHTML = readFixtures('mock/mock-container-xblock.underscore'); + + respondWithMockXBlockFragment = function (requests, response) { + var requestIndex = requests.length - 1; + create_sinon.respondWithJson(requests, response, requestIndex); + }; + + beforeEach(function () { + view_helpers.installViewTemplates(); + appendSetFixtures('
'); + model = new XBlockInfo({ + id: rootLocator, + display_name: 'Test AB Test', + category: 'split_test' + }); + + containerView = new ContainerView({ + model: model, + view: 'container_preview', + el: $('.wrapper-xblock') + }); + }); + + afterEach(function () { + containerView.remove(); + }); + + init = function (caller) { + var requests = create_sinon.requests(caller); + containerView.render(); + + respondWithMockXBlockFragment(requests, { + html: mockContainerHTML, + "resources": [] + }); + + $('body').append(containerView.$el); + return requests; + }; + + getComponent = function(locator) { + return containerView.$('[data-locator="' + locator + '"]'); + }; + + getDragHandle = function(locator) { + var component = getComponent(locator); + return component.prev(); + }; + + dragComponentVertically = function (locator, dy) { + var handle = getDragHandle(locator); + handle.simulate("drag", {dy: dy}); + }; + + dragComponentAbove = function (sourceLocator, targetLocator) { + var targetElement = getComponent(targetLocator), + targetTop = targetElement.offset().top + 1, + handle = getDragHandle(sourceLocator), + handleY = handle.offset().top + (handle.height() / 2), + dy = targetTop - handleY; + handle.simulate("drag", {dy: dy}); + }; + + verifyRequest = function (requests, reorderCallIndex, expectedURL, expectedChildren) { + var actualIndex, request, children, i; + // 0th call is the response to the initial render call to get HTML. + actualIndex = reorderCallIndex + 1; + expect(requests.length).toBeGreaterThan(actualIndex); + request = requests[actualIndex]; + expect(request.url).toEqual(expectedURL); + children = (JSON.parse(request.requestBody)).children; + expect(children.length).toEqual(expectedChildren.length); + for (i = 0; i < children.length; i++) { + expect(children[i]).toEqual(expectedChildren[i]); + } + }; + + verifyNumReorderCalls = function (requests, expectedCalls) { + // Number of calls will be 1 more than expected because of the initial render call to get HTML. + expect(requests.length).toEqual(expectedCalls + 1); + }; + + respondToRequest = function (requests, reorderCallIndex, status) { + var actualIndex; + // Number of calls will be 1 more than expected because of the initial render call to get HTML. + actualIndex = reorderCallIndex + 1; + expect(requests.length).toBeGreaterThan(actualIndex); + requests[actualIndex].respond(status); + }; + + it('does nothing if item not moved far enough', function () { + var requests = init(this); + // Drag the first component in Group A down very slightly but not enough to move it. + dragComponentVertically(groupAComponent1, 5); + verifyNumReorderCalls(requests, 0); + }); + + it('can reorder within a group', function () { + var requests = init(this); + // Drag the third component in Group A to be the first + dragComponentAbove(groupAComponent3, groupAComponent1); + respondToRequest(requests, 0, 200); + verifyRequest(requests, 0, groupAUrl, [groupAComponent3, groupAComponent1, groupAComponent2]); + }); + + it('can drag from one group to another', function () { + var requests = init(this); + // Drag the first component in Group B to the top of group A. + dragComponentAbove(groupBComponent1, groupAComponent1); + + // Respond to the two requests: add the component to Group A, then remove it from Group B. + respondToRequest(requests, 0, 200); + respondToRequest(requests, 1, 200); + + verifyRequest(requests, 0, groupAUrl, + [groupBComponent1, groupAComponent1, groupAComponent2, groupAComponent3]); + verifyRequest(requests, 1, groupBUrl, [groupBComponent2, groupBComponent3]); + }); + + it('does not remove from old group if addition to new group fails', function () { + var requests = init(this); + // Drag the first component in Group B to the first group. + dragComponentAbove(groupBComponent1, groupAComponent1); + respondToRequest(requests, 0, 500); + // Send failure for addition to new group -- no removal event should be received. + verifyRequest(requests, 0, groupAUrl, + [groupBComponent1, groupAComponent1, groupAComponent2, groupAComponent3]); + // Verify that a second request was not issued + verifyNumReorderCalls(requests, 1); + }); + + it('can swap group A and group B', function () { + var requests = init(this); + // Drag Group B before group A. + dragComponentAbove(groupB, groupA); + respondToRequest(requests, 0, 200); + verifyRequest(requests, 0, containerTestUrl, [groupB, groupA]); + }); + + describe("Shows a saving message", function () { + var savingSpies; + + beforeEach(function () { + savingSpies = spyOnConstructor(Notification, "Mini", + ["show", "hide"]); + savingSpies.show.andReturn(savingSpies); + }); + + it('hides saving message upon success', function () { + var requests, savingOptions; + requests = init(this); + + // Drag the first component in Group B to the first group. + dragComponentAbove(groupBComponent1, groupAComponent1); + + expect(savingSpies.constructor).toHaveBeenCalled(); + expect(savingSpies.show).toHaveBeenCalled(); + expect(savingSpies.hide).not.toHaveBeenCalled(); + savingOptions = savingSpies.constructor.mostRecentCall.args[0]; + expect(savingOptions.title).toMatch(/Saving/); + + respondToRequest(requests, 0, 200); + expect(savingSpies.hide).not.toHaveBeenCalled(); + respondToRequest(requests, 1, 200); + expect(savingSpies.hide).toHaveBeenCalled(); + }); + + it('does not hide saving message if failure', function () { + var requests = init(this); + + // Drag the first component in Group B to the first group. + dragComponentAbove(groupBComponent1, groupAComponent1); + + expect(savingSpies.constructor).toHaveBeenCalled(); + expect(savingSpies.show).toHaveBeenCalled(); + expect(savingSpies.hide).not.toHaveBeenCalled(); + + respondToRequest(requests, 0, 500); + expect(savingSpies.hide).not.toHaveBeenCalled(); + + // Since the first reorder call failed, the removal will not be called. + verifyNumReorderCalls(requests, 1); + }); + }); + }); + }); + }); diff --git a/cms/static/js/spec/views/modals/edit_xblock_spec.js b/cms/static/js/spec/views/modals/edit_xblock_spec.js index e58759dea3..ecc12b43f6 100644 --- a/cms/static/js/spec/views/modals/edit_xblock_spec.js +++ b/cms/static/js/spec/views/modals/edit_xblock_spec.js @@ -12,7 +12,7 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers beforeEach(function () { edit_helpers.installEditTemplates(); - appendSetFixtures('
'); + appendSetFixtures('
'); model = new XBlockInfo({ id: 'testCourse/branch/draft/block/verticalFFF', display_name: 'Test Unit', diff --git a/cms/static/js/spec_helpers/modal_helpers.js b/cms/static/js/spec_helpers/modal_helpers.js index 8556cce60a..97cd94e838 100644 --- a/cms/static/js/spec_helpers/modal_helpers.js +++ b/cms/static/js/spec_helpers/modal_helpers.js @@ -1,8 +1,8 @@ /** * Provides helper methods for invoking Studio modal windows in Jasmine tests. */ -define(["jquery"], - function($) { +define(["jquery", "js/spec_helpers/view_helpers"], + function($, view_helpers) { var basicModalTemplate = readFixtures('basic-modal.underscore'), modalButtonTemplate = readFixtures('modal-button.underscore'), feedbackTemplate = readFixtures('system-feedback.underscore'), @@ -14,11 +14,7 @@ define(["jquery"], cancelModalIfShowing; installModalTemplates = function(append) { - if (append) { - appendSetFixtures($("