MDL-67623 block overview: fix pagination regression
authorTom Dickman <tomdickman@catalyst-au.net>
Thu, 19 Nov 2020 04:40:09 +0000 (15:40 +1100)
committerTom Dickman <tomdickman@catalyst-au.net>
Mon, 14 Dec 2020 02:28:11 +0000 (13:28 +1100)
Fixed a regression caused by MDL-64194 resulting in Dashboard - Course
overview pagination not working from second page of courses onwards.

Course overview block should now do the following:
- Display no pagination controls when user has no courses
- Display no pagination controls when less than 12 courses to display
- Only display pagination controls up to the number of courses user is
enrolled in
- Work correctly when on a page greater than the second page.

blocks/myoverview/amd/build/view.min.js
blocks/myoverview/amd/build/view.min.js.map
blocks/myoverview/amd/src/view.js
blocks/myoverview/classes/output/main.php
blocks/myoverview/tests/behat/block_myoverview_pagelimit_persistence.feature
blocks/myoverview/tests/behat/block_myoverview_pagination.feature [new file with mode: 0644]
lib/amd/build/paged_content_factory.min.js
lib/amd/build/paged_content_factory.min.js.map
lib/amd/src/paged_content_factory.js
lib/templates/paged_content_paging_bar.mustache

index 8e18c99..17ed845 100644 (file)
Binary files a/blocks/myoverview/amd/build/view.min.js and b/blocks/myoverview/amd/build/view.min.js differ
index 723993f..e238948 100644 (file)
Binary files a/blocks/myoverview/amd/build/view.min.js.map and b/blocks/myoverview/amd/build/view.min.js.map differ
index 524c4ba..1c92361 100644 (file)
@@ -543,11 +543,9 @@ function(
 
         // Filter out all pagination options which are too large for the amount of courses user is enrolled in.
         var totalCourseCount = parseInt(root.find(Selectors.courseView.region).attr('data-totalcoursecount'), 10);
-        if (totalCourseCount) {
-            itemsPerPage = itemsPerPage.filter(function(pagingOption) {
-                return pagingOption.value < totalCourseCount;
-            });
-        }
+        itemsPerPage = itemsPerPage.filter(function(pagingOption) {
+            return pagingOption.value < totalCourseCount || pagingOption.value === 0;
+        });
 
         var filters = getFilterValues(root);
         var config = $.extend({}, DEFAULT_PAGED_CONTENT_CONFIG);
@@ -602,7 +600,8 @@ function(
                                 pageCourses = $.merge(loadedPages[currentPage].courses, courses.slice(0, nextPageStart));
                             }
                         } else {
-                            nextPageStart = pageData.limit;
+                            // When the page limit is zero, there is only one page of courses, no start for next page.
+                            nextPageStart = pageData.limit || false;
                             pageCourses = (pageData.limit > 0) ? courses.slice(0, pageData.limit) : courses;
                         }
 
@@ -611,8 +610,8 @@ function(
                             courses: pageCourses
                         };
 
