Allow to provide secondary sort column for reports (matomo-org#12128)

* Allow to provide secondy sort column for reports * rename method * Adds two simple tests to prove secondary column sorting * invoke sort callback with datatable as second parameter
InfinityVoid · Oct 11, 2018 · aee6629 · aee6629
1 parent bdd2fca
commit aee6629
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 8 deletions.
diff --git a/core/API/DataTableGenericFilter.php b/core/API/DataTableGenericFilter.php
@@ -100,7 +100,7 @@ public static function getGenericFiltersInformation()
                       'filter_sort_order'  => array('string', 'desc'),
                       $naturalSort = true,
                       $recursiveSort = true,
-                      $doSortBySecondaryColumn = true
+                      'filter_sort_column_secondary' => true
                   )),
             array('Truncate',
                   array(
@@ -124,6 +124,13 @@ private function getGenericFiltersHavingDefaultValues()
                 if ($filter[0] === 'Sort') {
                     $filters[$index][1]['filter_sort_column'] = array('string', $this->report->getDefaultSortColumn());
                     $filters[$index][1]['filter_sort_order']  = array('string', $this->report->getDefaultSortOrder());
+
+                    $callback = $this->report->getSecondarySortColumnCallback();
+
+                    if (is_callable($callback)) {
+                        $filters[$index][1]['filter_sort_column_secondary'] = $callback;
+                    }
+
                 }
             }
         }

diff --git a/core/DataTable/Filter/Sort.php b/core/DataTable/Filter/Sort.php
@@ -28,6 +28,7 @@ class Sort extends BaseFilter
     protected $order;
     protected $naturalSort;
     protected $isSecondaryColumnSortEnabled;
+    protected $secondaryColumnSortCallback;
 
     const ORDER_DESC = 'desc';
     const ORDER_ASC  = 'asc';
