Compute row percentages in PHP before metrics are formatted to avoid …

…percentage formatting WARNINGS. (matomo-org#15304)

* Compute row percentages in PHP before metrics are formatted to avoid percentage formatting WARNINGS.

* do not fail if site summary is not available

* Make sure siteSummary requests total data without segment.

* Make sure proper precision is used.

* try to fix tests

* update expected screenshots

core/Twig.php

@@ -198,6 +198,7 @@ public function __construct()
         $this->addTest_false();
         $this->addTest_true();
         $this->addTest_emptyString();
+        $this->addTest_isNumeric();
 
         $this->twig->addExtension(new PiwikTwigFilterExtension());
     }
@@ -575,4 +576,15 @@ private function addFilter_safelink()
         });
         $this->twig->addFilter($safelink);
     }
+
+    private function addTest_isNumeric()
+    {
+        $test = new Twig_SimpleTest(
+            'numeric',
+            function ($value) {
+                return is_numeric($value);
+            }
+        );
+        $this->twig->addTest($test);
+    }
 }

plugins/CoreVisualizations/Visualizations/HtmlTable.php

@@ -14,7 +14,9 @@
 use Piwik\DataTable\Row;
 use Piwik\Metrics;
 use Piwik\DataTable;
+use Piwik\NumberFormatter;
 use Piwik\Period;
