diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index bd2a57e1a9..847d9c91af 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -205,8 +205,7 @@ def xblock_view_handler(request, usage_key_string, view_name): if 'application/json' in accept_header: store = modulestore() xblock = store.get_item(usage_key) - container_views = ['container_preview', 'reorderable_container_child_preview'] - library = isinstance(usage_key, LibraryUsageLocator) + container_views = ['container_preview', 'reorderable_container_child_preview', 'container_child_preview'] # wrap the generated fragment in the xmodule_editor div so that the javascript # can bind to it correctly @@ -235,7 +234,7 @@ def xblock_view_handler(request, usage_key_string, view_name): # are being shown in a reorderable container, so the xblock is automatically # added to the list. reorderable_items = set() - if not library and view_name == 'reorderable_container_child_preview': + if view_name == 'reorderable_container_child_preview': reorderable_items.add(xblock.location) paging = None @@ -246,11 +245,15 @@ def xblock_view_handler(request, usage_key_string, view_name): 'page_size': int(request.REQUEST.get('page_size', 0)), } except ValueError: - log.exception( - "Couldn't parse paging parameters: enable_paging: %s, page_number: %s, page_size: %s", - request.REQUEST.get('enable_paging', 'false'), - request.REQUEST.get('page_number', 0), - request.REQUEST.get('page_size', 0) + return HttpResponse( + content="Couldn't parse paging parameters: enable_paging: " + "%s, page_number: %s, page_size: %s".format( + request.REQUEST.get('enable_paging', 'false'), + request.REQUEST.get('page_number', 0), + request.REQUEST.get('page_size', 0) + ), + status=400, + content_type="text/plain", ) # Set up the context to be passed to each XBlock's render method. diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index 1bd1177264..b83442a9a6 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -239,7 +239,7 @@ define([ "js/spec/views/assets_spec", "js/spec/views/baseview_spec", "js/spec/views/container_spec", - "js/spec/views/library_container_spec", + "js/spec/views/paged_container_spec", "js/spec/views/group_configuration_spec", "js/spec/views/paging_spec", "js/spec/views/unit_outline_spec", diff --git a/cms/static/js/factories/container.js b/cms/static/js/factories/container.js index ea48bb2a98..429ae58f51 100644 --- a/cms/static/js/factories/container.js +++ b/cms/static/js/factories/container.js @@ -7,11 +7,11 @@ function($, _, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { 'use strict'; return function (componentTemplates, XBlockInfoJson, action, options) { var main_options = { - el: $('#content'), - model: new XBlockInfo(XBlockInfoJson, {parse: true}), - action: action, - templates: new ComponentTemplates(componentTemplates, {parse: true}) - }; + el: $('#content'), + model: new XBlockInfo(XBlockInfoJson, {parse: true}), + action: action, + templates: new ComponentTemplates(componentTemplates, {parse: true}) + }; xmoduleLoader.done(function () { var view = new ContainerPage(_.extend(main_options, options)); diff --git a/cms/static/js/factories/library.js b/cms/static/js/factories/library.js index e7834f60ef..76ac47413d 100644 --- a/cms/static/js/factories/library.js +++ b/cms/static/js/factories/library.js @@ -1,20 +1,21 @@ define([ - 'jquery', 'underscore', 'js/models/xblock_info', 'js/views/pages/container', - 'js/collections/component_template', 'xmodule', 'coffee/src/main', + 'jquery', 'underscore', 'js/models/xblock_info', 'js/views/pages/paged_container', + 'js/views/library_container', 'js/collections/component_template', 'xmodule', 'coffee/src/main', 'xblock/cms.runtime.v1' ], -function($, _, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { +function($, _, XBlockInfo, PagedContainerPage, LibraryContainerView, ComponentTemplates, xmoduleLoader) { 'use strict'; return function (componentTemplates, XBlockInfoJson, options) { var main_options = { el: $('#content'), model: new XBlockInfo(XBlockInfoJson, {parse: true}), templates: new ComponentTemplates(componentTemplates, {parse: true}), - action: 'view' + action: 'view', + viewClass: LibraryContainerView }; xmoduleLoader.done(function () { - var view = new ContainerPage(_.extend(main_options, options)); + var view = new PagedContainerPage(_.extend(main_options, options)); view.render(); }); }; diff --git a/cms/static/js/spec/views/library_container_spec.js b/cms/static/js/spec/views/paged_container_spec.js similarity index 99% rename from cms/static/js/spec/views/library_container_spec.js rename to cms/static/js/spec/views/paged_container_spec.js index 2d39cdc358..524f88e552 100644 --- a/cms/static/js/spec/views/library_container_spec.js +++ b/cms/static/js/spec/views/paged_container_spec.js @@ -1,6 +1,6 @@ define([ "jquery", "underscore", "js/common_helpers/ajax_helpers", "URI", "js/models/xblock_info", - "js/views/library_container", "js/views/paging_header", "js/views/paging_footer"], - function ($, _, AjaxHelpers, URI, XBlockInfo, PagedContainer, PagingContainer, PagingFooter) { + "js/views/paged_container", "js/views/paging_header", "js/views/paging_footer"], + function ($, _, AjaxHelpers, URI, XBlockInfo, PagedContainer, PagingHeader, PagingFooter) { var htmlResponseTpl = _.template('' + '
' diff --git a/cms/static/js/spec/views/pages/container_spec.js b/cms/static/js/spec/views/pages/container_spec.js index d5ec6938dc..ce862aac7d 100644 --- a/cms/static/js/spec/views/pages/container_spec.js +++ b/cms/static/js/spec/views/pages/container_spec.js @@ -1,7 +1,7 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_helpers", "js/common_helpers/template_helpers", "js/spec_helpers/edit_helpers", - "js/views/pages/container", "js/models/xblock_info", "jquery.simulate"], - function ($, _, str, AjaxHelpers, TemplateHelpers, EditHelpers, ContainerPage, XBlockInfo) { + "js/views/pages/container", "js/views/pages/paged_container", "js/models/xblock_info"], + function ($, _, str, AjaxHelpers, TemplateHelpers, EditHelpers, ContainerPage, PagedContainerPage, XBlockInfo) { function parameterized_suite(label, global_page_options, fixtures) { describe(label + " ContainerPage", function () { @@ -13,7 +13,8 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel 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'); + mockXBlockEditorHtml = readFixtures('mock/mock-xblock-editor.underscore'), + PageClass = fixtures.page; beforeEach(function () { var newDisplayName = 'New Display Name'; @@ -62,7 +63,7 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel templates: EditHelpers.mockComponentTemplates, el: $('#content') }; - return new ContainerPage(_.extend(options || {}, global_page_options, default_options)); + return new PageClass(_.extend(options || {}, global_page_options, default_options)); }; renderContainerPage = function (test, html, options) { @@ -273,7 +274,7 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel }); describe("xblock operations", function () { - var getGroupElement, paginated, + var getGroupElement, paginated, getDeleteOffset, NUM_COMPONENTS_PER_GROUP = 3, GROUP_TO_TEST = "A", allComponentsInGroup = _.map( _.range(NUM_COMPONENTS_PER_GROUP), @@ -283,9 +284,13 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel ); paginated = function () { - return containerPage.enable_paging; + return containerPage instanceof PagedContainerPage; }; + getDeleteOffset = function () { + // Paginated containers will make an additional AJAX request. + return paginated() ? 3 : 2; + }; getGroupElement = function () { return containerPage.$("[data-locator='locator-group-" + GROUP_TO_TEST + "']"); @@ -316,8 +321,6 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel deleteComponent = function (componentIndex, requestOffset) { clickDelete(componentIndex); AjaxHelpers.respondWithJson(requests, {}); - - // second to last request contains given component's id (to delete the component) AjaxHelpers.expectJsonRequest(requests, 'DELETE', '/xblock/locator-component-' + GROUP_TO_TEST + (componentIndex + 1), null, requests.length - requestOffset); @@ -329,8 +332,7 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel deleteComponentWithSuccess = function (componentIndex) { var deleteOffset; - deleteOffset = paginated() ? 3 : 2; - + deleteOffset = getDeleteOffset(); deleteComponent(componentIndex, deleteOffset); // verify the new list of components within the group @@ -356,17 +358,12 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel }); it("can delete an xblock with broken JavaScript", function () { + var deleteOffset = getDeleteOffset(); renderContainerPage(this, mockBadContainerXBlockHtml); containerPage.$('.delete-button').first().click(); EditHelpers.confirmPrompt(promptSpy); AjaxHelpers.respondWithJson(requests, {}); - var deleteOffset; - if (paginated()) { - deleteOffset = 3; - } else { - deleteOffset = 2; - } // expect the second to last request to be a delete of the xblock AjaxHelpers.expectJsonRequest(requests, 'DELETE', '/xblock/locator-broken-javascript', null, requests.length - deleteOffset); @@ -528,7 +525,7 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel }); describe('Template Picker', function () { - var showTemplatePicker, verifyCreateHtmlComponent, call_count; + var showTemplatePicker, verifyCreateHtmlComponent; showTemplatePicker = function () { containerPage.$('.new-component .new-component-type a.multiple-templates')[0].click(); @@ -536,7 +533,6 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel verifyCreateHtmlComponent = function (test, templateIndex, expectedRequest) { var xblockCount; - // call_count = paginated() ? 18: 10; renderContainerPage(test, mockContainerXBlockHtml); showTemplatePicker(); xblockCount = containerPage.$('.studio-xblock-wrapper').length; @@ -568,12 +564,17 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel } parameterized_suite("Non paged", - { enable_paging: false }, - { initial: 'mock/mock-container-xblock.underscore', add_response: 'mock/mock-xblock.underscore' } + { }, + { + page: ContainerPage, + initial: 'mock/mock-container-xblock.underscore', + add_response: 'mock/mock-xblock.underscore' + } ); parameterized_suite("Paged", - { enable_paging: true, page_size: 42 }, + { page_size: 42 }, { + page: PagedContainerPage, initial: 'mock/mock-container-paged-xblock.underscore', add_response: 'mock/mock-xblock-paged.underscore' }); diff --git a/cms/static/js/views/container.js b/cms/static/js/views/container.js index ec89208b44..a99993fe5d 100644 --- a/cms/static/js/views/container.js +++ b/cms/static/js/views/container.js @@ -9,6 +9,8 @@ define(["jquery", "underscore", "js/views/xblock", "js/utils/module", "gettext", // child xblocks within the page. requestToken: "", + new_child_view: 'reorderable_container_child_preview', + xblockReady: function () { XBlockView.prototype.xblockReady.call(this); var reorderableClass, reorderableContainer, diff --git a/cms/static/js/views/library_container.js b/cms/static/js/views/library_container.js index 7c48e83cee..ea09c69c89 100644 --- a/cms/static/js/views/library_container.js +++ b/cms/static/js/views/library_container.js @@ -1,6 +1,5 @@ -define(["jquery", "underscore", "js/views/paged_container", "js/utils/module", "gettext", "js/views/feedback_notification", - "js/views/paging_header", "js/views/paging_footer"], - function ($, _, PagedContainerView) { +define(["js/views/paged_container"], + function (PagedContainerView) { // To be extended with Library-specific features later. var LibraryContainerView = PagedContainerView; return LibraryContainerView; diff --git a/cms/static/js/views/paged_container.js b/cms/static/js/views/paged_container.js index cd7590156a..a8cd7aec32 100644 --- a/cms/static/js/views/paged_container.js +++ b/cms/static/js/views/paged_container.js @@ -5,9 +5,13 @@ define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettex initialize: function(options){ var self = this; ContainerView.prototype.initialize.call(this); - this.page_size = this.options.page_size || 10; - this.page_reload_callback = options.page_reload_callback || function () {}; - // emulating Backbone.paginator interface + this.page_size = this.options.page_size; + // Reference to the page model + this.page = options.page; + // XBlocks are rendered via Django views and templates rather than underscore templates, and so don't + // have a Backbone model for us to manipulate in a backbone collection. Here, we emulate the interface + // of backbone.paginator so that we can use the Paging Header and Footer with this page. As a + // consequence, however, we have to manipulate its members manually. this.collection = { currentPage: 0, totalPages: 0, @@ -15,18 +19,23 @@ define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettex sortDirection: "desc", start: 0, _size: 0, - - bind: function() {}, // no-op + // Paging header and footer expect this to be a Backbone model they can listen to for changes, but + // they cannot. Provide the bind function for them, but have it do nothing. + bind: function() {}, + // size() on backbone collections shows how many objects are in the collection, or in the case + // of paginator, on the current page. size: function() { return self.collection._size; } }; }, + new_child_view: 'container_child_preview', + render: function(options) { - var eff_options = options || {}; - eff_options.page_number = typeof eff_options.page_number !== "undefined" - ? eff_options.page_number + options = options || {}; + options.page_number = typeof options.page_number !== "undefined" + ? options.page_number : this.collection.currentPage; - return this.renderPage(eff_options); + return this.renderPage(options); }, renderPage: function(options){ @@ -43,16 +52,15 @@ define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettex success: function(fragment) { self.handleXBlockFragment(fragment, options); self.processPaging({ requested_page: options.page_number }); - // This is expected to render the add xblock components menu. - self.page_reload_callback(self.$el) + self.page.renderAddXBlockComponents() } }); }, getRenderParameters: function(page_number) { return { - enable_paging: true, page_size: this.page_size, + enable_paging: true, page_number: page_number }; }, @@ -67,6 +75,8 @@ define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettex }, processPaging: function(options){ + // We have the Django template sneak us the pagination information, + // and we load it from a div here. var $element = this.$el.find('.xblock-container-paging-parameters'), total = $element.data('total'), displayed = $element.data('displayed'), @@ -82,6 +92,8 @@ define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettex }, processPagingHeaderAndFooter: function(){ + // Rendering the container view detaches the header and footer from the DOM. + // It's just as easy to recreate them as it is to try to shove them back into the tree. if (this.pagingHeader) this.pagingHeader.undelegateEvents(); if (this.pagingFooter) @@ -100,12 +112,6 @@ define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettex this.pagingFooter.render(); }, - xblockReady: function () { - ContainerView.prototype.xblockReady.call(this); - - this.requestToken = this.$('div.xblock').first().data('request-token'); - }, - refresh: function(block_added) { if (block_added) { this.collection.totalCount += 1; @@ -150,7 +156,7 @@ define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettex }, sortDisplayName: function() { - return "Date added"; // TODO add support for sorting + return gettext("Date added"); // TODO add support for sorting } }); diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 771e9afb1b..406e6e9b03 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -3,10 +3,10 @@ * This page allows the user to understand and manipulate the xblock and its children. */ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views/utils/view_utils", - "js/views/container", "js/views/library_container", "js/views/xblock", "js/views/components/add_xblock", "js/views/modals/edit_xblock", + "js/views/container", "js/views/xblock", "js/views/components/add_xblock", "js/views/modals/edit_xblock", "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, PagedContainerView, XBlockView, AddXBlockComponent, + function ($, _, gettext, BasePage, ViewUtils, ContainerView, XBlockView, AddXBlockComponent, EditXBlockModal, XBlockInfo, XBlockStringFieldEditor, ContainerSubviews, UnitOutlineView, XBlockUtils) { 'use strict'; @@ -25,12 +25,16 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views view: 'container_preview', + defaultViewClass: ContainerView, + + // Overridable by subclasses-- determines whether the XBlock component + // addition menu is added on initialization. You may set this to false + // if your subclass handles it. + components_on_init: true, + initialize: function(options) { BasePage.prototype.initialize.call(this, options); - this.enable_paging = options.enable_paging || false; - if (this.enable_paging) { - this.page_size = options.page_size || 10; - } + this.viewClass = options.viewClass || this.defaultViewClass; this.nameEditor = new XBlockStringFieldEditor({ el: this.$('.wrapper-xblock-field'), model: this.model @@ -75,26 +79,16 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views } }, - getXBlockView: function(){ - var self = this, - parameters = { - el: this.$('.wrapper-xblock'), - model: this.model, - view: this.view - }; + getViewParameters: function () { + return { + el: this.$('.wrapper-xblock'), + model: this.model, + view: this.view + } + }, - if (this.enable_paging) { - parameters = _.extend(parameters, { - page_size: this.page_size, - page_reload_callback: function($element) { - self.renderAddXBlockComponents(); - } - }); - return new PagedContainerView(parameters); - } - else { - return new ContainerView(parameters); - } + getXBlockView: function(){ + return new this.viewClass(this.getViewParameters()); }, render: function(options) { @@ -120,7 +114,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views xblockView.notifyRuntime('page-shown', self); // Render the add buttons. Paged containers should do this on their own. - if (!self.enable_paging) { + if (self.components_on_init) { // Render the add buttons self.renderAddXBlockComponents(); } @@ -277,7 +271,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views rootLocator = this.xblockView.model.id; if (xblockElement.length === 0 || xblockElement.data('locator') === rootLocator) { this.render({refresh: true, block_added: block_added}); - } else if (parentElement.hasClass('reorderable-container') || this.enable_paging) { + } else if (parentElement.hasClass('reorderable-container')) { this.refreshChildXBlock(xblockElement, block_added); } else { this.refreshXBlock(this.findXBlockElement(parentElement)); @@ -313,7 +307,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views }); temporaryView = new TemporaryXBlockView({ model: xblockInfo, - view: 'reorderable_container_child_preview', + view: self.xblockView.new_child_view, el: xblockElement }); return temporaryView.render({ diff --git a/cms/static/js/views/pages/paged_container.js b/cms/static/js/views/pages/paged_container.js new file mode 100644 index 0000000000..916bf3005e --- /dev/null +++ b/cms/static/js/views/pages/paged_container.js @@ -0,0 +1,36 @@ +/** + * PagedXBlockContainerPage is a variant of XBlockContainerPage that supports Pagination. + */ +define(["jquery", "underscore", "gettext", "js/views/pages/container", "js/views/paged_container"], + function ($, _, gettext, XBlockContainerPage, PagedContainerView) { + 'use strict'; + var PagedXBlockContainerPage = XBlockContainerPage.extend({ + + defaultViewClass: PagedContainerView, + components_on_init: false, + + initialize: function (options){ + this.page_size = options.page_size || 10; + XBlockContainerPage.prototype.initialize.call(this, options); + }, + + getViewParameters: function () { + return _.extend(XBlockContainerPage.prototype.getViewParameters.call(this), { + page_size: this.page_size, + page: this + }); + }, + + refreshXBlock: function(element, block_added) { + var xblockElement = this.findXBlockElement(element), + rootLocator = this.xblockView.model.id; + if (xblockElement.length === 0 || xblockElement.data('locator') === rootLocator) { + this.render({refresh: true, block_added: block_added}); + } else { + this.refreshChildXBlock(xblockElement, block_added); + } + } + + }); + return PagedXBlockContainerPage; + }); diff --git a/cms/static/js/views/paging_footer.js b/cms/static/js/views/paging_footer.js index 86d776aafd..a897e8275a 100644 --- a/cms/static/js/views/paging_footer.js +++ b/cms/static/js/views/paging_footer.js @@ -44,6 +44,8 @@ define(["underscore", "js/views/baseview"], function(_, BaseView) { if (pageNumber <= 0) { pageNumber = false; } + // If we still have a page number by this point, + // and it's not the current page, load it. if (pageNumber && pageNumber !== currentPage) { view.setPage(pageNumber - 1); } diff --git a/cms/static/js/views/paging_mixin.js b/cms/static/js/views/paging_mixin.js index d2c1700e5d..16d518f856 100644 --- a/cms/static/js/views/paging_mixin.js +++ b/cms/static/js/views/paging_mixin.js @@ -1,5 +1,5 @@ -define(["jquery", "underscore"], - function ($, _) { +define([], + function () { var PagedMixin = { setPage: function (page) { var self = this, diff --git a/cms/templates/library.html b/cms/templates/library.html index dc9baa5736..d367c333d2 100644 --- a/cms/templates/library.html +++ b/cms/templates/library.html @@ -25,7 +25,6 @@ from django.utils.translation import ugettext as _ ${component_templates | n}, ${json.dumps(xblock_info) | n}, { isUnitPage: false, - enable_paging: true, page_size: 10 } ); diff --git a/common/lib/xmodule/xmodule/library_root_xblock.py b/common/lib/xmodule/xmodule/library_root_xblock.py index 3118f9a258..6a58cf1b1d 100644 --- a/common/lib/xmodule/xmodule/library_root_xblock.py +++ b/common/lib/xmodule/xmodule/library_root_xblock.py @@ -50,8 +50,7 @@ class LibraryRoot(XBlock): def render_children(self, context, fragment, can_reorder=False, can_add=False): # pylint: disable=unused-argument """ - Renders the children of the module with HTML appropriate for Studio. If can_reorder is True, - then the children will be rendered to support drag and drop. + Renders the children of the module with HTML appropriate for Studio. Reordering is not supported. """ contents = [] @@ -77,7 +76,7 @@ class LibraryRoot(XBlock): contents.append({ 'id': unicode(child.location), - 'content': rendered_child.content + 'content': rendered_child.content, }) fragment.add_content( diff --git a/common/test/acceptance/pages/studio/library.py b/common/test/acceptance/pages/studio/library.py index 5572b1a91e..64f93f2116 100644 --- a/common/test/acceptance/pages/studio/library.py +++ b/common/test/acceptance/pages/studio/library.py @@ -3,14 +3,14 @@ Library edit page in Studio """ from bok_choy.page_object import PageObject -from selenium.webdriver.common.keys import Keys +from ...pages.studio.pagination import PaginatedMixin from .container import XBlockWrapper from ...tests.helpers import disable_animations from .utils import confirm_prompt, wait_for_notification from . import BASE_URL -class LibraryPage(PageObject): +class LibraryPage(PageObject, PaginatedMixin): """ Library page in Studio """ @@ -75,58 +75,6 @@ class LibraryPage(PageObject): confirm_prompt(self) # this will also wait_for_notification() self.wait_for_ajax() - def nav_disabled(self, position, arrows=('next', 'previous')): - """ - Verifies that pagination nav is disabled. Position can be 'top' or 'bottom'. - - To specify a specific arrow, pass an iterable with a single element, 'next' or 'previous'. - """ - return all([ - self.q(css='nav.%s * a.%s-page-link.is-disabled' % (position, arrow)) - for arrow in arrows - ]) - - def move_back(self, position): - """ - Clicks one of the forward nav buttons. Position can be 'top' or 'bottom'. - """ - self.q(css='nav.%s * a.previous-page-link' % position)[0].click() - self.wait_until_ready() - - def move_forward(self, position): - """ - Clicks one of the forward nav buttons. Position can be 'top' or 'bottom'. - """ - self.q(css='nav.%s * a.next-page-link' % position)[0].click() - self.wait_until_ready() - - def revisit(self): - """ - Visit the page's URL, instead of refreshing, so that a new state is created. - """ - self.browser.get(self.browser.current_url) - self.wait_until_ready() - - def go_to_page(self, number): - """ - Enter a number into the page number input field, and then try to navigate to it. - """ - page_input = self.q(css="#page-number-input")[0] - page_input.click() - page_input.send_keys(str(number)) - page_input.send_keys(Keys.RETURN) - self.wait_until_ready() - - def check_page_unchanged(self, first_block_name): - """ - Used to make sure that a page has not transitioned after a bogus number is given. - """ - if not self.xblocks[0].name == first_block_name: - return False - if not self.q(css='#page-number-input')[0].get_attribute('value') == '': - return False - return True - def _get_xblocks(self): """ Create an XBlockWrapper for each XBlock div found on the page. diff --git a/common/test/acceptance/pages/studio/pagination.py b/common/test/acceptance/pages/studio/pagination.py new file mode 100644 index 0000000000..a976149c37 --- /dev/null +++ b/common/test/acceptance/pages/studio/pagination.py @@ -0,0 +1,62 @@ +""" +Mixin to include for Paginated container pages +""" +from selenium.webdriver.common.keys import Keys + + +class PaginatedMixin(object): + """ + Mixin class used for paginated page tests. + """ + def nav_disabled(self, position, arrows=('next', 'previous')): + """ + Verifies that pagination nav is disabled. Position can be 'top' or 'bottom'. + + `top` is the header, `bottom` is the footer. + + To specify a specific arrow, pass an iterable with a single element, 'next' or 'previous'. + """ + return all([ + self.q(css='nav.%s * a.%s-page-link.is-disabled' % (position, arrow)) + for arrow in arrows + ]) + + def move_back(self, position): + """ + Clicks one of the forward nav buttons. Position can be 'top' or 'bottom'. + """ + self.q(css='nav.%s * a.previous-page-link' % position)[0].click() + self.wait_until_ready() + + def move_forward(self, position): + """ + Clicks one of the forward nav buttons. Position can be 'top' or 'bottom'. + """ + self.q(css='nav.%s * a.next-page-link' % position)[0].click() + self.wait_until_ready() + + def go_to_page(self, number): + """ + Enter a number into the page number input field, and then try to navigate to it. + """ + page_input = self.q(css="#page-number-input")[0] + page_input.click() + page_input.send_keys(str(number)) + page_input.send_keys(Keys.RETURN) + self.wait_until_ready() + + def get_page_number(self): + """ + Returns the page number as the page represents it, in string form. + """ + return self.q(css="span.current-page")[0].get_attribute('innerHTML') + + def check_page_unchanged(self, first_block_name): + """ + Used to make sure that a page has not transitioned after a bogus number is given. + """ + if not self.xblocks[0].name == first_block_name: + return False + if not self.q(css='#page-number-input')[0].get_attribute('value') == '': + return False + return True diff --git a/common/test/acceptance/tests/studio/test_studio_library.py b/common/test/acceptance/tests/studio/test_studio_library.py index 5529f36032..491c9093d0 100644 --- a/common/test/acceptance/tests/studio/test_studio_library.py +++ b/common/test/acceptance/tests/studio/test_studio_library.py @@ -4,6 +4,7 @@ Acceptance tests for Content Libraries in Studio from ddt import ddt, data from .base_studio_test import StudioLibraryTest +from ...fixtures.course import XBlockFixtureDesc from ...pages.studio.utils import add_component from ...pages.studio.library import LibraryPage @@ -137,109 +138,64 @@ class LibraryEditPageTest(StudioLibraryTest): Scenario: Ensure that the navigation buttons aren't active when there aren't enough XBlocks. Given that I have a library in Studio with no XBlocks The Navigation buttons should be disabled. - When I add 5 multiple Choice XBlocks + When I add a multiple choice problem The Navigation buttons should be disabled. """ self.assertEqual(len(self.lib_page.xblocks), 0) self.assertTrue(self.lib_page.nav_disabled(position)) - for _ in range(0, 5): - add_component(self.lib_page, "problem", "Multiple Choice") + add_component(self.lib_page, "problem", "Multiple Choice") self.assertTrue(self.lib_page.nav_disabled(position)) - @data('top', 'bottom') - def test_nav_buttons(self, position): + +@ddt +class LibraryNavigationTest(StudioLibraryTest): + """ + Test common Navigation actions + """ + def setUp(self): # pylint: disable=arguments-differ """ - Scenario: Ensure that the navigation buttons work. - Given that I have a library in Studio with no XBlocks - And I create 10 Multiple Choice XBlocks - And I create 10 Checkbox XBlocks - And I create 10 Dropdown XBlocks - And I revisit the page - The previous button should be disabled. - The first XBlock should be a Multiple Choice XBlock - Then if I hit the next button - The first XBlock should be a Checkboxes XBlock - Then if I hit the next button - The first XBlock should be a Dropdown XBlock - And the next button should be disabled - Then if I hit the previous button - The first XBlock should be an Checkboxes XBlock - Then if I hit the previous button - The first XBlock should be a Multipe Choice XBlock - And the previous button should be disabled + Ensure a library exists and navigate to the library edit page. """ - self.assertEqual(len(self.lib_page.xblocks), 0) - block_types = [('problem', 'Multiple Choice'), ('problem', 'Checkboxes'), ('problem', 'Dropdown')] - for block_type in block_types: - for _ in range(0, 10): - add_component(self.lib_page, *block_type) + super(LibraryNavigationTest, self).setUp(is_staff=True) + self.lib_page = LibraryPage(self.browser, self.library_key) + self.lib_page.visit() + self.lib_page.wait_until_ready() - # Don't refresh, as that may contain additional state. - self.lib_page.revisit() - - # Check forward navigation - self.assertTrue(self.lib_page.nav_disabled(position, ['previous'])) - self.assertEqual(self.lib_page.xblocks[0].name, 'Multiple Choice') - self.lib_page.move_forward(position) - self.assertEqual(self.lib_page.xblocks[0].name, 'Checkboxes') - self.lib_page.move_forward(position) - self.assertEqual(self.lib_page.xblocks[0].name, 'Dropdown') - self.lib_page.nav_disabled(position, ['next']) - - # Check backward navigation - self.lib_page.move_back(position) - self.assertEqual(self.lib_page.xblocks[0].name, 'Checkboxes') - self.lib_page.move_back(position) - self.assertEqual(self.lib_page.xblocks[0].name, 'Multiple Choice') - self.assertTrue(self.lib_page.nav_disabled(position, ['previous'])) + def populate_library_fixture(self, library_fixture): + """ + Create four pages worth of XBlocks, and offset by one so each is named + after the number they should be in line by the user's perception. + """ + # pylint: disable=attribute-defined-outside-init + self.blocks = [XBlockFixtureDesc('html', str(i)) for i in xrange(1, 41)] + library_fixture.add_children(*self.blocks) def test_arbitrary_page_selection(self): """ Scenario: I can pick a specific page number of a Library at will. - Given that I have a library in Studio with no XBlocks - And I create 10 Multiple Choice XBlocks - And I create 10 Checkboxes XBlocks - And I create 10 Dropdown XBlocks - And I create 10 Numerical Input XBlocks - And I revisit the page + Given that I have a library in Studio with 40 XBlocks When I go to the 3rd page - The first XBlock should be a Dropdown XBlock + The first XBlock should be the 21st XBlock When I go to the 4th Page - The first XBlock should be a Numerical Input XBlock + The first XBlock should be the 31st XBlock When I go to the 1st page - The first XBlock should be a Multiple Choice XBlock + The first XBlock should be the 1st XBlock When I go to the 2nd page - The first XBlock should be a Checkboxes XBlock + The first XBlock should be the 11th XBlock """ - self.assertEqual(len(self.lib_page.xblocks), 0) - block_types = [ - ('problem', 'Multiple Choice'), ('problem', 'Checkboxes'), ('problem', 'Dropdown'), - ('problem', 'Numerical Input'), - ] - for block_type in block_types: - for _ in range(0, 10): - add_component(self.lib_page, *block_type) - - # Don't refresh, as that may contain additional state. - self.lib_page.revisit() self.lib_page.go_to_page(3) - self.assertEqual(self.lib_page.xblocks[0].name, 'Dropdown') + self.assertEqual(self.lib_page.xblocks[0].name, '21') self.lib_page.go_to_page(4) - self.assertEqual(self.lib_page.xblocks[0].name, 'Numerical Input') + self.assertEqual(self.lib_page.xblocks[0].name, '31') self.lib_page.go_to_page(1) - self.assertEqual(self.lib_page.xblocks[0].name, 'Multiple Choice') + self.assertEqual(self.lib_page.xblocks[0].name, '1') self.lib_page.go_to_page(2) - self.assertEqual(self.lib_page.xblocks[0].name, 'Checkboxes') + self.assertEqual(self.lib_page.xblocks[0].name, '11') def test_bogus_page_selection(self): """ Scenario: I can't pick a nonsense page number of a Library - Given that I have a library in Studio with no XBlocks - And I create 10 Multiple Choice XBlocks - And I create 10 Checkboxes XBlocks - And I create 10 Dropdown XBlocks - And I create 10 Numerical Input XBlocks - And I revisit the page + Given that I have a library in Studio with 40 XBlocks When I attempt to go to the 'a'th page The input field will be cleared and no change of XBlocks will be made When I attempt to visit the 5th page @@ -249,22 +205,104 @@ class LibraryEditPageTest(StudioLibraryTest): When I attempt to visit the 0th page The input field will be cleared and no change of XBlocks will be made """ - self.assertEqual(len(self.lib_page.xblocks), 0) - block_types = [ - ('problem', 'Multiple Choice'), ('problem', 'Checkboxes'), ('problem', 'Dropdown'), - ('problem', 'Numerical Input'), - ] - for block_type in block_types: - for _ in range(0, 10): - add_component(self.lib_page, *block_type) - - self.lib_page.revisit() - self.assertEqual(self.lib_page.xblocks[0].name, 'Multiple Choice') + self.assertEqual(self.lib_page.xblocks[0].name, '1') self.lib_page.go_to_page('a') - self.assertTrue(self.lib_page.check_page_unchanged('Multiple Choice')) + self.assertTrue(self.lib_page.check_page_unchanged('1')) self.lib_page.go_to_page(-1) - self.assertTrue(self.lib_page.check_page_unchanged('Multiple Choice')) + self.assertTrue(self.lib_page.check_page_unchanged('1')) self.lib_page.go_to_page(5) - self.assertTrue(self.lib_page.check_page_unchanged('Multiple Choice')) + self.assertTrue(self.lib_page.check_page_unchanged('1')) self.lib_page.go_to_page(0) - self.assertTrue(self.lib_page.check_page_unchanged('Multiple Choice')) + self.assertTrue(self.lib_page.check_page_unchanged('1')) + + @data('top', 'bottom') + def test_nav_buttons(self, position): + """ + Scenario: Ensure that the navigation buttons work. + Given that I have a library in Studio with 40 XBlocks + The previous button should be disabled. + The first XBlock should be the 1st XBlock + Then if I hit the next button + The first XBlock should be the 11th XBlock + Then if I hit the next button + The first XBlock should be the 21st XBlock + Then if I hit the next button + The first XBlock should be the 31st XBlock + And the next button should be disabled + Then if I hit the previous button + The first XBlock should be the 21st XBlock + Then if I hit the previous button + The first XBlock should be the 11th XBlock + Then if I hit the previous button + The first XBlock should be the 1st XBlock + And the previous button should be disabled + """ + # Check forward navigation + self.assertTrue(self.lib_page.nav_disabled(position, ['previous'])) + self.assertEqual(self.lib_page.xblocks[0].name, '1') + self.lib_page.move_forward(position) + self.assertEqual(self.lib_page.xblocks[0].name, '11') + self.lib_page.move_forward(position) + self.assertEqual(self.lib_page.xblocks[0].name, '21') + self.lib_page.move_forward(position) + self.assertEqual(self.lib_page.xblocks[0].name, '31') + self.lib_page.nav_disabled(position, ['next']) + + # Check backward navigation + self.lib_page.move_back(position) + self.assertEqual(self.lib_page.xblocks[0].name, '21') + self.lib_page.move_back(position) + self.assertEqual(self.lib_page.xblocks[0].name, '11') + self.lib_page.move_back(position) + self.assertEqual(self.lib_page.xblocks[0].name, '1') + self.assertTrue(self.lib_page.nav_disabled(position, ['previous'])) + + def test_library_pagination(self): + """ + Scenario: Ensure that adding several XBlocks to a library results in pagination. + Given that I have a library in Studio with 40 XBlocks + Then 10 are displayed + And the first XBlock will be the 1st one + And I'm on the 1st page + When I add 1 Multiple Choice XBlock + Then 1 XBlock will be displayed + And I'm on the 5th page + The first XBlock will be the newest one + When I delete that XBlock + Then 10 are displayed + And I'm on the 4th page + And the first XBlock is the 31st one + And the last XBlock is the 40th one. + """ + self.assertEqual(len(self.lib_page.xblocks), 10) + self.assertEqual(self.lib_page.get_page_number(), '1') + self.assertEqual(self.lib_page.xblocks[0].name, '1') + add_component(self.lib_page, "problem", "Multiple Choice") + self.assertEqual(len(self.lib_page.xblocks), 1) + self.assertEqual(self.lib_page.get_page_number(), '5') + self.assertEqual(self.lib_page.xblocks[0].name, "Multiple Choice") + self.lib_page.click_delete_button(self.lib_page.xblocks[0].locator) + self.assertEqual(len(self.lib_page.xblocks), 10) + self.assertEqual(self.lib_page.get_page_number(), '4') + self.assertEqual(self.lib_page.xblocks[0].name, '31') + self.assertEqual(self.lib_page.xblocks[-1].name, '40') + + def test_delete_shifts_blocks(self): + """ + Scenario: Ensure that removing an XBlock shifts other blocks back. + Given that I have a library in Studio with 40 XBlocks + Then 10 are displayed + And I will be on the first page + When I delete the third XBlock + There will be 10 displayed + And the first XBlock will be the first one + And the last XBlock will be the 11th one + And I will be on the first page + """ + self.assertEqual(len(self.lib_page.xblocks), 10) + self.assertEqual(self.lib_page.get_page_number(), '1') + self.lib_page.click_delete_button(self.lib_page.xblocks[2].locator, confirm=True) + self.assertEqual(len(self.lib_page.xblocks), 10) + self.assertEqual(self.lib_page.xblocks[0].name, '1') + self.assertEqual(self.lib_page.xblocks[-1].name, '11') + self.assertEqual(self.lib_page.get_page_number(), '1')