MDL-52036 tablelib: Fix the behaviour of resetting table preferences
authorDavid Mudrák <david@moodle.com>
Wed, 4 Nov 2015 14:07:17 +0000 (15:07 +0100)
committerDavid Mudrák <david@moodle.com>
Wed, 4 Nov 2015 14:07:17 +0000 (15:07 +0100)
The problem with the previous implementation was that the table's
$this->prefs can contain valid non-empty value in its default state -
the default column to sort by. On resetting, we must not throw away
these default prefs.

This patch simplifies the TABLE_VAR_RESET interpretation. If such an
HTTP parameter is passed via the request, the table simply behaves as if
there were no previously stored preferences (does not matter if coming
from the current session, or from the persistent cross-session storage).

The logic that decides on whether or not the reset widget should be
displayed is put into a new method can_be_reset() with unit tests
attached.

Finally, the previously private method render_reset_button() is now
protected and the reset widget is given a new semantic CSS class.

lib/tablelib.php
lib/tests/fixtures/testable_flexible_table.php [new file with mode: 0644]
lib/tests/tablelib_test.php

index 06516e9..e2c4b7f 100644 (file)
@@ -458,7 +458,9 @@ class flexible_table {
         } else if (isset($SESSION->flextable[$this->uniqueid])) {
             $this->prefs = $SESSION->flextable[$this->uniqueid];
         }
-        if (!$this->prefs) {
+
+        // Set up default preferences if needed.
+        if (!$this->prefs or optional_param($this->request[TABLE_VAR_RESET], false, PARAM_BOOL)) {
             $this->prefs = array(
                 'collapse' => array(),
                 'sortby'   => array(),
@@ -526,17 +528,6 @@ class flexible_table {
             $this->prefs['i_first'] = $ifirst;
         }
 
-        // Allow user to reset table preferences.
-        if (optional_param($this->request[TABLE_VAR_RESET], 0, PARAM_BOOL) === 1) {
-            $this->prefs = array(
-                'collapse' => array(),
-                'sortby'   => array(),
-                'i_first'  => '',
-                'i_last'   => '',
-                'textsort' => $this->column_textsort,
-            );
-        }
-
         // Save user preferences if they have changed.
         if ($this->prefs != $oldprefs) {
             if ($this->persistent) {
@@ -1416,32 +1407,57 @@ class flexible_table {
     /**
      * Generate the HTML for the table preferences reset button.
      *
-     * @return string HTML fragment.
+     * @return string HTML fragment, empty string if no need to reset
      */
-    private function render_reset_button() {
-        $userprefs = false;
-        // Loop through the user table preferences for a setting.
-        foreach ($this->prefs as $preference) {
-            if (!empty($preference)) {
-                // We have a preference.
-                $userprefs = true;
-                // We only need one.
-                break;
-            }
-        }
-        // If no table preferences have been set then don't show the reset button.
-        if (!$userprefs) {
+    protected function render_reset_button() {
+
+        if (!$this->can_be_reset()) {
             return '';
         }
 
         $url = $this->baseurl->out(false, array($this->request[TABLE_VAR_RESET] => 1));
 
-        $html  = html_writer::start_div('mdl-right');
+        $html  = html_writer::start_div('resettable mdl-right');
         $html .= html_writer::link($url, get_string('resettable'));
         $html .= html_writer::end_div();
 
         return $html;
     }
+
+    /**
+     * Are there some table preferences that can be reset?
+     *
+     * If true, then the "reset table preferences" widget should be displayed.
+     *
+     * @return bool
+     */
+    protected function can_be_reset() {
+
+        // Loop through preferences and make sure they are empty or set to the default value.
+        foreach ($this->prefs as $prefname => $prefval) {
+
+            if ($prefname === 'sortby' and !empty($this->sort_default_column)) {
+                // Check if the actual sorting differs from the default one.
+                if (empty($prefval) or $prefval !== array($this->sort_default_column => $this->sort_default_order)) {
+                    return true;
+                }
+
+            } else if ($prefname === 'collapse' and !empty($prefval)) {
+                // Check if there are some collapsed columns (all are expanded by default).
+                foreach ($prefval as $columnname => $iscollapsed) {
+                    if ($iscollapsed) {
+                        return true;
+                    }
+                }
+
+            } else if (!empty($prefval)) {
+                // For all other cases, we just check if some preference is set.
+                return true;
+            }
+        }
+
+        return false;
+    }
 }
 
 
diff --git a/lib/tests/fixtures/testable_flexible_table.php b/lib/tests/fixtures/testable_flexible_table.php
new file mode 100644 (file)
index 0000000..9787f72
--- /dev/null
@@ -0,0 +1,45 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Provides testable_flexible_table class.
+ *
+ * @package     core
+ * @subpackage  fixtures
+ * @category    test
+ * @copyright   2015 David Mudrak <david@moodle.com>
+ * @license     http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+/**
+ * Testable subclass of flexible_table providing access to some protected methods.
+ *
+ * @copyright 2015 David Mudrak <david@moodle.com>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class testable_flexible_table extends flexible_table {
+
+    /**
+     * Provides access to flexible_table::can_be_reset() method.
+     *
+     * @return bool
+     */
+    public function can_be_reset() {
+        return parent::can_be_reset();
+    }
+}
index 3db08c3..1e1fe7b 100644 (file)
@@ -27,6 +27,7 @@ defined('MOODLE_INTERNAL') || die();
 
 global $CFG;
 require_once($CFG->libdir . '/tablelib.php');
+require_once($CFG->libdir . '/tests/fixtures/testable_flexible_table.php');
 
 /**
  * Test some of tablelib.
@@ -498,4 +499,110 @@ class core_tablelib_testcase extends basic_testcase {
 
         $this->assertEquals($table5, $table6);
     }
+
+    /**
+     * Helper method for preparing tables instances in {@link self::test_can_be_reset()}.
+     *
+     * @param string $tableid
+     * @return testable_flexible_table
+     */
+    protected function prepare_table_for_reset_test($tableid) {
+        global $SESSION;
+
+        unset($SESSION->flextable[$tableid]);
+
+        $data = $this->generate_data(25, 3);
+        $columns = array('column0', 'column1', 'column2');
+        $headers = $this->generate_headers(3);
+
+        $table = new testable_flexible_table($tableid);
+        $table->define_baseurl('/invalid.php');
+        $table->define_columns($columns);
+        $table->define_headers($headers);
+        $table->collapsible(true);
+        $table->is_persistent(false);
+
+        return $table;
+    }
+
+    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();
+        $this->assertFalse($table->can_be_reset());
+
+        // Table in its default state with default sorting defined, nothing to reset.
+        $table = $this->prepare_table_for_reset_test(uniqid('tablelib_test_'));
+        $table->sortable(true, 'column1', SORT_DESC);
+        $table->setup();
+        $this->assertFalse($table->can_be_reset());
+
+        // Table explicitly sorted by the default column (reverses the order), can be reset.
+        $table = $this->prepare_table_for_reset_test(uniqid('tablelib_test_'));
+        $table->sortable(true, 'column1', SORT_DESC);
+        $_GET['tsort'] = 'column1';
+        $table->setup();
+        unset($_GET['tsort']);
+        $this->assertTrue($table->can_be_reset());
+
+        // Table explicitly sorted twice by the default column (puts back to default order), nothing to reset.
+        $table = $this->prepare_table_for_reset_test(uniqid('tablelib_test_'));
+        $table->sortable(true, 'column1', SORT_DESC);
+        $_GET['tsort'] = 'column1';
+        $table->setup();
+        $table->setup(); // Set up again to simulate the second page request.
+        unset($_GET['tsort']);
+        $this->assertFalse($table->can_be_reset());
+
+        // Table sorted by other than default column, can be reset.
+        $table = $this->prepare_table_for_reset_test(uniqid('tablelib_test_'));
+        $table->sortable(true, 'column1', SORT_DESC);
+        $_GET['tsort'] = 'column2';
+        $table->setup();
+        unset($_GET['tsort']);
+        $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_'));
+        $table->sortable(true, 'column1', SORT_DESC);
+        $_GET['tsort'] = 'column0';
+        $table->setup();
+        $_GET['tsort'] = 'column1';
+        $table->setup();
+        unset($_GET['tsort']);
+        $this->assertTrue($table->can_be_reset());
+
+        // Table having some column collapsed, can be reset.
+        $table = $this->prepare_table_for_reset_test(uniqid('tablelib_test_'));
+        $_GET['thide'] = 'column2';
+        $table->setup();
+        unset($_GET['thide']);
+        $this->assertTrue($table->can_be_reset());
+
+        // Table having some column explicitly expanded, nothing to reset.
+        $table = $this->prepare_table_for_reset_test(uniqid('tablelib_test_'));
+        $_GET['tshow'] = 'column2';
+        $table->setup();
+        unset($_GET['tshow']);
+        $this->assertFalse($table->can_be_reset());
+
+        // Table after expanding a collapsed column, nothing to reset.
+        $table = $this->prepare_table_for_reset_test(uniqid('tablelib_test_'));
+        $_GET['thide'] = 'column0';
+        $table->setup();
+        $_GET['tshow'] = 'column0';
+        $table->setup();
+        unset($_GET['thide']);
+        unset($_GET['tshow']);
+        $this->assertFalse($table->can_be_reset());
+
+        // Table with some name filtering enabled, can be reset.
+        $table = $this->prepare_table_for_reset_test(uniqid('tablelib_test_'));
+        $_GET['tifirst'] = 'A';
+        $table->setup();
+        unset($_GET['tifirst']);
+        $this->assertTrue($table->can_be_reset());
+    }
 }