Merge pull request #7171 from piwik/6705_2

Scheduled reports: do not show "Users" metric when value is zero 

4 files changed, 347 insertions, 3 deletions

diff --git a/plugins/VisitsSummary/Reports/Get.php b/plugins/VisitsSummary/Reports/Get.php
index f2a087a517..55696873be 100644
--- a/plugins/VisitsSummary/Reports/Get.php
+++ b/plugins/VisitsSummary/Reports/Get.php
@@ -8,6 +8,7 @@
  */
 namespace Piwik\Plugins\VisitsSummary\Reports;
 
+use Piwik\DataTable\DataTableInterface;
 use Piwik\Piwik;
 use Piwik\Plugins\CoreHome\Columns\Metrics\ActionsPerVisit;
 use Piwik\Plugins\CoreHome\Columns\Metrics\AverageTimeOnSite;
@@ -15,6 +16,8 @@ use Piwik\Plugins\CoreHome\Columns\Metrics\BounceRate;
 
 class Get extends \Piwik\Plugin\Report
 {
+    private $usersColumn = 'nb_users';
+
     protected function init()
     {
         parent::init();
@@ -29,7 +32,7 @@ class Get extends \Piwik\Plugin\Report
         $this->metrics       = array(
             'nb_uniq_visitors',
             'nb_visits',
-            'nb_users',
+            $this->usersColumn,
             'nb_actions',
             'max_actions'
         );
@@ -43,7 +46,7 @@ class Get extends \Piwik\Plugin\Report
     {
         $metrics = parent::getMetrics();
 
-        $metrics['max_actions']      = Piwik::translate('General_ColumnMaxActions');
+        $metrics['max_actions'] = Piwik::translate('General_ColumnMaxActions');
 
         return $metrics;
     }
@@ -56,4 +59,25 @@ class Get extends \Piwik\Plugin\Report
 
         return $metrics;
     }
+
+    public function removeUsersFromProcessedReport(&$response)
+    {
+        if (!empty($response['metadata']['metrics'][$this->usersColumn])) {
+            unset($response['metadata']['metrics'][$this->usersColumn]);
+        }
+
+        if (!empty($response['metadata']['metricsDocumentation'][$this->usersColumn])) {
+            unset($response['metadata']['metricsDocumentation'][$this->usersColumn]);
+        }
+
+        if (!empty($response['columns'][$this->usersColumn])) {
+            unset($response['columns'][$this->usersColumn]);
+        }
+
+        if (!empty($response['reportData'])) {
+            $dataTable = $response['reportData'];
+            $dataTable->deleteColumn($this->usersColumn, true);
+        }
+    }
+
 }
\ No newline at end of file
diff --git a/plugins/VisitsSummary/VisitsSummary.php b/plugins/VisitsSummary/VisitsSummary.php
index cc6782f216..f0e7215aac 100644
--- a/plugins/VisitsSummary/VisitsSummary.php
+++ b/plugins/VisitsSummary/VisitsSummary.php
@@ -7,6 +7,9 @@
  *
  */
 namespace Piwik\Plugins\VisitsSummary;
