Addressed further review notes for Library Pagination

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.

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",

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

cms/static/js/factories/library.js

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

cms/static/js/spec/views/library_container_spec.js → cms/static/js/spec/views/paged_container_spec.js

 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('' +
             '<div class="xblock-container-paging-parameters" data-start="<%= start %>" data-displayed="<%= displayed %>" data-total="<%= total %>"/>'

cms/static/js/spec/views/pages/container_spec.js

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

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,

cms/static/js/views/library_container.js

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

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

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,28 +79,18 @@ 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
-                    };
-
-                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);
+            getViewParameters: function () {
+                return {
+                    el: this.$('.wrapper-xblock'),
+                    model: this.model,
+                    view: this.view
                 }
             },
 
+            getXBlockView: function(){
+                return new this.viewClass(this.getViewParameters());
+            },
+
             render: function(options) {
                 var self = this,
                     xblockView = this.xblockView,
@@ -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({

cms/static/js/views/pages/paged_container.js

+/**
+ * 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;
+    });

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

cms/static/js/views/paging_mixin.js

-define(["jquery", "underscore"],
-    function ($, _) {
+define([],
+    function () {
         var PagedMixin = {
             setPage: function (page) {
                 var self = this,

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

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(

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.

common/test/acceptance/pages/studio/pagination.py

+"""
+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

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)
-
-        # 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'])
+        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()
 
-        # 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')