Merge branch 'MDL-64784' of https://github.com/paulholden/moodle

lib/tablelib.php

@@ -35,6 +35,7 @@ define('TABLE_VAR_IFIRST', 4);
 define('TABLE_VAR_ILAST',  5);
 define('TABLE_VAR_PAGE',   6);
 define('TABLE_VAR_RESET',  7);
+define('TABLE_VAR_DIR',    8);
 /**#@-*/
 
 /**#@+
@@ -150,7 +151,8 @@ class flexible_table {
             TABLE_VAR_IFIRST => 'tifirst',
             TABLE_VAR_ILAST  => 'tilast',
             TABLE_VAR_PAGE   => 'page',
-            TABLE_VAR_RESET  => 'treset'
+            TABLE_VAR_RESET  => 'treset',
+            TABLE_VAR_DIR    => 'tdir',
         );
     }
 
@@ -516,14 +518,16 @@ class flexible_table {
                 (isset($this->columns[$sortcol]) || in_array($sortcol, get_all_user_name_fields())
                 && isset($this->columns['fullname']))) {
 
+            $sortdir = optional_param($this->request[TABLE_VAR_DIR], $this->sort_default_order, PARAM_INT);
+
             if (array_key_exists($sortcol, $this->prefs['sortby'])) {
                 // This key already exists somewhere. Change its sortorder and bring it to the top.
-                $sortorder = $this->prefs['sortby'][$sortcol] == SORT_ASC ? SORT_DESC : SORT_ASC;
+                $sortorder = $this->prefs['sortby'][$sortcol] = $sortdir;
                 unset($this->prefs['sortby'][$sortcol]);
                 $this->prefs['sortby'] = array_merge(array($sortcol => $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($sortcol => SORT_ASC), $this->prefs['sortby']);
+                $this->prefs['sortby'] = array_merge(array($sortcol => $sortdir), $this->prefs['sortby']);
             }
 
             // Finally, make sure that no more than $this->maxsortkeys are present into the array
@@ -1362,8 +1366,19 @@ class flexible_table {
      * @return string HTML fragment.
      */
     protected function sort_link($text, $column, $isprimary, $order) {
-        return html_writer::link($this->baseurl->out(false,
+        // If we are already sorting by this column, switch direction.
-                array($this->request[TABLE_VAR_SORT] => $column)),
+        if (array_key_exists($column, $this->prefs['sortby'])) {
+            $sortorder = $this->prefs['sortby'][$column] == SORT_ASC ? SORT_DESC : SORT_ASC;
+        } else {
+            $sortorder = $order;
+        }
+
+        $params = [
+            $this->request[TABLE_VAR_SORT] => $column,
+            $this->request[TABLE_VAR_DIR] => $sortorder,
+        ];
+
+        return html_writer::link($this->baseurl->out(false, $params),
                 $text . get_accesshide(get_string('sortby') . ' ' .
                 $text . ' ' . $this->sort_order_name($isprimary, $order))) . ' ' .
                 $this->sort_icon($isprimary, $order);

lib/tests/tablelib_test.php

@@ -526,7 +526,6 @@ class core_tablelib_testcase extends basic_testcase {
     }
 
     public function test_can_be_reset() {
-
         // Table in its default state (as if seen for the first time), nothing to reset.
         $table = $this->prepare_table_for_reset_test(uniqid('tablelib_test_'));
         $table->setup();
@@ -538,21 +537,25 @@ class core_tablelib_testcase extends basic_testcase {
         $table->setup();
         $this->assertFalse($table->can_be_reset());
 
-        // Table explicitly sorted by the default column (reverses the order), can be reset.
+        // Table explicitly sorted by the default column & direction, nothing to reset.
         $table = $this->prepare_table_for_reset_test(uniqid('tablelib_test_'));
         $table->sortable(true, 'column1', SORT_DESC);
         $_GET['tsort'] = 'column1';
+        $_GET['tdir'] = SORT_DESC;
         $table->setup();
         unset($_GET['tsort']);
-        $this->assertTrue($table->can_be_reset());
+        unset($_GET['tdir']);
+        $this->assertFalse($table->can_be_reset());
 
-        // Table explicitly sorted twice by the default column (puts back to default order), nothing to reset.
+        // Table explicitly sorted twice by the default column & direction, nothing to reset.
         $table = $this->prepare_table_for_reset_test(uniqid('tablelib_test_'));
         $table->sortable(true, 'column1', SORT_DESC);
         $_GET['tsort'] = 'column1';
+        $_GET['tdir'] = SORT_DESC;
         $table->setup();
         $table->setup(); // Set up again to simulate the second page request.
         unset($_GET['tsort']);
+        unset($_GET['tdir']);
         $this->assertFalse($table->can_be_reset());
 
         // Table sorted by other than default column, can be reset.
@@ -563,6 +566,16 @@ class core_tablelib_testcase extends basic_testcase {
         unset($_GET['tsort']);
         $this->assertTrue($table->can_be_reset());
 
+        // Table sorted by other than default direction, can be reset.
+        $table = $this->prepare_table_for_reset_test(uniqid('tablelib_test_'));
+        $table->sortable(true, 'column1', SORT_DESC);
+        $_GET['tsort'] = 'column1';
+        $_GET['tdir'] = SORT_ASC;
+        $table->setup();
+        unset($_GET['tsort']);
+        unset($_GET['tdir']);
+        $this->assertTrue($table->can_be_reset());
+
         // Table sorted by the default column after another sorting previously selected.
         // This leads to different ORDER BY than just having a single sort defined, can be reset.
         $table = $this->prepare_table_for_reset_test(uniqid('tablelib_test_'));