diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 91c4f75bfb66..5e902c5898b3 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -739,6 +739,8 @@ def _move_item(source_usage_key, target_parent_usage_key, user, target_index=Non error = 'You can not move an item into itself.' elif is_source_item_in_target_parents(source_item, target_parent): error = 'You can not move an item into it\'s child.' + elif target_parent_type == 'split_test': + error = 'You can not move an item directly into content experiment.' elif source_index is None: error = '{source_usage_key} not found in {parent_usage_key}.'.format( source_usage_key=unicode(source_usage_key), @@ -1119,7 +1121,8 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F xblock_info = { 'id': unicode(xblock.location), 'display_name': xblock.display_name_with_default, - 'category': xblock.category + 'category': xblock.category, + 'has_children': xblock.has_children } if is_concise: if child_info and len(child_info.get('children', [])) > 0: @@ -1127,7 +1130,6 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F # Groups are labelled with their internal ids, rather than with the group name. Replace id with display name. group_display_name = get_group_display_name(user_partitions, xblock_info['display_name']) xblock_info['display_name'] = group_display_name if group_display_name else xblock_info['display_name'] - xblock_info['has_children'] = xblock.has_children else: xblock_info.update({ 'edited_on': get_default_time_display(xblock.subtree_edited_on) if xblock.subtree_edited_on else None, diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index afad862fd2c4..28e2b5f11f25 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -948,17 +948,17 @@ def test_invalid_move(self): """ Test invalid move. """ - parent_loc = self.store.get_parent_location(self.chapter_usage_key) - response = self._move_component(self.chapter_usage_key, self.usage_key) + parent_loc = self.store.get_parent_location(self.html_usage_key) + response = self._move_component(self.html_usage_key, self.seq_usage_key) self.assertEqual(response.status_code, 400) response = json.loads(response.content) expected_error = 'You can not move {source_type} into {target_type}.'.format( - source_type=self.chapter_usage_key.block_type, - target_type=self.usage_key.block_type + source_type=self.html_usage_key.block_type, + target_type=self.seq_usage_key.block_type ) self.assertEqual(expected_error, response['error']) - new_parent_loc = self.store.get_parent_location(self.chapter_usage_key) + new_parent_loc = self.store.get_parent_location(self.html_usage_key) self.assertEqual(new_parent_loc, parent_loc) def test_move_current_parent(self): @@ -1039,11 +1039,23 @@ def test_move_content_experiment_components(self): def test_move_into_content_experiment_groups(self): """ - Test that a component can be moved to content experiment. + Test that a component can be moved to content experiment groups. """ split_test = self.setup_and_verify_content_experiment(0) self.assert_move_item(self.html_usage_key, split_test.children[0]) + def test_can_not_move_into_content_experiment_level(self): + """ + Test that a component can not be moved directly to content experiment level. + """ + self.setup_and_verify_content_experiment(0) + response = self._move_component(self.html_usage_key, self.split_test_usage_key) + self.assertEqual(response.status_code, 400) + response = json.loads(response.content) + + self.assertEqual(response['error'], 'You can not move an item directly into content experiment.') + self.assertEqual(self.store.get_parent_location(self.html_usage_key), self.vert_usage_key) + def test_can_not_move_content_experiment_into_its_children(self): """ Test that a content experiment can not be moved inside any of it's children. diff --git a/cms/static/js/models/xblock_info.js b/cms/static/js/models/xblock_info.js index e3fd1c706cc5..fd7fdd8d9054 100644 --- a/cms/static/js/models/xblock_info.js +++ b/cms/static/js/models/xblock_info.js @@ -49,6 +49,10 @@ function(Backbone, _, str, ModuleUtils) { * publishing info was explicitly requested. */ 'published_by': null, + /** + * True if the xblock is a parentable xblock. + */ + has_children: null, /** * True if the xblock has changes. * Note: this is not always provided as a performance optimization. It is only provided for diff --git a/cms/static/js/spec/views/move_xblock_spec.js b/cms/static/js/spec/views/move_xblock_spec.js index 6e7f9664089d..3911124f0cb5 100644 --- a/cms/static/js/spec/views/move_xblock_spec.js +++ b/cms/static/js/spec/views/move_xblock_spec.js @@ -1,17 +1,19 @@ -define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', +define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', 'js/spec_helpers/edit_helpers', 'common/js/spec_helpers/template_helpers', 'common/js/spec_helpers/view_helpers', - 'js/views/modals/move_xblock_modal', 'edx-ui-toolkit/js/utils/html-utils', + 'js/views/modals/move_xblock_modal', 'js/views/pages/container', 'edx-ui-toolkit/js/utils/html-utils', 'edx-ui-toolkit/js/utils/string-utils', 'js/models/xblock_info'], - function($, _, AjaxHelpers, TemplateHelpers, ViewHelpers, MoveXBlockModal, HtmlUtils, StringUtils, XBlockInfo) { + function($, _, AjaxHelpers, EditHelpers, TemplateHelpers, ViewHelpers, MoveXBlockModal, ContainerPage, HtmlUtils, + StringUtils, XBlockInfo) { 'use strict'; describe('MoveXBlock', function() { var modal, showModal, renderViews, createXBlockInfo, createCourseOutline, courseOutlineOptions, parentChildMap, categoryMap, createChildXBlockInfo, xblockAncestorInfo, courseOutline, verifyBreadcrumbViewInfo, verifyListViewInfo, getDisplayedInfo, clickForwardButton, clickBreadcrumbButton, verifyXBlockInfo, nextCategory, verifyMoveEnabled, getSentRequests, - verifyNotificationStatus, sendMoveXBlockRequest, moveXBlockWithSuccess, - verifyConfirmationFeedbackTitleHtml, verifyConfirmationFeedbackRedirectLinkHtml, - verifyUndoConfirmationFeedbackTitleHtml, verifyConfirmationFeedbackUndoMoveActionHtml, + verifyNotificationStatus, sendMoveXBlockRequest, moveXBlockWithSuccess, getMovedAlertNotification, + verifyConfirmationFeedbackTitleText, verifyConfirmationFeedbackRedirectLinkText, + verifyUndoConfirmationFeedbackTitleText, verifyConfirmationFeedbackUndoMoveActionText, + sourceParentXBlockInfo, mockContainerPage, createContainerPage, containerPage, sourceDisplayName = 'component_display_name_0', sourceLocator = 'component_ID_0', sourceParentLocator = 'unit_ID_0'; @@ -62,13 +64,31 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe ] }; + sourceParentXBlockInfo = new XBlockInfo({ + id: sourceParentLocator, + display_name: 'unit_display_name_0', + category: 'vertical' + }); + + createContainerPage = function() { + containerPage = new ContainerPage({ + model: sourceParentXBlockInfo, + templates: EditHelpers.mockComponentTemplates, + el: $('#content'), + isUnitPage: true + }); + }; + beforeEach(function() { setFixtures("
"); + mockContainerPage = readFixtures('mock/mock-container-page.underscore'); TemplateHelpers.installTemplates([ 'basic-modal', 'modal-button', 'move-xblock-modal' ]); + appendSetFixtures(mockContainerPage); + createContainerPage(); courseOutline = createCourseOutline(courseOutlineOptions); showModal(); }); @@ -76,6 +96,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe afterEach(function() { modal.hide(); courseOutline = null; + containerPage.remove(); }); showModal = function() { @@ -85,11 +106,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe display_name: sourceDisplayName, category: 'component' }), - sourceParentXBlockInfo: new XBlockInfo({ - id: sourceParentLocator, - display_name: 'unit_display_name_0', - category: 'vertical' - }), + sourceParentXBlockInfo: sourceParentXBlockInfo, XBlockUrlRoot: '/xblock' }); modal.show(); @@ -338,6 +355,13 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe ViewHelpers.verifyNotificationHidden(notificationSpy); }; + /** + * Get move alert confirmation message HTML + */ + getMovedAlertNotification = function() { + return $('#page-alert'); + }; + /** * Send move xblock request. * @@ -384,33 +408,36 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe }); modal.$el.find('.modal-actions .action-move').click(); sendMoveXBlockRequest(requests, sourceLocator); - expect(modal.movedAlertView).toBeDefined(); - verifyConfirmationFeedbackTitleHtml(sourceDisplayName); - verifyConfirmationFeedbackRedirectLinkHtml(); - verifyConfirmationFeedbackUndoMoveActionHtml(); + AjaxHelpers.expectJsonRequest(requests, 'GET', '/xblock/' + sourceParentLocator); + AjaxHelpers.respondWithJson(requests, sourceParentXBlockInfo); + expect(getMovedAlertNotification().html().length).not.toEqual(0); + verifyConfirmationFeedbackTitleText(sourceDisplayName); + verifyConfirmationFeedbackRedirectLinkText(); + verifyConfirmationFeedbackUndoMoveActionText(); }; /** - * Verify success banner message html has correct title html. + * Verify success banner message html has correct title text. * * @param {String} displayName XBlock display name */ - verifyConfirmationFeedbackTitleHtml = function(displayName) { - expect(modal.movedAlertView.$el.find('.title').html().trim()) - .toEqual(StringUtils.interpolate('Success! "{displayName}" has been moved.', - { - displayName: displayName - }) - ); + verifyConfirmationFeedbackTitleText = function(displayName) { + expect(getMovedAlertNotification().find('.title').html() + .trim()) + .toEqual(StringUtils.interpolate('Success! "{displayName}" has been moved.', + { + displayName: displayName + }) + ); }; /** - * Verify undo success banner message html has correct title html. + * Verify undo success banner message html has correct title text. * * @param {String} displayName XBlock display name */ - verifyUndoConfirmationFeedbackTitleHtml = function(displayName) { - expect(modal.movedAlertView.$el.find('.title').html()).toEqual( + verifyUndoConfirmationFeedbackTitleText = function(displayName) { + expect(getMovedAlertNotification().find('.title').html()).toEqual( StringUtils.interpolate( 'Move cancelled. "{sourceDisplayName}" has been moved back to its original location.', { @@ -421,25 +448,18 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe }; /** - * Verify success banner message html has correct redirect link html. + * Verify success banner message html has correct redirect link text. */ - verifyConfirmationFeedbackRedirectLinkHtml = function() { - expect(modal.movedAlertView.$el.find('.copy').html().indexOf( - HtmlUtils.HTML( - '' - ) !== -1 - )).toBeTruthy(); + verifyConfirmationFeedbackRedirectLinkText = function() { + expect(getMovedAlertNotification().find('.nav-actions .action-secondary').html()) + .toEqual('Take me to the new location'); }; /** - * Verify success banner message html has correct undo move button html. + * Verify success banner message html has correct undo move text. */ - verifyConfirmationFeedbackUndoMoveActionHtml = function() { - expect(modal.movedAlertView.$el.find('.copy').html().indexOf( - HtmlUtils.HTML( - '' - ) !== -1 - )).toBeTruthy(); + verifyConfirmationFeedbackUndoMoveActionText = function() { + expect(getMovedAlertNotification().find('.nav-actions .action-primary').html()).toEqual('Undo move'); }; /** @@ -633,6 +653,24 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe expect(modal.$el.find('.modal-actions .action-move').hasClass('is-disabled')).toBeFalsy(); }); + it('is enabled when moving a component inside a parentable component', function() { + // create a source parent with has_childern set true + modal.sourceParentXBlockInfo = new XBlockInfo({ + category: 'conditional', + display_name: 'Parentable Component', + has_children: true, + id: 'PARENTABLE_ID' + }); + // navigate and verify move button is enabled + renderViews(courseOutline); + _.each(_.range(3), function() { + clickForwardButton(0); + }); + + // move is enabled when moving a component. + expect(modal.$el.find('.modal-actions .action-move').hasClass('is-disabled')).toBeFalsy(); + }); + it('is disabled when navigating to any non-parentable component', function() { var nonParentableXBlockInfo = { category: 'html', @@ -651,7 +689,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe it('can not move in a disabled state', function() { verifyMoveEnabled(false); modal.$el.find('.modal-actions .action-move').click(); - expect(modal.movedAlertView).toBeNull(); + expect(getMovedAlertNotification().html().length).toEqual(0); expect(getSentRequests().length).toEqual(0); }); @@ -662,7 +700,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe it('do not move an xblock when cancel button is clicked', function() { modal.$el.find('.modal-actions .action-cancel').click(); - expect(modal.movedAlertView).toBeNull(); + expect(getMovedAlertNotification().html().length).toEqual(0); expect(getSentRequests().length).toEqual(0); }); @@ -670,13 +708,13 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe var sourceIndex = 0, requests = AjaxHelpers.requests(this); moveXBlockWithSuccess(requests); - modal.movedAlertView.$el.find('.action-save').click(); + getMovedAlertNotification().find('.action-save').click(); AjaxHelpers.respondWithJson(requests, { move_source_locator: sourceLocator, parent_locator: sourceParentLocator, target_index: sourceIndex }); - verifyUndoConfirmationFeedbackTitleHtml(sourceDisplayName); + verifyUndoConfirmationFeedbackTitleText(sourceDisplayName); }); }); @@ -698,7 +736,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe requests = AjaxHelpers.requests(this); moveXBlockWithSuccess(requests); notificationSpy = ViewHelpers.createNotificationSpy(); - modal.movedAlertView.$el.find('.action-save').click(); + getMovedAlertNotification().find('.action-save').click(); verifyNotificationStatus(requests, notificationSpy, 'Undo moving'); }); @@ -719,7 +757,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe var requests = AjaxHelpers.requests(this), notificationSpy = ViewHelpers.createNotificationSpy('Error'); moveXBlockWithSuccess(requests); - modal.movedAlertView.$el.find('.action-save').click(); + getMovedAlertNotification().find('.action-save').click(); AjaxHelpers.respondWithError(requests); ViewHelpers.verifyNotificationShowing(notificationSpy, "Studio's having trouble saving your work"); }); diff --git a/cms/static/js/spec/views/pages/container_spec.js b/cms/static/js/spec/views/pages/container_spec.js index 717e5318f770..4b56b837b66c 100644 --- a/cms/static/js/spec/views/pages/container_spec.js +++ b/cms/static/js/spec/views/pages/container_spec.js @@ -49,6 +49,9 @@ define(['jquery', 'underscore', 'underscore.string', 'edx-ui-toolkit/js/utils/sp afterEach(function() { EditHelpers.uninstallMockXBlock(); + if (containerPage !== undefined) { + containerPage.remove(); + } }); respondWithHtml = function(html) { diff --git a/cms/static/js/spec/views/pages/container_subviews_spec.js b/cms/static/js/spec/views/pages/container_subviews_spec.js index 947e73c1dc13..13bfc8d3e271 100644 --- a/cms/static/js/spec/views/pages/container_subviews_spec.js +++ b/cms/static/js/spec/views/pages/container_subviews_spec.js @@ -35,6 +35,9 @@ define(['jquery', 'underscore', 'underscore.string', 'edx-ui-toolkit/js/utils/sp afterEach(function() { delete window.course; + if (containerPage !== undefined) { + containerPage.remove(); + } }); defaultXBlockInfo = { diff --git a/cms/static/js/views/modals/move_xblock_modal.js b/cms/static/js/views/modals/move_xblock_modal.js index 932dd86c2c40..2da157b4be96 100644 --- a/cms/static/js/views/modals/move_xblock_modal.js +++ b/cms/static/js/views/modals/move_xblock_modal.js @@ -41,7 +41,6 @@ function($, Backbone, _, gettext, BaseView, XBlockViewUtils, MoveXBlockUtils, Ht initialize: function() { var self = this; BaseModal.prototype.initialize.call(this); - this.listenTo(Backbone, 'move:breadcrumbRendered', this.focusModal); this.sourceXBlockInfo = this.options.sourceXBlockInfo; this.sourceParentXBlockInfo = this.options.sourceParentXBlockInfo; this.targetParentXBlockInfo = null; @@ -57,9 +56,9 @@ function($, Backbone, _, gettext, BaseView, XBlockViewUtils, MoveXBlockUtils, Ht $('.breadcrumb-container').removeClass('is-hidden'); self.renderViews(courseOutlineInfo, ancestorInfo); }); - this.movedAlertView = null; - this.isValidMove = false; + this.listenTo(Backbone, 'move:breadcrumbRendered', this.focusModal); this.listenTo(Backbone, 'move:enableMoveOperation', this.enableMoveOperation); + this.listenTo(Backbone, 'move:hideMoveModal', this.hide); }, getTitle: function() { @@ -137,15 +136,22 @@ function($, Backbone, _, gettext, BaseView, XBlockViewUtils, MoveXBlockUtils, Ht } }, - isValidCategory: function(sourceParentType, targetParentType, targetHasChildren) { - var basicBlockTypes = ['course', 'chapter', 'sequential', 'vertical']; + isValidCategory: function(targetParentXBlockInfo) { + var basicBlockTypes = ['course', 'chapter', 'sequential', 'vertical'], + sourceParentType = this.sourceParentXBlockInfo.get('category'), + targetParentType = targetParentXBlockInfo.get('category'), + sourceParentHasChildren = this.sourceParentXBlockInfo.get('has_children'), + targetParentHasChildren = targetParentXBlockInfo.get('has_children'); + // Treat source parent component as vertical to support move child components under content experiment // and other similar xblocks. - // eslint-disable-next-line no-param-reassign - sourceParentType = sourceParentType === 'split_test' ? 'vertical' : sourceParentType; + if (sourceParentHasChildren && !_.contains(basicBlockTypes, sourceParentType)) { + sourceParentType = 'vertical'; // eslint-disable-line no-param-reassign + } + // Treat target parent component as a vertical to support move to parentable target parent components. // Also, moving a component directly to content experiment is not allowed, we need to visit to group level. - if (targetHasChildren && !_.contains(basicBlockTypes, targetParentType) && + if (targetParentHasChildren && !_.contains(basicBlockTypes, targetParentType) && targetParentType !== 'split_test') { targetParentType = 'vertical'; // eslint-disable-line no-param-reassign } @@ -153,44 +159,28 @@ function($, Backbone, _, gettext, BaseView, XBlockViewUtils, MoveXBlockUtils, Ht }, enableMoveOperation: function(targetParentXBlockInfo) { - var isValidMove = false, - sourceParentType = this.sourceParentXBlockInfo.get('category'), - targetParentType = targetParentXBlockInfo.get('category'), - targetHasChildren = targetParentXBlockInfo.get('has_children'); + var isValidMove = false; - if (this.isValidCategory(sourceParentType, targetParentType, targetHasChildren) && + // update target parent on navigation + this.targetParentXBlockInfo = targetParentXBlockInfo; + if (this.isValidCategory(targetParentXBlockInfo) && this.sourceParentXBlockInfo.id !== targetParentXBlockInfo.id && // same parent case this.sourceXBlockInfo.id !== targetParentXBlockInfo.id) { // same source item case isValidMove = true; - this.targetParentXBlockInfo = targetParentXBlockInfo; } this.updateMoveState(isValidMove); }, moveXBlock: function() { - var self = this; - XBlockViewUtils.moveXBlock(self.sourceXBlockInfo.id, self.targetParentXBlockInfo.id) - .done(function(response) { - // hide modal - self.hide(); - // hide xblock element - $("li.studio-xblock-wrapper[data-locator='" + self.sourceXBlockInfo.id + "']").hide(); - self.movedAlertView = MoveXBlockUtils.showMovedNotification( - StringUtils.interpolate( - gettext('Success! "{displayName}" has been moved.'), - { - displayName: self.sourceXBlockInfo.get('display_name') - } - ), - { - sourceDisplayName: self.sourceXBlockInfo.get('display_name'), - sourceLocator: self.sourceXBlockInfo.id, - sourceParentLocator: self.sourceParentXBlockInfo.id, - targetParentLocator: response.parent_locator, - targetIndex: response.source_index - } - ); - }); + MoveXBlockUtils.moveXBlock( + { + sourceXBlockElement: $("li.studio-xblock-wrapper[data-locator='" + this.sourceXBlockInfo.id + "']"), + sourceDisplayName: this.sourceXBlockInfo.get('display_name'), + sourceLocator: this.sourceXBlockInfo.id, + sourceParentLocator: this.sourceParentXBlockInfo.id, + targetParentLocator: this.targetParentXBlockInfo.id + } + ); } }); diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 6334ace61885..f26723270b43 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -2,11 +2,12 @@ * XBlockContainerPage is used to display Studio's container page for an xblock which has children. * This page allows the user to understand and manipulate the xblock and its children. */ -define(['jquery', 'underscore', 'gettext', 'js/views/pages/base_page', 'common/js/components/utils/view_utils', - 'js/views/container', 'js/views/xblock', 'js/views/components/add_xblock', 'js/views/modals/edit_xblock', - 'js/views/modals/move_xblock_modal', 'js/models/xblock_info', 'js/views/xblock_string_field_editor', - 'js/views/pages/container_subviews', 'js/views/unit_outline', 'js/views/utils/xblock_utils'], - function($, _, gettext, BasePage, ViewUtils, ContainerView, XBlockView, AddXBlockComponent, +define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page', + 'common/js/components/utils/view_utils', 'js/views/container', 'js/views/xblock', + 'js/views/components/add_xblock', 'js/views/modals/edit_xblock', 'js/views/modals/move_xblock_modal', + 'js/models/xblock_info', 'js/views/xblock_string_field_editor', 'js/views/pages/container_subviews', + 'js/views/unit_outline', 'js/views/utils/xblock_utils'], + function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView, AddXBlockComponent, EditXBlockModal, MoveXBlockModal, XBlockInfo, XBlockStringFieldEditor, ContainerSubviews, UnitOutlineView, XBlockUtils) { 'use strict'; @@ -81,6 +82,8 @@ define(['jquery', 'underscore', 'gettext', 'js/views/pages/base_page', 'common/j }); this.unitOutlineView.render(); } + + this.listenTo(Backbone, 'move:onXBlockMoved', this.onXBlockMoved); }, getViewParameters: function() { @@ -283,6 +286,13 @@ define(['jquery', 'underscore', 'gettext', 'js/views/pages/base_page', 'common/j this.model.fetch(); }, + /* + After move operation is complete, updates the xblock information from server . + */ + onXBlockMoved: function() { + this.model.fetch(); + }, + onNewXBlock: function(xblockElement, scrollOffset, is_duplicate, data) { ViewUtils.setScrollOffset(xblockElement, scrollOffset); xblockElement.data('locator', data.locator); diff --git a/cms/static/js/views/utils/move_xblock_utils.js b/cms/static/js/views/utils/move_xblock_utils.js index 820c8abc59b9..e3344c5cb801 100644 --- a/cms/static/js/views/utils/move_xblock_utils.js +++ b/cms/static/js/views/utils/move_xblock_utils.js @@ -4,25 +4,53 @@ define([ 'jquery', 'underscore', + 'backbone', 'common/js/components/views/feedback', 'common/js/components/views/feedback_alert', 'js/views/utils/xblock_utils', 'js/views/utils/move_xblock_utils', 'edx-ui-toolkit/js/utils/string-utils' ], -function($, _, Feedback, AlertView, XBlockViewUtils, MoveXBlockUtils, StringUtils) { +function($, _, Backbone, Feedback, AlertView, XBlockViewUtils, MoveXBlockUtils, StringUtils) { 'use strict'; - var redirectLink, undoMoveXBlock, showMovedNotification, hideMovedNotification; + var redirectLink, moveXBlock, undoMoveXBlock, showMovedNotification, hideMovedNotification; redirectLink = function(link) { window.location.href = link; }; + moveXBlock = function(data) { + XBlockViewUtils.moveXBlock(data.sourceLocator, data.targetParentLocator) + .done(function(response) { + // hide modal + Backbone.trigger('move:hideMoveModal'); + // hide xblock element + data.sourceXBlockElement.hide(); + showMovedNotification( + StringUtils.interpolate( + gettext('Success! "{displayName}" has been moved.'), + { + displayName: data.sourceDisplayName + } + ), + { + sourceXBlockElement: data.sourceXBlockElement, + sourceDisplayName: data.sourceDisplayName, + sourceLocator: data.sourceLocator, + sourceParentLocator: data.sourceParentLocator, + targetParentLocator: data.targetParentLocator, + targetIndex: response.source_index + } + ); + Backbone.trigger('move:onXBlockMoved'); + }); + }; + undoMoveXBlock = function(data) { XBlockViewUtils.moveXBlock(data.sourceLocator, data.sourceParentLocator, data.targetIndex) - .done(function(response) { + .done(function() { // show XBlock element - $('.studio-xblock-wrapper[data-locator="' + response.move_source_locator + '"]').show(); + data.sourceXBlockElement.show(); showMovedNotification( StringUtils.interpolate( gettext('Move cancelled. "{sourceDisplayName}" has been moved back to its original location.'), @@ -31,6 +59,7 @@ function($, _, Feedback, AlertView, XBlockViewUtils, MoveXBlockUtils, StringUtil } ) ); + Backbone.trigger('move:onXBlockMoved'); }); }; @@ -44,15 +73,10 @@ function($, _, Feedback, AlertView, XBlockViewUtils, MoveXBlockUtils, StringUtil primary: { text: gettext('Undo move'), class: 'action-save', - data: JSON.stringify({ - sourceDisplayName: data.sourceDisplayName, - sourceLocator: data.sourceLocator, - sourceParentLocator: data.sourceParentLocator, - targetIndex: data.targetIndex - }), click: function() { undoMoveXBlock( { + sourceXBlockElement: data.sourceXBlockElement, sourceDisplayName: data.sourceDisplayName, sourceLocator: data.sourceLocator, sourceParentLocator: data.sourceParentLocator, @@ -65,9 +89,6 @@ function($, _, Feedback, AlertView, XBlockViewUtils, MoveXBlockUtils, StringUtil { text: gettext('Take me to the new location'), class: 'action-cancel', - data: JSON.stringify({ - targetParentLocator: data.targetParentLocator - }), click: function() { redirectLink('/container/' + data.targetParentLocator); } @@ -100,6 +121,8 @@ function($, _, Feedback, AlertView, XBlockViewUtils, MoveXBlockUtils, StringUtil return { redirectLink: redirectLink, + moveXBlock: moveXBlock, + undoMoveXBlock: undoMoveXBlock, showMovedNotification: showMovedNotification, hideMovedNotification: hideMovedNotification }; diff --git a/cms/static/js/views/utils/xblock_utils.js b/cms/static/js/views/utils/xblock_utils.js index 860d055f3ad8..970ad6162dd4 100644 --- a/cms/static/js/views/utils/xblock_utils.js +++ b/cms/static/js/views/utils/xblock_utils.js @@ -272,7 +272,8 @@ define(['jquery', 'underscore', 'gettext', 'common/js/components/utils/view_util findXBlockInfo = function(xblockWrapperElement, defaultXBlockInfo) { var xblockInfo = defaultXBlockInfo, xblockElement, - displayName; + displayName, + hasChildren; if (xblockWrapperElement.length > 0) { xblockElement = xblockWrapperElement.find('.xblock'); displayName = xblockWrapperElement.find( @@ -283,11 +284,13 @@ define(['jquery', 'underscore', 'gettext', 'common/js/components/utils/view_util if (!displayName) { displayName = xblockElement.find('.component-header').text().trim(); } + hasChildren = defaultXBlockInfo ? defaultXBlockInfo.get('has_children') : false; xblockInfo = new XBlockInfo({ id: xblockWrapperElement.data('locator'), courseKey: xblockWrapperElement.data('course-key'), category: xblockElement.data('block-type'), - display_name: displayName + display_name: displayName, + has_children: hasChildren }); } return xblockInfo; diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index 85a6f83a77cb..99d1e41fa892 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -229,6 +229,18 @@ def view_published_version(self): self.q(css='.button-view').first.click() self._switch_to_lms() + def verify_publish_title(self, expected_title): + """ + Waits for the publish title to change to the expected value. + """ + def wait_for_title_change(): + """ + Promise function to check publish title. + """ + return (self.publish_title == expected_title, self.publish_title) + + Promise(wait_for_title_change, "Publish title incorrect. Found '" + self.publish_title + "'").fulfill() + def preview(self): """ Clicks "Preview", which will open the draft version of the unit page in the LMS. diff --git a/common/test/acceptance/tests/studio/test_studio_container.py b/common/test/acceptance/tests/studio/test_studio_container.py index e496a14a0921..ab3ae1e9283c 100644 --- a/common/test/acceptance/tests/studio/test_studio_container.py +++ b/common/test/acceptance/tests/studio/test_studio_container.py @@ -664,7 +664,7 @@ def test_publishing(self): And the last saved text contains "Last published" """ unit = self.go_to_unit_page() - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) # Start date set in course fixture to 1970. self._verify_release_date_info( unit, self.RELEASE_TITLE_RELEASED, 'Jan 01, 1970 at 00:00 UTC\nwith Section "Test Section"' @@ -675,11 +675,11 @@ def test_publishing(self): # Add a component to the page so it will have unpublished changes. add_discussion(unit) - self._verify_publish_title(unit, self.DRAFT_STATUS) + unit.verify_publish_title(self.DRAFT_STATUS) self._verify_last_published_and_saved(unit, self.LAST_PUBLISHED, self.LAST_SAVED) unit.publish_action.click() unit.wait_for_ajax() - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) self._verify_last_published_and_saved(unit, self.LAST_PUBLISHED, self.LAST_PUBLISHED) def test_discard_changes(self): @@ -696,9 +696,9 @@ def test_discard_changes(self): """ unit = self.go_to_unit_page() add_discussion(unit) - self._verify_publish_title(unit, self.DRAFT_STATUS) + unit.verify_publish_title(self.DRAFT_STATUS) unit.discard_changes() - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) def test_view_live_no_changes(self): """ @@ -757,7 +757,7 @@ def test_initially_unlocked_visible_to_students(self): Then I see the content in the unit """ unit = self.go_to_unit_page("Unlocked Section", "Unlocked Subsection", "Unlocked Unit") - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) self.assertTrue(unit.currently_visible_to_students) self._verify_release_date_info( unit, self.RELEASE_TITLE_RELEASED, self.past_start_date_text + '\n' + 'with Section "Unlocked Section"' @@ -783,7 +783,7 @@ def test_locked_visible_to_staff_only(self): self.assertTrue(checked) self.assertFalse(unit.currently_visible_to_students) self.assertFalse(unit.shows_inherited_staff_lock()) - self._verify_publish_title(unit, self.LOCKED_STATUS) + unit.verify_publish_title(self.LOCKED_STATUS) self._view_published_version(unit) # Will initially be in staff view, locked component should be visible. self._verify_components_visible(['problem']) @@ -802,7 +802,7 @@ def test_initially_locked_not_visible_to_students(self): Then I do not see any content in the unit """ unit = self.go_to_unit_page("Section With Locked Unit", "Subsection With Locked Unit", "Locked Unit") - self._verify_publish_title(unit, self.LOCKED_STATUS) + unit.verify_publish_title(self.LOCKED_STATUS) self.assertFalse(unit.currently_visible_to_students) self._verify_release_date_info( unit, self.RELEASE_TITLE_RELEASE, @@ -826,7 +826,7 @@ def test_unlocked_visible_to_all(self): unit = self.go_to_unit_page("Section With Locked Unit", "Subsection With Locked Unit", "Locked Unit") checked = unit.toggle_staff_lock() self.assertFalse(checked) - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) self.assertTrue(unit.currently_visible_to_students) self._view_published_version(unit) # Will initially be in staff view, components always visible. @@ -894,10 +894,10 @@ def test_published_unit_with_draft_child(self): component.edit() HtmlComponentEditorView(self.browser, component.locator).set_content_and_save(modified_content) self.assertEqual(component.student_content, modified_content) - self._verify_publish_title(unit, self.DRAFT_STATUS) + unit.verify_publish_title(self.DRAFT_STATUS) unit.publish_action.click() unit.wait_for_ajax() - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) self._view_published_version(unit) self.assertIn(modified_content, self.courseware.xblock_component_html_content(0)) @@ -917,10 +917,10 @@ def test_cancel_does_not_create_draft(self): component.edit() HtmlComponentEditorView(self.browser, component.locator).set_content_and_cancel("modified content") self.assertEqual(component.student_content, "Body of HTML Unit.") - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) self.browser.refresh() unit.wait_for_page() - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) def test_delete_child_in_published_unit(self): """ @@ -936,10 +936,10 @@ def test_delete_child_in_published_unit(self): """ unit = self.go_to_unit_page() unit.delete(0) - self._verify_publish_title(unit, self.DRAFT_STATUS) + unit.verify_publish_title(self.DRAFT_STATUS) unit.publish_action.click() unit.wait_for_ajax() - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) self._view_published_version(unit) self.assertEqual(0, self.courseware.num_xblock_components) @@ -955,12 +955,12 @@ def test_published_not_live(self): Then the title in the Publish information box is "Published (not yet released)" """ unit = self.go_to_unit_page('Unreleased Section', 'Unreleased Subsection', 'Unreleased Unit') - self._verify_publish_title(unit, self.PUBLISHED_STATUS) + unit.verify_publish_title(self.PUBLISHED_STATUS) add_discussion(unit) - self._verify_publish_title(unit, self.DRAFT_STATUS) + unit.verify_publish_title(self.DRAFT_STATUS) unit.publish_action.click() unit.wait_for_ajax() - self._verify_publish_title(unit, self.PUBLISHED_STATUS) + unit.verify_publish_title(self.PUBLISHED_STATUS) def _view_published_version(self, unit): """ @@ -1007,15 +1007,6 @@ def _verify_release_date_info(self, unit, expected_title, expected_date): self.assertEqual(expected_title, unit.release_title) self.assertEqual(expected_date, unit.release_date) - def _verify_publish_title(self, unit, expected_title): - """ - Waits for the publish title to change to the expected value. - """ - def wait_for_title_change(): - return (unit.publish_title == expected_title, unit.publish_title) - - Promise(wait_for_title_change, "Publish title incorrect. Found '" + unit.publish_title + "'").fulfill() - def _verify_last_published_and_saved(self, unit, expected_published_prefix, expected_saved_prefix): """ Verifies that last published and last saved messages respectively contain the given strings. @@ -1144,6 +1135,9 @@ class MoveComponentTest(ContainerBase): """ Tests of moving an XBlock to another XBlock. """ + PUBLISHED_LIVE_STATUS = "Publishing Status\nPublished and Live" + DRAFT_STATUS = "Publishing Status\nDraft (Unpublished changes)" + def setUp(self, is_staff=True): super(MoveComponentTest, self).setUp(is_staff=is_staff) self.container = ContainerPage(self.browser, None) @@ -1181,26 +1175,36 @@ def populate_course_fixture(self, course_fixture): ) ) - def verify_move_opertions(self, unit_page, source_component, operation, component_display_names_after_operation): + def verify_move_opertions(self, unit_page, source_component, operation, component_display_names_after_operation, + should_verify_publish_title=True): """ Verify move operations. Arguments: unit_page (Object) Unit container page. - source_component (Object) source XBlock object to be moved. + source_component (Object) Source XBlock object to be moved. operation (str), `move` or `undo move` operation. - component_display_names_after_operation (dict) display names of components after operation in source/dest + component_display_names_after_operation (dict) Display names of components after operation in source/dest + should_verify_publish_title (Boolean) Should verify publish title ot not. Default is True. """ source_component.open_move_modal() self.move_modal_view.navigate_to_category(self.source_xblock_category, self.navigation_options) self.assertEqual(self.move_modal_view.is_move_button_enabled, True) + # Verify unit is in published state before move operation + if should_verify_publish_title: + self.container.verify_publish_title(self.PUBLISHED_LIVE_STATUS) + self.move_modal_view.click_move_button() self.container.verify_confirmation_message( self.message_move.format(display_name=self.source_component_display_name) ) self.assertEqual(len(unit_page.displayed_children), 1) + # Verify unit in draft state now + if should_verify_publish_title: + self.container.verify_publish_title(self.DRAFT_STATUS) + if operation == 'move': self.container.click_take_me_there_link() elif operation == 'undo_move': @@ -1333,7 +1337,8 @@ def test_content_experiment(self): unit_page=group_container_page, source_component=components[0], operation='undo_move', - component_display_names_after_operation=['HTML 311', 'HTML 312'] + component_display_names_after_operation=['HTML 311', 'HTML 312'], + should_verify_publish_title=False ) # Verify move operation for content experiment. @@ -1341,7 +1346,8 @@ def test_content_experiment(self): unit_page=group_container_page, source_component=components[0], operation='move', - component_display_names_after_operation=['HTML 21', 'HTML 22', 'HTML 311'] + component_display_names_after_operation=['HTML 21', 'HTML 22', 'HTML 311'], + should_verify_publish_title=False ) def test_a11y(self):