From 5d46f802f0485c504c3923aa625032ca998048cf Mon Sep 17 00:00:00 2001
From: Stefan Giehl <stefan@piwik.org>
Date: Wed, 29 Mar 2017 10:09:18 +0200
Subject: [PATCH] Make it possible to disable flattening for an report (#11529)

* use report property to enable/disable flattener

* update tests

* Update changelog [ci skip]

* minor renamings
---
 CHANGELOG.md                                  |   1 +
 core/API/DataTablePostProcessor.php           |   6 ++
 core/DataTable/Filter/RemoveSubtables.php     |  47 +++++++++++++
 core/Plugin/Report.php                        |  17 +++++
 core/ViewDataTable/Config.php                 |  10 +++
 plugins/Referrers/Reports/GetReferrerType.php |   1 +
 plugins/UserId/Reports/GetUsers.php           |  12 ++--
 ...attened__Referrers.getReferrerType_day.xml |  62 +++---------------
 .../UIIntegrationTest_metric_tooltip.png      | Bin 131 -> 131 bytes
 9 files changed, 98 insertions(+), 58 deletions(-)
 create mode 100644 core/DataTable/Filter/RemoveSubtables.php

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7a0c7a7ba4..4e71995db5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@ The Product Changelog at **[piwik.org/changelog](http://piwik.org/changelog)** l
 
 ### Breaking Changes
 * New config setting `enable_plugin_upload` lets you enable uploading and installing a Piwik plugin ZIP file by a Super User. This used to be enabled by default, but it is now disabled by default now for security reasons.
+* New Report class property `Report::$supportsFlatten` lets you define if a report supports flattening (defaults to `true`). If set to `false` it will also set `ViewDataTable\Config::$show_flatten_table` to `false`
 
 ### New APIs
 * A new event `Controller.triggerAdminNotifications` has been added to let plugins know when they are supposed to trigger notifications in the admin.
diff --git a/core/API/DataTablePostProcessor.php b/core/API/DataTablePostProcessor.php
index c2fdd1569c..c4eb7441d0 100644
--- a/core/API/DataTablePostProcessor.php
+++ b/core/API/DataTablePostProcessor.php
@@ -170,6 +170,12 @@ class DataTablePostProcessor
     public function applyFlattener($dataTable)
     {
         if (Common::getRequestVar('flat', '0', 'string', $this->request) == '1') {
+            // skip flattening if not supported by report and remove subtables only
+            if ($this->report && !$this->report->supportsFlatten()) {
+                $dataTable->filter('RemoveSubtables');
+                return $dataTable;
+            }
+
             $flattener = new Flattener($this->apiModule, $this->apiMethod, $this->request);
             if (Common::getRequestVar('include_aggregate_rows', '0', 'string', $this->request) == '1') {
                 $flattener->includeAggregateRows();
diff --git a/core/DataTable/Filter/RemoveSubtables.php b/core/DataTable/Filter/RemoveSubtables.php
new file mode 100644
index 0000000000..e84244119c
--- /dev/null
+++ b/core/DataTable/Filter/RemoveSubtables.php
@@ -0,0 +1,47 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ */
+namespace Piwik\DataTable\Filter;
+
+use Piwik\DataTable;
+use Piwik\DataTable\BaseFilter;
+
+/**
+ * Delete all existing subtables from rows.
+ *
+ * **Basic example usage**
+ *
+ *     $dataTable->filter('RemoveSubtables');
+ *
+ * @api
+ */
+class RemoveSubtables extends BaseFilter
+{
+    /**
+     * Constructor.
+     *
+     * @param DataTable $table The DataTable that will be filtered eventually.
+     */
+    public function __construct($table)
+    {
+        parent::__construct($table);
+    }
+
+    /**
+     * See {@link Limit}.
+     *
+     * @param DataTable $table
+     */
+    public function filter($table)
+    {
+        $rows = $table->getRows();
+        foreach ($rows as $row) {
+            $row->removeSubtable();
+        }
+    }
+}
diff --git a/core/Plugin/Report.php b/core/Plugin/Report.php
index 7bdbf0710e..df28f450c9 100644
--- a/core/Plugin/Report.php
+++ b/core/Plugin/Report.php
@@ -119,6 +119,14 @@ class Report
      */
     protected $hasGoalMetrics = false;
 
+    /**
+     * Set this property to false in case your report can't/shouldn't be flattened.
+     * In this case, flattener won't be applied even if parameter is provided in a request
+     * @var bool
+     * @api
+     */
+    protected $supportsFlatten = true;
+
     /**
      * Set it to boolean `true` if your report always returns a constant count of rows, for instance always 24 rows
      * for 1-24 hours.
@@ -493,6 +501,15 @@ class Report
         return $this->hasGoalMetrics;
     }
 
+    /**
+     * @return bool
+     * @ignore
+     */
+    public function supportsFlatten()
+    {
+        return $this->supportsFlatten;
+    }
+
     /**
      * If the report is enabled the report metadata for this report will be built and added to the list of available
      * reports. Overwrite this method and leave it empty in case you do not want your report to be added to the report
diff --git a/core/ViewDataTable/Config.php b/core/ViewDataTable/Config.php
index 2ccd975c7b..0eb99aa9f5 100644
--- a/core/ViewDataTable/Config.php
+++ b/core/ViewDataTable/Config.php
@@ -502,6 +502,7 @@ class Config
 
         $this->loadDocumentation();
         $this->setShouldShowPivotBySubtable();
+        $this->setShouldShowFlattener();
     }
 
     /** Load documentation from the API */
@@ -755,6 +756,15 @@ class Config
         }
     }
 
+    private function setShouldShowFlattener()
+    {
+        $report = ReportsProvider::factory($this->controllerName, $this->controllerAction);
+
+        if ($report && !$report->supportsFlatten()) {
+            $this->show_flatten_table = false;
+        }
+    }
+
     public function disablePivotBySubtableIfTableHasNoSubtables(DataTable $table)
     {
         foreach ($table->getRows() as $row) {
diff --git a/plugins/Referrers/Reports/GetReferrerType.php b/plugins/Referrers/Reports/GetReferrerType.php
index 16a11d295b..dea44ccb5a 100644
--- a/plugins/Referrers/Reports/GetReferrerType.php
+++ b/plugins/Referrers/Reports/GetReferrerType.php
@@ -37,6 +37,7 @@ class GetReferrerType extends Base
         $this->hasGoalMetrics = true;
         $this->order = 1;
         $this->subcategoryId = 'Referrers_WidgetGetAll';
+        $this->supportsFlatten = false;
     }
 
     public function getDefaultTypeViewDataTable()
diff --git a/plugins/UserId/Reports/GetUsers.php b/plugins/UserId/Reports/GetUsers.php
index 776c29e02f..7fba70f472 100644
--- a/plugins/UserId/Reports/GetUsers.php
+++ b/plugins/UserId/Reports/GetUsers.php
@@ -25,11 +25,12 @@ class GetUsers extends Base
     {
         parent::init();
 
-        $this->name = Piwik::translate('UserId_UserReportTitle');
-        $this->subcategoryId = 'UserId_UserReportTitle';
-        $this->documentation = '';
-        $this->dimension = new UserId();
-        $this->metrics = array('label', 'nb_visits', 'nb_actions', 'nb_visits_converted');
+        $this->name            = Piwik::translate('UserId_UserReportTitle');
+        $this->subcategoryId   = 'UserId_UserReportTitle';
+        $this->documentation   = '';
+        $this->dimension       = new UserId();
+        $this->metrics         = array('label', 'nb_visits', 'nb_actions', 'nb_visits_converted');
+        $this->supportsFlatten = false;
 
         // This defines in which order your report appears in the mobile app, in the menu and in the list of widgets
         $this->order = 9;
@@ -60,7 +61,6 @@ class GetUsers extends Base
         $view->config->show_related_reports = false;
         $view->config->show_insights = false;
         $view->config->show_pivot_by_subtable = false;
-        $view->config->show_flatten_table = false;
 
         if ($view->isViewDataTableId(HtmlTable::ID)) {
             $view->config->disable_row_evolution = false;
diff --git a/tests/PHPUnit/System/expected/test_reportLimiting_flattened__Referrers.getReferrerType_day.xml b/tests/PHPUnit/System/expected/test_reportLimiting_flattened__Referrers.getReferrerType_day.xml
index 2f781c0adb..e8636937d0 100644
--- a/tests/PHPUnit/System/expected/test_reportLimiting_flattened__Referrers.getReferrerType_day.xml
+++ b/tests/PHPUnit/System/expected/test_reportLimiting_flattened__Referrers.getReferrerType_day.xml
@@ -14,68 +14,26 @@
 	</row>
 	<row>
 		<label>Search Engines</label>
-		<nb_uniq_visitors>7</nb_uniq_visitors>
-		<nb_visits>7</nb_visits>
-		<nb_actions>7</nb_actions>
-		<nb_users>0</nb_users>
-		<max_actions>1</max_actions>
-		<sum_visit_length>0</sum_visit_length>
-		<bounce_count>7</bounce_count>
-		<nb_visits_converted>0</nb_visits_converted>
-	</row>
-	<row>
-		<label>Websites</label>
-		<nb_uniq_visitors>3</nb_uniq_visitors>
-		<nb_visits>4</nb_visits>
-		<nb_actions>4</nb_actions>
-		<nb_users>0</nb_users>
-		<max_actions>1</max_actions>
-		<sum_visit_length>0</sum_visit_length>
-		<bounce_count>4</bounce_count>
-		<nb_visits_converted>0</nb_visits_converted>
-	</row>
-	<row>
-		<label>Search Engines</label>
-		<nb_uniq_visitors>3</nb_uniq_visitors>
-		<nb_visits>3</nb_visits>
-		<nb_actions>3</nb_actions>
+		<nb_uniq_visitors>12</nb_uniq_visitors>
+		<nb_visits>12</nb_visits>
+		<nb_actions>12</nb_actions>
 		<nb_users>0</nb_users>
 		<max_actions>1</max_actions>
 		<sum_visit_length>0</sum_visit_length>
-		<bounce_count>3</bounce_count>
-		<nb_visits_converted>0</nb_visits_converted>
-	</row>
-	<row>
-		<label>Search Engines</label>
-		<nb_uniq_visitors>2</nb_uniq_visitors>
-		<nb_visits>2</nb_visits>
-		<nb_actions>2</nb_actions>
-		<nb_users>0</nb_users>
-		<max_actions>1</max_actions>
-		<sum_visit_length>0</sum_visit_length>
-		<bounce_count>2</bounce_count>
+		<bounce_count>12</bounce_count>
 		<nb_visits_converted>0</nb_visits_converted>
+		<segment>referrerType==search</segment>
 	</row>
 	<row>
 		<label>Websites</label>
-		<nb_uniq_visitors>2</nb_uniq_visitors>
-		<nb_visits>2</nb_visits>
-		<nb_actions>2</nb_actions>
-		<nb_users>0</nb_users>
-		<max_actions>1</max_actions>
-		<sum_visit_length>0</sum_visit_length>
-		<bounce_count>2</bounce_count>
-		<nb_visits_converted>0</nb_visits_converted>
-	</row>
-	<row>
-		<label>Websites</label>
-		<nb_uniq_visitors>2</nb_uniq_visitors>
-		<nb_visits>2</nb_visits>
-		<nb_actions>2</nb_actions>
+		<nb_uniq_visitors>7</nb_uniq_visitors>
+		<nb_visits>8</nb_visits>
+		<nb_actions>8</nb_actions>
 		<nb_users>0</nb_users>
 		<max_actions>1</max_actions>
 		<sum_visit_length>0</sum_visit_length>
-		<bounce_count>2</bounce_count>
+		<bounce_count>8</bounce_count>
 		<nb_visits_converted>0</nb_visits_converted>
+		<segment>referrerType==website</segment>
 	</row>
 </result>
\ No newline at end of file
diff --git a/tests/UI/expected-screenshots/UIIntegrationTest_metric_tooltip.png b/tests/UI/expected-screenshots/UIIntegrationTest_metric_tooltip.png
index ace02fe4d0b84a52dd0f6c37e84c93d2b4be0325..0bf02f72092da459b0fe2504ae60d1978df4bf30 100644
GIT binary patch
delta 84
zcmZo>Y-XHb=9^-XmS%2fnr3NfnrfbEXkck(YMgACY?NqjZjxe{mX=~*WN4Xenv!H{
nkZh5dY;I|umY8f{Zfs$kY;J0nV#ZaRS(U0_Xl7w-VZsFf=|&dl

delta 84
zcmZo>Y-XHb=9_G2Y@BS6Xkwn0YGG(>X=HAmVrZ0Vkd|U$lxS*bVwz%Ml4xX<VwRMa
nnwpkoXpv-+oRVsuXpm}QY-pBbV8T_LS(U0_Xl7wyWXJ^o<)arH

-- 
GitLab