Commit a3e26e68 by Andy Armstrong

Implement next round of code reviews

parent d55ad618
......@@ -273,7 +273,6 @@ def unit_handler(request, tag=None, package_id=None, branch=None, version_guid=N
'context_course': course,
'unit': item,
'unit_locator': locator,
'xblocks': xblocks,
'locators': locators,
'component_templates': component_templates,
'draft_preview_link': preview_lms_link,
......
......@@ -5,6 +5,7 @@ Unit tests for the container view.
from contentstore.tests.utils import CourseTestCase
from contentstore.utils import compute_publish_state, PublishState
from contentstore.views.helpers import xblock_studio_url
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.factories import ItemFactory
......@@ -47,13 +48,33 @@ class ContainerViewTestCase(CourseTestCase):
Create the scenario of an xblock with children (non-vertical) on the container page.
This should create a container page that is a child of another container page.
"""
xblock_with_child = ItemFactory.create(parent_location=self.child_vertical.location,
category="wrapper", display_name="Wrapper")
ItemFactory.create(parent_location=xblock_with_child.location,
category="html", display_name="Child HTML")
published_xblock_with_child = ItemFactory.create(
parent_location=self.child_vertical.location,
category="wrapper", display_name="Wrapper"
)
ItemFactory.create(
parent_location=published_xblock_with_child.location,
category="html", display_name="Child HTML"
)
draft_xblock_with_child = modulestore('draft').convert_to_draft(published_xblock_with_child.location)
branch_name = "MITx.999.Robot_Super_Course/branch/draft/block"
self._test_html_content(
xblock_with_child,
published_xblock_with_child,
branch_name=branch_name,
expected_section_tag=(
'<section class="wrapper-xblock level-page is-hidden" '
'data-locator="{branch_name}/Wrapper">'.format(branch_name=branch_name)
),
expected_breadcrumbs=(
r'<a href="/unit/{branch_name}/Unit"\s*'
r'class="navigation-link navigation-parent">Unit</a>\s*'
r'<a href="/container/{branch_name}/Child_Vertical"\s*'
r'class="navigation-link navigation-parent">Child Vertical</a>\s*'
r'<a href="#" class="navigation-link navigation-current">Wrapper</a>'
).format(branch_name=branch_name)
)
self._test_html_content(
draft_xblock_with_child,
branch_name=branch_name,
expected_section_tag=(
'<section class="wrapper-xblock level-page is-hidden" '
......
......@@ -35,7 +35,7 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers"
containerPage.render();
respondWithMockXBlockEditorFragment(requests, {
html: mockContainerXBlockHtml,
"resources": []
resources: []
});
expect(containerPage.$el.select('.xblock-header')).toBeTruthy();
......@@ -49,7 +49,7 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers"
expect(containerPage.$('.ui-loading')).not.toHaveClass('is-hidden');
respondWithMockXBlockEditorFragment(requests, {
html: mockContainerXBlockHtml,
"resources": []
resources: []
});
expect(containerPage.$('.ui-loading')).toHaveClass('is-hidden');
});
......@@ -84,19 +84,17 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers"
containerPage.render();
respondWithMockXBlockEditorFragment(requests, {
html: mockContainerXBlockHtml,
"resources": []
resources: []
});
editButtons = containerPage.$('.edit-button');
// The container renders four mock xblocks, so there should be four edit buttons
expect(editButtons.length).toBe(4);
// 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/testCourse/branch/draft/block/html447/studio_view'
);
expect(requests[requests.length - 1].url).toBe('/xblock/locator-component-A1/studio_view');
create_sinon.respondWithJson(requests, {
html: mockXBlockEditorHtml,
"resources": []
resources: []
});
expect(edit_helpers.isShowingModal()).toBeTruthy();
});
......@@ -108,15 +106,15 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers"
containerPage.render();
respondWithMockXBlockEditorFragment(requests, {
html: mockContainerXBlockHtml,
"resources": []
resources: []
});
editButtons = containerPage.$('.edit-button');
// The container renders four mock xblocks, so there should be four edit buttons
expect(editButtons.length).toBe(4);
// The container renders six mock xblocks, so there should be an equal number of edit buttons
expect(editButtons.length).toBe(6);
editButtons.first().click();
create_sinon.respondWithJson(requests, {
html: mockXBlockEditorHtml,
"resources": []
resources: []
});
modal = $('.edit-xblock-modal');
......@@ -133,11 +131,10 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers"
// Respond to the request to refresh
respondWithMockXBlockEditorFragment(requests, {
html: mockUpdatedXBlockHtml,
"resources": []
resources: []
});
// Verify that the xblock was updated
expect(containerPage.$('.mock-updated-content').text()).toBe('Mock Update');
expect(edit_helpers.hasSavedMockXBlock()).toBe(true);
});
});
......@@ -149,7 +146,7 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/edit_helpers"
containerPage.render();
respondWithMockXBlockEditorFragment(requests, {
html: mockContainerXBlockHtml,
"resources": []
resources: []
});
expect(containerPage.$('.no-container-content')).not.toHaveClass('is-hidden');
......
......@@ -43,7 +43,7 @@ define([ "jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helper
editor.render();
create_sinon.respondWithJson(requests, {
html: mockXBlockEditorHtml,
"resources": []
resources: []
});
expect(editor.$el.select('.xblock-header')).toBeTruthy();
......@@ -55,12 +55,11 @@ define([ "jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helper
editor.render();
create_sinon.respondWithJson(requests, {
html: mockXBlockEditorHtml,
"resources": []
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');
});
......@@ -84,7 +83,7 @@ define([ "jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helper
editor.render();
create_sinon.respondWithJson(requests, {
html: mockXModuleEditorHtml,
"resources": []
resources: []
});
expect(editor.$el.select('.xblock-header')).toBeTruthy();
......@@ -96,14 +95,13 @@ define([ "jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helper
editor.render();
create_sinon.respondWithJson(requests, {
html: mockXModuleEditorHtml,
"resources": []
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');
});
......@@ -115,7 +113,7 @@ define([ "jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helper
editor.render();
create_sinon.respondWithJson(requests, {
html: mockXModuleEditorHtml,
"resources": []
resources: []
});
expect(editor.$el.select('.xblock-header')).toBeTruthy();
......
......@@ -28,7 +28,7 @@ define([ "jquery", "js/spec_helpers/create_sinon", "URI", "js/views/xblock", "js
xblockView.render();
respondWithMockXBlockFragment(requests, {
html: mockXBlockHtml,
"resources": []
resources: []
});
expect(xblockView.$el.select('.xblock-header')).toBeTruthy();
......
......@@ -10,11 +10,9 @@ 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,
......@@ -22,11 +20,9 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/modal_helpers
showEditModal;
installMockXBlock = function(mockResult) {
xblockSaved = false;
window.MockXBlock = function(runtime, element) {
return {
save: function() {
xblockSaved = true;
return mockResult;
}
};
......@@ -37,15 +33,9 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/modal_helpers
window.MockXBlock = null;
};
hasSavedMockXBlock = function() {
return xblockSaved;
};
installMockXModule = function(mockResult) {
xmoduleSaved = false;
window.MockDescriptor = _.extend(XModule.Descriptor, {
save: function() {
xmoduleSaved = true;
return mockResult;
}
});
......@@ -55,10 +45,6 @@ define(["jquery", "js/spec_helpers/create_sinon", "js/spec_helpers/modal_helpers
window.MockDescriptor = null;
};
hasSavedMockXModule = function() {
return xmoduleSaved;
};
installEditTemplates = function(append) {
modal_helpers.installModalTemplates(append);
......@@ -84,10 +70,8 @@ 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
......
define(["js/views/baseview", "underscore", "codemirror", "js/views/feedback_notification", "js/views/course_info_helper", "js/utils/modal"],
function(BaseView, _, CodeMirror, NotificationView, CourseInfoHelper, ModalUtils) {
define(["js/views/baseview", "codemirror", "js/views/feedback_notification", "js/views/course_info_helper", "js/utils/modal"],
function(BaseView, CodeMirror, NotificationView, CourseInfoHelper, ModalUtils) {
// the handouts view is dumb right now; it needs tied to a model and all that jazz
var CourseInfoHandoutsView = BaseView.extend({
......
define(["js/views/baseview", "underscore", "codemirror", "js/models/course_update",
define(["js/views/baseview", "codemirror", "js/models/course_update",
"js/views/feedback_prompt", "js/views/feedback_notification", "js/views/course_info_helper", "js/utils/modal"],
function(BaseView, _, CodeMirror, CourseUpdateModel, PromptView, NotificationView, CourseInfoHelper, ModalUtils) {
function(BaseView, CodeMirror, CourseUpdateModel, PromptView, NotificationView, CourseInfoHelper, ModalUtils) {
var CourseInfoUpdateView = BaseView.extend({
// collection is CourseUpdateCollection
......
define(["js/views/baseview", "underscore", "jquery", "js/views/edit_textbook", "js/views/show_textbook"],
function(BaseView, _, $, EditTextbookView, ShowTextbookView) {
define(["js/views/baseview", "jquery", "js/views/edit_textbook", "js/views/show_textbook"],
function(BaseView, $, EditTextbookView, ShowTextbookView) {
var ListTextbooks = BaseView.extend({
initialize: function() {
this.emptyTemplate = this.loadTemplate('no-textbooks');
......
......@@ -139,12 +139,14 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal",
this.$el.html("");
},
findXBlockInfo: function(xblockElement, defaultXBlockInfo) {
var xblockInfo = defaultXBlockInfo;
if (xblockElement.length > 0) {
findXBlockInfo: function(xblockWrapperElement, defaultXBlockInfo) {
var xblockInfo = defaultXBlockInfo,
xblockElement;
if (xblockWrapperElement.length > 0) {
xblockElement = xblockWrapperElement.find('.xblock');
xblockInfo = new XBlockInfo({
id: xblockElement.data('locator'),
category: xblockElement.data('category')
id: xblockWrapperElement.data('locator'),
category: xblockElement.data('block-type')
});
}
return xblockInfo;
......
......@@ -11,7 +11,7 @@ define(["underscore", "js/views/baseview"], function(_, BaseView) {
var view = options.view,
collection = view.collection;
this.view = view;
this.template = this.loadTemplate("paging-footer");
this.template = this.loadTemplate('paging-footer');
collection.bind('add', _.bind(this.render, this));
collection.bind('remove', _.bind(this.render, this));
collection.bind('reset', _.bind(this.render, this));
......
......@@ -10,7 +10,7 @@ define(["underscore", "gettext", "js/views/baseview"], function(_, gettext, Base
var view = options.view,
collection = view.collection;
this.view = view;
this.template = this.loadTemplate("paging-header");
this.template = this.loadTemplate('paging-header');
collection.bind('add', _.bind(this.render, this));
collection.bind('remove', _.bind(this.render, this));
collection.bind('reset', _.bind(this.render, this));
......
......@@ -173,7 +173,7 @@
.modal-window {
.CodeMirror {
height: 390px;
height: 100%;
}
.wrapper-comp-settings {
......
......@@ -60,8 +60,8 @@ require(["domReady!", "jquery", "js/models/module_info", "coffee/src/views/unit"
<article class="unit-body window">
<p class="unit-name-input"><label for="unit-display-name-input">${_("Display Name:")}</label><input type="text" value="${unit.display_name_with_default | h}" id="unit-display-name-input" class="unit-display-name-input" /></p>
<ol class="components">
% for (locator, xblock) in zip(locators, xblocks):
<li class="component" data-locator="${locator}" data-display-name="${xblock.display_name_with_default |h}" data-category="${xblock.category}"/>
% for locator in locators:
<li class="component" data-locator="${locator}"/>
% endfor
<li class="new-component-item adding">
<div class="new-component">
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment