MDL-68508 core_table: Improve sorting to support sorting by multiple columns
authorAndrew Nicols <andrew@nicols.co.uk>
Fri, 24 Apr 2020 08:08:45 +0000 (16:08 +0800)
committerAndrew Nicols <andrew@nicols.co.uk>
Fri, 22 May 2020 02:42:30 +0000 (10:42 +0800)
lib/table/amd/build/dynamic.min.js
lib/table/amd/build/dynamic.min.js.map
lib/table/amd/build/local/dynamic/repository.min.js
lib/table/amd/build/local/dynamic/repository.min.js.map
lib/table/amd/src/dynamic.js
lib/table/amd/src/local/dynamic/repository.js
lib/table/classes/external/dynamic/fetch.php
lib/table/tests/external/dynamic/fetch_test.php
lib/tablelib.php

index b8a5c03..f4b2bdc 100644 (file)
Binary files a/lib/table/amd/build/dynamic.min.js and b/lib/table/amd/build/dynamic.min.js differ
index 5c5e815..822e013 100644 (file)
Binary files a/lib/table/amd/build/dynamic.min.js.map and b/lib/table/amd/build/dynamic.min.js.map differ
index ec438f1..ff5ca26 100644 (file)
Binary files a/lib/table/amd/build/local/dynamic/repository.min.js and b/lib/table/amd/build/local/dynamic/repository.min.js differ
index af07af3..a8cb000 100644 (file)
Binary files a/lib/table/amd/build/local/dynamic/repository.min.js.map and b/lib/table/amd/build/local/dynamic/repository.min.js.map differ
index a53dc6f..235dd5e 100644 (file)
@@ -74,8 +74,7 @@ export const refreshTableContent = (tableRoot, resetContent = false) => {
         tableRoot.dataset.tableHandler,
         tableRoot.dataset.tableUniqueid,
         {
-            sortBy: tableRoot.dataset.tableSortBy,
-            sortOrder: tableRoot.dataset.tableSortOrder,
+            sortData: JSON.parse(tableRoot.dataset.tableSortData),
             joinType: filterset.jointype,
             filters: filterset.filters,
             firstinitial: tableRoot.dataset.tableFirstInitial,
@@ -116,8 +115,12 @@ export const updateTable = (tableRoot, {
 
     // Update sort fields.
     if (sortBy && sortOrder) {
-        tableRoot.dataset.tableSortBy = sortBy;
-        tableRoot.dataset.tableSortOrder = sortOrder;
+        const sortData = JSON.parse(tableRoot.dataset.tableSortData);
+        sortData.unshift({
+            sortby: sortBy,
+            sortorder: parseInt(sortOrder, 10),
+        });
+        tableRoot.dataset.tableSortData = JSON.stringify(sortData);
     }
 
     // Update initials.
index a8e6141..3e43ac9 100644 (file)
@@ -31,18 +31,20 @@ import {call as fetchMany} from 'core/ajax';
  * @param {String} component The component
  * @param {String} handler The name of the handler
  * @param {String} uniqueid The unique id of the table
- * @param {Object} filters The filters to apply when searching
- * @param {String} firstinitial The first name initial to filter on
- * @param {String} lastinitial The last name initial to filter on
- * @param {String} pageNumber The page number
- * @param {Number} pageSize The page size
- * @param {Number} params parameters to request table
+ * @param {Object} options The options to use when updating the table
+ * @param {Array} options.sortData The list of columns to sort by
+ * @param {Number} options.joinType The filterset join type
+ * @param {Object} options.filters The filters to apply when searching
+ * @param {String} options.firstinitial The first name initial to filter on
+ * @param {String} options.lastinitial The last name initial to filter on
+ * @param {String} options.pageNumber The page number
+ * @param {Number} options.pageSize The page size
+ * @param {Object} options.hiddenColumns The columns to hide
  * @param {Bool} resetPreferences
  * @return {Promise} Resolved with requested table view
  */
 export const fetch = (component, handler, uniqueid, {
-        sortBy = null,
-        sortOrder = null,
+        sortData = [],
         joinType = null,
         filters = {},
         firstinitial = null,
@@ -57,8 +59,7 @@ export const fetch = (component, handler, uniqueid, {
             component,
             handler,
             uniqueid,
-            sortby: sortBy,
-            sortorder: sortOrder,
+            sortdata: sortData,
             jointype: joinType,
             filters,
             firstinitial,
index 15b9583..b63c6c2 100644 (file)
@@ -67,15 +67,22 @@ class fetch extends external_api {
                 'Unique ID for the container',
                 VALUE_REQUIRED
             ),
-            'sortby' => new external_value(
-                PARAM_ALPHANUMEXT,
-                'The name of a sortable column',
-                VALUE_REQUIRED
-            ),
-            'sortorder' => new external_value(
-                PARAM_ALPHANUMEXT,
-                'The sort order',
-                VALUE_REQUIRED
+            'sortdata' => new external_multiple_structure(
+                new external_single_structure([
+                    'sortby' => new external_value(
+                        PARAM_ALPHANUMEXT,
+                        'The name of a sortable column',
+                        VALUE_REQUIRED
+                    ),
+                    'sortorder' => new external_value(
+                        PARAM_ALPHANUMEXT,
+                        'The direction that this column should be sorted by',
+                        VALUE_REQUIRED
+                    ),
+                ]),
+                'The combined sort order of the table. Multiple fields can be specified.',
+                VALUE_OPTIONAL,
+                []
             ),
             'filters' => new external_multiple_structure(
                 new external_single_structure([
@@ -138,8 +145,7 @@ class fetch extends external_api {
      * @param string $component The component.
      * @param string $handler Dynamic table class name.
      * @param string $uniqueid Unique ID for the container.
-     * @param string $sortby The name of a sortable column.
-     * @param string $sortorder The sort order.
+     * @param array $sortdata The columns and order to sort by
      * @param array $filters The filters that will be applied in the request.
      * @param string $jointype The join type.
      * @param string $firstinitial The first name initial to filter on
@@ -155,8 +161,7 @@ class fetch extends external_api {
         string $component,
         string $handler,
         string $uniqueid,
-        string $sortby,
-        string $sortorder,
+        array $sortdata,
         ?array $filters = null,
         ?string $jointype = null,
         ?string $firstinitial = null,
@@ -166,15 +171,13 @@ class fetch extends external_api {
         ?array $hiddencolumns = null,
         ?bool $resetpreferences = null
     ) {
-
         global $PAGE;
 
         [
             'component' => $component,
             'handler' => $handler,
             'uniqueid' => $uniqueid,
-            'sortby' => $sortby,
-            'sortorder' => $sortorder,
+            'sortdata' => $sortdata,
             'filters' => $filters,
             'jointype' => $jointype,
             'firstinitial' => $firstinitial,
@@ -187,8 +190,7 @@ class fetch extends external_api {
             'component' => $component,
             'handler' => $handler,
             'uniqueid' => $uniqueid,
-            'sortby' => $sortby,
-            'sortorder' => $sortorder,
+            'sortdata' => $sortdata,
             'filters' => $filters,
             'jointype' => $jointype,
             'firstinitial' => $firstinitial,
@@ -227,7 +229,7 @@ class fetch extends external_api {
         $instance->set_filterset($filterset);
         self::validate_context($instance->get_context());
 
-        $instance->set_sorting($sortby, $sortorder);
+        $instance->set_sortdata($sortdata);
 
         if ($firstinitial !== null) {
             $instance->set_first_initial($firstinitial);
index 111b221..c825079 100644 (file)
@@ -55,7 +55,21 @@ class fetch_test extends advanced_testcase {
         $this->resetAfterTest();
 
         $this->expectException(\invalid_parameter_exception::class);
-        fetch::execute("core-user", "participants", "", "email", "4", [], "1");
+        fetch::execute(
+            "core-user",
+            "participants",
+            "",
+            $this->get_sort_array(['email' => SORT_ASC]),
+            [],
+            (string) filter::JOINTYPE_ANY,
+            null,
+            null,
+            null,
+            null,
+            [],
+            null
+
+        );
     }
 
     /**
@@ -65,8 +79,20 @@ class fetch_test extends advanced_testcase {
         $this->resetAfterTest();
 
         $this->expectException(\UnexpectedValueException::class);
-        fetch::execute("core_users", "participants", "", "email", "4", [], (string)filter::JOINTYPE_ANY,
-            null, null, null, null, []);
+        fetch::execute(
+            "core_users",
+            "participants",
+            "",
+            $this->get_sort_array(['email' => SORT_ASC]),
+            [],
+            (string) filter::JOINTYPE_ANY,
+            null,
+            null,
+            null,
+            null,
+            [],
+            null
+        );
     }
 
     /**
@@ -80,8 +106,21 @@ class fetch_test extends advanced_testcase {
         $this->expectExceptionMessage("Table handler class {$handler} not found. Please make sure that your table handler class is under the \\core_user\\table namespace.");
 
         // Tests that invalid users_participants_table class gets an exception.
-        fetch::execute("core_user", "users_participants_table", "", "email", "4", [], (string)filter::JOINTYPE_ANY,
-            null, null, null, null, []);
+        fetch::execute(
+            "core_user",
+            "users_participants_table",
+            "",
+            $this->get_sort_array(['email' => SORT_ASC]),
+            [],
+            (string) filter::JOINTYPE_ANY,
+            null,
+            null,
+            null,
+            null,
+            [],
+            null
+
+        );
     }
 
     /**
@@ -97,15 +136,20 @@ class fetch_test extends advanced_testcase {
             [
                 'fullname' => 'courseid',
                 'jointype' => filter::JOINTYPE_ANY,
-                'values' => [(int)$course->id]
+                'values' => [(int) $course->id]
             ]
         ];
         $this->expectException(\invalid_parameter_exception::class);
         $this->expectExceptionMessage("Invalid parameter value detected (filters => Invalid parameter value detected " .
         "(Missing required key in single structure: name): Missing required key in single structure: name");
 
-        fetch::execute("core_user", "participants", "user-index-participants-{$course->id}",
-        "firstname", "4", $filter, (string)filter::JOINTYPE_ANY);
+        fetch::execute(
+            "core_user",
+            "participants", "user-index-participants-{$course->id}",
+            $this->get_sort_array(['firstname' => SORT_ASC]),
+            $filter,
+            (string) filter::JOINTYPE_ANY
+        );
     }
 
     /**
@@ -122,18 +166,30 @@ class fetch_test extends advanced_testcase {
 
         $this->setUser($teacher);
 
+        $this->get_sort_array(['email' => SORT_ASC]);
+
         $filter = [
             [
                 'name' => 'courseid',
                 'jointype' => filter::JOINTYPE_ANY,
-                'values' => [(int)$course->id]
+                'values' => [(int) $course->id]
             ]
         ];
 
-        $participantstable = fetch::execute("core_user", "participants",
-            "user-index-participants-{$course->id}", "firstname", "4", $filter, (string)filter::JOINTYPE_ANY,
-            null, null, null, null, []);
-
+        $participantstable = fetch::execute(
+            "core_user",
+            "participants",
+            "user-index-participants-{$course->id}",
+            $this->get_sort_array(['firstname' => SORT_ASC]),
+            $filter,
+            (string) filter::JOINTYPE_ANY,
+            null,
+            null,
+            null,
+            null,
+            [],
+            null
+        );
         $html = $participantstable['html'];
 
         $this->assertStringContainsString($user1->email, $html);
@@ -141,4 +197,23 @@ class fetch_test extends advanced_testcase {
         $this->assertStringContainsString($teacher->email, $html);
         $this->assertStringNotContainsString($user3->email, $html);
     }
+
+
+    /**
+     * Convert a traditional sort order into a sortorder for the web service.
+     *
+     * @param array $sortdata
+     * @return array
+     */
+    protected function get_sort_array(array $sortdata): array {
+        $newsortorder = [];
+        foreach ($sortdata as $sortby => $sortorder) {
+            $newsortorder[] = [
+                'sortby' => $sortby,
+                'sortorder' => $sortorder,
+            ];
+        }
+
+        return $newsortorder;
+    }
 }
index 997a965..73db35a 100644 (file)
@@ -87,14 +87,9 @@ class flexible_table {
     var $is_sortable    = false;
 
     /**
-     * @var string The field name to sort by.
+     * @var array The fields to sort.
      */
-    protected $sortby;
-
-    /**
-     * @var string $sortorder The direction for sorting.
-     */
-    protected $sortorder;
+    protected $sortdata;
 
     /** @var string The manually set first name initial preference */
     protected $ifirst;
@@ -1301,40 +1296,43 @@ class flexible_table {
      * Calculate the preferences for sort order based on user-supplied values and get params.
      */
     protected function set_sorting_preferences(): void {
-        $sortorder = $this->sortorder;
-        $sortby = $this->sortby;
+        $sortdata = $this->sortdata;
+
+        if ($sortdata === null) {
+            $sortdata = $this->prefs['sortby'];
 
-        if ($sortorder === null || $sortby === null) {
             $sortorder = optional_param($this->request[TABLE_VAR_DIR], $this->sort_default_order, PARAM_INT);
             $sortby = optional_param($this->request[TABLE_VAR_SORT], '', PARAM_ALPHANUMEXT);
-        }
-
-        $isvalidsort = $sortby && $this->is_sortable($sortby);
-        $isvalidsort = $isvalidsort && empty($this->prefs['collapse'][$sortby]);
-        $isrealcolumn = isset($this->columns[$sortby]);
-        $isfullnamefield = isset($this->columns['fullname']) && in_array($sortby, get_all_user_name_fields());
 
-        if ($isvalidsort && ($isrealcolumn || $isfullnamefield)) {
-            if (array_key_exists($sortby, $this->prefs['sortby'])) {
+            if (array_key_exists($sortby, $sortdata)) {
                 // This key already exists somewhere. Change its sortorder and bring it to the top.
-                $sortorder = $this->prefs['sortby'][$sortby] = $sortorder;
-                unset($this->prefs['sortby'][$sortby]);
-                $this->prefs['sortby'] = array_merge(array($sortby => $sortorder), $this->prefs['sortby']);
-            } else {
-                // Key doesn't exist, so just add it to the beginning of the array, ascending order.
-                $this->prefs['sortby'] = array_merge(array($sortby => $sortorder), $this->prefs['sortby']);
+                //$sortorder = $sortdata[$sortby] = $sortorder;
+                unset($sortdata['sortby'][$sortby]);
             }
-
-            // Finally, make sure that no more than $this->maxsortkeys are present into the array.
-            $this->prefs['sortby'] = array_slice($this->prefs['sortby'], 0, $this->maxsortkeys);
+            $sortdata = array_merge([$sortby => $sortorder], $sortdata);
         }
 
+        $usernamefields = get_all_user_name_fields();
+        $sortdata = array_filter($sortdata, function($sortby) use ($usernamefields) {
+            $isvalidsort = $sortby && $this->is_sortable($sortby);
+            $isvalidsort = $isvalidsort && empty($this->prefs['collapse'][$sortby]);
+            $isrealcolumn = isset($this->columns[$sortby]);
+            $isfullnamefield = isset($this->columns['fullname']) && in_array($sortby, $usernamefields);
+
+            return $isvalidsort && ($isrealcolumn || $isfullnamefield);
+        }, ARRAY_FILTER_USE_KEY);
+
+        // Finally, make sure that no more than $this->maxsortkeys are present into the array.
+        $sortdata = array_slice($sortdata, 0, $this->maxsortkeys);
+
         // If a default order is defined and it is not in the current list of order by columns, add it at the end.
         // This prevents results from being returned in a random order if the only order by column contains equal values.
-        if (!empty($this->sort_default_column) && !array_key_exists($this->sort_default_column, $this->prefs['sortby'])) {
-            $defaultsort = array($this->sort_default_column => $this->sort_default_order);
-            $this->prefs['sortby'] = array_merge($this->prefs['sortby'], $defaultsort);
+        if (!empty($this->sort_default_column) && !array_key_exists($this->sort_default_column, $sortdata)) {
+            $sortdata = array_merge($sortdata, [$this->sort_default_column => $this->sort_default_order]);
         }
+
+        // Apply the sortdata to the preference.
+        $this->prefs['sortby'] = $sortdata;
     }
 
     /**
@@ -1431,8 +1429,7 @@ class flexible_table {
 
         // Save user preferences if they have changed.
         if ($this->is_resetting_preferences()) {
-            $this->sortorder = null;
-            $this->sortby = null;
+            $this->sortdata = null;
             $this->ifirst = null;
             $this->ilast = null;
         }
@@ -1502,9 +1499,13 @@ class flexible_table {
      * @param string $sortby The field to sort by.
      * @param int $sortorder The sort order.
      */
-    public function set_sorting(string $sortby, int $sortorder): void {
-        $this->sortby = $sortby;
-        $this->sortorder = $sortorder;
+    public function set_sortdata(array $sortdata): void {
+        $this->sortdata = [];
+        foreach ($sortdata as $sortitem) {
+            if (!array_key_exists($sortitem['sortby'], $this->sortdata)) {
+                $this->sortdata[$sortitem['sortby']] = $sortitem['sortorder'];
+            }
+        }
     }
 
     /**
@@ -1643,7 +1644,13 @@ class flexible_table {
      */
     protected function get_dynamic_table_html_start(): string {
         if (is_a($this, \core_table\dynamic::class)) {
-            $sortdata = $this->get_sort_order();
+            $sortdata = array_map(function($sortby, $sortorder) {
+                return [
+                    'sortby' => $sortby,
+                    'sortorder' => $sortorder,
+                ];
+            }, array_keys($this->prefs['sortby']), array_values($this->prefs['sortby']));;
+
             return html_writer::start_tag('div', [
                 'class' => 'table-dynamic position-relative',
                 'data-region' => 'core_table/dynamic',
@@ -1651,8 +1658,7 @@ class flexible_table {
                 'data-table-component' => $this->get_component(),
                 'data-table-uniqueid' => $this->uniqueid,
                 'data-table-filters' => json_encode($this->get_filterset()),
-                'data-table-sort-by' => $sortdata['sortby'],
-                'data-table-sort-order' => $sortdata['sortorder'],
+                'data-table-sort-data' => json_encode($sortdata),
                 'data-table-first-initial' => $this->prefs['i_first'],
                 'data-table-last-initial' => $this->prefs['i_last'],
                 'data-table-page-number' => $this->currpage + 1,