Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #4913 from edx/andya/handle-component-errors

Browse Source 
Fix Studio to gracefully handle xblock JavaScript errors
This commit is contained in:
Andy Armstrong
2014-08-21 18:20:16 -04:00
parent 30fc0441d1 d451c3e91d
commit 3ac3a3152a
13 changed files with 281 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
cms/static/js/spec/views/modals/edit_xblock_spec.js
View File

@@ -71,8 +71,8 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers
refreshed = true;
};
modal = showModal(requests, mockXBlockEditorHtml, { refresh: refresh });
modal.runtime.notify('save', { state: 'start' });
modal.runtime.notify('save', { state: 'end' });
modal.editorView.notifyRuntime('save', { state: 'start' });
modal.editorView.notifyRuntime('save', { state: 'end' });
expect(edit_helpers.isShowingModal(modal)).toBeFalsy();
expect(refreshed).toBeTruthy();
});
@@ -84,7 +84,7 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers
refreshed = true;
};
modal = showModal(requests, mockXBlockEditorHtml, { refresh: refresh });
modal.runtime.notify('cancel');
modal.editorView.notifyRuntime('cancel');
expect(edit_helpers.isShowingModal(modal)).toBeFalsy();
expect(refreshed).toBeFalsy();
});

59
cms/static/js/spec/views/pages/container_spec.js
View File

