From 459b65143e2f3d06d5b9c9cdb41a13af003c0acd Mon Sep 17 00:00:00 2001 From: "E. Kolpakov" Date: Mon, 3 Nov 2014 20:20:29 +0700 Subject: [PATCH 1/5] Paging for LibraryView added with JS tests. --- cms/djangoapps/contentstore/views/item.py | 18 +- cms/static/coffee/spec/main.coffee | 1 + cms/static/js/factories/container.js | 22 +- cms/static/js/factories/library.js | 22 +- .../js/spec/views/library_container_spec.js | 489 ++++++++++ .../js/spec/views/pages/container_spec.js | 893 +++++++++--------- cms/static/js/views/container.js | 4 + cms/static/js/views/library_container.js | 164 ++++ cms/static/js/views/pages/container.js | 52 +- cms/static/js/views/paging_footer.js | 6 + cms/static/sass/elements/_pagination.scss | 119 +++ cms/static/sass/elements/_xblocks.scss | 31 + cms/static/sass/style-app-extend1-rtl.scss | 1 + cms/static/sass/style-app-extend1.scss | 1 + cms/static/sass/views/_assets.scss | 122 +-- cms/templates/container.html | 5 +- ...ontainer-paged-after-add-xblock.underscore | 283 ++++++ .../mock-container-paged-xblock.underscore | 257 +++++ cms/templates/library.html | 8 +- .../xmodule/xmodule/library_root_xblock.py | 54 +- .../xmodule/video_module/video_handlers.py | 1 - .../studio_render_paged_children_view.html | 23 + 22 files changed, 1963 insertions(+), 613 deletions(-) create mode 100644 cms/static/js/spec/views/library_container_spec.js create mode 100644 cms/static/js/views/library_container.js create mode 100644 cms/static/sass/elements/_pagination.scss create mode 100644 cms/templates/js/mock/mock-container-paged-after-add-xblock.underscore create mode 100644 cms/templates/js/mock/mock-container-paged-xblock.underscore create mode 100644 lms/templates/studio_render_paged_children_view.html diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index d3123aa0d04d..5d05a27e1781 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -238,12 +238,28 @@ def xblock_view_handler(request, usage_key_string, view_name): if view_name == 'reorderable_container_child_preview': reorderable_items.add(xblock.location) + paging = None + try: + if request.REQUEST.get('enable_paging', 'false') == 'true': + paging = { + 'page_number': int(request.REQUEST.get('page_number', 0)), + '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) + ) + # Set up the context to be passed to each XBlock's render method. context = { 'is_pages_view': is_pages_view, # This setting disables the recursive wrapping of xblocks 'is_unit_page': is_unit(xblock), 'root_xblock': xblock if (view_name == 'container_preview') else None, - 'reorderable_items': reorderable_items + 'reorderable_items': reorderable_items, + 'paging': paging } fragment = get_preview_fragment(request, xblock, context) diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index b9b3546dbff3..e2ba18a82198 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -232,6 +232,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/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 93cdeb8fd991..ea48bb2a989f 100644 --- a/cms/static/js/factories/container.js +++ b/cms/static/js/factories/container.js @@ -1,22 +1,20 @@ define([ - 'jquery', 'js/models/xblock_info', 'js/views/pages/container', + 'jquery', 'underscore', 'js/models/xblock_info', 'js/views/pages/container', 'js/collections/component_template', 'xmodule', 'coffee/src/main', 'xblock/cms.runtime.v1' ], -function($, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { +function($, _, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { 'use strict'; - return function (componentTemplates, XBlockInfoJson, action, isUnitPage) { - var templates = new ComponentTemplates(componentTemplates, {parse: true}), - mainXBlockInfo = new XBlockInfo(XBlockInfoJson, {parse: true}); - - xmoduleLoader.done(function () { - var view = new ContainerPage({ + return function (componentTemplates, XBlockInfoJson, action, options) { + var main_options = { el: $('#content'), - model: mainXBlockInfo, + model: new XBlockInfo(XBlockInfoJson, {parse: true}), action: action, - templates: templates, - isUnitPage: isUnitPage - }); + templates: new ComponentTemplates(componentTemplates, {parse: true}) + }; + + xmoduleLoader.done(function () { + var view = new ContainerPage(_.extend(main_options, options)); view.render(); }); }; diff --git a/cms/static/js/factories/library.js b/cms/static/js/factories/library.js index 2729a3cf279d..e7834f60ef3c 100644 --- a/cms/static/js/factories/library.js +++ b/cms/static/js/factories/library.js @@ -1,22 +1,20 @@ define([ - 'jquery', 'js/models/xblock_info', 'js/views/pages/container', + 'jquery', 'underscore', 'js/models/xblock_info', 'js/views/pages/container', 'js/collections/component_template', 'xmodule', 'coffee/src/main', 'xblock/cms.runtime.v1' ], -function($, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { +function($, _, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { 'use strict'; - return function (componentTemplates, XBlockInfoJson) { - var templates = new ComponentTemplates(componentTemplates, {parse: true}), - mainXBlockInfo = new XBlockInfo(XBlockInfoJson, {parse: true}); + return function (componentTemplates, XBlockInfoJson, options) { + var main_options = { + el: $('#content'), + model: new XBlockInfo(XBlockInfoJson, {parse: true}), + templates: new ComponentTemplates(componentTemplates, {parse: true}), + action: 'view' + }; xmoduleLoader.done(function () { - var view = new ContainerPage({ - el: $('#content'), - model: mainXBlockInfo, - action: "view", - templates: templates, - isUnitPage: false - }); + var view = new ContainerPage(_.extend(main_options, options)); view.render(); }); }; diff --git a/cms/static/js/spec/views/library_container_spec.js b/cms/static/js/spec/views/library_container_spec.js new file mode 100644 index 000000000000..2d39cdc35819 --- /dev/null +++ b/cms/static/js/spec/views/library_container_spec.js @@ -0,0 +1,489 @@ +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) { + + var htmlResponseTpl = _.template('' + + '
' + ); + + function getResponseHtml(options){ + return '
' + + '
' + + htmlResponseTpl(options) + + '
' + + '
' + } + + var PAGE_SIZE = 3; + + var mockFirstPage = { + resources: [], + html: getResponseHtml({ + start: 0, + displayed: PAGE_SIZE, + total: PAGE_SIZE + 1 + }) + }; + + var mockSecondPage = { + resources: [], + html: getResponseHtml({ + start: PAGE_SIZE, + displayed: 1, + total: PAGE_SIZE + 1 + }) + }; + + var mockEmptyPage = { + resources: [], + html: getResponseHtml({ + start: 0, + displayed: 0, + total: 0 + }) + }; + + var respondWithMockPage = function(requests) { + var requestIndex = requests.length - 1; + var request = requests[requestIndex]; + var url = new URI(request.url); + var queryParameters = url.query(true); // Returns an object with each query parameter stored as a value + var page = queryParameters.page_number; + var response = page === "0" ? mockFirstPage : mockSecondPage; + AjaxHelpers.respondWithJson(requests, response, requestIndex); + }; + + var MockPagingView = PagedContainer.extend({ + view: 'container_preview', + el: $("
"), + model: new XBlockInfo({}, {parse: true}) + }); + + describe("Paging Container", function() { + var pagingContainer; + + beforeEach(function () { + var feedbackTpl = readFixtures('system-feedback.underscore'); + setFixtures($(" + + +
+ +
+
+
+
+
+ + + Expand or Collapse + + Group A +
+
+
    +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ + + Expand or Collapse + + Group B +
+
+
    +
  • + +
    • +
+
+
+
+ +
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
+ diff --git a/cms/templates/js/mock/mock-container-paged-xblock.underscore b/cms/templates/js/mock/mock-container-paged-xblock.underscore new file mode 100644 index 000000000000..2314bb892577 --- /dev/null +++ b/cms/templates/js/mock/mock-container-paged-xblock.underscore @@ -0,0 +1,257 @@ +
+
+
+ Test Container +
+
+
    +
+
+
+
+
+
+ + + + +
+ +
+
+
+
+
+ + + Expand or Collapse + + Group A +
+
+
    +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ + + Expand or Collapse + + Group B +
+
+
    +
  • + +
    • +
+
+
+
+ +
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/cms/templates/library.html b/cms/templates/library.html index 70bd836baddd..dc9baa5736c3 100644 --- a/cms/templates/library.html +++ b/cms/templates/library.html @@ -22,8 +22,12 @@ <%block name="requirejs"> require(["js/factories/library"], function(LibraryFactory) { LibraryFactory( - ${component_templates | n}, - ${json.dumps(xblock_info) | n} + ${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 dc00aaa97fa9..497a145b79bd 100644 --- a/common/lib/xmodule/xmodule/library_root_xblock.py +++ b/common/lib/xmodule/xmodule/library_root_xblock.py @@ -3,10 +3,10 @@ """ import logging -from .studio_editable import StudioEditableModule from xblock.core import XBlock from xblock.fields import Scope, String, List from xblock.fragment import Fragment +from xmodule.studio_editable import StudioEditableModule log = logging.getLogger(__name__) @@ -42,29 +42,55 @@ def __str__(self): def author_view(self, context): """ - Renders the Studio preview view, which supports drag and drop. + Renders the Studio preview view. """ fragment = Fragment() + self.render_children(context, fragment, can_reorder=False, can_add=True) + return fragment + + 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. + """ contents = [] - for child_key in self.children: # pylint: disable=E1101 - context['reorderable_items'].add(child_key) + paging = context.get('paging', None) + + children_count = len(self.children) # pylint: disable=no-member + item_start, item_end = 0, children_count + + # TODO sort children + if paging: + page_number = paging.get('page_number', 0) + raw_page_size = paging.get('page_size', None) + page_size = raw_page_size if raw_page_size is not None else children_count + item_start, item_end = page_size * page_number, page_size * (page_number + 1) + + children_to_show = self.children[item_start:item_end] # pylint: disable=no-member + + for child_key in children_to_show: # pylint: disable=E1101 child = self.runtime.get_block(child_key) - rendered_child = self.runtime.render_child(child, StudioEditableModule.get_preview_view_name(child), context) + child_view_name = StudioEditableModule.get_preview_view_name(child) + rendered_child = self.runtime.render_child(child, child_view_name, context) fragment.add_frag_resources(rendered_child) contents.append({ - 'id': unicode(child_key), - 'content': rendered_child.content, + 'id': child.location.to_deprecated_string(), + 'content': rendered_child.content }) - fragment.add_content(self.runtime.render_template("studio_render_children_view.html", { - 'items': contents, - 'xblock_context': context, - 'can_add': True, - 'can_reorder': True, - })) - return fragment + fragment.add_content( + self.runtime.render_template("studio_render_paged_children_view.html", { + 'items': contents, + 'xblock_context': context, + 'can_add': can_add, + 'can_reorder': False, + 'first_displayed': item_start, + 'total_children': children_count, + 'displayed_children': len(children_to_show) + }) + ) @property def display_org_with_default(self): diff --git a/common/lib/xmodule/xmodule/video_module/video_handlers.py b/common/lib/xmodule/xmodule/video_module/video_handlers.py index 9e9db860ca54..1ba427c35785 100644 --- a/common/lib/xmodule/xmodule/video_module/video_handlers.py +++ b/common/lib/xmodule/xmodule/video_module/video_handlers.py @@ -155,7 +155,6 @@ def get_static_transcript(self, request): if transcript_name: # Get the asset path for course - asset_path = None course = self.descriptor.runtime.modulestore.get_course(self.course_id) if course.static_asset_path: asset_path = course.static_asset_path diff --git a/lms/templates/studio_render_paged_children_view.html b/lms/templates/studio_render_paged_children_view.html new file mode 100644 index 000000000000..fe5b5403e1ab --- /dev/null +++ b/lms/templates/studio_render_paged_children_view.html @@ -0,0 +1,23 @@ +<%! from django.utils.translation import ugettext as _ %> + +<%namespace name='static' file='static_content.html'/> + +% for template_name in ["paging-header", "paging-footer"]: + +% endfor + +
+ +
+ +% for item in items: + ${item['content']} +% endfor + +% if can_add: +
+% endif + +
From 523e8d98a92c9e7d8a4adc49d1c25ed55151242f Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Thu, 4 Dec 2014 21:25:52 +0000 Subject: [PATCH 2/5] Added tests for Library pagination. --- .../test/acceptance/pages/studio/library.py | 53 ++++++ .../tests/studio/test_studio_library.py | 161 ++++++++++++++++++ 2 files changed, 214 insertions(+) diff --git a/common/test/acceptance/pages/studio/library.py b/common/test/acceptance/pages/studio/library.py index e87c556da968..5572b1a91e43 100644 --- a/common/test/acceptance/pages/studio/library.py +++ b/common/test/acceptance/pages/studio/library.py @@ -3,6 +3,7 @@ """ from bok_choy.page_object import PageObject +from selenium.webdriver.common.keys import Keys from .container import XBlockWrapper from ...tests.helpers import disable_animations from .utils import confirm_prompt, wait_for_notification @@ -74,6 +75,58 @@ def click_delete_button(self, xblock_id, confirm=True): 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/tests/studio/test_studio_library.py b/common/test/acceptance/tests/studio/test_studio_library.py index b505ac140dbc..5529f3603293 100644 --- a/common/test/acceptance/tests/studio/test_studio_library.py +++ b/common/test/acceptance/tests/studio/test_studio_library.py @@ -1,11 +1,14 @@ """ Acceptance tests for Content Libraries in Studio """ +from ddt import ddt, data + from .base_studio_test import StudioLibraryTest from ...pages.studio.utils import add_component from ...pages.studio.library import LibraryPage +@ddt class LibraryEditPageTest(StudioLibraryTest): """ Test the functionality of the library edit page. @@ -107,3 +110,161 @@ def test_no_discussion_button(self): Ensure the UI is not loaded for adding discussions. """ self.assertFalse(self.browser.find_elements_by_css_selector('span.large-discussion-icon')) + + 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 no XBlocks + And I create 10 Multiple Choice XBlocks + Then 10 are displayed. + When I add one more Multiple Choice XBlock + Then 1 XBlock will be displayed + When I delete that XBlock + Then 10 are displayed. + """ + self.assertEqual(len(self.lib_page.xblocks), 0) + for _ in range(0, 10): + add_component(self.lib_page, "problem", "Multiple Choice") + self.assertEqual(len(self.lib_page.xblocks), 10) + add_component(self.lib_page, "problem", "Multiple Choice") + self.assertEqual(len(self.lib_page.xblocks), 1) + self.lib_page.click_delete_button(self.lib_page.xblocks[0].locator) + self.assertEqual(len(self.lib_page.xblocks), 10) + + @data('top', 'bottom') + def test_nav_present_but_disabled(self, position): + """ + 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 + 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") + self.assertTrue(self.lib_page.nav_disabled(position)) + + @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 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 + """ + 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']) + + # 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 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 + When I go to the 3rd page + The first XBlock should be a Dropdown XBlock + When I go to the 4th Page + The first XBlock should be a Numerical Input XBlock + When I go to the 1st page + The first XBlock should be a Multiple Choice XBlock + When I go to the 2nd page + The first XBlock should be a Checkboxes 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.lib_page.go_to_page(4) + self.assertEqual(self.lib_page.xblocks[0].name, 'Numerical Input') + self.lib_page.go_to_page(1) + self.assertEqual(self.lib_page.xblocks[0].name, 'Multiple Choice') + self.lib_page.go_to_page(2) + self.assertEqual(self.lib_page.xblocks[0].name, 'Checkboxes') + + 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 + 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 + The input field will be cleared and no change of XBlocks will be made + When I attempt to visit the -1st page + The input field will be cleared and no change of XBlocks will be made + 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.lib_page.go_to_page('a') + self.assertTrue(self.lib_page.check_page_unchanged('Multiple Choice')) + self.lib_page.go_to_page(-1) + self.assertTrue(self.lib_page.check_page_unchanged('Multiple Choice')) + self.lib_page.go_to_page(5) + self.assertTrue(self.lib_page.check_page_unchanged('Multiple Choice')) + self.lib_page.go_to_page(0) + self.assertTrue(self.lib_page.check_page_unchanged('Multiple Choice')) From 26e9399b39b16381de57e97bde7f2061c09ea0f5 Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Mon, 8 Dec 2014 22:22:02 +0000 Subject: [PATCH 3/5] Addressed notes from reviewers on Library Pagination. --- cms/djangoapps/contentstore/views/item.py | 5 +- .../js/spec/views/pages/container_spec.js | 32 +- cms/static/js/views/library_container.js | 71 +++-- cms/static/js/views/pages/container.js | 23 +- cms/static/sass/elements/_pagination.scss | 8 +- cms/static/sass/elements/_xblocks.scss | 2 +- ...ontainer-paged-after-add-xblock.underscore | 283 ------------------ .../js/mock/mock-xblock-paged.underscore | 21 ++ .../xmodule/xmodule/library_root_xblock.py | 3 +- .../xmodule/video_module/video_handlers.py | 1 + 10 files changed, 113 insertions(+), 336 deletions(-) delete mode 100644 cms/templates/js/mock/mock-container-paged-after-add-xblock.underscore create mode 100644 cms/templates/js/mock/mock-xblock-paged.underscore diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 5d05a27e1781..7f68dcd78481 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -207,6 +207,7 @@ def xblock_view_handler(request, usage_key_string, view_name): store = modulestore() xblock = store.get_item(usage_key) container_views = ['container_preview', 'reorderable_container_child_preview'] + library = isinstance(usage_key, LibraryUsageLocator) # wrap the generated fragment in the xmodule_editor div so that the javascript # can bind to it correctly @@ -235,7 +236,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 view_name == 'reorderable_container_child_preview': + if not library and view_name == 'reorderable_container_child_preview': reorderable_items.add(xblock.location) paging = None @@ -259,7 +260,7 @@ def xblock_view_handler(request, usage_key_string, view_name): 'is_unit_page': is_unit(xblock), 'root_xblock': xblock if (view_name == 'container_preview') else None, 'reorderable_items': reorderable_items, - 'paging': paging + 'paging': paging, } fragment = get_preview_fragment(request, xblock, context) diff --git a/cms/static/js/spec/views/pages/container_spec.js b/cms/static/js/spec/views/pages/container_spec.js index 6f4b4baf46af..d5ec6938dc25 100644 --- a/cms/static/js/spec/views/pages/container_spec.js +++ b/cms/static/js/spec/views/pages/container_spec.js @@ -273,7 +273,7 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel }); describe("xblock operations", function () { - var getGroupElement, + var getGroupElement, paginated, NUM_COMPONENTS_PER_GROUP = 3, GROUP_TO_TEST = "A", allComponentsInGroup = _.map( _.range(NUM_COMPONENTS_PER_GROUP), @@ -282,6 +282,11 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel } ); + paginated = function () { + return containerPage.enable_paging; + }; + + getGroupElement = function () { return containerPage.$("[data-locator='locator-group-" + GROUP_TO_TEST + "']"); }; @@ -294,6 +299,7 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel promptSpy = EditHelpers.createPromptSpy(); }); + clickDelete = function (componentIndex, clickNo) { // find all delete buttons for the given group @@ -307,21 +313,25 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel EditHelpers.confirmPrompt(promptSpy, clickNo); }; - deleteComponent = function (componentIndex) { + 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 - 2); + null, requests.length - requestOffset); // final request to refresh the xblock info AjaxHelpers.expectJsonRequest(requests, 'GET', '/xblock/locator-container'); }; deleteComponentWithSuccess = function (componentIndex) { - deleteComponent(componentIndex); + var deleteOffset; + + deleteOffset = paginated() ? 3 : 2; + + deleteComponent(componentIndex, deleteOffset); // verify the new list of components within the group expectComponents( @@ -350,9 +360,16 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel 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 - 2); + null, requests.length - deleteOffset); // expect the last request to be a fetch of the xblock info for the parent container AjaxHelpers.expectJsonRequest(requests, 'GET', '/xblock/locator-container'); }); @@ -511,7 +528,7 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel }); describe('Template Picker', function () { - var showTemplatePicker, verifyCreateHtmlComponent; + var showTemplatePicker, verifyCreateHtmlComponent, call_count; showTemplatePicker = function () { containerPage.$('.new-component .new-component-type a.multiple-templates')[0].click(); @@ -519,6 +536,7 @@ 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; @@ -557,6 +575,6 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel { enable_paging: true, page_size: 42 }, { initial: 'mock/mock-container-paged-xblock.underscore', - add_response: 'mock/mock-container-paged-after-add-xblock.underscore' + add_response: 'mock/mock-xblock-paged.underscore' }); }); diff --git a/cms/static/js/views/library_container.js b/cms/static/js/views/library_container.js index b655833289c9..a8c15999ab3f 100644 --- a/cms/static/js/views/library_container.js +++ b/cms/static/js/views/library_container.js @@ -1,19 +1,16 @@ -define(["jquery", "underscore", "js/views/xblock", "js/utils/module", "gettext", "js/views/feedback_notification", +define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettext", "js/views/feedback_notification", "js/views/paging_header", "js/views/paging_footer"], - function ($, _, XBlockView, ModuleUtils, gettext, NotificationView, PagingHeader, PagingFooter) { - var LibraryContainerView = XBlockView.extend({ + function ($, _, ContainerView, ModuleUtils, gettext, NotificationView, PagingHeader, PagingFooter) { + var LibraryContainerView = ContainerView.extend({ // Store the request token of the first xblock on the page (which we know was rendered by Studio when // the page was generated). Use that request token to filter out user-defined HTML in any // child xblocks within the page. - requestToken: "", initialize: function(options){ var self = this; - XBlockView.prototype.initialize.call(this); + ContainerView.prototype.initialize.call(this); this.page_size = this.options.page_size || 10; - if (options) { - this.page_reload_callback = options.page_reload_callback; - } + this.page_reload_callback = options.page_reload_callback || function () {}; // emulating Backbone.paginator interface this.collection = { currentPage: 0, @@ -30,9 +27,6 @@ define(["jquery", "underscore", "js/views/xblock", "js/utils/module", "gettext", render: function(options) { var eff_options = options || {}; - if (eff_options.block_added) { - this.collection.currentPage = this.getPageCount(this.collection.totalCount+1) - 1; - } eff_options.page_number = typeof eff_options.page_number !== "undefined" ? eff_options.page_number : this.collection.currentPage; @@ -53,9 +47,8 @@ define(["jquery", "underscore", "js/views/xblock", "js/utils/module", "gettext", success: function(fragment) { self.handleXBlockFragment(fragment, options); self.processPaging({ requested_page: options.page_number }); - if (options.paging && self.page_reload_callback){ - self.page_reload_callback(self.$el); - } + // This is expected to render the add xblock components menu. + self.page_reload_callback(self.$el) } }); }, @@ -69,12 +62,12 @@ define(["jquery", "underscore", "js/views/xblock", "js/utils/module", "gettext", }, getPageCount: function(total_count){ - if (total_count==0) return 1; + if (total_count===0) return 1; return Math.ceil(total_count / this.page_size); }, setPage: function(page_number) { - this.render({ page_number: page_number, paging: true }); + this.render({ page_number: page_number}); }, nextPage: function() { @@ -129,32 +122,54 @@ define(["jquery", "underscore", "js/views/xblock", "js/utils/module", "gettext", }, xblockReady: function () { - XBlockView.prototype.xblockReady.call(this); + ContainerView.prototype.xblockReady.call(this); this.requestToken = this.$('div.xblock').first().data('request-token'); }, - refresh: function() { }, + refresh: function(block_added) { + if (block_added) { + this.collection.totalCount += 1; + this.collection._size +=1; + if (this.collection.totalCount == 1) { + this.render(); + return + } + this.collection.totalPages = this.getPageCount(this.collection.totalCount); + var new_page = this.collection.totalPages - 1; + // If we're on a new page due to overflow, or this is the first item, set the page. + if (((this.collection.currentPage) != new_page) || this.collection.totalCount == 1) { + this.setPage(new_page); + } else { + this.pagingHeader.render(); + this.pagingFooter.render(); + } + } + }, acknowledgeXBlockDeletion: function (locator){ this.notifyRuntime('deleted-child', locator); this.collection._size -= 1; this.collection.totalCount -= 1; - // pages are counted from 0 - thus currentPage == 1 if we're on second page - if (this.collection._size == 0 && this.collection.currentPage >= 1) { - this.setPage(this.collection.currentPage - 1); - this.collection.totalPages -= 1; - } - else { + var current_page = this.collection.currentPage; + var total_pages = this.getPageCount(this.collection.totalCount); + this.collection.totalPages = total_pages; + // Starts counting from 0 + if ((current_page + 1) > total_pages) { + // The number of total pages has changed. Move down. + // Also, be mindful of the off-by-one. + this.setPage(total_pages - 1) + } else if ((current_page + 1) != total_pages) { + // Refresh page to get any blocks shifted from the next page. + this.setPage(current_page) + } else { + // We're on the last page, just need to update the numbers in the + // pagination interface. this.pagingHeader.render(); this.pagingFooter.render(); } }, - makeRequestSpecificSelector: function(selector) { - return 'div.xblock[data-request-token="' + this.requestToken + '"] > ' + selector; - }, - sortDisplayName: function() { return "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 c63e7f00f6e0..771e9afb1b89 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -119,8 +119,11 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views // Notify the runtime that the page has been successfully shown xblockView.notifyRuntime('page-shown', self); - // Render the add buttons - self.renderAddXBlockComponents(); + // Render the add buttons. Paged containers should do this on their own. + if (!self.enable_paging) { + // Render the add buttons + self.renderAddXBlockComponents(); + } // Refresh the views now that the xblock is visible self.onXBlockRefresh(xblockView); @@ -141,8 +144,8 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views return this.xblockView.model.urlRoot; }, - onXBlockRefresh: function(xblockView) { - this.xblockView.refresh(); + onXBlockRefresh: function(xblockView, block_added) { + this.xblockView.refresh(block_added); // Update publish and last modified information from the server. this.model.fetch(); }, @@ -274,10 +277,10 @@ 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.refreshChildXBlock(xblockElement); + } else if (parentElement.hasClass('reorderable-container') || this.enable_paging) { + this.refreshChildXBlock(xblockElement, block_added); } else { - this.refreshXBlock(this.findXBlockElement(parentElement), block_added); + this.refreshXBlock(this.findXBlockElement(parentElement)); } }, @@ -285,9 +288,11 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views * Refresh an xblock element inline on the page, using the specified xblockInfo. * Note that the element is removed and replaced with the newly rendered xblock. * @param xblockElement The xblock element to be refreshed. + * @param block_added Specifies if a block has been added, rather than just needs + * refreshing. * @returns {jQuery promise} A promise representing the complete operation. */ - refreshChildXBlock: function(xblockElement) { + refreshChildXBlock: function(xblockElement, block_added) { var self = this, xblockInfo, TemporaryXBlockView, @@ -313,7 +318,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views }); return temporaryView.render({ success: function() { - self.onXBlockRefresh(temporaryView); + self.onXBlockRefresh(temporaryView, block_added); temporaryView.unbind(); // Remove the temporary view } }); diff --git a/cms/static/sass/elements/_pagination.scss b/cms/static/sass/elements/_pagination.scss index f3ba465b8086..379d8785e315 100644 --- a/cms/static/sass/elements/_pagination.scss +++ b/cms/static/sass/elements/_pagination.scss @@ -2,7 +2,7 @@ // ========================== %pagination { - @include clearfix; + @include clearfix(); display: inline-block; width: flex-grid(3, 12); @@ -48,7 +48,7 @@ } .nav-label { - @extend .sr; + @extend %cont-text-sr; } .pagination-form, @@ -89,7 +89,7 @@ .page-number-label, .submit-pagination-form { - @extend .sr; + @extend %cont-text-sr; } .page-number-input { @@ -116,4 +116,4 @@ } } } -} \ No newline at end of file +} diff --git a/cms/static/sass/elements/_xblocks.scss b/cms/static/sass/elements/_xblocks.scss index 938f000d8e53..0473d748e7d6 100644 --- a/cms/static/sass/elements/_xblocks.scss +++ b/cms/static/sass/elements/_xblocks.scss @@ -105,7 +105,7 @@ .container-paging-header { .meta-wrap { - margin: $baseline $baseline/2; + margin: $baseline ($baseline/2); } .meta { @extend %t-copy-sub2; diff --git a/cms/templates/js/mock/mock-container-paged-after-add-xblock.underscore b/cms/templates/js/mock/mock-container-paged-after-add-xblock.underscore deleted file mode 100644 index cb260c9bca9d..000000000000 --- a/cms/templates/js/mock/mock-container-paged-after-add-xblock.underscore +++ /dev/null @@ -1,283 +0,0 @@ - -
-
-
- Test Container -
-
-
    -
-
-
-
-
-
- - - - -
- -
-
-
-
-
- - - Expand or Collapse - - Group A -
-
-
    -
  • - -
    • -
-
-
-
-
-
-
-
-
-
-
-
    -
  • - -
    • -
  • - -
    • -
  • - -
    • -
  • - -
    • -
-
-
-
-
-
-
-
-
-
-
-
-
    -
  • - -
    • -
  • - -
    • -
  • - -
    • -
  • - -
    • -
-
-
-
-
-
-
-
-
-
-
-
-
    -
  • - -
    • -
  • - -
    • -
  • - -
    • -
  • - -
    • -
-
-
-
-
-
-
-
-
-
-
-
-
    -
  • - -
    • -
  • - -
    • -
  • - -
    • -
  • - -
    • -
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
- - - Expand or Collapse - - Group B -
-
-
    -
  • - -
    • -
-
-
-
- -
-
-
-
-
-
-
-
    -
  • - -
    • -
  • - -
    • -
  • - -
    • -
  • - -
    • -
-
-
-
-
-
-
-
-
-
-
-
-
    -
  • - -
    • -
  • - -
    • -
  • - -
    • -
  • - -
    • -
-
-
-
-
-
-
-
-
-
-
-
-
    -
  • - -
    • -
  • - -
    • -
  • - -
    • -
  • - -
    • -
-
-
-
-
-
-
-
-
-
-
-
-
-
diff --git a/cms/templates/js/mock/mock-xblock-paged.underscore b/cms/templates/js/mock/mock-xblock-paged.underscore new file mode 100644 index 000000000000..c6c2c881d8d2 --- /dev/null +++ b/cms/templates/js/mock/mock-xblock-paged.underscore @@ -0,0 +1,21 @@ +
+
+
+ Mock XBlock +
+
+
    +
+ + Add Missing Groups + +
+
+
+
+

Mock XBlock

+
+
+
diff --git a/common/lib/xmodule/xmodule/library_root_xblock.py b/common/lib/xmodule/xmodule/library_root_xblock.py index 497a145b79bd..3118f9a25842 100644 --- a/common/lib/xmodule/xmodule/library_root_xblock.py +++ b/common/lib/xmodule/xmodule/library_root_xblock.py @@ -76,7 +76,7 @@ def render_children(self, context, fragment, can_reorder=False, can_add=False): fragment.add_frag_resources(rendered_child) contents.append({ - 'id': child.location.to_deprecated_string(), + 'id': unicode(child.location), 'content': rendered_child.content }) @@ -85,7 +85,6 @@ def render_children(self, context, fragment, can_reorder=False, can_add=False): 'items': contents, 'xblock_context': context, 'can_add': can_add, - 'can_reorder': False, 'first_displayed': item_start, 'total_children': children_count, 'displayed_children': len(children_to_show) diff --git a/common/lib/xmodule/xmodule/video_module/video_handlers.py b/common/lib/xmodule/xmodule/video_module/video_handlers.py index 1ba427c35785..9e9db860ca54 100644 --- a/common/lib/xmodule/xmodule/video_module/video_handlers.py +++ b/common/lib/xmodule/xmodule/video_module/video_handlers.py @@ -155,6 +155,7 @@ def get_static_transcript(self, request): if transcript_name: # Get the asset path for course + asset_path = None course = self.descriptor.runtime.modulestore.get_course(self.course_id) if course.static_asset_path: asset_path = course.static_asset_path From 1581a398248e54a85505706a216ccc6a9a4c2eb5 Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Thu, 11 Dec 2014 19:53:08 +0000 Subject: [PATCH 4/5] Factored out Pagination into its own Container view. --- cms/static/js/views/library_container.js | 180 +---------------------- cms/static/js/views/paged_container.js | 158 ++++++++++++++++++++ cms/static/js/views/paging.js | 40 +---- cms/static/js/views/paging_mixin.js | 37 +++++ 4 files changed, 202 insertions(+), 213 deletions(-) create mode 100644 cms/static/js/views/paged_container.js create mode 100644 cms/static/js/views/paging_mixin.js diff --git a/cms/static/js/views/library_container.js b/cms/static/js/views/library_container.js index a8c15999ab3f..7c48e83cee8f 100644 --- a/cms/static/js/views/library_container.js +++ b/cms/static/js/views/library_container.js @@ -1,179 +1,7 @@ -define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettext", "js/views/feedback_notification", +define(["jquery", "underscore", "js/views/paged_container", "js/utils/module", "gettext", "js/views/feedback_notification", "js/views/paging_header", "js/views/paging_footer"], - function ($, _, ContainerView, ModuleUtils, gettext, NotificationView, PagingHeader, PagingFooter) { - var LibraryContainerView = ContainerView.extend({ - // Store the request token of the first xblock on the page (which we know was rendered by Studio when - // the page was generated). Use that request token to filter out user-defined HTML in any - // child xblocks within the page. - - 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.collection = { - currentPage: 0, - totalPages: 0, - totalCount: 0, - sortDirection: "desc", - start: 0, - _size: 0, - - bind: function() {}, // no-op - size: function() { return self.collection._size; } - }; - }, - - render: function(options) { - var eff_options = options || {}; - eff_options.page_number = typeof eff_options.page_number !== "undefined" - ? eff_options.page_number - : this.collection.currentPage; - return this.renderPage(eff_options); - }, - - renderPage: function(options){ - var self = this, - view = this.view, - xblockInfo = this.model, - xblockUrl = xblockInfo.url(); - return $.ajax({ - url: decodeURIComponent(xblockUrl) + "/" + view, - type: 'GET', - cache: false, - data: this.getRenderParameters(options.page_number), - headers: { Accept: 'application/json' }, - 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) - } - }); - }, - - getRenderParameters: function(page_number) { - return { - enable_paging: true, - page_size: this.page_size, - page_number: page_number - }; - }, - - getPageCount: function(total_count){ - if (total_count===0) return 1; - return Math.ceil(total_count / this.page_size); - }, - - setPage: function(page_number) { - this.render({ page_number: page_number}); - }, - - nextPage: function() { - var collection = this.collection, - currentPage = collection.currentPage, - lastPage = collection.totalPages - 1; - if (currentPage < lastPage) { - this.setPage(currentPage + 1); - } - }, - - previousPage: function() { - var collection = this.collection, - currentPage = collection.currentPage; - if (currentPage > 0) { - this.setPage(currentPage - 1); - } - }, - - processPaging: function(options){ - var $element = this.$el.find('.xblock-container-paging-parameters'), - total = $element.data('total'), - displayed = $element.data('displayed'), - start = $element.data('start'); - - this.collection.currentPage = options.requested_page; - this.collection.totalCount = total; - this.collection.totalPages = this.getPageCount(total); - this.collection.start = start; - this.collection._size = displayed; - - this.processPagingHeaderAndFooter(); - }, - - processPagingHeaderAndFooter: function(){ - if (this.pagingHeader) - this.pagingHeader.undelegateEvents(); - if (this.pagingFooter) - this.pagingFooter.undelegateEvents(); - - this.pagingHeader = new PagingHeader({ - view: this, - el: this.$el.find('.container-paging-header') - }); - this.pagingFooter = new PagingFooter({ - view: this, - el: this.$el.find('.container-paging-footer') - }); - - this.pagingHeader.render(); - 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; - this.collection._size +=1; - if (this.collection.totalCount == 1) { - this.render(); - return - } - this.collection.totalPages = this.getPageCount(this.collection.totalCount); - var new_page = this.collection.totalPages - 1; - // If we're on a new page due to overflow, or this is the first item, set the page. - if (((this.collection.currentPage) != new_page) || this.collection.totalCount == 1) { - this.setPage(new_page); - } else { - this.pagingHeader.render(); - this.pagingFooter.render(); - } - } - }, - - acknowledgeXBlockDeletion: function (locator){ - this.notifyRuntime('deleted-child', locator); - this.collection._size -= 1; - this.collection.totalCount -= 1; - var current_page = this.collection.currentPage; - var total_pages = this.getPageCount(this.collection.totalCount); - this.collection.totalPages = total_pages; - // Starts counting from 0 - if ((current_page + 1) > total_pages) { - // The number of total pages has changed. Move down. - // Also, be mindful of the off-by-one. - this.setPage(total_pages - 1) - } else if ((current_page + 1) != total_pages) { - // Refresh page to get any blocks shifted from the next page. - this.setPage(current_page) - } else { - // We're on the last page, just need to update the numbers in the - // pagination interface. - this.pagingHeader.render(); - this.pagingFooter.render(); - } - }, - - sortDisplayName: function() { - return "Date added"; // TODO add support for sorting - } - }); - + function ($, _, PagedContainerView) { + // To be extended with Library-specific features later. + var LibraryContainerView = PagedContainerView; return LibraryContainerView; }); // end define(); diff --git a/cms/static/js/views/paged_container.js b/cms/static/js/views/paged_container.js new file mode 100644 index 000000000000..cd7590156a17 --- /dev/null +++ b/cms/static/js/views/paged_container.js @@ -0,0 +1,158 @@ +define(["jquery", "underscore", "js/views/container", "js/utils/module", "gettext", + "js/views/feedback_notification", "js/views/paging_header", "js/views/paging_footer", "js/views/paging_mixin"], + function ($, _, ContainerView, ModuleUtils, gettext, NotificationView, PagingHeader, PagingFooter, PagingMixin) { + var PagedContainerView = ContainerView.extend(PagingMixin).extend({ + 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.collection = { + currentPage: 0, + totalPages: 0, + totalCount: 0, + sortDirection: "desc", + start: 0, + _size: 0, + + bind: function() {}, // no-op + size: function() { return self.collection._size; } + }; + }, + + render: function(options) { + var eff_options = options || {}; + eff_options.page_number = typeof eff_options.page_number !== "undefined" + ? eff_options.page_number + : this.collection.currentPage; + return this.renderPage(eff_options); + }, + + renderPage: function(options){ + var self = this, + view = this.view, + xblockInfo = this.model, + xblockUrl = xblockInfo.url(); + return $.ajax({ + url: decodeURIComponent(xblockUrl) + "/" + view, + type: 'GET', + cache: false, + data: this.getRenderParameters(options.page_number), + headers: { Accept: 'application/json' }, + 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) + } + }); + }, + + getRenderParameters: function(page_number) { + return { + enable_paging: true, + page_size: this.page_size, + page_number: page_number + }; + }, + + getPageCount: function(total_count){ + if (total_count===0) return 1; + return Math.ceil(total_count / this.page_size); + }, + + setPage: function(page_number) { + this.render({ page_number: page_number}); + }, + + processPaging: function(options){ + var $element = this.$el.find('.xblock-container-paging-parameters'), + total = $element.data('total'), + displayed = $element.data('displayed'), + start = $element.data('start'); + + this.collection.currentPage = options.requested_page; + this.collection.totalCount = total; + this.collection.totalPages = this.getPageCount(total); + this.collection.start = start; + this.collection._size = displayed; + + this.processPagingHeaderAndFooter(); + }, + + processPagingHeaderAndFooter: function(){ + if (this.pagingHeader) + this.pagingHeader.undelegateEvents(); + if (this.pagingFooter) + this.pagingFooter.undelegateEvents(); + + this.pagingHeader = new PagingHeader({ + view: this, + el: this.$el.find('.container-paging-header') + }); + this.pagingFooter = new PagingFooter({ + view: this, + el: this.$el.find('.container-paging-footer') + }); + + this.pagingHeader.render(); + 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; + this.collection._size +=1; + if (this.collection.totalCount == 1) { + this.render(); + return + } + this.collection.totalPages = this.getPageCount(this.collection.totalCount); + var new_page = this.collection.totalPages - 1; + // If we're on a new page due to overflow, or this is the first item, set the page. + if (((this.collection.currentPage) != new_page) || this.collection.totalCount == 1) { + this.setPage(new_page); + } else { + this.pagingHeader.render(); + this.pagingFooter.render(); + } + } + }, + + acknowledgeXBlockDeletion: function (locator){ + this.notifyRuntime('deleted-child', locator); + this.collection._size -= 1; + this.collection.totalCount -= 1; + var current_page = this.collection.currentPage; + var total_pages = this.getPageCount(this.collection.totalCount); + this.collection.totalPages = total_pages; + // Starts counting from 0 + if ((current_page + 1) > total_pages) { + // The number of total pages has changed. Move down. + // Also, be mindful of the off-by-one. + this.setPage(total_pages - 1) + } else if ((current_page + 1) != total_pages) { + // Refresh page to get any blocks shifted from the next page. + this.setPage(current_page) + } else { + // We're on the last page, just need to update the numbers in the + // pagination interface. + this.pagingHeader.render(); + this.pagingFooter.render(); + } + }, + + sortDisplayName: function() { + return "Date added"; // TODO add support for sorting + } + }); + + return PagedContainerView; + }); // end define(); diff --git a/cms/static/js/views/paging.js b/cms/static/js/views/paging.js index c6c3a491ca04..c4d9b1b602f5 100644 --- a/cms/static/js/views/paging.js +++ b/cms/static/js/views/paging.js @@ -1,7 +1,7 @@ -define(["underscore", "js/views/baseview", "js/views/feedback_alert", "gettext"], - function(_, BaseView, AlertView, gettext) { +define(["underscore", "js/views/baseview", "js/views/feedback_alert", "gettext", "js/views/paging_mixin"], + function(_, BaseView, AlertView, gettext, PagingMixin) { - var PagingView = BaseView.extend({ + var PagingView = BaseView.extend(PagingMixin).extend({ // takes a Backbone Paginator as a model sortableColumns: {}, @@ -21,43 +21,10 @@ define(["underscore", "js/views/baseview", "js/views/feedback_alert", "gettext"] this.$('#' + sortColumn).addClass('current-sort'); }, - setPage: function(page) { - var self = this, - collection = self.collection, - oldPage = collection.currentPage; - collection.goTo(page, { - reset: true, - success: function() { - window.scrollTo(0, 0); - }, - error: function(collection) { - collection.currentPage = oldPage; - self.onError(); - } - }); - }, - onError: function() { // Do nothing by default }, - nextPage: function() { - var collection = this.collection, - currentPage = collection.currentPage, - lastPage = collection.totalPages - 1; - if (currentPage < lastPage) { - this.setPage(currentPage + 1); - } - }, - - previousPage: function() { - var collection = this.collection, - currentPage = collection.currentPage; - if (currentPage > 0) { - this.setPage(currentPage - 1); - } - }, - /** * Registers information about a column that can be sorted. * @param columnName The element name of the column. @@ -110,6 +77,5 @@ define(["underscore", "js/views/baseview", "js/views/feedback_alert", "gettext"] this.setPage(0); } }); - return PagingView; }); // end define(); diff --git a/cms/static/js/views/paging_mixin.js b/cms/static/js/views/paging_mixin.js new file mode 100644 index 000000000000..d2c1700e5d64 --- /dev/null +++ b/cms/static/js/views/paging_mixin.js @@ -0,0 +1,37 @@ +define(["jquery", "underscore"], + function ($, _) { + var PagedMixin = { + setPage: function (page) { + var self = this, + collection = self.collection, + oldPage = collection.currentPage; + collection.goTo(page, { + reset: true, + success: function () { + window.scrollTo(0, 0); + }, + error: function (collection) { + collection.currentPage = oldPage; + self.onError(); + } + }); + }, + nextPage: function() { + var collection = this.collection, + currentPage = collection.currentPage, + lastPage = collection.totalPages - 1; + if (currentPage < lastPage) { + this.setPage(currentPage + 1); + } + }, + + previousPage: function() { + var collection = this.collection, + currentPage = collection.currentPage; + if (currentPage > 0) { + this.setPage(currentPage - 1); + } + } + }; + return PagedMixin; + }); From be3371ee85cb629f91705c2ae045cf418d3a286a Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Fri, 12 Dec 2014 19:19:56 +0000 Subject: [PATCH 5/5] Addressed further review notes for Library Pagination --- cms/djangoapps/contentstore/views/item.py | 19 +- cms/static/coffee/spec/main.coffee | 2 +- cms/static/js/factories/container.js | 10 +- cms/static/js/factories/library.js | 11 +- ...tainer_spec.js => paged_container_spec.js} | 4 +- .../js/spec/views/pages/container_spec.js | 43 ++-- cms/static/js/views/container.js | 2 + cms/static/js/views/library_container.js | 5 +- cms/static/js/views/paged_container.js | 44 ++-- cms/static/js/views/pages/container.js | 50 ++-- cms/static/js/views/pages/paged_container.js | 36 +++ cms/static/js/views/paging_footer.js | 2 + cms/static/js/views/paging_mixin.js | 4 +- cms/templates/library.html | 1 - .../xmodule/xmodule/library_root_xblock.py | 5 +- .../test/acceptance/pages/studio/library.py | 56 +---- .../acceptance/pages/studio/pagination.py | 62 +++++ .../tests/studio/test_studio_library.py | 222 ++++++++++-------- 18 files changed, 334 insertions(+), 244 deletions(-) rename cms/static/js/spec/views/{library_container_spec.js => paged_container_spec.js} (99%) create mode 100644 cms/static/js/views/pages/paged_container.js create mode 100644 common/test/acceptance/pages/studio/pagination.py diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 7f68dcd78481..c1f627009cd0 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -206,8 +206,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 @@ -236,7 +235,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 @@ -247,11 +246,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 e2ba18a82198..ba732d20c166 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -232,7 +232,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 ea48bb2a989f..429ae58f5151 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 e7834f60ef3c..76ac47413ddc 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 2d39cdc35819..524f88e552f7 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 d5ec6938dc25..ce862aac7d7f 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 ec89208b4435..a99993fe5da3 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 7c48e83cee8f..ea09c69c8929 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 cd7590156a17..a8cd7aec3242 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 771e9afb1b89..406e6e9b0354 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,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({ 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 000000000000..916bf3005e43 --- /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 5a42c1d03c92..4ec3501d4482 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 d2c1700e5d64..16d518f856d4 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 dc9baa5736c3..d367c333d270 100644 --- a/cms/templates/library.html +++ b/cms/templates/library.html @@ -25,7 +25,6 @@ ${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 3118f9a25842..6a58cf1b1d18 100644 --- a/common/lib/xmodule/xmodule/library_root_xblock.py +++ b/common/lib/xmodule/xmodule/library_root_xblock.py @@ -50,8 +50,7 @@ def author_view(self, context): 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 @@ def render_children(self, context, fragment, can_reorder=False, can_add=False): 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 5572b1a91e43..64f93f21167e 100644 --- a/common/test/acceptance/pages/studio/library.py +++ b/common/test/acceptance/pages/studio/library.py @@ -3,14 +3,14 @@ """ 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 @@ def click_delete_button(self, xblock_id, confirm=True): 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 000000000000..a976149c37dd --- /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 5529f3603293..491c9093d0fc 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 @@ 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 @@ def test_nav_present_but_disabled(self, position): 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 @@ def test_bogus_page_selection(self): 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')