Refs matomo-org#4041, matomo-org#4077, make sure treemap subtable loa…

…ding determines row limit dynamically, and allow visualizations to apply viewdatatable queued filters on their own time.
sabl0r · Sep 20, 2013 · ed569f2 · ed569f2
1 parent 035204a
commit ed569f2
Show file tree

Hide file tree

Showing 6 changed files with 100 additions and 100 deletions.
diff --git a/core/ViewDataTable.php b/core/ViewDataTable.php
@@ -610,14 +610,15 @@ protected function postDataTableLoadedFromAPI()
             $genericFilter->filter($this->dataTable);
         }
 
+        // queue other filters so they can be applied later if queued filters are disabled
+        foreach ($otherFilters as $filter) {
+            $this->dataTable->queueFilter($filter[0], $filter[1]);
+        }
+
         // Finally, apply datatable filters that were queued (should be 'presentation' filters that
         // do not affect the number of rows)
         if (!$this->areQueuedFiltersDisabled()) {
             $this->dataTable->applyQueuedFilters();
-
-            foreach ($otherFilters as $filter) {
-                $this->dataTable->filter($filter[0], $filter[1]);
-            }
         }
 
         return true;

diff --git a/plugins/TreemapVisualization/API.php b/plugins/TreemapVisualization/API.php
@@ -37,12 +37,14 @@ public static function getInstance()
      *                                          the first is used and the rest discarded.
      * @param string   $period
      * @param string   $date
-     * @param bool     $truncateAfter
+     * @param bool     $availableWidth          Available screen width in pixels.
+     * @param bool     $availableHeight         Available screen height in pixels.
      * @param int|bool $show_evolution_values   Whether to calculate evolution values for each row or not.
      *
      * @return array
      */