@@ -7,6 +7,8 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin
model, containerPage, requests, initialDisplayName,
mockContainerPage = readFixtures('mock/mock-container-page.underscore'),
mockContainerXBlockHtml = readFixtures('mock/mock-container-xblock.underscore'),
mockBadContainerXBlockHtml = readFixtures('mock/mock-bad-javascript-container-xblock.underscore'),
mockBadXBlockContainerXBlockHtml = readFixtures('mock/mock-bad-xblock-container-xblock.underscore'),
mockUpdatedContainerXBlockHtml = readFixtures('mock/mock-updated-container-xblock.underscore'),
mockXBlockEditorHtml = readFixtures('mock/mock-xblock-editor.underscore');
@@ -15,6 +17,7 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin
edit_helpers.installEditTemplates();
edit_helpers.installTemplate('xblock-string-field-editor');
edit_helpers.installTemplate('container-message');
appendSetFixtures(mockContainerPage);
edit_helpers.installMockXBlock({
@@ -83,6 +86,18 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin
expect(containerPage.$('.ui-loading')).toHaveClass('is-hidden');
});
it('can show an xblock with broken JavaScript', function() {
renderContainerPage(this, mockBadContainerXBlockHtml);
expect(containerPage.$('.wrapper-xblock .level-nesting')).not.toHaveClass('is-hidden');
expect(containerPage.$('.ui-loading')).toHaveClass('is-hidden');
});
it('can show an xblock with an invalid XBlock', function() {
renderContainerPage(this, mockBadXBlockContainerXBlockHtml);
expect(containerPage.$('.wrapper-xblock .level-nesting')).not.toHaveClass('is-hidden');
expect(containerPage.$('.ui-loading')).toHaveClass('is-hidden');
});
it('inline edits the display name when performing a new action', function() {
renderContainerPage(this, mockContainerXBlockHtml, {
action: 'new'
@@ -138,6 +153,9 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin
resources: []
});
// Respond to the subsequent xblock info fetch request.
create_sinon.respondWithJson(requests, {"display_name": updatedDisplayName});
// Expect the title to have been updated
expect(displayNameElement.text().trim()).toBe(updatedDisplayName);
});
@@ -177,6 +195,18 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin
});
expect(edit_helpers.isShowingModal()).toBeTruthy();
});
it('can show an edit modal for a child xblock with broken JavaScript', function() {
var editButtons;
renderContainerPage(this, mockBadContainerXBlockHtml);
editButtons = containerPage.$('.wrapper-xblock .edit-button');
editButtons[0].click();
create_sinon.respondWithJson(requests, {
html: mockXBlockEditorHtml,
resources: []
});
expect(edit_helpers.isShowingModal()).toBeTruthy();
});
});
describe("Editing an xmodule", function() {
@@ -268,10 +298,10 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin
clickDelete(componentIndex);
create_sinon.respondWithJson(requests, {});
// first request contains given component's id (to delete the component)
expect(requests[requests.length - 2].url).toMatch(
new RegExp("locator-component-" + GROUP_TO_TEST + (componentIndex + 1))
);
// second to last request contains given component's id (to delete the component)
create_sinon.expectJsonRequest(requests, 'DELETE',
'/xblock/locator-component-' + GROUP_TO_TEST + (componentIndex + 1),
null, requests.length - 2);
// final request to refresh the xblock info
create_sinon.expectJsonRequest(requests, 'GET', '/xblock/locator-container');
@@ -302,6 +332,18 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin
deleteComponentWithSuccess(NUM_COMPONENTS_PER_GROUP - 1);
});
it("can delete an xblock with broken JavaScript", function() {
renderContainerPage(this, mockBadContainerXBlockHtml);
containerPage.$('.delete-button').first().click();
edit_helpers.confirmPrompt(promptSpy);
create_sinon.respondWithJson(requests, {});
// expect the second to last request to be a delete of the xblock
create_sinon.expectJsonRequest(requests, 'DELETE', '/xblock/locator-broken-javascript',
null, requests.length - 2);
// expect the last request to be a fetch of the xblock info for the parent container
create_sinon.expectJsonRequest(requests, 'GET', '/xblock/locator-container');
});
it('does not delete when clicking No in prompt', function () {
var numRequests;
@@ -387,6 +429,15 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin
duplicateComponentWithSuccess(NUM_COMPONENTS_PER_GROUP - 1);
});
it("can duplicate an xblock with broken JavaScript", function() {
renderContainerPage(this, mockBadContainerXBlockHtml);
containerPage.$('.duplicate-button').first().click();
create_sinon.expectJsonRequest(requests, 'POST', '/xblock/', {
'duplicate_source_locator': 'locator-broken-javascript',
'parent_locator': 'locator-container'
});
});
it('shows a notification when duplicating', function () {
var notificationSpy = edit_helpers.createNotificationSpy();
renderContainerPage(this, mockContainerXBlockHtml);

15
cms/static/js/views/modals/edit_xblock.js
View File

@@ -65,15 +65,10 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie
onDisplayXBlock: function() {
var editorView = this.editorView,
title = this.getTitle(),
xblock = editorView.xblock,
runtime = xblock.runtime;
title = this.getTitle();
// Notify the runtime that the modal has been shown
if (runtime) {
this.runtime = runtime;
runtime.notify('modal-shown', this);
}
editorView.notifyRuntime('modal-shown', this);
// Update the modal's header
if (editorView.hasCustomTabs()) {
@@ -93,7 +88,7 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie
// If the xblock is not using custom buttons then choose which buttons to show
if (!editorView.hasCustomButtons()) {
// If the xblock does not support save then disable the save button
if (!xblock.save) {
if (!editorView.xblock.save) {
this.disableSave();
}
this.getActionBar().show();
@@ -175,9 +170,7 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie
BaseModal.prototype.hide.call(this);
// Notify the runtime that the modal has been hidden
if (this.runtime) {
this.runtime.notify('modal-hidden');
}
this.editorView.notifyRuntime('modal-hidden');
},
findXBlockInfo: function(xblockWrapperElement, defaultXBlockInfo) {

39
cms/static/js/views/pages/container.js
View File

@@ -9,6 +9,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views
function ($, _, gettext, BasePage, ViewUtils, ContainerView, XBlockView, AddXBlockComponent,
EditXBlockModal, XBlockInfo, XBlockStringFieldEditor, ContainerSubviews, UnitOutlineView,
XBlockUtils) {
'use strict';
var XBlockContainerPage = BasePage.extend({
// takes XBlockInfo as a model
@@ -88,14 +89,22 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views
// Render the xblock
xblockView.render({
success: function() {
xblockView.xblock.runtime.notify("page-shown", self);
done: function() {
// Show the xblock and hide the loading indicator
xblockView.$el.removeClass(hiddenCss);
self.renderAddXBlockComponents();
self.onXBlockRefresh(xblockView);
self.refreshDisplayName();
loadingElement.addClass(hiddenCss);
// Notify the runtime that the page has been successfully shown
xblockView.notifyRuntime('page-shown', self);
// Render the add buttons
self.renderAddXBlockComponents();
// Refresh the views now that the xblock is visible
self.onXBlockRefresh(xblockView);
unitLocationTree.removeClass(hiddenCss);
// Re-enable Backbone events for any updated DOM elements
self.delegateEvents();
}
});
@@ -109,11 +118,6 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views
return this.xblockView.model.urlRoot;
},
refreshDisplayName: function() {
var displayName = this.$('.xblock-header .header-details .xblock-display-name').first().text().trim();
this.model.set('display_name', displayName);
},
onXBlockRefresh: function(xblockView) {
this.addButtonActions(xblockView.$el);
this.xblockView.refresh();
@@ -159,6 +163,10 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views
});
},
createPlaceholderElement: function() {
return $("<div/>", { class: "studio-xblock-wrapper" });
},
createComponent: function(template, target) {
// A placeholder element is created in the correct location for the new xblock
// and then onNewXBlock will replace it with a rendering of the xblock. Note that
@@ -168,7 +176,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views
buttonPanel = target.closest('.add-xblock-component'),
listPanel = buttonPanel.prev(),
scrollOffset = ViewUtils.getScrollOffset(buttonPanel),
placeholderElement = $('<div class="studio-xblock-wrapper"></div>').appendTo(listPanel),
placeholderElement = this.createPlaceholderElement().appendTo(listPanel),
requestData = _.extend(template, {
parent_locator: parentLocator
});
@@ -189,7 +197,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views
ViewUtils.runOperationShowingMessage(gettext('Duplicating&hellip;'),
function() {
var scrollOffset = ViewUtils.getScrollOffset(xblockElement),
placeholderElement = $('<div class="studio-xblock-wrapper"></div>').insertAfter(xblockElement),
placeholderElement = self.createPlaceholderElement().insertAfter(xblockElement),
parentElement = self.findXBlockElement(parent),
requestData = {
duplicate_source_locator: xblockElement.data('locator'),
@@ -217,10 +225,13 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views
onDelete: function(xblockElement) {
// get the parent so we can remove this component from its parent.
var xblockView = this.xblockView,
xblock = xblockView.xblock,
parent = this.findXBlockElement(xblockElement.parent());
xblockElement.remove();
xblock.runtime.notify('deleted-child', parent.data('locator'));
// Inform the runtime that the child has been deleted in case
// other views are listening to deletion events.
xblockView.notifyRuntime('deleted-child', parent.data('locator'));
// Update publish and last modified information from the server.
this.model.fetch();
},

65
cms/static/js/views/xblock.js
View File

@@ -29,34 +29,58 @@ define(["jquery", "underscore", "js/views/baseview", "xblock/runtime.v1"],
var self = this,
wrapper = this.$el,
xblockElement,
success = options ? options.success : null,
successCallback = options ? options.success || options.done : null,
errorCallback = options ? options.error || options.done : null,
xblock,
fragmentsRendered;
fragmentsRendered = this.renderXBlockFragment(fragment, wrapper);
fragmentsRendered.done(function() {
fragmentsRendered.always(function() {
xblockElement = self.$('.xblock').first();
xblock = XBlock.initializeBlock(xblockElement);
self.xblock = xblock;
self.xblockReady(xblock);
if (success) {
success(xblock);
try {
xblock = XBlock.initializeBlock(xblockElement);
self.xblock = xblock;
self.xblockReady(xblock);
if (successCallback) {
successCallback(xblock);
}
} catch (e) {
console.error(e.stack);
// Add 'xblock-initialization-failed' class to every xblock
self.$('.xblock').addClass('xblock-initialization-failed');
// If the xblock was rendered but failed then still call xblockReady to allow
// drag-and-drop to be initialized.
if (xblockElement) {
self.xblockReady(null);
}
if (errorCallback) {
errorCallback();
}
}
});
},
/**
* This method is called upon successful rendering of an xblock.
* Sends a notification event to the runtime, if one is available. Note that the runtime
* is only available once the xblock has been rendered and successfully initialized.
* @param eventName The name of the event to be fired.
* @param data The data to be passed to any listener's of the event.
*/
xblockReady: function(xblock) {
// Do nothing
notifyRuntime: function(eventName, data) {
var runtime = this.xblock && this.xblock.runtime;
if (runtime) {
runtime.notify(eventName, data);
}
},
/**
* Returns true if the specified xblock has children.
* This method is called upon successful rendering of an xblock. Note that the xblock
* may have thrown JavaScript errors after rendering in which case the xblock parameter
* will be null.
*/
hasChildXBlocks: function() {
return this.$('.wrapper-xblock').length > 0;
xblockReady: function(xblock) {
// Do nothing
},
/**
@@ -77,9 +101,16 @@ define(["jquery", "underscore", "js/views/baseview", "xblock/runtime.v1"],
}
// Render the HTML first as the scripts might depend upon it, and then
// asynchronously add the resources to the page.
this.updateHtml(element, html);
return this.addXBlockFragmentResources(resources);
// asynchronously add the resources to the page. Any errors that are thrown
// by included scripts are logged to the console but are then ignored assuming
// that at least the rendered HTML will be in place.
try {
this.updateHtml(element, html);
return this.addXBlockFragmentResources(resources);
} catch(e) {
console.error(e.stack);
return $.Deferred().resolve();
}
},
/**
@@ -106,7 +137,7 @@ define(["jquery", "underscore", "js/views/baseview", "xblock/runtime.v1"],
numResources = resources.length;
deferred = $.Deferred();
applyResource = function(index) {
var hash, resource, head, value, promise;
var hash, resource, value, promise;
if (index >= numResources) {
deferred.resolve();
return;