From f5c463e3d7deb1b80aacdc2b7d8c96a0add6897b Mon Sep 17 00:00:00 2001
From: Thomas Steur <thomas.steur@gmail.com>
Date: Tue, 10 Feb 2015 02:56:47 +0000
Subject: [PATCH] refs #6705 added some tests and some code tweaks
---
plugins/CoreHome/Columns/UserId.php | 2 +-
.../tests/Integration/Column/UserIdTest.php | 68 ++++++++
plugins/VisitsSummary/Reports/Get.php | 27 ++-
plugins/VisitsSummary/VisitsSummary.php | 49 ++----
.../tests/Integration/VisitsSummaryTest.php | 165 ++++++++++++++++++
.../tests/Unit/Reports/GetTest.php | 100 +++++++++++
6 files changed, 378 insertions(+), 33 deletions(-)
create mode 100644 plugins/VisitsSummary/tests/Integration/VisitsSummaryTest.php
create mode 100644 plugins/VisitsSummary/tests/Unit/Reports/GetTest.php
diff --git a/plugins/CoreHome/Columns/UserId.php b/plugins/CoreHome/Columns/UserId.php
index c556407a61..4cdc13c5d1 100644
--- a/plugins/CoreHome/Columns/UserId.php
+++ b/plugins/CoreHome/Columns/UserId.php
@@ -95,7 +95,7 @@ class UserId extends VisitDimension
return $this->hasDataTableUsers($result);
}
- public function hasDataTableUsers(DataTable $result)
+ public function hasDataTableUsers(DataTable\DataTableInterface $result)
{
if ($result instanceof Map) {
$result = $result->mergeChildren();
diff --git a/plugins/CoreHome/tests/Integration/Column/UserIdTest.php b/plugins/CoreHome/tests/Integration/Column/UserIdTest.php
index 7861a4a65b..f2ca415b5a 100644
--- a/plugins/CoreHome/tests/Integration/Column/UserIdTest.php
+++ b/plugins/CoreHome/tests/Integration/Column/UserIdTest.php
@@ -131,6 +131,74 @@ class UserIdTest extends IntegrationTestCase
$this->assertNotUsedInAtLeastOneSite($idSites = array(1), 'range', '2014-04-01,2014-04-03');
}
+ public function test_hasDataTableUsers_shouldReturnFalse_IfEmptyTablesAreGiven()
+ {
+ $this->assertNotDataTableHasUsers(new DataTable\Map());
+ $this->assertNotDataTableHasUsers(new DataTable());
+ }
+
+ public function test_hasDataTableUsers_shouldHandleADataTableMap()
+ {
+ $map = new DataTable\Map();
+ $map->addTable(new DataTable(), 'label1');
+ $map->addTable(new DataTable(), 'label2');
+ $map->addTable($this->getDataTableWithoutUsersColumn(), 'label3');
+
+ $this->assertNotDataTableHasUsers($map);
+
+ $map->addTable($this->getDataTableWithZeroUsers(), 'label4');
+ $map->addTable(new DataTable(), 'label5');
+
+ $this->assertNotDataTableHasUsers($map);
+
+ $map->addTable($this->getDataTableWithUsers(), 'label6');
+
+ $this->assertDataTableHasUsers($map);
+ }
+
+ public function test_hasDataTableUsers_shouldHandleADataTable()
+ {
+ $this->assertNotDataTableHasUsers($this->getDataTableWithoutUsersColumn());
+ $this->assertNotDataTableHasUsers($this->getDataTableWithZeroUsers());
+ $this->assertDataTableHasUsers($this->getDataTableWithUsers());
+ }
+
+ private function getDataTableWithoutUsersColumn()
+ {
+ $tableWithoutUsers = new DataTable();
+ $tableWithoutUsers->addRowFromSimpleArray(array('label' => 'test', 'nb_visits' => 0));
+
+ return $tableWithoutUsers;
+ }
+
+ private function getDataTableWithZeroUsers()
+ {
+ $tableWithZeroUsers = new DataTable();
+ $tableWithZeroUsers->addRowFromSimpleArray(array('label' => 'test', 'nb_users' => 0));
+
+ return $tableWithZeroUsers;
+ }
+
+ private function getDataTableWithUsers()
+ {
+ $tableWithUsers = new DataTable();
+ $tableWithUsers->addRowFromSimpleArray(array('label' => 'test', 'nb_users' => 10));
+
+ return $tableWithUsers;
+ }
+
+ private function assertNotDataTableHasUsers($table)
+ {
+ $has = $this->userId->hasDataTableUsers($table);
+ $this->assertFalse($has);
+ }
+
+ private function assertDataTableHasUsers($table)
+ {
+ $has = $this->userId->hasDataTableUsers($table);
+ $this->assertTrue($has);
+ }
+
private function assertUsedInAtLeastOneSite($idSites, $period, $date)
{
$result = $this->userId->isUsedInAtLeastOneSite($idSites, $period, $date);
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;
+ }
+}
--
GitLab