+use Piwik\DataTable;
+use Piwik\Plugins\CoreHome\Columns\UserId;
+use Piwik\Plugins\VisitsSummary\Reports\Get;
 
 /**
  * Note: This plugin does not hook on Daily and Period Archiving like other Plugins because it reports the
@@ -23,10 +26,45 @@ class VisitsSummary extends \Piwik\Plugin
     public function getListHooksRegistered()
     {
         return array(
-            'AssetManager.getStylesheetFiles' => 'getStylesheetFiles'
+            'AssetManager.getStylesheetFiles' => 'getStylesheetFiles',
+            'API.API.getProcessedReport.end' => 'enrichProcessedReportIfVisitsSummaryGet',
         );
     }
 
+    private function isRequestingVisitsSummaryGet($module, $method)
+    {
+        return ($module === 'VisitsSummary' && $method === 'get');
+    }
+
+    public function enrichProcessedReportIfVisitsSummaryGet(&$response, $infos)
+    {
+        if (empty($infos['parameters'][4]) || empty($response['reportData'])) {
+            return;
+        }
+
+        $params  = $infos['parameters'];
+        $idSites = array($params[0]);
+        $period  = $params[1];
+        $date    = $params[2];
+        $module  = $params[3];
+        $method  = $params[4];
+
+        if (!$this->isRequestingVisitsSummaryGet($module, $method)) {
+            return;
+        }
+
+        $userId = new UserId();
+
+        /** @var DataTable|DataTable\Map $dataTable */
+        $dataTable = $response['reportData'];
+
+        if (!$userId->hasDataTableUsers($dataTable) &&
+            !$userId->isUsedInAtLeastOneSite($idSites, $period, $date)) {
+            $report = new Get();
+            $report->removeUsersFromProcessedReport($response);
+        }
+    }
+
     public function getStylesheetFiles(&$stylesheets)
     {
         $stylesheets[] = "plugins/VisitsSummary/stylesheets/datatable.less";
diff --git a/plugins/VisitsSummary/tests/Integration/VisitsSummaryTest.php b/plugins/VisitsSummary/tests/Integration/VisitsSummaryTest.php
new file mode 100644
index 0000000000..dc71cc4848
--- /dev/null
+++ b/plugins/VisitsSummary/tests/Integration/VisitsSummaryTest.php
@@ -0,0 +1,182 @@
+<?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\Plugins\VisitsSummary\tests\Integration;
+
+use Piwik\Access;
+use Piwik\API\Request;
+use Piwik\DataAccess\ArchiveTableCreator;
+use Piwik\Db;
+use Piwik\Plugins\Live\API;
+use Piwik\Plugins\VisitsSummary\VisitsSummary;
+use Piwik\Tests\Framework\Fixture;
+use Piwik\Tests\Framework\Mock\FakeAccess;
+use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
+
+/**
+ * @group VisitsSummary
+ * @group VisitsSummaryTest
+ * @group Plugins
+ */
+class VisitsSummaryTest extends IntegrationTestCase
+{
+    /**
+     * @var VisitsSummary
+     */
+    private $plugin;
+
+    protected $date = '2014-04-04';
+    private $column = 'nb_users';
+
+    public function setUp()
+    {
+        parent::setUp();
+        $this->plugin = new VisitsSummary();
+
+        $this->setSuperUser();
+
+        Fixture::createSuperUser();
+        Fixture::createWebsite('2014-01-01 00:00:00');
+    }
+
+    public function tearDown()
+    {
+        // clean up your test here if needed
+        $tables = ArchiveTableCreator::getTablesArchivesInstalled();
+        if (!empty($tables)) {
+            Db::dropTables($tables);
+        }
+        parent::tearDown();
+    }
+
+    public function test_enrichProcessedReportIfVisitsSummaryGet_shouldNotRemoveUsers_IfSomeWereTracked()
+    {
+        $this->trackPageviewsWithUsers();
+
+        $response = $this->requestProcessedGetReport();
+
+        $this->assertUsersNotRemovedFromProcessedReport($response, $expectedUsers = null, $minExpectedUsers = 1);
+    }
+
+    public function test_enrichProcessedReportIfVisitsSummaryGet_shouldNotRemoveUsers_IfNoneWereTrackedThatDay_ButThatMonth()
+    {
+        $this->date = '2014-04-04';
+        $this->trackPageviewsWithUsers();
+
+        $this->date = '2014-04-05';
+        $this->trackPageviewsWithoutUsers();
+
+        $response = $this->requestProcessedGetReport();
+
+        $this->assertUsersNotRemovedFromProcessedReport($response, $expectedUsers = 0);
+    }
+
+    public function test_isUsedInAtLeastOneSite_shouldRemoveUsers_IfNoneWereTracked()
+    {
+        $this->trackPageviewsWithoutUsers();
+
+        $response = $this->requestProcessedGetReport();
+
+        $this->assertUsersRemovedFromProcessedReport($response);
+    }
+
+    private function assertUsersNotRemovedFromProcessedReport($response, $exactNumUsers = null, $minNumUsers = 0)
+    {
+        $table = $response['reportData'];
+
+        if ($exactNumUsers !== null) {
+            $this->assertSame(array($exactNumUsers), $table->getColumn($this->column));
+        }
+
+        if ($minNumUsers != 0) {
+            $users = $table->getColumn($this->column);
+            $user = array_shift($users);
+            $this->assertGreaterThanOrEqual($minNumUsers, $user);
+        }
+
+        $numVisits = $table->getColumn('nb_visits');
+        $numVisits = array_shift($numVisits);
+        $this->assertGreaterThanOrEqual(2, $numVisits);
+
+        $this->assertNotEmpty($response['metadata']['metrics'][$this->column]);
+        $this->assertNotEmpty($response['metadata']['metricsDocumentation'][$this->column]);
+        $this->assertNotEmpty($response['columns'][$this->column]);
+    }
+
+    private function assertUsersRemovedFromProcessedReport($response)
+    {
+        $table = $response['reportData'];
+        $this->assertEquals(array(false), $table->getColumn($this->column));
+        
+        $numVisits = $table->getColumn('nb_visits');
+        $numVisits = array_shift($numVisits);
+        $this->assertGreaterThanOrEqual(2, $numVisits);
+
+        $this->assertArrayNotHasKey($this->column, $response['metadata']['metrics']);
+        $this->assertArrayNotHasKey($this->column, $response['metadata']['metricsDocumentation']);
+        $this->assertArrayNotHasKey($this->column, $response['columns']);
+    }
+
+    private function requestProcessedGetReport()
+    {
+        return Request::processRequest('API.getProcessedReport', array(
+            'idSite' => 1,
+            'period' => 'day',
+            'date'   => $this->date,
+            'apiModule' => 'VisitsSummary',
+            'apiAction' => 'get'
+        ));
+    }
+
+    private function trackPageviewsWithUsers()
+    {
+        $this->trackPageviewsWithDifferentUsers(array('user1', false, 'user3'));
+    }
+
+    private function trackPageviewsWithoutUsers()
+    {
+        $this->trackPageviewsWithDifferentUsers(array(false, false, false));
+    }
+
+    private function trackPageviewsWithDifferentUsers($userIds)
+    {
+        foreach ($userIds as $index => $userId) {
+            $tracker = $this->getTracker($index);
+            $tracker->setForceNewVisit();
+            $this->trackPageview($tracker, $userId, '/index/' . $index . '.html');
+        }
+    }
+
+    private function trackPageview(\PiwikTracker $tracker, $userId, $url)
+    {
+        $tracker->setUrl('http://www.example.org' . $url);
+        $tracker->setUserId($userId);
+        $tracker->doTrackPageView($url);
+    }
+
+    private function getTracker($hoursOffset = 0)
+    {
+        $hour = 10;
+        if ($hoursOffset) {
+            $hour += $hoursOffset;
+        }
+
+        $date = sprintf('%s %d:20:01', $this->date, $hour);
+
+        $tracker = Fixture::getTracker(1, $date, true, true);
+        $tracker->setTokenAuth(Fixture::getTokenAuth());
+        return $tracker;
+    }
+
+    private function setSuperUser()
+    {
+        $pseudoMockAccess = new FakeAccess();
+        $pseudoMockAccess::setSuperUserAccess(true);
+        Access::setSingletonInstance($pseudoMockAccess);
+    }
+}
diff --git a/plugins/VisitsSummary/tests/Unit/Reports/GetTest.php b/plugins/VisitsSummary/tests/Unit/Reports/GetTest.php
new file mode 100644
index 0000000000..df185a971e
--- /dev/null
+++ b/plugins/VisitsSummary/tests/Unit/Reports/GetTest.php
@@ -0,0 +1,100 @@
+<?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\Plugins\VisitsSummary\tests\Unit\Reports;
+
+use Piwik\DataTable;
+use Piwik\Plugins\VisitsSummary\Reports\Get;
+use Piwik\Tests\Framework\TestCase\UnitTestCase;
+
+/**
+ * @group VisitsSummary
+ * @group Reports
+ * @group GetTest
+ * @group Plugins
+ */
+class GetTest extends UnitTestCase
+{
+    /**
+     * @var Get
+     */
+    private $get;
+
+    private $column = 'nb_users';
+
+    public function setUp()
+    {
+        parent::setUp();
+        $this->get = new Get();
+    }
+
+    public function test_removeUsersFromProcessedReport_shouldNotDoAnything_IfNothingRelatedToUsersIsGiven()
+    {
+        $response = array();
+        $this->get->removeUsersFromProcessedReport($response);
+        $this->assertSame(array(), $response);
+
+        $response = array($this->column => '10', 'test' => 'whatever', 'columns' => array($this->column));
+        $this->get->removeUsersFromProcessedReport($response);
+        $this->assertSame(array($this->column => '10', 'test' => 'whatever', 'columns' => array($this->column)), $response);
+    }
+
+    public function test_removeUsersFromProcessedReport_shouldRemoveMetrics_IfUserIsGiven()
+    {
+        $response = array('metadata' => array('metrics' => array('nb_visits' => 'Visits', $this->column => 'Users')));
+        $this->get->removeUsersFromProcessedReport($response);
+        $this->assertSame(array('metadata' => array('metrics' => array('nb_visits' => 'Visits'))), $response);
+    }
+
+    public function test_removeUsersFromProcessedReport_shouldRemoveMetricsDocumentation_IfUserIsGiven()
+    {
+        $response = array('metadata' => array('metricsDocumentation' => array('nb_visits' => 'Visits', $this->column => 'Users')));
+        $this->get->removeUsersFromProcessedReport($response);
+        $this->assertSame(array('metadata' => array('metricsDocumentation' => array('nb_visits' => 'Visits'))), $response);
+    }
+
+    public function test_removeUsersFromProcessedReport_shouldRemoveColumn_IfUserIsGiven()
+    {
+        $response = array('columns' => array('nb_visits' => 'Visits', $this->column => 'Users'));
+        $this->get->removeUsersFromProcessedReport($response);
+        $this->assertSame(array('columns' => array('nb_visits' => 'Visits')), $response);
+    }
+
+    public function test_removeUsersFromProcessedReport_shouldRemoveUsersColumnFromDataTable_IfUserIsGiven()
+    {
+        $table = $this->getDataTableWithUsers();
+        $this->assertSame(array(20), $table->getColumn($this->column)); // verify column present
+
+        $response = array('reportData' => $table);
+        $this->get->removeUsersFromProcessedReport($response);
+
+        $this->assertSame(array(false), $table->getColumn($this->column));
+        $this->assertSame(array(10), $table->getColumn('nb_visits'));
+    }
+
+    public function test_removeUsersFromProcessedReport_shouldRemoveUsersColumnFromDataTableMap_IfUserIsGiven()
+    {
+        $table = new DataTable\Map();
+        $table->addTable($this->getDataTableWithUsers(), 'label');
+        $this->assertSame(array(20), $table->getColumn($this->column)); // verify column present
+
+        $response = array('reportData' => $table);
+        $this->get->removeUsersFromProcessedReport($response);
+
+        $this->assertSame(array(false), $table->getColumn($this->column));
+        $this->assertSame(array(10), $table->getColumn('nb_visits'));
+    }
+
+    private function getDataTableWithUsers()
+    {
+        $table = new DataTable();
+        $table->addRowFromSimpleArray(array('nb_visits' => 10, $this->column => 20));
+
+        return $table;
+    }
+}