refs #6705 added some tests and some code tweaks

4 files changed, 309 insertions, 32 deletions

diff --git a/plugins/VisitsSummary/Reports/Get.php b/plugins/VisitsSummary/Reports/Get.php
index bced863b07..cbd7ddab35 100644
--- a/plugins/VisitsSummary/Reports/Get.php
+++ b/plugins/VisitsSummary/Reports/Get.php
@@ -8,14 +8,16 @@
  */
 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;
 use Piwik\Plugins\CoreHome\Columns\Metrics\BounceRate;
-use Piwik\Plugins\CoreHome\Columns\UserId;
 
 class Get extends \Piwik\Plugin\Report
 {
+    private $usersColumn = 'nb_users';
+
     protected function init()
     {
         parent::init();
@@ -30,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'
         );
@@ -57,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);
+        }
+    }
+
 }
\ No newline at end of file
diff --git a/plugins/VisitsSummary/VisitsSummary.php b/plugins/VisitsSummary/VisitsSummary.php
index 4150f7dfaf..5db1cb33e7 100644
--- a/plugins/VisitsSummary/VisitsSummary.php
+++ b/plugins/VisitsSummary/VisitsSummary.php
@@ -9,6 +9,7 @@
 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
@@ -30,50 +31,38 @@ class VisitsSummary extends \Piwik\Plugin
         );
     }
 
-    public function enrichProcessedReportIfVisitsSummaryGet(&$response, $infos)
+    private function isRequestingVisitsSummaryGet($module, $method)
     {
-        $params = $infos['parameters'];
-        $module = $params[3];
-        $method = $params[4];
+        return ($module === 'VisitsSummary' && $method === 'get');
+    }
 
-        if ($module !== 'VisitsSummary' || $method !== 'get') {
+    public function enrichProcessedReportIfVisitsSummaryGet(&$response, $infos)
+    {
+        if (empty($infos['parameters'][4]) || empty($response['reportData'])) {
             return;
         }
 
-        $userId = new UserId();
+        $params  = $infos['parameters'];
+        $idSites = array($params[0]);
+        $period  = $params[1];
+        $date    = $params[2];
+        $module  = $params[3];
+        $method  = $params[4];
 
         /** @var DataTable|DataTable\Map $dataTable */
         $dataTable = $response['reportData'];
 
-        if ($userId->hasDataTableUsers($dataTable)) {
-            return;
-        }
-
-        $idSites = $params[0];
-        if (!is_array($idSites)) {
-            $idSites = array($idSites);
-        }
-
-        $period = $params[1];
-        $date   = $params[2];
-
-        if ($userId->isUsedInAtLeastOneSite($idSites, $period, $date)) {
+        if (!$this->isRequestingVisitsSummaryGet($module, $method)) {
             return;
         }
 
-        if (!empty($response['metadata']['metrics']['nb_users'])) {
-            unset($response['metadata']['metrics']['nb_users']);
-        }
-
-        if (!empty($response['metadata']['metricsDocumentation']['nb_users'])) {
-            unset($response['metadata']['metricsDocumentation']['nb_users']);
-        }
+        $userId = new UserId();
 
-        if (!empty($response['columns']['nb_users'])) {
-            unset($response['columns']['nb_users']);
+        if (!$userId->hasDataTableUsers($dataTable) &&
+            !$userId->isUsedInAtLeastOneSite($idSites, $period, $date)) {
+            $report = new Get();
+            $report->removeUsersFromProcessedReport($response, $dataTable);
         }
-
-        $dataTable->deleteColumn('nb_users');
     }
 
     public function getStylesheetFiles(&$stylesheets)
diff --git a/plugins/VisitsSummary/tests/Integration/VisitsSummaryTest.php b/plugins/VisitsSummary/tests/Integration/VisitsSummaryTest.php
new file mode 100644
index 0000000000..cc30bbc345
--- /dev/null
+++ b/plugins/VisitsSummary/tests/Integration/VisitsSummaryTest.php
@@ -0,0 +1,165 @@
+<?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\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');
+        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 = 2);
+    }
+
+    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, $numUsers)
+    {
+        $table = $response['reportData'];
+        $this->assertEquals(array($numUsers), $table->getColumn($this->column));
+        $this->assertEquals(array(3), $table->getColumn('nb_visits'));
+        $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));
+        $this->assertEquals(array(3), $table->getColumn('nb_visits'));
+        $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)
+    {
+        $tracker = $this->getTracker();
+
+        foreach ($userIds as $index => $userId) {
+            $tracker->setForceNewVisit();
+            $this->trackPageview($tracker, $userId, '/index/' . $index . '.html');
+        }
+    }
+
+    private function trackPageview(\PiwikTracker $tracker, $userId, $url = null)
+    {
+        if (null !== $url) {
+            $tracker->setUrl('http://www.example.org' . $url);
+        }
+
+        $tracker->setUserId($userId);
+
+        $title = $url ? : 'test';
+
+        $tracker->doTrackPageView($title);
+    }
+
+    private function getTracker()
+    {
+        $tracker = Fixture::getTracker(1, $this->date . ' 00:01:01', 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;
+    }
+}