Fixed Backend: Move component directly to content experiment level (component will now be the sibling of groups)

Fixed Move button is disabled when moving components inside conditional module

Fixed When navigating upward/backward, target parent is not set correct, resulting source to be moved in the previously selected parent
This commit is contained in:
Mushtaq Ali
2017-03-03 16:20:27 +05:00
parent de29ef94c6
commit e83bee65bd
12 changed files with 249 additions and 143 deletions

View File

@@ -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

View File

@@ -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("<div id='page-alert'></div>");
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(
'<button class="action-secondary action-cancel">Take me to the new location</button>'
) !== -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(
'<button class="action-primary action-save">Undo Move</button>'
) !== -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");
});

View File

@@ -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) {

View File

@@ -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 = {

View File

@@ -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
}
);
}
});

View File

@@ -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);

View File

@@ -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
};

View File

@@ -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;