From dd3c8c268c14556223812a42c85e6bc0fb13180f Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Fri, 4 Apr 2014 13:56:20 -0400 Subject: [PATCH 1/2] Support for Duplicate and Delete of XBlock Container Leaves (disabled under Feature Flag). --- cms/envs/common.py | 6 + .../js/spec/views/pages/container_spec.js | 303 +++++++++++++++--- cms/static/js/views/pages/container.js | 100 +++++- cms/templates/studio_xblock_wrapper.html | 39 ++- 4 files changed, 387 insertions(+), 61 deletions(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index 022e9dd94b..9e199e3372 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -99,6 +99,12 @@ FEATURES = { # Turn off Advanced Security by default 'ADVANCED_SECURITY': False, + + # Temporary feature flag for duplicating xblock leaves + 'ENABLE_DUPLICATE_XBLOCK_LEAF_COMPONENT': False, + + # Temporary feature flag for deleting xblock leaves + 'ENABLE_DELETE_XBLOCK_LEAF_COMPONENT': False, } ENABLE_JASMINE = False diff --git a/cms/static/js/spec/views/pages/container_spec.js b/cms/static/js/spec/views/pages/container_spec.js index 6c28dac98f..a135ca4509 100644 --- a/cms/static/js/spec/views/pages/container_spec.js +++ b/cms/static/js/spec/views/pages/container_spec.js @@ -1,11 +1,13 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers", + "js/views/feedback_notification", "js/views/feedback_prompt", "js/views/pages/container", "js/models/xblock_info"], - function ($, create_sinon, edit_helpers, ContainerPage, XBlockInfo) { + function ($, create_sinon, edit_helpers, Notification, Prompt, ContainerPage, XBlockInfo) { describe("ContainerPage", function() { - var model, containerPage, respondWithMockXBlockEditorFragment, mockContainerPage; - - mockContainerPage = readFixtures('mock/mock-container-page.underscore'); + var lastRequest, renderContainerPage, expectComponents, respondWithHtml, + model, containerPage, requests, + mockContainerPage = readFixtures('mock/mock-container-page.underscore'), + ABTestFixture = readFixtures('mock/mock-container-xblock.underscore'); beforeEach(function () { edit_helpers.installEditTemplates(); @@ -22,40 +24,50 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers" }); }); - respondWithMockXBlockEditorFragment = function(requests, response) { + lastRequest = function() { return requests[requests.length - 1]; }; + + respondWithHtml = function(html) { var requestIndex = requests.length - 1; - create_sinon.respondWithJson(requests, response, requestIndex); + create_sinon.respondWithJson( + requests, + { html: html, "resources": [] }, + requestIndex + ); + }; + + renderContainerPage = function(html, that) { + requests = create_sinon.requests(that); + containerPage.render(); + respondWithHtml(html); + }; + + expectComponents = function (container, locators) { + // verify expected components (in expected order) by their locators + var components = $(container).find('[data-locator]'); + expect(components.length).toBe(locators.length); + _.each(locators, function(locator, locator_index) { + expect($(components[locator_index]).data('locator')).toBe(locator); + }); }; describe("Basic display", function() { var mockContainerXBlockHtml = readFixtures('mock/mock-container-xblock.underscore'); it('can render itself', function() { - var requests = create_sinon.requests(this); - containerPage.render(); - respondWithMockXBlockEditorFragment(requests, { - html: mockContainerXBlockHtml, - resources: [] - }); - + renderContainerPage(mockContainerXBlockHtml, this); expect(containerPage.$el.select('.xblock-header')).toBeTruthy(); expect(containerPage.$('.wrapper-xblock')).not.toHaveClass('is-hidden'); expect(containerPage.$('.no-container-content')).toHaveClass('is-hidden'); }); - it('shows a loading indicator', function() { - var requests = create_sinon.requests(this); + requests = create_sinon.requests(this); containerPage.render(); expect(containerPage.$('.ui-loading')).not.toHaveClass('is-hidden'); - respondWithMockXBlockEditorFragment(requests, { - html: mockContainerXBlockHtml, - resources: [] - }); + respondWithHtml(mockContainerXBlockHtml); expect(containerPage.$('.ui-loading')).toHaveClass('is-hidden'); }); }); - describe("Editing an xblock", function() { var mockContainerXBlockHtml, mockXBlockEditorHtml, @@ -79,19 +91,16 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers" mockXBlockEditorHtml = readFixtures('mock/mock-xblock-editor.underscore'); it('can show an edit modal for a child xblock', function() { - var requests = create_sinon.requests(this), - editButtons; - containerPage.render(); - respondWithMockXBlockEditorFragment(requests, { - html: mockContainerXBlockHtml, - resources: [] - }); + var editButtons; + renderContainerPage(mockContainerXBlockHtml, this); editButtons = containerPage.$('.edit-button'); // The container renders six mock xblocks, so there should be an equal number of edit buttons expect(editButtons.length).toBe(6); editButtons.first().click(); // Make sure that the correct xblock is requested to be edited - expect(requests[requests.length - 1].url).toBe('/xblock/locator-component-A1/studio_view'); + expect(lastRequest().url).toBe( + '/xblock/locator-component-A1/studio_view' + ); create_sinon.respondWithJson(requests, { html: mockXBlockEditorHtml, resources: [] @@ -100,14 +109,9 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers" }); it('can save changes to settings', function() { - var requests, editButtons, modal, mockUpdatedXBlockHtml; + var editButtons, modal, mockUpdatedXBlockHtml; mockUpdatedXBlockHtml = readFixtures('mock/mock-updated-xblock.underscore'); - requests = create_sinon.requests(this); - containerPage.render(); - respondWithMockXBlockEditorFragment(requests, { - html: mockContainerXBlockHtml, - resources: [] - }); + renderContainerPage(mockContainerXBlockHtml, this); editButtons = containerPage.$('.edit-button'); // The container renders six mock xblocks, so there should be an equal number of edit buttons expect(editButtons.length).toBe(6); @@ -128,11 +132,10 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers" create_sinon.respondWithJson(requests, { id: model.id }); + // Respond to the request to refresh - respondWithMockXBlockEditorFragment(requests, { - html: mockUpdatedXBlockHtml, - resources: [] - }); + respondWithHtml(mockUpdatedXBlockHtml); + // Verify that the xblock was updated expect(containerPage.$('.mock-updated-content').text()).toBe('Mock Update'); }); @@ -142,16 +145,224 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers" var mockContainerXBlockHtml = readFixtures('mock/mock-empty-container-xblock.underscore'); it('shows the "no children" message', function() { - var requests = create_sinon.requests(this); - containerPage.render(); - respondWithMockXBlockEditorFragment(requests, { - html: mockContainerXBlockHtml, - resources: [] - }); - + renderContainerPage(mockContainerXBlockHtml, this); expect(containerPage.$('.no-container-content')).not.toHaveClass('is-hidden'); expect(containerPage.$('.wrapper-xblock')).toHaveClass('is-hidden'); }); }); + + describe("xblock operations", function() { + var getGroupElement, expectNumComponents, expectNotificationToBeShown, + NUM_GROUPS = 2, NUM_COMPONENTS_PER_GROUP = 3, GROUP_TO_TEST = "A", + notificationSpies, + allComponentsInGroup = _.map( + _.range(NUM_COMPONENTS_PER_GROUP), + function(index) { return 'locator-component-' + GROUP_TO_TEST + (index + 1); } + ); + + beforeEach(function () { + notificationSpies = spyOnConstructor(Notification, "Mini", ["show", "hide"]); + notificationSpies.show.andReturn(notificationSpies); + }); + + getGroupElement = function() { + return containerPage.$("[data-locator='locator-group-" + GROUP_TO_TEST + "']"); + }; + expectNumComponents = function(numComponents) { + expect(containerPage.$('.wrapper-xblock.level-element').length).toBe( + numComponents * NUM_GROUPS + ); + }; + expectNotificationToBeShown = function(expectedTitle) { + expect(notificationSpies.constructor).toHaveBeenCalled(); + expect(notificationSpies.show).toHaveBeenCalled(); + expect(notificationSpies.hide).not.toHaveBeenCalled(); + expect(notificationSpies.constructor.mostRecentCall.args[0].title).toMatch(expectedTitle); + }; + + describe("Deleting an xblock", function() { + var clickDelete, deleteComponent, deleteComponentWithSuccess, + promptSpies; + + beforeEach(function() { + promptSpies = spyOnConstructor(Prompt, "Warning", ["show", "hide"]); + promptSpies.show.andReturn(this.promptSpies); + }); + + clickDelete = function(componentIndex) { + + // find all delete buttons for the given group + var deleteButtons = getGroupElement().find(".delete-button"); + expect(deleteButtons.length).toBe(NUM_COMPONENTS_PER_GROUP); + + // click the requested delete button + deleteButtons[componentIndex].click(); + + // expect delete confirmation + expect(promptSpies.constructor).toHaveBeenCalled(); + + // no components should be deleted yet + expectNumComponents(NUM_COMPONENTS_PER_GROUP); + }; + + deleteComponent = function(componentIndex, responseCode) { + + // click delete button for given component + clickDelete(componentIndex); + + // click 'Yes' on delete confirmation + promptSpies.constructor.mostRecentCall.args[0].actions.primary.click(promptSpies); + + // expect 'deleting' notification to be shown + expectNotificationToBeShown(/Deleting/); + + // respond to request with given response code + lastRequest().respond(responseCode, {}, ""); + + // expect request URL to contain given component's id + expect(lastRequest().url).toMatch( + new RegExp("locator-component-" + GROUP_TO_TEST + (componentIndex + 1)) + ); + }; + + deleteComponentWithSuccess = function(componentIndex) { + + // delete component with an 'OK' response code + deleteComponent(componentIndex, 200); + + // expect 'deleting' notification to be hidden + expect(notificationSpies.hide).toHaveBeenCalled(); + + // verify the new list of components within the group + expectComponents( + getGroupElement(), + _.without(allComponentsInGroup, allComponentsInGroup[componentIndex]) + ); + }; + + it("deletes first xblock", function() { + renderContainerPage(ABTestFixture, this); + deleteComponentWithSuccess(0); + }); + + it("deletes middle xblock", function() { + renderContainerPage(ABTestFixture, this); + deleteComponentWithSuccess(1); + }); + + it("deletes last xblock", function() { + renderContainerPage(ABTestFixture, this); + deleteComponentWithSuccess(NUM_COMPONENTS_PER_GROUP - 1); + }); + + it('does not delete xblock when clicking No in prompt', function () { + var numRequests; + + renderContainerPage(ABTestFixture, this); + numRequests = requests.length; + + // click delete on the first component + clickDelete(0); + + // click 'No' on delete confirmation + promptSpies.constructor.mostRecentCall.args[0].actions.secondary.click(promptSpies); + + // all components should still exist + expectComponents(getGroupElement(), allComponentsInGroup); + + // no requests should have been sent to the server + expect(requests.length).toBe(numRequests); + }); + + it('does not delete xblock upon failure', function () { + renderContainerPage(ABTestFixture, this); + deleteComponent(0, 500); + expectComponents(getGroupElement(), allComponentsInGroup); + expect(notificationSpies.hide).not.toHaveBeenCalled(); + }); + }); + + describe("Duplicating an xblock", function() { + var clickDuplicate, duplicateComponentWithResponse, duplicateComponentWithSuccess, + refreshXBlockSpies; + + beforeEach(function() { + refreshXBlockSpies = spyOn(containerPage, "refreshXBlock"); + }); + + clickDuplicate = function(componentIndex) { + + // find all duplicate buttons for the given group + var duplicateButtons = getGroupElement().find(".duplicate-button"); + expect(duplicateButtons.length).toBe(NUM_COMPONENTS_PER_GROUP); + + // click the requested duplicate button + duplicateButtons[componentIndex].click(); + }; + + duplicateComponentWithResponse = function(componentIndex, responseCode) { + var request; + + // click duplicate button for given component + clickDuplicate(componentIndex); + + // expect 'duplicating' notification to be shown + expectNotificationToBeShown(/Duplicating/); + + // verify content of request + request = lastRequest(); + request.respond( + responseCode, + { "Content-Type": "application/json" }, + JSON.stringify({'locator': 'locator-duplicated-component'}) + ); + expect(request.url).toEqual("/xblock"); + expect(request.method).toEqual("POST"); + expect(JSON.parse(request.requestBody)).toEqual( + JSON.parse( + '{' + + '"duplicate_source_locator": "locator-component-' + GROUP_TO_TEST + (componentIndex + 1) + '",' + + '"parent_locator": "locator-group-' + GROUP_TO_TEST + + '"}' + ) + ); + }; + + duplicateComponentWithSuccess = function(componentIndex) { + + // duplicate component with an 'OK' response code + duplicateComponentWithResponse(componentIndex, 200); + + // expect 'duplicating' notification to be hidden + expect(notificationSpies.hide).toHaveBeenCalled(); + + // expect parent container to be refreshed + expect(refreshXBlockSpies).toHaveBeenCalled(); + }; + + it("duplicates first xblock", function() { + renderContainerPage(ABTestFixture, this); + duplicateComponentWithSuccess(0); + }); + + it("duplicates middle xblock", function() { + renderContainerPage(ABTestFixture, this); + duplicateComponentWithSuccess(1); + }); + + it("duplicates last xblock", function() { + renderContainerPage(ABTestFixture, this); + duplicateComponentWithSuccess(NUM_COMPONENTS_PER_GROUP - 1); + }); + + it('does not duplicate xblock upon failure', function () { + renderContainerPage(ABTestFixture, this); + duplicateComponentWithResponse(0, 500); + expectComponents(getGroupElement(), allComponentsInGroup); + expect(notificationSpies.hide).not.toHaveBeenCalled(); + expect(refreshXBlockSpies).not.toHaveBeenCalled(); + }); + }); + }); }); }); diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index ad6d5d95ce..9354af97c1 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -2,8 +2,8 @@ * XBlockContainerView is used to display an xblock which has children, and allows the * user to interact with the children. */ -define(["jquery", "underscore", "js/views/baseview", "js/views/xblock", "js/views/modals/edit_xblock"], - function ($, _, BaseView, XBlockView, EditXBlockModal) { +define(["jquery", "underscore", "gettext", "js/views/feedback_notification", "js/views/feedback_prompt", "js/views/baseview", "js/views/xblock", "js/views/modals/edit_xblock", "js/models/xblock_info"], + function ($, _, gettext, NotificationView, PromptView, BaseView, XBlockView, EditXBlockModal, XBlockInfo) { var XBlockContainerView = BaseView.extend({ // takes XBlockInfo as a model @@ -53,6 +53,10 @@ define(["jquery", "underscore", "js/views/baseview", "js/views/xblock", "js/view return $(target).closest('[data-locator]'); }, + getURLRoot: function() { + return this.xblockView.model.urlRoot; + }, + addButtonActions: function(element) { var self = this; element.find('.edit-button').click(function(event) { @@ -68,11 +72,98 @@ define(["jquery", "underscore", "js/views/baseview", "js/views/xblock", "js/view } }); }); + element.find('.duplicate-button').click(function(event) { + event.preventDefault(); + self.duplicateComponent( + self.findXBlockElement(event.target) + ); + }); + element.find('.delete-button').click(function(event) { + event.preventDefault(); + self.deleteComponent( + self.findXBlockElement(event.target) + ); + }); + }, + + duplicateComponent: function(xblockElement) { + var self = this, + parentElement = self.findXBlockElement(xblockElement.parent()), + duplicating = new NotificationView.Mini({ + title: gettext('Duplicating…') + }); + + duplicating.show(); + return $.postJSON(self.getURLRoot(), { + duplicate_source_locator: xblockElement.data('locator'), + parent_locator: parentElement.data('locator') + }, function(data) { + // copy the element + var duplicatedElement = xblockElement.clone(false); + + // place it after the original element + xblockElement.after(duplicatedElement); + + // update its locator id + duplicatedElement.attr('data-locator', data.locator); + + // have it refresh itself + self.refreshXBlockElement(duplicatedElement); + + // hide the notification + duplicating.hide(); + }); + }, + + + deleteComponent: function(xblockElement) { + var self = this, deleting; + return new PromptView.Warning({ + title: gettext('Delete this component?'), + message: gettext('Deleting this component is permanent and cannot be undone.'), + actions: { + primary: { + text: gettext('Yes, delete this component'), + click: function(prompt) { + prompt.hide(); + deleting = new NotificationView.Mini({ + title: gettext('Deleting…') + }); + deleting.show(); + return $.ajax({ + type: 'DELETE', + url: + self.getURLRoot() + "/" + + xblockElement.data('locator') + "?" + + $.param({recurse: true, all_versions: true}) + }).success(function() { + deleting.hide(); + xblockElement.remove(); + }); + } + }, + secondary: { + text: gettext('Cancel'), + click: function(prompt) { + return prompt.hide(); + } + } + } + }).show(); + }, + + refreshXBlockElement: function(xblockElement) { + this.refreshXBlock( + new XBlockInfo({ + id: xblockElement.data('locator') + }), + xblockElement + ); }, refreshXBlock: function(xblockInfo, xblockElement) { - var self = this, - temporaryView; + var self = this, temporaryView; + // There is only one Backbone view created on the container page, which is // for the container xblock itself. Any child xblocks rendered inside the // container do not get a Backbone view. Thus, create a temporary XBlock @@ -93,3 +184,4 @@ define(["jquery", "underscore", "js/views/baseview", "js/views/xblock", "js/view return XBlockContainerView; }); // end define(); + diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index c2e447a9b6..f656006856 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -1,11 +1,11 @@ <%! from django.utils.translation import ugettext as _ %> +<%! from django.conf import settings %> + % if xblock.location != xblock_context['root_xblock'].location: - % if xblock.has_children: -
- % else: -
- % endif + <% section_class = "level-nesting" if xblock.has_children else "level-element" %> +
% endif +
${xblock.display_name_with_default | h} @@ -13,12 +13,28 @@
@@ -26,6 +42,7 @@
${content}
+ % if xblock.location != xblock_context['root_xblock'].location:
% endif From 48d935d715ce2c8f40ffd0013327d55e3a74c677 Mon Sep 17 00:00:00 2001 From: Frances Botsford Date: Thu, 10 Apr 2014 11:51:54 -0400 Subject: [PATCH 2/2] fix for fuzzy prompt on container page --- cms/templates/container.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/templates/container.html b/cms/templates/container.html index f3f7f292c5..9893cf735e 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -131,6 +131,6 @@ main_xblock_info = { -
+