+use Piwik\Piwik;
 use Piwik\Plugin\Visualization;
 
 /**
@@ -81,31 +83,9 @@ public function beforeRender()
             }
         }
 
-        // we do not want to get a datatable\map
-        $period = Common::getRequestVar('period', 'day', 'string');
-        if (Period\Range::parseDateRange($period)) {
-            $period = 'range';
-        }
-
         if ($this->dataTable->getRowsCount()) {
-            $request = new ApiRequest(array(
-                'method' => 'API.get',
-                'module' => 'API',
-                'action' => 'get',
-                'format' => 'original',
-                'filter_limit'  => '-1',
-                'disable_generic_filters' => 1,
-                'expanded'      => 0,
-                'flat'          => 0,
-                'filter_offset' => 0,
-                'period'        => $period,
-                'showColumns'   => implode(',', $this->config->columns_to_display),
-                'columns'       => implode(',', $this->config->columns_to_display),
-                'pivotBy'       => ''
-            ));
-
-            $dataTable = $request->process();
-            $this->assignTemplateVar('siteSummary', $dataTable);
+            $siteTotalRow = $this->getSiteSummary() ? $this->getSiteSummary()->getFirstRow() : null;
+            $this->assignTemplateVar('siteTotalRow', $siteTotalRow);
         }
 
         if ($this->isPivoted()) {
@@ -222,6 +202,55 @@ public function beforeGenericFiltersAreAppliedToLoadedDataTable()
         }
     }
 
+    public function afterGenericFiltersAreAppliedToLoadedDataTable()
+    {
+        parent::afterGenericFiltersAreAppliedToLoadedDataTable();
+
+        $this->calculateTotalPercentages(); // this must be done before metrics are formatted
+    }
+
+    private function calculateTotalPercentages()
+    {
+        if (empty($this->report)) {
+            return;
+        }
+
+        $columnNamesToIndices = Metrics::getMappingFromNameToId();
+        $formatter = NumberFormatter::getInstance();
+
+        $totals = $this->dataTable->getMetadata('totals');
+
+        $siteSummary = $this->getSiteSummary();
+        $siteTotalRow = $siteSummary ? $siteSummary->getFirstRow() : null;
+
+        foreach ($this->dataTable->getRows() as $row) {
+            foreach ($this->report->getMetrics() as $column => $translation) {
+                $indexColumn = $columnNamesToIndices[$column]; // TODO: This might fail for goal specific columns...
+
+                $value = $row->getColumn($indexColumn) ?: $row->getColumn($column) ?: 0;
+                if ($column == 'label') {
+                    continue;
+                }
+
+                $reportTotal = isset($totals[$column]) ? $totals[$column] : 0;
+
+                $percentageColumnName = $column . '_row_percentage';
+                $rowPercentage = $formatter->formatPercent(Piwik::getPercentageSafe($value, $reportTotal, $precision = 1), $precision);
+                $row->setMetadata($percentageColumnName, $rowPercentage);
+
+                if ($siteTotalRow) {
+                    $siteTotal = $siteTotalRow->getColumn($column) ?: 0;
+
+                    $siteTotalPercentage = $column . '_site_total_percentage';
+                    if ($siteTotal && $siteTotal > $reportTotal) {
+                        $rowPercentage = $formatter->formatPercent(Piwik::getPercentageSafe($value, $siteTotal, $precision = 1), $precision);
+                        $row->setMetadata($siteTotalPercentage, $rowPercentage);
+                    }
+                }
+            }
+        }
+    }
+
     protected function isPivoted()
     {
         return $this->requestConfig->pivotBy || Common::getRequestVar('pivotBy', '');
@@ -248,4 +277,30 @@ protected function isFlattened()
     {
         return $this->requestConfig->flat || Common::getRequestVar('flat', '');
     }
+
+    private function getSiteSummary()
+    {
+        if (empty($this->siteSummary)) {
+            // we do not want to get a datatable\map
+            $period = Common::getRequestVar('period', 'day', 'string');
+            if (Period\Range::parseDateRange($period)) {
+                $period = 'range';
+            }
+
+            $this->siteSummary = ApiRequest::processRequest('API.get', [
+                'module' => 'API',
+                'action' => 'get',
+                'filter_limit'  => '-1',
+                'disable_generic_filters' => 1,
+                'filter_offset' => 0,
+                'date' => Common::getRequestVar('date', null, 'string'),
+                'idSite' => Common::getRequestVar('idSite', null, 'int'),
+                'period'        => $period,
+                'showColumns'   => implode(',', $this->config->columns_to_display),
+                'columns'       => implode(',', $this->config->columns_to_display),
+            ], $default = []);
+        }
+
+        return $this->siteSummary;
+    }
 }

plugins/CoreVisualizations/templates/_dataTableViz_htmlTable_comparisons.twig

@@ -67,7 +67,6 @@
                 'totals': rowComparisonTotals,
                 'label': comparedRow.getColumn('label'),
                 'labelColumn': properties.columns_to_display|first,
-                'changePercantage': columnChange,
                 'forceZero': true,
                 'tooltipSuffix': comparisonTooltipSuffix,
                 'translations': properties.translations,

plugins/CoreVisualizations/templates/_dataTableViz_htmlTable_ratio.twig

@@ -1,26 +1,41 @@
 {% if column in properties.report_ratio_columns and (column in totals|keys or forceZero|default) -%}
     {% set reportTotal   = totals[column]|default(0) %}
-    {% if siteTotalRow|default is not empty %}
+
+    {% if siteTotalRow is defined and siteTotalRow is not empty %}
         {% set siteTotal = siteTotalRow.getColumn(column) %}
-    {% elseif siteSummary is defined and siteSummary is not empty and siteSummary.getFirstRow %}
-        {% set siteTotal = siteSummary.getFirstRow.getColumn(column) %}
     {% else %}
         {% set siteTotal = 0 %}
     {% endif %}
 
-    {% set rowPercentage = row.getColumn(column)|percentage(reportTotal, 1) %}
+    {% if rowPercentage|default is empty %}
+        {% if row.getMetadata(column ~ '_row_percentage') != false %}
+            {% set rowPercentage = row.getMetadata(column ~ '_row_percentage') %}
+        {% elseif row.getColumn(column)|default(0) is numeric and reportTotal is numeric %}
+            {% set rowPercentage = row.getColumn(column)|percentage(reportTotal, 1) %}
+        {% else %}
+            {% set rowPercentage = '' %} {# should not happen #}
+        {% endif %}
+    {% endif %}
+
     {% set metricTitle   = translations[column]|default(column) %}
 
     {% set reportRatioTooltip = 'General_ReportRatioTooltip'|translate(label, rowPercentage|e('html_attr'), reportTotal|e('html_attr'), metricTitle|e('html_attr'), '"' ~ segmentTitlePretty ~ '"', translations[labelColumn]|default(labelColumn)|e('html_attr')) %}
 
-    {% if siteTotal and siteTotal > reportTotal %}
-        {% set totalPercentage   = row.getColumn(column)|percentage(siteTotal, 1) %}
+    {% if totalPercentage|default is empty %}
+        {% if row.getMetadata(column ~ '_site_total_percentage') != false %}
+            {% set totalPercentage = row.getMetadata(column ~ '_site_total_percentage') %}
+        {% elseif row.getColumn(column)|default(0) is numeric and siteTotal is numeric %}
+            {% set totalPercentage = row.getColumn(column)|percentage(siteTotal, 1) %}
+        {% endif %}
+    {% endif %}
+
+    {% if totalPercentage|default is not empty %}
         {% set totalRatioTooltip = 'General_TotalRatioTooltip'|translate(totalPercentage, siteTotal|number(2,0), metricTitle, periodTitlePretty) %}
     {% else %}
         {% set totalRatioTooltip = '' %}
     {% endif %}
 
     <span class="ratio"
           title="{{ reportRatioTooltip|rawSafeDecoded|raw }} {{ totalRatioTooltip|rawSafeDecoded|e('html_attr') }}{% if tooltipSuffix|default is not empty %}<br/><br/> {{ tooltipSuffix|rawSafeDecoded|e('html_attr') }}{% endif %}"
-    >&nbsp;{{ rowPercentage }} {% if changePercantage|default is not empty %}({{ changePercentage }}){% endif %}</span>
+    >&nbsp;{{ rowPercentage }} {% if changePercentage|default is not empty %}({{ changePercentage }}){% endif %}</span>
 {%- endif %}