From 2ae28c5db7d2d1bdd51d34d467ce349d9e2aa020 Mon Sep 17 00:00:00 2001 From: Andy Armstrong Date: Tue, 8 Apr 2014 19:35:18 -0400 Subject: [PATCH] Improve unit testing of xblock editing --- .../js/spec/views/modals/edit_xblock_spec.js | 6 +- .../js/spec/views/pages/container_spec.js | 77 +++++++++--------- .../js/spec/views/xblock_editor_spec.js | 78 ++++++++++++------- cms/static/js/spec_helpers/edit_helpers.js | 52 +++++++++++++ cms/static/js/views/xblock_editor.js | 17 ++-- cms/templates/container.html | 4 +- .../js/mock/mock-updated-xblock.underscore | 17 ++++ .../js/mock/mock-xblock-editor.underscore | 8 +- .../js/mock/mock-xmodule-editor.underscore | 3 +- 9 files changed, 178 insertions(+), 84 deletions(-) create mode 100644 cms/templates/js/mock/mock-updated-xblock.underscore diff --git a/cms/static/js/spec/views/modals/edit_xblock_spec.js b/cms/static/js/spec/views/modals/edit_xblock_spec.js index ac33fa59d2..0605b269a4 100644 --- a/cms/static/js/spec/views/modals/edit_xblock_spec.js +++ b/cms/static/js/spec/views/modals/edit_xblock_spec.js @@ -30,13 +30,11 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers mockXBlockEditorHtml = readFixtures('mock/mock-xblock-editor.underscore'); beforeEach(function () { - window.MockXBlock = function(runtime, element) { - return { }; - }; + edit_helpers.installMockXBlock(); }); afterEach(function() { - window.MockXBlock = null; + edit_helpers.uninstallMockXBlock(); }); it('can show itself', function() { diff --git a/cms/static/js/spec/views/pages/container_spec.js b/cms/static/js/spec/views/pages/container_spec.js index 015befe2fb..a969d71997 100644 --- a/cms/static/js/spec/views/pages/container_spec.js +++ b/cms/static/js/spec/views/pages/container_spec.js @@ -1,9 +1,9 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers", "js/views/pages/container", "js/models/xblock_info"], - function ($, create_sinon, edit_helpers, XBlockContainerPage, XBlockInfo) { + function ($, create_sinon, edit_helpers, ContainerPage, XBlockInfo) { - describe("XBlockContainerView", function() { - var model, containerView, respondWithMockXBlockEditorFragment, mockContainerPage; + describe("ContainerPage", function() { + var model, containerPage, respondWithMockXBlockEditorFragment, mockContainerPage; mockContainerPage = readFixtures('mock/mock-container-page.underscore'); @@ -16,7 +16,7 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers" display_name: 'Test Unit', category: 'vertical' }); - containerView = new XBlockContainerPage({ + containerPage = new ContainerPage({ model: model, el: $('#content') }); @@ -32,26 +32,26 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers" it('can render itself', function() { var requests = create_sinon.requests(this); - containerView.render(); + containerPage.render(); respondWithMockXBlockEditorFragment(requests, { html: mockContainerXBlockHtml, "resources": [] }); - expect(containerView.$el.select('.xblock-header')).toBeTruthy(); - expect(containerView.$('.wrapper-xblock')).not.toHaveClass('is-hidden'); - expect(containerView.$('.no-container-content')).toHaveClass('is-hidden'); + 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); - containerView.render(); - expect(containerView.$('.ui-loading')).not.toHaveClass('is-hidden'); + containerPage.render(); + expect(containerPage.$('.ui-loading')).not.toHaveClass('is-hidden'); respondWithMockXBlockEditorFragment(requests, { html: mockContainerXBlockHtml, "resources": [] }); - expect(containerView.$('.ui-loading')).toHaveClass('is-hidden'); + expect(containerPage.$('.ui-loading')).toHaveClass('is-hidden'); }); }); @@ -59,28 +59,19 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers" describe("Editing an xblock", function() { var mockContainerXBlockHtml, mockXBlockEditorHtml, - saved, newDisplayName = 'New Display Name'; beforeEach(function () { - saved = false; - window.MockXBlock = function(runtime, element) { - return { - save: function() { - saved = true; - return { - data: "

Some HTML

", - metadata: { - display_name: newDisplayName - } - }; - } - }; - }; + edit_helpers.installMockXBlock({ + data: "

Some HTML

", + metadata: { + display_name: newDisplayName + } + }); }); afterEach(function() { - window.MockXBlock = null; + edit_helpers.uninstallMockXBlock(); edit_helpers.cancelModalIfShowing(); }); @@ -90,12 +81,12 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers" it('can show an edit modal for a child xblock', function() { var requests = create_sinon.requests(this), editButtons; - containerView.render(); + containerPage.render(); respondWithMockXBlockEditorFragment(requests, { html: mockContainerXBlockHtml, "resources": [] }); - editButtons = containerView.$('.edit-button'); + editButtons = containerPage.$('.edit-button'); // The container renders four mock xblocks, so there should be four edit buttons expect(editButtons.length).toBe(4); editButtons.first().click(); @@ -111,14 +102,15 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers" }); it('can save changes to settings', function() { - var requests, editButtons, modal; + var requests, editButtons, modal, mockUpdatedXBlockHtml; + mockUpdatedXBlockHtml = readFixtures('mock/mock-updated-xblock.underscore'); requests = create_sinon.requests(this); - containerView.render(); + containerPage.render(); respondWithMockXBlockEditorFragment(requests, { html: mockContainerXBlockHtml, "resources": [] }); - editButtons = containerView.$('.edit-button'); + editButtons = containerPage.$('.edit-button'); // The container renders four mock xblocks, so there should be four edit buttons expect(editButtons.length).toBe(4); editButtons.first().click(); @@ -131,10 +123,21 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers" // Click on the settings tab modal.find('.settings-button').click(); // Change the display name's text - modal.find('.setting-input').text("New display name"); + modal.find('.setting-input').text("Mock Update"); // Press the save button modal.find('.action-save').click(); - expect(saved).toBe(true); + // Respond to the save + create_sinon.respondWithJson(requests, { + id: model.id + }); + // Respond to the request to refresh + respondWithMockXBlockEditorFragment(requests, { + html: mockUpdatedXBlockHtml, + "resources": [] + }); + // Verify that the xblock was updated + expect(containerPage.$('.mock-updated-content').text()).toBe('Mock Update'); + expect(edit_helpers.hasSavedMockXBlock()).toBe(true); }); }); @@ -143,14 +146,14 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers" it('shows the "no children" message', function() { var requests = create_sinon.requests(this); - containerView.render(); + containerPage.render(); respondWithMockXBlockEditorFragment(requests, { html: mockContainerXBlockHtml, "resources": [] }); - expect(containerView.$('.no-container-content')).not.toHaveClass('is-hidden'); - expect(containerView.$('.wrapper-xblock')).toHaveClass('is-hidden'); + expect(containerPage.$('.no-container-content')).not.toHaveClass('is-hidden'); + expect(containerPage.$('.wrapper-xblock')).toHaveClass('is-hidden'); }); }); }); diff --git a/cms/static/js/spec/views/xblock_editor_spec.js b/cms/static/js/spec/views/xblock_editor_spec.js index 7d2b9b47d7..ac313342c3 100644 --- a/cms/static/js/spec/views/xblock_editor_spec.js +++ b/cms/static/js/spec/views/xblock_editor_spec.js @@ -1,9 +1,17 @@ -define([ "jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers", - "js/views/xblock_editor", "js/models/xblock_info"], - function ($, create_sinon, edit_helpers, XBlockEditorView, XBlockInfo) { +define([ "jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers", + "js/views/xblock_editor", "js/models/xblock_info"], + function ($, _, create_sinon, edit_helpers, XBlockEditorView, XBlockInfo) { describe("XBlockEditorView", function() { - var model, editor; + var model, editor, testDisplayName, mockSaveResponse; + + testDisplayName = 'Test Display Name'; + mockSaveResponse = { + data: "

Some HTML

", + metadata: { + display_name: testDisplayName + } + }; beforeEach(function () { edit_helpers.installEditTemplates(); @@ -21,13 +29,11 @@ define([ "jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers var mockXBlockEditorHtml; beforeEach(function () { - window.MockXBlock = function(runtime, element) { - return { }; - }; + edit_helpers.installMockXBlock(mockSaveResponse); }); afterEach(function() { - window.MockXBlock = null; + edit_helpers.uninstallMockXBlock(); }); mockXBlockEditorHtml = readFixtures('mock/mock-xblock-editor.underscore'); @@ -41,7 +47,22 @@ define([ "jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers }); expect(editor.$el.select('.xblock-header')).toBeTruthy(); - expect(editor.getMode()).toEqual('editor'); + expect(editor.getMode()).toEqual('settings'); + }); + + it('saves any custom metadata', function() { + var requests = create_sinon.requests(this), request, response; + editor.render(); + create_sinon.respondWithJson(requests, { + html: mockXBlockEditorHtml, + "resources": [] + }); + editor.save(); + request = requests[requests.length - 1]; + response = JSON.parse(request.requestBody); + expect(edit_helpers.hasSavedMockXBlock()).toBeTruthy(); + expect(response.metadata.display_name).toBe(testDisplayName); + expect(response.metadata.custom_field).toBe('Custom Value'); }); }); @@ -51,12 +72,11 @@ define([ "jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers mockXModuleEditorHtml = readFixtures('mock/mock-xmodule-editor.underscore'); beforeEach(function() { - // Mock the VerticalDescriptor so that the module can be rendered - window.VerticalDescriptor = XModule.Descriptor; + edit_helpers.installMockXModule(mockSaveResponse); }); afterEach(function () { - window.VerticalDescriptor = null; + edit_helpers.uninstallMockXModule(); }); it('can render itself', function() { @@ -70,26 +90,28 @@ define([ "jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers expect(editor.$el.select('.xblock-header')).toBeTruthy(); expect(editor.getMode()).toEqual('editor'); }); - }); - describe("Editing an xmodule (settings only)", function() { - var mockXModuleEditorHtml; - - mockXModuleEditorHtml = readFixtures('mock/mock-xmodule-settings-only-editor.underscore'); - - beforeEach(function() { - edit_helpers.installEditTemplates(); - - // Mock the VerticalDescriptor so that the module can be rendered - window.VerticalDescriptor = XModule.Descriptor; + it('saves any custom metadata', function() { + var requests = create_sinon.requests(this), request, response; + editor.render(); + create_sinon.respondWithJson(requests, { + html: mockXModuleEditorHtml, + "resources": [] + }); + // Give the mock xblock a save method... + editor.xblock.save = window.MockDescriptor.save; + editor.save(); + request = requests[requests.length - 1]; + response = JSON.parse(request.requestBody); + expect(edit_helpers.hasSavedMockXModule()).toBeTruthy(); + expect(response.metadata.display_name).toBe(testDisplayName); + expect(response.metadata.custom_field).toBe('Custom Value'); }); - afterEach(function () { - window.VerticalDescriptor = null; - }); + it('can render a module with only settings', function() { + var requests = create_sinon.requests(this), mockXModuleEditorHtml; + mockXModuleEditorHtml = readFixtures('mock/mock-xmodule-settings-only-editor.underscore'); - it('can render itself', function() { - var requests = create_sinon.requests(this); editor.render(); create_sinon.respondWithJson(requests, { html: mockXModuleEditorHtml, diff --git a/cms/static/js/spec_helpers/edit_helpers.js b/cms/static/js/spec_helpers/edit_helpers.js index 0c291cde28..ba81532f19 100644 --- a/cms/static/js/spec_helpers/edit_helpers.js +++ b/cms/static/js/spec_helpers/edit_helpers.js @@ -10,9 +10,55 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/modal_helpers stringEntryTemplate = readFixtures('metadata-string-entry.underscore'), editXBlockModalTemplate = readFixtures('edit-xblock-modal.underscore'), editorModeButtonTemplate = readFixtures('editor-mode-button.underscore'), + xblockSaved, + installMockXBlock, + uninstallMockXBlock, + hasSavedMockXBlock, + xmoduleSaved, + installMockXModule, + uninstallMockXModule, + hasSavedMockXModule, installEditTemplates, showEditModal; + installMockXBlock = function(mockResult) { + xblockSaved = false; + window.MockXBlock = function(runtime, element) { + return { + save: function() { + xblockSaved = true; + return mockResult; + } + }; + }; + }; + + uninstallMockXBlock = function() { + window.MockXBlock = null; + }; + + hasSavedMockXBlock = function() { + return xblockSaved; + }; + + installMockXModule = function(mockResult) { + xmoduleSaved = false; + window.MockDescriptor = _.extend(XModule.Descriptor, { + save: function() { + xmoduleSaved = true; + return mockResult; + } + }); + }; + + uninstallMockXModule = function() { + window.MockDescriptor = null; + }; + + hasSavedMockXModule = function() { + return xmoduleSaved; + }; + installEditTemplates = function(append) { modal_helpers.installModalTemplates(append); @@ -37,6 +83,12 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/modal_helpers }; return $.extend(modal_helpers, { + 'installMockXBlock': installMockXBlock, + 'hasSavedMockXBlock': hasSavedMockXBlock, + 'uninstallMockXBlock': uninstallMockXBlock, + 'installMockXModule': installMockXModule, + 'hasSavedMockXModule': hasSavedMockXModule, + 'uninstallMockXModule': uninstallMockXModule, 'installEditTemplates': installEditTemplates, 'showEditModal': showEditModal }); diff --git a/cms/static/js/views/xblock_editor.js b/cms/static/js/views/xblock_editor.js index 38f8df1371..336d2fef1e 100644 --- a/cms/static/js/views/xblock_editor.js +++ b/cms/static/js/views/xblock_editor.js @@ -85,8 +85,7 @@ define(["jquery", "underscore", "gettext", "js/views/feedback_notification", "js }, save: function(options) { - var xblock = this.xblock, - xblockInfo = this.model, + var xblockInfo = this.model, data, saving; data = this.getXBlockData(); @@ -131,14 +130,14 @@ define(["jquery", "underscore", "gettext", "js/views/feedback_notification", "js // Walk through the set of elements which have the 'data-metadata_name' attribute and // build up an object to pass back to the server on the subsequent POST. // Note that these values will always be sent back on POST, even if they did not actually change. - var metadata = {}, - metadataNameElements; + var metadata, metadataNameElements, i, element, metadataName; + metadata = {}; metadataNameElements = this.$('[data-metadata-name]'); - metadataNameElements.each(function (element) { - var key = $(element).data("metadata-name"), - value = element.value; - metadata[key] = value; - }); + for (i = 0; i < metadataNameElements.length; i++) { + element = metadataNameElements[i]; + metadataName = $(element).data("metadata-name"); + metadata[metadataName] = element.value; + } return metadata; }, diff --git a/cms/templates/container.html b/cms/templates/container.html index 77917aa343..f3f7f292c5 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -32,12 +32,12 @@ main_xblock_info = {
+