@@ -40,8 +41,10 @@ class Sort extends BaseFilter
      * @param string $order order `'asc'` or `'desc'`.
      * @param bool $naturalSort Whether to use a natural sort or not (see {@link http://php.net/natsort}).
      * @param bool $recursiveSort Whether to sort all subtables or not.
-     * @param bool $doSortBySecondaryColumn If true will sort by a secondary column. The column is automatically
-     *                                    detected and will be either nb_visits or label, if possible.
+     * @param bool|callback $doSortBySecondaryColumn If true will sort by a secondary column. The column is automatically
+     *                                               detected and will be either nb_visits or label, if possible.
+     *                                               If callback given it will sort by the column returned by the callback (if any)
+     *                                               callback will be called with 2 parameters: primaryColumnToSort and table
      */
     public function __construct($table, $columnToSort, $order = 'desc', $naturalSort = true, $recursiveSort = true, $doSortBySecondaryColumn = false)
     {
@@ -54,7 +57,8 @@ public function __construct($table, $columnToSort, $order = 'desc', $naturalSort
         $this->columnToSort = $columnToSort;
         $this->naturalSort = $naturalSort;
         $this->order = strtolower($order);
-        $this->isSecondaryColumnSortEnabled = $doSortBySecondaryColumn;
+        $this->isSecondaryColumnSortEnabled = !empty($doSortBySecondaryColumn);
+        $this->secondaryColumnSortCallback = is_callable($doSortBySecondaryColumn) ? $doSortBySecondaryColumn : null;
     }
 
     /**
@@ -90,7 +94,11 @@ public function filter($table)
         $config->primaryColumnToSort   = $sorter->getPrimaryColumnToSort($table, $this->columnToSort);
         $config->primarySortOrder      = $sorter->getPrimarySortOrder($this->order);
         $config->primarySortFlags      = $sorter->getBestSortFlags($table, $config->primaryColumnToSort);
-        $config->secondaryColumnToSort = $sorter->getSecondaryColumnToSort($row, $config->primaryColumnToSort);
+        if (!empty($this->secondaryColumnSortCallback)) {
+            $config->secondaryColumnToSort = call_user_func($this->secondaryColumnSortCallback, $config->primaryColumnToSort, $table);
+        } else {
+            $config->secondaryColumnToSort = $sorter->getSecondaryColumnToSort($row, $config->primaryColumnToSort);
+        }
         $config->secondarySortOrder    = $sorter->getSecondarySortOrder($this->order, $config->secondaryColumnToSort);
         $config->secondarySortFlags    = $sorter->getBestSortFlags($table, $config->secondaryColumnToSort);
 

diff --git a/core/Plugin/Report.php b/core/Plugin/Report.php
@@ -10,16 +10,13 @@
 
 use Piwik\API\Proxy;
 use Piwik\API\Request;
-use Piwik\Cache;
 use Piwik\Columns\Dimension;
 use Piwik\Common;
 use Piwik\DataTable;
 use Piwik\DataTable\Filter\Sort;
 use Piwik\Metrics;
-use Piwik\Cache as PiwikCache;
 use Piwik\Piwik;
 use Piwik\Plugins\CoreVisualizations\Visualizations\HtmlTable;
-use Piwik\Plugins\CoreVisualizations\Visualizations\JqplotGraph\Evolution;
 use Piwik\ViewDataTable\Factory as ViewDataTableFactory;
 use Exception;
 use Piwik\Widget\WidgetsList;
@@ -648,6 +645,30 @@ public function getDefaultSortOrder()
         return Sort::ORDER_ASC;
     }
 
+    /**
+     * Allows to define a callback that will be used to determine the secondary column to sort by
+     *
+     * ```
+     * public function getSecondarySortColumnCallback()
+     * {
+     *     return function ($primaryColumn) {
+     *         switch ($primaryColumn) {
+     *             case Metrics::NB_CLICKS:
+     *                 return Metrics::NB_IMPRESSIONS;
+     *             case 'label':
+     *             default:
+     *                 return Metrics::NB_CLICKS;
+     *         }
+     *     };
+     * }
+     * ```
+     * @return null|callable
+     */
+    public function getSecondarySortColumnCallback()
+    {
+        return null;
+    }
+
     /**
      * Get the list of related reports if there are any. They will be displayed for instance below a report as a
      * recommended related report.

diff --git a/tests/PHPUnit/Unit/DataTable/Filter/SortTest.php b/tests/PHPUnit/Unit/DataTable/Filter/SortTest.php
@@ -254,6 +254,34 @@ public function testFilter_shouldPickStringSearchEvenIfFirstLabelIsNumeric()
         $this->assertTrue(DataTable::isEqual($table, $expectedtableReverse));
     }
 
+    public function testSecondarySort()
+    {
+        $table = new DataTable();
+        $table->addRowsFromArray(array(
+            array(Row::COLUMNS => array('label' => 'ask', 'count' => 10, 'count2' => 10)),
+            array(Row::COLUMNS => array('label' => 'nintendo', 'count' => 10, 'count2' => 5)),
+            array(Row::COLUMNS => array('label' => 'yahoo', 'count' => 10, 'count2' => 100)
+            )));
+        $filter = new Sort($table, 'count', 'desc', true, true, function(){return 'count2';});
+        $filter->filter($table);
+        $expectedOrder = array('yahoo', 'ask', 'nintendo');
+        $this->assertEquals($expectedOrder, $table->getColumn('label'));
+    }
+
+    public function testSecondarySort2()
+    {
+        $table = new DataTable();
+        $table->addRowsFromArray(array(
+            array(Row::COLUMNS => array('label' => 'ask', 'count' => 1, 'count2' => 10)),
+            array(Row::COLUMNS => array('label' => 'nintendo', 'count' => 10, 'count2' => 5)),
+            array(Row::COLUMNS => array('label' => 'yahoo', 'count' => 10, 'count2' => 100)
+            )));
+        $filter = new Sort($table, 'count', 'desc', true, true, function(){return 'count2';});
+        $filter->filter($table);
+        $expectedOrder = array('yahoo', 'nintendo', 'ask');
+        $this->assertEquals($expectedOrder, $table->getColumn('label'));
+    }
+
     private function createDataTable($rows)
     {
         $table = new DataTable();