-                        // Set up the next page
-                        var remainingCourses = nextPageStart ? courses.slice(nextPageStart, courses.length) : [];
+                        // Set up the next page (if there is more than one page).
+                        var remainingCourses = nextPageStart !== false ? courses.slice(nextPageStart, courses.length) : [];
                         if (remainingCourses.length) {
                             loadedPages[currentPage + 1] = {
                                 courses: remainingCourses
index c290c98..b0236fd 100644 (file)
@@ -202,8 +202,9 @@ class main implements renderable, templatable {
         // Check and remember the given view.
         $this->view = $view ? $view : BLOCK_MYOVERVIEW_VIEW_CARD;
 
-        // Check and remember the given page size.
-        if ($paging == BLOCK_MYOVERVIEW_PAGING_ALL) {
+        // Check and remember the given page size, `null` indicates no page size set
+        // while a `0` indicates a paging size of `All`.
+        if (!is_null($paging) && $paging == BLOCK_MYOVERVIEW_PAGING_ALL) {
             $this->paging = BLOCK_MYOVERVIEW_PAGING_ALL;
         } else {
             $this->paging = $paging ? $paging : BLOCK_MYOVERVIEW_PAGING_12;
index a192c3d..fef9a8b 100644 (file)
@@ -7,30 +7,30 @@ Feature: The my overview block allows users to persistence of their page limits
       | student1 | Student   | X        | student1@example.com | S1       |
     And the following "courses" exist:
       | fullname | shortname | category |
-      | Course 1 | C1        | 0        |
-      | Course 2 | C2        | 0        |
-      | Course 3 | C3        | 0        |
-      | Course 4 | C4        | 0        |
-      | Course 5 | C5        | 0        |
-      | Course 6 | C6        | 0        |
-      | Course 7 | C7        | 0        |
-      | Course 8 | C8        | 0        |
-      | Course 9 | C9        | 0        |
+      | Course 1 | C01        | 0        |
+      | Course 2 | C02        | 0        |
+      | Course 3 | C03        | 0        |
+      | Course 4 | C04        | 0        |
+      | Course 5 | C05        | 0        |
+      | Course 6 | C06        | 0        |
+      | Course 7 | C07        | 0        |
+      | Course 8 | C08        | 0        |
+      | Course 9 | C09        | 0        |
       | Course 10 | C10        | 0        |
       | Course 11 | C11        | 0        |
       | Course 12 | C12        | 0        |
       | Course 13 | C13        | 0        |
     And the following "course enrolments" exist:
       | user | course | role |
-      | student1 | C1 | student |
-      | student1 | C2 | student |
-      | student1 | C3 | student |
-      | student1 | C4 | student |
-      | student1 | C5 | student |
-      | student1 | C6 | student |
-      | student1 | C7 | student |
-      | student1 | C8 | student |
-      | student1 | C9 | student |
+      | student1 | C01 | student |
+      | student1 | C02 | student |
+      | student1 | C03 | student |
+      | student1 | C04 | student |
+      | student1 | C05 | student |
+      | student1 | C06 | student |
+      | student1 | C07 | student |
+      | student1 | C08 | student |
+      | student1 | C09 | student |
       | student1 | C10 | student |
       | student1 | C11 | student |
       | student1 | C12 | student |
@@ -38,8 +38,8 @@ Feature: The my overview block allows users to persistence of their page limits
 
   Scenario: Toggle the page limit between page reloads
     Given I log in as "student1"
-    When I click on "[data-toggle='dropdown']" "css_element" in the "Course overview" "block"
-    And I click on "All" "link"
+    When I click on "[data-action='limit-toggle']" "css_element" in the "Course overview" "block"
+    And I click on "All" "link" in the ".dropdown-menu.show" "css_element"
     Then I should see "Course 13"
     And I reload the page
     Then I should see "Course 13"
@@ -47,8 +47,8 @@ Feature: The my overview block allows users to persistence of their page limits
 
   Scenario: Toggle the page limit between grouping changes
     Given I log in as "student1"
-    When I click on "[data-toggle='dropdown']" "css_element" in the "Course overview" "block"
-    And I click on "All" "link"
+    When I click on "[data-action='limit-toggle']" "css_element" in the "Course overview" "block"
+    And I click on "All" "link" in the ".dropdown-menu.show" "css_element"
     And I click on "All (except removed from view)" "button" in the "Course overview" "block"
     And I click on "In progress" "link" in the "Course overview" "block"
     Then I should see "Course 13"
diff --git a/blocks/myoverview/tests/behat/block_myoverview_pagination.feature b/blocks/myoverview/tests/behat/block_myoverview_pagination.feature
new file mode 100644 (file)
index 0000000..9792252
--- /dev/null
@@ -0,0 +1,191 @@
+@block @block_myoverview @javascript
+Feature: My overview block pagination
+
+  Background:
+    Given the following "users" exist:
+      | username | firstname | lastname | email                | idnumber |
+      | student1 | Student   | X        | student1@example.com | S1       |
+    And the following "courses" exist:
+      | fullname | shortname | category |
+      | Course 01 | C1       | 0        |
+      | Course 02 | C2       | 0        |
+      | Course 03 | C3       | 0        |
+      | Course 04 | C4       | 0        |
+      | Course 05 | C5       | 0        |
+      | Course 06 | C6       | 0        |
+      | Course 07 | C7       | 0        |
+      | Course 08 | C8       | 0        |
+      | Course 09 | C9       | 0        |
+      | Course 10 | C10      | 0        |
+      | Course 11 | C11      | 0        |
+      | Course 12 | C12      | 0        |
+      | Course 13 | C13      | 0        |
+      | Course 14 | C14      | 0        |
+      | Course 15 | C15      | 0        |
+      | Course 16 | C16      | 0        |
+      | Course 17 | C17      | 0        |
+      | Course 18 | C18      | 0        |
+      | Course 19 | C19      | 0        |
+      | Course 20 | C20      | 0        |
+      | Course 21 | C21      | 0        |
+      | Course 22 | C22      | 0        |
+      | Course 23 | C23      | 0        |
+      | Course 24 | C24      | 0        |
+      | Course 25 | C25      | 0        |
+
+  Scenario: The pagination controls should be hidden if I am not enrolled in any courses
+    When I log in as "student1"
+    Then I should see "No courses" in the "Course overview" "block"
+    And I should not see "Show" in the "Course overview" "block"
+    And ".block_myoverview .dropdown-menu.show" "css_element" should not be visible
+    And ".block_myoverview [data-control='next']" "css_element" should not be visible
+    And ".block_myoverview [data-control='previous']" "css_element" should not be visible
+    And I log out
+
+  Scenario: The pagination controls should be hidden if I am enrolled in 12 courses or less
+    Given the following "course enrolments" exist:
+      | user | course | role |
+      | student1 | C1 | student |
+      | student1 | C2 | student |
+      | student1 | C3 | student |
+      | student1 | C4 | student |
+      | student1 | C5 | student |
+      | student1 | C6 | student |
+      | student1 | C7 | student |
+      | student1 | C8 | student |
+      | student1 | C9 | student |
+      | student1 | C10 | student |
+      | student1 | C11 | student |
+      | student1 | C12 | student |
+    When I log in as "student1"
+    Then I should not see "Show" in the "Course overview" "block"
+    And ".block_myoverview .dropdown-menu.show" "css_element" should not be visible
+    And ".block_myoverview [data-control='next']" "css_element" should not be visible
+    And ".block_myoverview [data-control='previous']" "css_element" should not be visible
+    And I log out
+
+  Scenario: The default pagination should be 12 courses
+    Given the following "course enrolments" exist:
+      | user | course | role |
+      | student1 | C1 | student |
+      | student1 | C2 | student |
+      | student1 | C3 | student |
+      | student1 | C4 | student |
+      | student1 | C5 | student |
+      | student1 | C6 | student |
+      | student1 | C7 | student |
+      | student1 | C8 | student |
+      | student1 | C9 | student |
+      | student1 | C10 | student |
+      | student1 | C11 | student |
+      | student1 | C12 | student |
+      | student1 | C13 | student |
+    When I log in as "student1"
+    Then I should see "12" in the "[data-action='limit-toggle']" "css_element"
+    And I log out
+
+  Scenario: I should only see pagination limit options less than total number of enrolled courses
+    Given the following "course enrolments" exist:
+      | user | course | role |
+      | student1 | C1 | student |
+      | student1 | C2 | student |
+      | student1 | C3 | student |
+      | student1 | C4 | student |
+      | student1 | C5 | student |
+      | student1 | C6 | student |
+      | student1 | C7 | student |
+      | student1 | C8 | student |
+      | student1 | C9 | student |
+      | student1 | C10 | student |
+      | student1 | C11 | student |
+      | student1 | C12 | student |
+      | student1 | C13 | student |
+    And I log in as "student1"
+    When I click on "[data-action='limit-toggle']" "css_element" in the "Course overview" "block"
+    Then I should see "All" in the ".dropdown-menu.show" "css_element"
+    And I should see "12" in the ".dropdown-menu.show" "css_element"
+    And ".block_myoverview [data-control='next']" "css_element" should be visible
+    And ".block_myoverview [data-control='previous']" "css_element" should be visible
+    But I should not see "24" in the ".block_myoverview .dropdown-menu.show" "css_element"
+    And I log out
+
+  Scenario: Previous page button should be disabled when on the first page of courses
+    Given the following "course enrolments" exist:
+      | user | course | role |
+      | student1 | C1 | student |
+      | student1 | C2 | student |
+      | student1 | C3 | student |
+      | student1 | C4 | student |
+      | student1 | C5 | student |
+      | student1 | C6 | student |
+      | student1 | C7 | student |
+      | student1 | C8 | student |
+      | student1 | C9 | student |
+      | student1 | C10 | student |
+      | student1 | C11 | student |
+      | student1 | C12 | student |
+      | student1 | C13 | student |
+    When I log in as "student1"
+    Then the "class" attribute of ".block_myoverview [data-control='previous']" "css_element" should contain "disabled"
+    And I log out
+
+  Scenario: Next page button should be disabled when on the last page of courses
+    Given the following "course enrolments" exist:
+      | user | course | role |
+      | student1 | C1 | student |
+      | student1 | C2 | student |
+      | student1 | C3 | student |
+      | student1 | C4 | student |
+      | student1 | C5 | student |
+      | student1 | C6 | student |
+      | student1 | C7 | student |
+      | student1 | C8 | student |
+      | student1 | C9 | student |
+      | student1 | C10 | student |
+      | student1 | C11 | student |
+      | student1 | C12 | student |
+      | student1 | C13 | student |
+    When I log in as "student1"
+    And I click on "[data-control='next']" "css_element" in the "Course overview" "block"
+    And I wait until ".block_myoverview [data-control='next']" "css_element" exists
+    Then the "class" attribute of ".block_myoverview [data-control='next']" "css_element" should contain "disabled"
+    And I log out
+
+  Scenario: Next and previous page buttons should both be enabled when not on last or first page of courses
+    Given the following "course enrolments" exist:
+      | user | course | role |
+      | student1 | C1 | student |
+      | student1 | C2 | student |
+      | student1 | C3 | student |
+      | student1 | C4 | student |
+      | student1 | C5 | student |
+      | student1 | C6 | student |
+      | student1 | C7 | student |
+      | student1 | C8 | student |
+      | student1 | C9 | student |
+      | student1 | C10 | student |
+      | student1 | C11 | student |
+      | student1 | C12 | student |
+      | student1 | C13 | student |
+      | student1 | C14 | student |
+      | student1 | C15 | student |
+      | student1 | C16 | student |
+      | student1 | C17 | student |
+      | student1 | C18 | student |
+      | student1 | C19 | student |
+      | student1 | C20 | student |
+      | student1 | C21 | student |
+      | student1 | C22 | student |
+      | student1 | C23 | student |
+      | student1 | C24 | student |
+      | student1 | C25 | student |
+    When I log in as "student1"
+    And I click on "[data-control='next']" "css_element" in the "Course overview" "block"
+    And I wait until ".block_myoverview [data-control='next']" "css_element" exists
+    Then the "class" attribute of ".block_myoverview [data-control='next']" "css_element" should not contain "disabled"
+    And the "class" attribute of ".block_myoverview [data-control='previous']" "css_element" should not contain "disabled"
+    And I should see "Course 13" in the "Course overview" "block"
+    And I should see "Course 24" in the "Course overview" "block"
+    But I should not see "Course 12" in the "Course overview" "block"
+    And I should not see "Course 25" in the "Course overview" "block"
+    And I log out
index 349aade..6843e1b 100644 (file)
Binary files a/lib/amd/build/paged_content_factory.min.js and b/lib/amd/build/paged_content_factory.min.js differ
index 9c4ea67..e541e28 100644 (file)
Binary files a/lib/amd/build/paged_content_factory.min.js.map and b/lib/amd/build/paged_content_factory.min.js.map differ
index 577ccec..359e16a 100644 (file)
@@ -166,9 +166,11 @@ function(
      * @return {int|array}
      */
     var buildItemsPerPagePagingBarContext = function(itemsPerPage) {
+        var context = [];
+
         if ($.isArray(itemsPerPage)) {
             // Convert the array into a format accepted by the template.
-            var context = itemsPerPage.map(function(num) {
+            context = itemsPerPage.map(function(num) {
                 if (typeof num === 'number') {
                     // If the item is just a plain number then convert it into
                     // an object with value and active keys.
@@ -190,11 +192,12 @@ function(
             if (!activeItems.length) {
                 context[0].active = true;
             }
-
-            return context;
         } else {
-            return itemsPerPage;
+            // Convert the integer into a format accepted by the template.
+            context = [{value: itemsPerPage, active: true}];
         }
+
+        return context;
     };
 
     /**
index f1b3e79..365dec7 100644 (file)
         {{^activepagenumber}}
             data-active-page-number="1"
         {{/activepagenumber}}
-        {{#showitemsperpageselector}}
-            {{#itemsperpage}}
-                {{#active}}
-                    data-items-per-page="{{value}}"
-                {{/active}}
-            {{/itemsperpage}}
-        {{/showitemsperpageselector}}
-        {{^showitemsperpageselector}}
-            data-items-per-page="{{itemsperpage}}"
-        {{/showitemsperpageselector}}
+        {{#itemsperpage}}
+            {{#active}}
+                data-items-per-page="{{value}}"
+            {{/active}}
+        {{/itemsperpage}}
         {{#arialabels.paginationnav}}
             aria-label="{{.}}"
         {{/arialabels.paginationnav}}