@tsteur opened this Pull Request on October 8th 2018 Member

@mattab I would need someone to check all reports and various features to check if the totals make sense and when it breaks and when it works etc.

A bonus is that it shows on hover percentage values for some % rate columns. The shown percentage here is basically compared to the average (would need to somehow adjust hover text). Doesn't work for time metrics though because then other percentages break.

While this implementation could work in some cases, I have no idea if that always works or shows correct data.

If this implementation doesn't work as expected, we may need to schedule this for 3.8 or 3.9 if it is important as it would be a lot of work.

refs https://github.com/matomo-org/matomo/issues/5267

@tsteur commented on October 9th 2018 Member

Should work

@diosmosis commented on December 5th 2018 Member

Would it help to make the totals row bold to differentiate it from other rows (since this row will stay even while paginating)? Eg:

image

@diosmosis commented on December 5th 2018 Member

I suppose the percent doesn't need to be shown here:

image

@diosmosis commented on December 5th 2018 Member

Doesn't seem to work w/ goals columns:

image

@diosmosis commented on December 5th 2018 Member

don't think it's a big deal, but I noticed the total row option will display, even if there's no data in the report:

image

I'm sure most will understand, but it seems a bit odd.

@tsteur commented on December 6th 2018 Member

@diosmosis for now added the supportsTotalsRow method... the totals row should be bold. Because the change is in a _dataTable.less starting with underscore you might need to clear your tmp/assets cache.

@diosmosis commented on December 6th 2018 Member

Still need support for goal columns if possible. Unless it's hard to do I guess?

@tsteur commented on December 6th 2018 Member

@diosmosis I just remember a report can already disable totals row through viewDataTable config show_totals_row and can also set a request parameter totals=0 so I'm thinking it is actually better to remove the supportsTotalsRow method again which only applies to one visualisation kind of anyway (and the API but there it shouldn't cause a problem, it's more visualisation specific). Keeps the report class more simple.

@tsteur commented on December 6th 2018 Member

@diosmosis just pushed another commit with various fixes/changes

  • It wasn't showing the totals row when it was saved from previous usage (saved viewDataTable params)
  • No longer show it when report is empty
  • Disabled it for goals visualisation (still works for all columns visualisation in the goals report etc). Tried to make this work for over an hour but gave up after a while. There's so much special code around it and don't understand everything. It looked like this what I had but didn't quite produce correct numbers... I think there might not be much to do to make it work but couldn't quite figure out where things are replaced in what way...
diff --git a/plugins/Goals/Visualizations/Goals.php b/plugins/Goals/Visualizations/Goals.php
index 4453b2f352..2283b84dba 100644
--- a/plugins/Goals/Visualizations/Goals.php
+++ b/plugins/Goals/Visualizations/Goals.php
@@ -8,8 +8,10 @@

 namespace Piwik\Plugins\Goals\Visualizations;

+use Piwik\API\DataTablePostProcessor;
 use Piwik\API\Request;
 use Piwik\Common;
+use Piwik\DataTable;
 use Piwik\DataTable\Filter\AddColumnsProcessedMetricsGoal;
 use Piwik\Piwik;
 use Piwik\Plugins\CoreVisualizations\Visualizations\HtmlTable;
@@ -41,6 +43,16 @@ class Goals extends HtmlTable

     public function beforeRender()
     {
+        if ($this->dataTable->getTotalsRow()){
+            $dataTable = new DataTable();
+            $dataTable->addRow($this->dataTable->getTotalsRow());
+            $this->setShowGoalsColumnsProperties($dataTable);
+            $processor = new DataTablePostProcessor($this->requestConfig->getApiModuleToRequest(), $this->requestConfig->getApiMethodToRequest(), $this->request->getRequestArray());
+            $processor->applyComputeProcessedMetrics($dataTable);
+            $dataTable = $processor->applyMetricsFormatting($dataTable);
+            $this->dataTable->setTotalsRow($dataTable->getFirstRow());
+        }
+
         $this->config->show_goals = true;
         $this->config->show_goals_columns  = true;
         $this->config->datatable_css_class = 'dataTableVizGoals';
@@ -57,7 +69,7 @@ class Goals extends HtmlTable
         parent::beforeRender();
     }

-    private function setShowGoalsColumnsProperties()
+    private function setShowGoalsColumnsProperties($dataTable = null)
     {
         // set view properties based on goal requested
         $idSite = Common::getRequestVar('idSite', null, 'int');
@@ -83,7 +95,11 @@ class Goals extends HtmlTable
         }

         // add goals columns
-        $this->config->filters[] = array('AddColumnsProcessedMetricsGoal', array($enable = true, $idGoal, $goalsToProcess), $priority = true);
+        if (!empty($dataTable)) {
+            $dataTable->filter('AddColumnsProcessedMetricsGoal', array($enable = true, $idGoal, $goalsToProcess));
+        } else {
+            $this->config->filters[] = array('AddColumnsProcessedMetricsGoal', array($enable = true, $idGoal, $goalsToProcess), $priority = true);
+        }
     }

     private function setPropertiesForEcommerceView()
This Pull Request was closed on December 7th 2018
Powered by GitHub Issue Mirror