-    public function getTreemapData($apiMethod, $column, $period, $date, $truncateAfter = false, $show_evolution_values = false)
+    public function getTreemapData($apiMethod, $column, $period, $date, $availableWidth = false, $availableHeight = false,
+                                   $show_evolution_values = false)
     {
         if ($period == 'range') {
             $show_evolution_values = false;
@@ -67,15 +69,10 @@ public function getTreemapData($apiMethod, $column, $period, $date, $truncateAft
 
         $generator = new TreemapDataGenerator($column, $translation);
         $generator->setInitialRowOffset(Common::getRequestVar('filter_offset', 0, 'int'));
+        $generator->setAvailableDimensions($availableWidth, $availableHeight);
         if ($show_evolution_values) {
             $generator->showEvolutionValues();
         }
-
-        $truncateAfter = (int)$truncateAfter;
-        if ($truncateAfter > 0) {
-            $generator->setTruncateAfter($truncateAfter);
-        }
-
         return $generator->generate($dataTable);
     }
 }
diff --git a/plugins/TreemapVisualization/Treemap.php b/plugins/TreemapVisualization/Treemap.php
@@ -30,9 +30,6 @@ class Treemap extends Graph
     const FOOTER_ICON = 'plugins/TreemapVisualization/images/treemap-icon.png';
     const FOOTER_ICON_TITLE = 'Treemap';
 
-    const DEFAULT_MAX_ELEMENTS = 10;
-    const MIN_NODE_AREA = 400; // 20px * 20px
-
     /**
      * Controls whether the treemap nodes should be colored based on the evolution percent of
      * individual metrics, or not. If false, the jqPlot pie graph's series colors are used to
@@ -42,7 +39,12 @@ class Treemap extends Graph
      */
     const SHOW_EVOLUTION_VALUES = 'show_evolution_values';
 
-    public static $clientSideProperties = array('filter_offset', 'max_graph_elements', 'show_evolution_values');
+    public static $clientSideProperties = array(
+        'filter_offset',
+        'max_graph_elements',
+        'show_evolution_values',
+        'subtable_controller_action'
+    );
 
     /**
      * Constructor.
@@ -66,28 +68,21 @@ public function __construct($view)
             $view->custom_parameters['columns'] = $metric;
         };
 
+        $this->createTreemapDataGenerator($view);
         $this->handleShowEvolutionValues($view);
-        $this->handleDynamicTruncation($view, $metric);
     }
 
-    private function handleDynamicTruncation($view, $metric)
+    private function createTreemapDataGenerator($view)
     {
-        $currentPeriod = Period::makePeriodFromQueryParams(
-            $timezone = null, $period = Common::getRequestVar('period'), $date = Common::getRequestVar('date'));
-
-        $self = $this;
-        $doTruncate = function ($dataTable) use ($self, $metric, $currentPeriod) {
-            // only truncate current data
-            if ($dataTable->getMetadata('period')->getRangeString() != $currentPeriod->getRangeString()) {
-                return;
-            }
-
-            $truncateAfter = $self->getDynamicMaxElementCount($dataTable, $metric);
-            if ($truncateAfter > 0) {
-                $dataTable->filter('Truncate', array($truncateAfter));
-            }
-        };
-        $view->filters[] = array($doTruncate, array(), $priority = true);
+        $metric = $this->getMetricToGraph($view->columns_to_display);
+        $translation = empty($view->translations[$metric]) ? $metric : $view->translations[$metric];
+
+        $this->generator = new TreemapDataGenerator($metric, $translation);
+        $this->generator->setInitialRowOffset($view->filter_offset ?: 0);
+
+        $availableWidth = Common::getRequestVar('availableWidth', false);
+        $availableHeight = Common::getRequestVar('availableHeight', false);
+        $this->generator->setAvailableDimensions($availableWidth, $availableHeight);
     }
 
     /**
@@ -116,20 +111,6 @@ public function isThereDataToDisplay($dataTable, $view)
         return $this->getCurrentData($dataTable)->getRowsCount() != 0;
     }
 
-    public function getGraphData($dataTable, $properties)
-    {
-        $metric = $this->getMetricToGraph($properties['columns_to_display']);
-        $translation = empty($properties['translations'][$metric]) ? $metric : $properties['translations'][$metric];
-
-        $generator = new TreemapDataGenerator($metric, $translation);
-        $generator->setInitialRowOffset($properties['filter_offset'] ?: 0);
-        if ($dataTable instanceof Map) {
-            $generator->showEvolutionValues();
-        }
-
-        return $generator->generate($dataTable);
-    }
-
     public function getMetricToGraph($columnsToDisplay)
     {
         $firstColumn = reset($columnsToDisplay);
@@ -152,40 +133,8 @@ private function handleShowEvolutionValues($view)
             list($previousDate, $ignore) = Range::getLastDate($date, $period);
 
             $view->request_parameters_to_modify['date'] = $previousDate . ',' . $date;
-        }
-    }
 
-    public function getDynamicMaxElementCount($dataTable, $metricName)
-    {
-        $availableWidth = Common::getRequestVar('availableWidth', false);
-        $availableHeight = Common::getRequestVar('availableHeight', false);
-
-        if (!is_numeric($availableWidth)
-            || !is_numeric($availableHeight)
-        ) {
-            return self::DEFAULT_MAX_ELEMENTS - 1;
-        } else {
-            $totalArea = $availableWidth * $availableHeight;
-
-            $dataTable->filter('ReplaceColumnNames');
-
-            $metricValues = $dataTable->getColumn($metricName);
-            $metricSum = array_sum($metricValues);
-
-            // find the row index in $dataTable for which all rows after it will have treemap
-            // nodes that are too small. this is the row from which we truncate.
-            // Note: $dataTable is sorted at this point, so $metricValues is too
-            $result = 0;
-            foreach ($metricValues as $value) {
-                $nodeArea = ($totalArea * $value) / $metricSum;
-
-                if ($nodeArea < self::MIN_NODE_AREA) {
-                    break;
-                } else {
-                    ++$result;
-                }
-            }
-            return $result;
+            $this->generator->showEvolutionValues();
         }
     }
 

diff --git a/plugins/TreemapVisualization/TreemapDataGenerator.php b/plugins/TreemapVisualization/TreemapDataGenerator.php
@@ -21,6 +21,9 @@
  */
 class TreemapDataGenerator
 {
+    const DEFAULT_MAX_ELEMENTS = 10;
+    const MIN_NODE_AREA = 400; // 20px * 20px
+
     /**
      * The list of row metadata that should appear in treemap JSON data, if in the row.
      * 
@@ -67,19 +70,25 @@ class TreemapDataGenerator
     private $showEvolutionValues = false;
 
     /**
-     * The row offset to apply an additional truncation to (the first truncation occurs in
-     * DataTableGenericFilter).
+     * Holds the date of the past period. Implementation detail.
+     * 
+     * @var string
+     */
+    private $pastDataDate = null;
+
+    /**
+     * Holds the available screen width in pixels for the treemap.
      * 
      * @var int
      */
-    private $truncateAfter = false;
+    private $availableWidth = false;
 
     /**
-     * Holds the date of the past period. Implementation detail.
+     * Holds the available screen height in pixels for the treemap.
      * 
-     * @var string
+     * @var int
      */
-    private $pastDataDate = null;
+    private $availableHeight = false;
 
     /**
      * Constructor.
@@ -123,13 +132,15 @@ public function showEvolutionValues()
     }
 
     /**
-     * Sets the row offset to apply additional truncation after.
+     * Sets the available screen width & height for this treemap.
      * 
-     * @param int $truncateAfter
+     * @param int $availableWidth
+     * @param int $availableHeight
      */
-    public function setTruncateAfter($truncateAfter)
+    public function setAvailableDimensions($availableWidth, $availableHeight)
     {
-        $this->truncateAfter = $truncateAfter;
+        $this->availableWidth = $availableWidth;
+        $this->availableHeight = $availableHeight;
     }
 
     /**
@@ -154,8 +165,9 @@ public function generate($dataTable)
         }
 
         // handle extra truncation (only for current data)
-        if ($this->truncateAfter) {
-            $dataTable->filter('Truncate', array($this->truncateAfter));
+        $truncateAfter = $this->getDynamicMaxElementCount($dataTable);
+        if ($truncateAfter > 0) {
+            $dataTable->filter('Truncate', array($truncateAfter));
         }
 
         $tableId = Common::getRequestVar('idSubtable', '');
@@ -165,12 +177,46 @@ public function generate($dataTable)
         return $root;
     }
 
+    private function getDynamicMaxElementCount($dataTable)
+    {
+        if (!is_numeric($this->availableWidth)
+            || !is_numeric($this->availableHeight)
+        ) {
+            return self::DEFAULT_MAX_ELEMENTS - 1;
+        } else {
+            $totalArea = $this->availableWidth * $this->availableHeight;
+
+            $dataTable->filter('ReplaceColumnNames');
+
+            $metricValues = $dataTable->getColumn($this->metricToGraph);
+            $metricSum = array_sum($metricValues);
+
+            // find the row index in $dataTable for which all rows after it will have treemap
+            // nodes that are too small. this is the row from which we truncate.
+            // Note: $dataTable is sorted at this point, so $metricValues is too
+            $result = 0;
+            foreach ($metricValues as $value) {
+                $nodeArea = ($totalArea * $value) / $metricSum;
+
+                if ($nodeArea < self::MIN_NODE_AREA) {
+                    break;
+                } else {
+                    ++$result;
+                }
+            }
+            return $result;
+        }
+    }
+
     private function addDataTableToNode(&$node, $dataTable, $pastData = false, $tableId = '', $offset = 0)
     {
         foreach ($dataTable->getRows() as $rowId => $row) {
             $pastRow = $pastData ? $pastData->getRowFromLabel($row->getColumn('label')) : false;
 
             $childNode = $this->makeNodeFromRow($tableId, $rowId, $row, $pastRow);
+            if (empty($childNode)) {
+                continue;
+            }
 
             if ($rowId == DataTable::ID_SUMMARY_ROW) {
                 $childNode['data']['aggregate_offset'] = $offset + $dataTable->getRowsCount() - 1;
@@ -187,6 +233,10 @@ private function makeNodeFromRow($tableId, $rowId, $row, $pastRow)
         $label = $row->getColumn('label');
         $columnValue = $row->getColumn($this->metricToGraph) ?: 0;
 
+        if ($columnValue == 0) { // avoid issues in JIT w/ 0 $area values
+            return false;
+        }
+
         $data = array();
         $data['$area'] = $columnValue;
 

diff --git a/plugins/TreemapVisualization/javascripts/treemapViz.js b/plugins/TreemapVisualization/javascripts/treemapViz.js
@@ -417,7 +417,10 @@
          * Loads data for a node's subtable without reloading the datatable view.
          */
         _loadSubtableNodeChildren: function (node, callback) {
-            var ajax = this._getNodeChildrenAjax({idSubtable: node.data.idSubtable}, node, callback);
+            var ajax = this._getNodeChildrenAjax({
+                idSubtable: node.data.idSubtable,
+
+            }, node, callback);
             ajax.send();
         },
 
@@ -426,15 +429,15 @@
          */
         _getNodeChildrenAjax: function (overrideParams, node, callback) {
             var self = this,
-                dataNode = this._findNodeWithId(node.id),
-                params = $.extend({}, this.param, overrideParams, {
+                dataNode = self._findNodeWithId(node.id),
+                params = $.extend({}, self.param, overrideParams, {
                     module: 'API',
                     method: 'TreemapVisualization.getTreemapData',
                     action: 'index',
-                    apiMethod: this.param.module + '.' + this.param.action, // TODO: will this work for all subtables?
+                    apiMethod: self.param.module + '.' + self.props.subtable_controller_action,
                     format: 'json',
-                    column: this.param.columns,
-                    show_evolution_values: this.props.show_evolution_values || 0
+                    column: self.param.columns,
+                    show_evolution_values: self.props.show_evolution_values || 0,
                 });
 
             // make sure parallel load data requests aren't made

diff --git a/plugins/TreemapVisualization/templates/_dataTableViz_treemap.twig b/plugins/TreemapVisualization/templates/_dataTableViz_treemap.twig
@@ -1,3 +1,3 @@
-<div class="infoviz-treemap" data-data="{{ visualization.getGraphData(dataTable, properties)|json_encode }}">
+<div class="infoviz-treemap" data-data="{{ generator.generate(dataTable)|json_encode }}">
     <img class="infoviz-treemap-zoom-out" style="visibility:hidden;" src="plugins/Zeitgeist/images/zoom-out.png"/>
 </div>