From 4165176308adc982bb127bb19ff8825340573f67 Mon Sep 17 00:00:00 2001 From: Thomas Steur Date: Fri, 7 Mar 2014 01:44:59 +0100 Subject: [PATCH] refs #57 some code improvements and tests, also some adjustments for sites having not many visits --- plugins/Insights/API.php | 27 ++-- plugins/Insights/Controller.php | 47 +++++-- plugins/Insights/InsightReport.php | 54 ++++++- plugins/Insights/Model.php | 13 +- plugins/Insights/Visualizations/Insight.php | 65 ++++----- .../Visualizations/Insight/RequestConfig.php | 8 +- .../templates/insightVisualization.twig | 4 +- plugins/Insights/templates/table_row.twig | 2 +- plugins/Insights/tests/ApiTest.php | 12 ++ .../tests/{BaseUnitTest.php => BaseUnit.php} | 2 +- plugins/Insights/tests/FilterAverageTest.php | 2 +- .../tests/FilterExcludeLowValueTest.php | 2 +- plugins/Insights/tests/FilterInsightTest.php | 2 +- plugins/Insights/tests/FilterLimitTest.php | 2 +- .../Insights/tests/FilterMinGrowthTest.php | 2 +- plugins/Insights/tests/FilterOrderByTest.php | 2 +- plugins/Insights/tests/InsightReportTest.php | 133 ++++++++++++++++-- plugins/Insights/tests/ModelTest.php | 16 ++- 18 files changed, 298 insertions(+), 97 deletions(-) rename plugins/Insights/tests/{BaseUnitTest.php => BaseUnit.php} (95%) diff --git a/plugins/Insights/API.php b/plugins/Insights/API.php index 3be7ac495e1..b1ee45b65fe 100644 --- a/plugins/Insights/API.php +++ b/plugins/Insights/API.php @@ -56,8 +56,10 @@ protected function __construct() $this->model = new Model(); } - public function canGenerateInsights($period, $date) + public function canGenerateInsights($date, $period) { + Piwik::checkUserHasSomeViewAccess(); + try { $model = new Model(); $lastDate = $model->getLastDate($date, $period, 1); @@ -74,12 +76,12 @@ public function canGenerateInsights($period, $date) public function getInsightsOverview($idSite, $period, $date, $segment = false) { - Piwik::checkUserHasViewAccess(array($idSite)); + Piwik::checkUserHasViewAccess($idSite); $defaultParams = array( 'limitIncreaser' => 3, 'limitDecreaser' => 3, - 'minImpactPercent' => 2, + 'minImpactPercent' => 1, 'minGrowthPercent' => 25, ); @@ -90,7 +92,7 @@ public function getInsightsOverview($idSite, $period, $date, $segment = false) public function getMoversAndShakersOverview($idSite, $period, $date, $segment = false) { - Piwik::checkUserHasViewAccess(array($idSite)); + Piwik::checkUserHasViewAccess($idSite); $defaultParams = array( 'limitIncreaser' => 4, @@ -164,16 +166,20 @@ public function getInsights( $reportMetadata = $this->model->getReportByUniqueId($idSite, $reportUniqueId); + $totalValue = $this->model->getTotalValue($idSite, $period, $date, $metric); $currentReport = $this->model->requestReport($idSite, $period, $date, $reportUniqueId, $metric, $segment); - $totalValue = $this->model->getRelevantTotalValue($currentReport, $idSite, $period, $date, $metric); $lastDate = $this->model->getLastDate($date, $period, $comparedToXPeriods); + $lastTotalValue = $this->model->getTotalValue($idSite, $period, $lastDate, $metric); $lastReport = $this->model->requestReport($idSite, $period, $lastDate, $reportUniqueId, $metric, $segment); $minGrowthPercentPositive = abs($minGrowthPercent); $minGrowthPercentNegative = -1 * $minGrowthPercentPositive; - $minMoversPercent = -1; - $minNewPercent = -1; + + $relevantTotal = $this->model->getRelevantTotalValue($currentReport, $metric, $totalValue); + + $minMoversPercent = -1; + $minNewPercent = -1; $minDisappearedPercent = -1; switch ($filterBy) { @@ -193,7 +199,10 @@ public function getInsights( } $insight = new InsightReport(); - return $insight->generateInsight($reportMetadata, $period, $date, $lastDate, $metric, $currentReport, $lastReport, $totalValue, $minMoversPercent, $minNewPercent, $minDisappearedPercent, $minGrowthPercentPositive, $minGrowthPercentNegative, $orderBy, $limitIncreaser, $limitDecreaser); + $table = $insight->generateInsight($reportMetadata, $period, $date, $lastDate, $metric, $currentReport, $lastReport, $relevantTotal, $minMoversPercent, $minNewPercent, $minDisappearedPercent, $minGrowthPercentPositive, $minGrowthPercentNegative, $orderBy, $limitIncreaser, $limitDecreaser); + $insight->markMoversAndShakers($table, $currentReport, $lastReport, $totalValue, $lastTotalValue); + + return $table; } private function requestApiMethod($method, $idSite, $period, $date, $reportId, $segment, $additionalParams) @@ -207,7 +216,7 @@ private function requestApiMethod($method, $idSite, $period, $date, $reportId, $ 'reportUniqueId' => $reportId, ); - if ($segment) { + if (!empty($segment)) { $params['segment'] = $segment; } diff --git a/plugins/Insights/Controller.php b/plugins/Insights/Controller.php index 73765276bc5..649cb056d38 100644 --- a/plugins/Insights/Controller.php +++ b/plugins/Insights/Controller.php @@ -19,29 +19,32 @@ class Controller extends \Piwik\Plugin\Controller { + public function __construct() + { + $idSite = Common::getRequestVar('idSite', null, 'int'); + + Piwik::checkUserHasViewAccess($idSite); + } + public function getInsightsOverview() { - $view = $this->prepareWidget('insightsOverviewWidget.twig', $apiReport = 'getInsightsOverview'); + $view = $this->prepareWidgetView('insightsOverviewWidget.twig'); + $view->reports = $this->requestApiReport('getInsightsOverview'); return $view->render(); } public function getOverallMoversAndShakers() { - $view = $this->prepareWidget('moversAndShakersOverviewWidget.twig', $apiReport = 'getMoversAndShakersOverview'); + $view = $this->prepareWidgetView('moversAndShakersOverviewWidget.twig'); + $view->reports = $this->requestApiReport('getMoversAndShakersOverview'); return $view->render(); } - private function prepareWidget($template, $apiReport) + private function prepareWidgetView($template) { - $idSite = Common::getRequestVar('idSite', null, 'int'); - $period = Common::getRequestVar('period', null, 'string'); - $date = Common::getRequestVar('date', null, 'string'); - - Piwik::checkUserHasViewAccess(array($idSite)); - - if (!API::getInstance()->canGenerateInsights($period, $date)) { + if (!$this->canGenerateInsights()) { $view = new View('@Insights/cannotDisplayReport.twig'); $this->setBasicVariablesView($view); @@ -51,11 +54,31 @@ private function prepareWidget($template, $apiReport) $view = new View('@Insights/' . $template); $this->setBasicVariablesView($view); - $view->reports = API::getInstance()->$apiReport($idSite, $period, $date); $view->properties = array( - 'order_by' => 'absolute' + 'order_by' => InsightReport::ORDER_BY_ABSOLUTE ); return $view; } + + private function requestApiReport($apiReport) + { + if (!$this->canGenerateInsights()) { + return; + } + + $idSite = Common::getRequestVar('idSite', null, 'int'); + $period = Common::getRequestVar('period', null, 'string'); + $date = Common::getRequestVar('date', null, 'string'); + + return API::getInstance()->$apiReport($idSite, $period, $date); + } + + private function canGenerateInsights() + { + $period = Common::getRequestVar('period', null, 'string'); + $date = Common::getRequestVar('date', null, 'string'); + + return API::getInstance()->canGenerateInsights($date, $period); + } } diff --git a/plugins/Insights/InsightReport.php b/plugins/Insights/InsightReport.php index 3a3cbb9d271..3666a366dba 100644 --- a/plugins/Insights/InsightReport.php +++ b/plugins/Insights/InsightReport.php @@ -12,9 +12,7 @@ use Piwik\Piwik; /** - * API for plugin Insights - * - * @method static \Piwik\Plugins\Insights\API getInstance() + * Insight report generator */ class InsightReport { @@ -41,13 +39,13 @@ public function generateMoverAndShaker($reportMetadata, $period, $date, $lastDat { $totalEvolution = Piwik::getPercentageSafe($totalValue - $lastTotalValue, $lastTotalValue, 1); - $minMoversPercent = 0.5; + $minMoversPercent = 1; if ($totalEvolution >= 100) { // eg change from 50 to 150 = 200% // $evolutionReverse = Piwik::getPercentageSafe($totalValue > $lastTotal ? $lastTotal : $totalValue, $totalValue > $lastTotal ? $totalValue : $lastTotal, 1); - - $minGrowthPercentPositive = $totalEvolution + 40; // min +240% + $factor = (int) ceil($totalEvolution / 500); + $minGrowthPercentPositive = $totalEvolution + ($factor * 40); // min +240% $minGrowthPercentNegative = -70; // min -70% $minDisappearedPercent = 8; // min 12 $minNewPercent = min(($totalEvolution / 100) * 3, 10); // min 6% = min 10 of total visits up to max 10% @@ -66,6 +64,13 @@ public function generateMoverAndShaker($reportMetadata, $period, $date, $lastDat $minNewPercent = 5; } + if ($totalValue < 100) { + // force at least a change of 2 visits + $minMoversPercent = (int) ceil(2 / ($totalValue / 100)); + $minNewPercent = max($minNewPercent, $minMoversPercent); + $minDisappearedPercent = max($minDisappearedPercent, $minMoversPercent); + } + $dataTable = $this->generateInsight($reportMetadata, $period, $date, $lastDate, $metric, $currentReport, $lastReport, $totalValue, $minMoversPercent, $minNewPercent, $minDisappearedPercent, $minGrowthPercentPositive, $minGrowthPercentNegative, $orderBy, $limitIncreaser, $limitDecreaser); $dataTable->setMetadata('lastTotalValue', $lastTotalValue); @@ -75,6 +80,41 @@ public function generateMoverAndShaker($reportMetadata, $period, $date, $lastDat return $dataTable; } + /** + * Extends an already generated insight report by adding a column "isMoverAndShaker" whether a row is also a + * "Mover and Shaker" or not. + * + * Avoids the need to fetch all reports again when we already have the currentReport/lastReport + */ + public function markMoversAndShakers(DataTable $insight, $currentReport, $lastReport, $totalValue, $lastTotalValue) + { + if (!$insight->getRowsCount()) { + return; + } + + $limitIncreaser = max($insight->getRowsCount(), 3); + $limitDecreaser = max($insight->getRowsCount(), 3); + + $lastDate = $insight->getMetadata('lastDate'); + $date = $insight->getMetadata('date'); + $period = $insight->getMetadata('period'); + $metric = $insight->getMetadata('metric'); + $orderBy = $insight->getMetadata('orderBy'); + $reportMetadata = $insight->getMetadata('report'); + + $shakers = $this->generateMoverAndShaker($reportMetadata, $period, $date, $lastDate, $metric, $currentReport, $lastReport, $totalValue, $lastTotalValue, $orderBy, $limitIncreaser, $limitDecreaser); + + foreach ($insight->getRows() as $row) { + $label = $row->getColumn('label'); + + if ($shakers->getRowFromLabel($label)) { + $row->setColumn('isMoverAndShaker', true); + } else { + $row->setColumn('isMoverAndShaker', false); + } + } + } + /** * @param array $reportMetadata * @param string $period @@ -186,6 +226,8 @@ public function generateInsight($reportMetadata, $period, $date, $lastDate, $met 'period' => $period, 'report' => $reportMetadata, 'totalValue' => $totalValue, + 'orderBy' => $orderBy, + 'metric' => $metric, 'minChangeMovers' => $minChangeMovers, 'minIncreaseNew' => $minIncreaseNew, 'minDecreaseDisappeared' => $minDecreaseDisappeared, diff --git a/plugins/Insights/Model.php b/plugins/Insights/Model.php index 380f80b2246..4245a48a74b 100644 --- a/plugins/Insights/Model.php +++ b/plugins/Insights/Model.php @@ -9,10 +9,7 @@ namespace Piwik\Plugins\Insights; use Piwik\DataTable; -use Piwik\Date; -use Piwik\Log; use Piwik\Period\Range; -use Piwik\Piwik; use Piwik\Plugins\API\ProcessedReport; use Piwik\API\Request as ApiRequest; use Piwik\Plugins\VisitsSummary\API as VisitsSummaryAPI; @@ -64,10 +61,16 @@ public function getLastDate($date, $period, $comparedToXPeriods) return $pastDate[0]; } - public function getRelevantTotalValue(DataTable $currentReport, $idSite, $period, $date, $metric) + /** + * Returns either the $totalValue (eg 5500 visits) or the total value of the report + * (eg 2500 visits of 5500 total visits as for instance only 2500 visits came to the website using a search engine). + * + * If the metric total (2500) is much lower than $totalValue, the metric total will be returned, otherwise the + * $totalValue + */ + public function getRelevantTotalValue(DataTable $currentReport, $metric, $totalValue) { $totalMetric = $this->getMetricTotalValue($currentReport, $metric); - $totalValue = $this->getTotalValue($idSite, $period, $date, $metric); if ($totalMetric > $totalValue) { return $totalMetric; diff --git a/plugins/Insights/Visualizations/Insight.php b/plugins/Insights/Visualizations/Insight.php index 6853776ccad..0c24c1f5caf 100644 --- a/plugins/Insights/Visualizations/Insight.php +++ b/plugins/Insights/Visualizations/Insight.php @@ -35,39 +35,49 @@ public function beforeLoadDataTable() return; } - $report = $this->requestConfig->apiMethodToRequestDataTable; - $report = str_replace('.', '_', $report); - if (!$this->requestConfig->filter_limit) { $this->requestConfig->filter_limit = 10; } - $limit = $this->requestConfig->filter_limit; - $limitDecrease = 0; - $limitIncrease = 0; - - if ($this->requestConfig->limit_increaser && !$this->requestConfig->limit_decreaser) { - $limitIncrease = $limit; - } elseif (!$this->requestConfig->limit_increaser && $this->requestConfig->limit_decreaser) { - $limitDecrease = $limit; - } elseif ($this->requestConfig->limit_increaser && $this->requestConfig->limit_decreaser) { - $limitIncrease = round($limit / 2); - $limitDecrease = $limit - $limitIncrease; - } + $report = $this->requestConfig->apiMethodToRequestDataTable; + $report = str_replace('.', '_', $report); $this->requestConfig->apiMethodToRequestDataTable = 'Insights.getInsights'; + $this->requestConfig->request_parameters_to_modify = array( 'reportUniqueId' => $report, 'minImpactPercent' => $this->requestConfig->min_impact_percent, 'minGrowthPercent' => $this->requestConfig->min_growth_percent, 'comparedToXPeriods' => $this->requestConfig->compared_to_x_periods_ago, - 'orderBy' => $this->requestConfig->order_by, + 'orderBy' => $this->requestConfig->order_by, 'filterBy' => $this->requestConfig->filter_by, - 'limitIncreaser' => $limitIncrease, - 'limitDecreaser' => $limitDecrease, + 'limitIncreaser' => $this->getLimitIncrease(), + 'limitDecreaser' => $this->getLimitDecrease(), ); } + private function getLimitIncrease() + { + $filterLimit = $this->requestConfig->filter_limit; + $limitIncrease = 0; + + if ($this->requestConfig->limit_increaser && !$this->requestConfig->limit_decreaser) { + $limitIncrease = $filterLimit; + } elseif ($this->requestConfig->limit_increaser && $this->requestConfig->limit_decreaser) { + $limitIncrease = round($filterLimit / 2); + } + + return $limitIncrease; + } + + private function getLimitDecrease() + { + $filterLimit = $this->requestConfig->filter_limit; + $limitDecrease = $filterLimit - $this->getLimitIncrease(); + + return abs($limitDecrease); + } + public static function getDefaultRequestConfig() { return new Insight\RequestConfig(); @@ -91,23 +101,8 @@ public function beforeRender() return; } - $period = Common::getRequestVar('period', null, 'string'); - $date = Common::getRequestVar('date', null, 'string'); - $idSite = Common::getRequestVar('idSite', null, 'string'); - $segment = Common::getRequestVar('segment', false, 'string'); - + $period = Common::getRequestVar('period', null, 'string'); $this->assignTemplateVar('period', $period); - - $reportId = $this->requestConfig->request_parameters_to_modify['reportUniqueId']; - $comparedToXPeriods = $this->requestConfig->compared_to_x_periods_ago; - $limitIncreaser = min($this->requestConfig->request_parameters_to_modify['limitIncreaser'], 2); - $limitDecreaser = min($this->requestConfig->request_parameters_to_modify['limitDecreaser'], 2); - - $shaker = API::getInstance()->getMoversAndShakers( - $idSite, $period, $date, $reportId, $segment, $comparedToXPeriods, $limitIncreaser, $limitDecreaser - ); - - $this->assignTemplateVar('shaker', $shaker); } public static function canDisplayViewDataTable(ViewDataTable $view) @@ -115,7 +110,7 @@ public static function canDisplayViewDataTable(ViewDataTable $view) $period = Common::getRequestVar('period', null, 'string'); $date = Common::getRequestVar('date', null, 'string'); - $canGenerateInsights = API::getInstance()->canGenerateInsights($period, $date); + $canGenerateInsights = API::getInstance()->canGenerateInsights($date, $period); if (!$canGenerateInsights) { return false; diff --git a/plugins/Insights/Visualizations/Insight/RequestConfig.php b/plugins/Insights/Visualizations/Insight/RequestConfig.php index 591f7d8fac8..43aea4b7662 100644 --- a/plugins/Insights/Visualizations/Insight/RequestConfig.php +++ b/plugins/Insights/Visualizations/Insight/RequestConfig.php @@ -9,6 +9,8 @@ namespace Piwik\Plugins\Insights\Visualizations\Insight; +use Piwik\Plugins\Insights\InsightReport; +use Piwik\Plugins\Insights\Visualizations\Insight; use Piwik\ViewDataTable\RequestConfig as VisualizationRequestConfig; class RequestConfig extends VisualizationRequestConfig @@ -16,10 +18,10 @@ class RequestConfig extends VisualizationRequestConfig public $min_impact_percent = '0.1'; public $min_growth_percent = 20; public $compared_to_x_periods_ago = 1; - public $order_by = 'absolute'; + public $order_by = InsightReport::ORDER_BY_ABSOLUTE; public $filter_by = ''; - public $limit_increaser = '13'; - public $limit_decreaser = '12'; + public $limit_increaser = '5'; + public $limit_decreaser = '5'; public function __construct() { diff --git a/plugins/Insights/templates/insightVisualization.twig b/plugins/Insights/templates/insightVisualization.twig index 8ddba92aeda..196b3909095 100644 --- a/plugins/Insights/templates/insightVisualization.twig +++ b/plugins/Insights/templates/insightVisualization.twig @@ -19,9 +19,7 @@ {% for row in dataTable.getRows %} - {% set isShaker = shaker.getRowFromLabel(row.getColumn('label')) %} - - {% include "@Insights/table_row.twig" with {"isShaker": isShaker} %} + {% include "@Insights/table_row.twig" %} {% endfor %} diff --git a/plugins/Insights/templates/table_row.twig b/plugins/Insights/templates/table_row.twig index 3c8c46576fc..d0ede5ae1d7 100644 --- a/plugins/Insights/templates/table_row.twig +++ b/plugins/Insights/templates/table_row.twig @@ -1,5 +1,5 @@ + {% if row.getColumn('isMoverAndShaker') %}class="isMoverAndShaker"{% endif %}> {{ row.getColumn('label') }} diff --git a/plugins/Insights/tests/ApiTest.php b/plugins/Insights/tests/ApiTest.php index 40a793a8eaa..4aaf4ceae89 100644 --- a/plugins/Insights/tests/ApiTest.php +++ b/plugins/Insights/tests/ApiTest.php @@ -59,6 +59,8 @@ public function test_getInsights_ShouldReturnCorrectMetadata() 'date' => self::$fixture->date1, 'lastDate' => self::$fixture->date2, 'period' => 'day', + 'orderBy' => 'absolute', + 'metric' => 'nb_visits', 'totalValue' => 50, 'minChangeMovers' => 1, 'minIncreaseNew' => 1, @@ -220,6 +222,16 @@ public function test_getInsights_ShouldBeAbleToShowOnlyDisappeared() $this->assertRows($expectedLabels, $insights); } + public function test_canGenerateInsights() + { + $this->assertTrue($this->api->canGenerateInsights('2012-12-12', 'day')); + $this->assertTrue($this->api->canGenerateInsights('2012-12-12', 'week')); + $this->assertTrue($this->api->canGenerateInsights('2012-12-12', 'month')); + + $this->assertFalse($this->api->canGenerateInsights('last10', 'day')); + $this->assertFalse($this->api->canGenerateInsights('2012-11-11,2012-12-12', 'range')); + } + private function requestInsights($requestParams) { $params = array( diff --git a/plugins/Insights/tests/BaseUnitTest.php b/plugins/Insights/tests/BaseUnit.php similarity index 95% rename from plugins/Insights/tests/BaseUnitTest.php rename to plugins/Insights/tests/BaseUnit.php index e3579a19065..a749242dbf6 100644 --- a/plugins/Insights/tests/BaseUnitTest.php +++ b/plugins/Insights/tests/BaseUnit.php @@ -16,7 +16,7 @@ * @group Unit * @group Core */ -class BaseUnitTest extends \PHPUnit_Framework_TestCase +class BaseUnit extends \PHPUnit_Framework_TestCase { /** * @var DataTable diff --git a/plugins/Insights/tests/FilterAverageTest.php b/plugins/Insights/tests/FilterAverageTest.php index 302cd2e2325..930025f32bb 100644 --- a/plugins/Insights/tests/FilterAverageTest.php +++ b/plugins/Insights/tests/FilterAverageTest.php @@ -18,7 +18,7 @@ * @group Unit * @group Core */ -class FilterAverageTest extends BaseUnitTest +class FilterAverageTest extends BaseUnit { public function setUp() { diff --git a/plugins/Insights/tests/FilterExcludeLowValueTest.php b/plugins/Insights/tests/FilterExcludeLowValueTest.php index ef87df895a4..faeaf38ddf6 100644 --- a/plugins/Insights/tests/FilterExcludeLowValueTest.php +++ b/plugins/Insights/tests/FilterExcludeLowValueTest.php @@ -18,7 +18,7 @@ * @group Unit * @group Core */ -class FilterExcludeLowValueTest extends BaseUnitTest +class FilterExcludeLowValueTest extends BaseUnit { public function setUp() { diff --git a/plugins/Insights/tests/FilterInsightTest.php b/plugins/Insights/tests/FilterInsightTest.php index 658a8460151..b0a1c91590a 100644 --- a/plugins/Insights/tests/FilterInsightTest.php +++ b/plugins/Insights/tests/FilterInsightTest.php @@ -18,7 +18,7 @@ * @group Unit * @group Core */ -class FilterInsightTest extends BaseUnitTest +class FilterInsightTest extends BaseUnit { /** * @var DataTable diff --git a/plugins/Insights/tests/FilterLimitTest.php b/plugins/Insights/tests/FilterLimitTest.php index 7eaa208b9c0..67870699f5b 100644 --- a/plugins/Insights/tests/FilterLimitTest.php +++ b/plugins/Insights/tests/FilterLimitTest.php @@ -18,7 +18,7 @@ * @group Unit * @group Core */ -class FilterLimitTest extends BaseUnitTest +class FilterLimitTest extends BaseUnit { public function setUp() { diff --git a/plugins/Insights/tests/FilterMinGrowthTest.php b/plugins/Insights/tests/FilterMinGrowthTest.php index 499311fc114..65db3fe99e4 100644 --- a/plugins/Insights/tests/FilterMinGrowthTest.php +++ b/plugins/Insights/tests/FilterMinGrowthTest.php @@ -18,7 +18,7 @@ * @group Unit * @group Core */ -class FilterMinGrowthTest extends BaseUnitTest +class FilterMinGrowthTest extends BaseUnit { public function setUp() diff --git a/plugins/Insights/tests/FilterOrderByTest.php b/plugins/Insights/tests/FilterOrderByTest.php index 1185b0d583b..55f12ffccfb 100644 --- a/plugins/Insights/tests/FilterOrderByTest.php +++ b/plugins/Insights/tests/FilterOrderByTest.php @@ -18,7 +18,7 @@ * @group Unit * @group Core */ -class FilterOrderByTest extends BaseUnitTest +class FilterOrderByTest extends BaseUnit { public function setUp() { diff --git a/plugins/Insights/tests/InsightReportTest.php b/plugins/Insights/tests/InsightReportTest.php index 1c19a24c0d6..003791f78f7 100644 --- a/plugins/Insights/tests/InsightReportTest.php +++ b/plugins/Insights/tests/InsightReportTest.php @@ -11,6 +11,7 @@ use Piwik\DataTable; use Piwik\DataTable\Row; use Piwik\Plugins\Insights\InsightReport; +use Piwik\Plugins\Insights\Visualizations\Insight; /** * @group Insights @@ -245,9 +246,86 @@ public function provideMinImpactDisappearedData() ); } + public function test_generateInsights_ShouldSetCorrectMetadata() + { + $report = $this->generateInsight(2, 4, 8, 17, -21); + $metadata = $report->getAllTableMetadata(); + + $expectedMetadata = array( + 'reportName' => 'TestReport', + 'metricName' => 'Visits', + 'date' => '2012-12-12', + 'lastDate' => '2012-12-11', + 'period' => 'day', + 'orderBy' => 'absolute', + 'metric' => 'nb_visits', + 'totalValue' => 200, + 'minChangeMovers' => 4, + 'minIncreaseNew' => 8, + 'minDecreaseDisappeared' => 16, + 'minGrowthPercentPositive' => 17, + 'minGrowthPercentNegative' => -21, + 'minMoversPercent' => 2, + 'minNewPercent' => 4, + 'minDisappearedPercent' => 8, + ); + + $this->assertInternalType('array', $metadata['report']); + $this->assertEquals('TestReport', $metadata['report']['name']); + unset($metadata['report']); + unset($metadata['totals']); + + $this->assertEquals($expectedMetadata, $metadata); + } + + public function test_markMoversAndShakers() + { + $report = $this->generateInsight(2, 2, 2, 5, -5); + $this->insightReport->markMoversAndShakers($report, $this->currentTable, $this->pastTable, 160, 100); + + // increase by 60% --> minGrowth 80% + $movers = array('val11', 'val12', 'val7', 'val9', 'val3', 'val10', 'val2', 'val6', 'val107', 'val102'); + $nonMovers = array('val1', 'val8', 'val4'); + + $this->assertMoversAndShakers($report, $movers, $nonMovers); + } + + public function test_generateMoversAndShakers() + { + // increase by 60% --> minGrowth 80% + $report = $this->generateMoverAndShaker(160, 100); + $this->assertOrder($report, array('val11', 'val7', 'val3', 'val10', 'val2', 'val12', 'val6', 'val107', 'val9', 'val102')); + + // increase by 1600% --> minGrowth 1640% + $report = $this->generateMoverAndShaker(1600, 100); + $this->assertOrder($report, array('val11', 'val6', 'val107', 'val9')); + + } + + private function assertMoversAndShakers(DataTable $report, $movers, $nonMovers) + { + foreach ($movers as $mover) { + $row = $report->getRowFromLabel($mover); + if (!$row) { + $this->fail("$mover is not a valid label"); + continue; + } + $this->assertTrue($row->getColumn('isMoverAndShaker'), "$mover is not a mover but should be"); + } + + foreach ($nonMovers as $nonMover) { + $row = $report->getRowFromLabel($nonMover); + if (!$row) { + $this->fail("$nonMover is not a valid label"); + continue; + } + $this->assertFalse($row->getColumn('isMoverAndShaker'), "$nonMover is a mover but should be not"); + } + } + public function test_generateMoversAndShakers_Metadata() { - $report = $this->generateMoverAndShaker(50, 150); + $report = $this->generateMoverAndShaker(150, 50); $metadata = $report->getAllTableMetadata(); $this->assertEquals(50, $metadata['lastTotalValue']); @@ -256,7 +334,7 @@ public function test_generateMoversAndShakers_Metadata() $this->assertEquals(200, $metadata['evolutionTotal']); - $report = $this->generateMoverAndShaker(50, 75); + $report = $this->generateMoverAndShaker(75, 50); $metadata = $report->getAllTableMetadata(); $this->assertEquals(50, $metadata['lastTotalValue']); @@ -265,7 +343,7 @@ public function test_generateMoversAndShakers_Metadata() $this->assertEquals(50, $metadata['evolutionTotal']); - $report = $this->generateMoverAndShaker(50, 25); + $report = $this->generateMoverAndShaker(25, 50); $metadata = $report->getAllTableMetadata(); $this->assertEquals(50, $metadata['lastTotalValue']); @@ -276,37 +354,68 @@ public function test_generateMoversAndShakers_Metadata() public function test_generateMoversAndShakers_ParameterCalculation() { - $report = $this->generateMoverAndShaker(50, 150); + $report = $this->generateMoverAndShaker(3000, 50); // evolution of 5900% + $metadata = $report->getAllTableMetadata(); + + $this->assertEquals(6380, $metadata['minGrowthPercentPositive']); + $this->assertEquals(-70, $metadata['minGrowthPercentNegative']); + $this->assertEquals(1, $metadata['minMoversPercent']); + $this->assertEquals(10, $metadata['minNewPercent']); + $this->assertEquals(8, $metadata['minDisappearedPercent']); + + $report = $this->generateMoverAndShaker(150, 50); $metadata = $report->getAllTableMetadata(); $this->assertEquals(240, $metadata['minGrowthPercentPositive']); $this->assertEquals(-70, $metadata['minGrowthPercentNegative']); - $this->assertEquals(0.5, $metadata['minMoversPercent']); + $this->assertEquals(1, $metadata['minMoversPercent']); $this->assertEquals(6, $metadata['minNewPercent']); $this->assertEquals(8, $metadata['minDisappearedPercent']); - $report = $this->generateMoverAndShaker(50, 75); + $report = $this->generateMoverAndShaker(225, 150); + $metadata = $report->getAllTableMetadata(); + + $this->assertEquals(70, $metadata['minGrowthPercentPositive']); + $this->assertEquals(-70, $metadata['minGrowthPercentNegative']); + $this->assertEquals(1, $metadata['minMoversPercent']); + $this->assertEquals(5, $metadata['minNewPercent']); + $this->assertEquals(7, $metadata['minDisappearedPercent']); + + + $report = $this->generateMoverAndShaker(150, 300); $metadata = $report->getAllTableMetadata(); $this->assertEquals(70, $metadata['minGrowthPercentPositive']); $this->assertEquals(-70, $metadata['minGrowthPercentNegative']); - $this->assertEquals(0.5, $metadata['minMoversPercent']); + $this->assertEquals(1, $metadata['minMoversPercent']); $this->assertEquals(5, $metadata['minNewPercent']); $this->assertEquals(7, $metadata['minDisappearedPercent']); - $report = $this->generateMoverAndShaker(50, 25); + // make sure to force a change of at least 2 visits in all rows if total is soooo low + $report = $this->generateMoverAndShaker(25, 50); + $metadata = $report->getAllTableMetadata(); + + $this->assertEquals(70, $metadata['minGrowthPercentPositive']); + $this->assertEquals(-70, $metadata['minGrowthPercentNegative']); + $this->assertEquals(8, $metadata['minMoversPercent']); + $this->assertEquals(8, $metadata['minNewPercent']); + $this->assertEquals(8, $metadata['minDisappearedPercent']); + + + // make sure to force a change of at least 2 visits + $report = $this->generateMoverAndShaker(75, 150); $metadata = $report->getAllTableMetadata(); $this->assertEquals(70, $metadata['minGrowthPercentPositive']); $this->assertEquals(-70, $metadata['minGrowthPercentNegative']); - $this->assertEquals(0.5, $metadata['minMoversPercent']); + $this->assertEquals(3, $metadata['minMoversPercent']); $this->assertEquals(5, $metadata['minNewPercent']); $this->assertEquals(7, $metadata['minDisappearedPercent']); } - private function generateMoverAndShaker($lastTotalValue, $totalValue, $orderBy = null, $limitIncreaser = 99, $limitDecreaser = 99) + private function generateMoverAndShaker($totalValue, $lastTotalValue, $orderBy = null, $limitIncreaser = 99, $limitDecreaser = 99) { if (is_null($orderBy)) { $orderBy = InsightReport::ORDER_BY_ABSOLUTE; @@ -315,7 +424,7 @@ private function generateMoverAndShaker($lastTotalValue, $totalValue, $orderBy = $reportMetadata = array('name' => 'TestReport', 'metrics' => array('nb_visits' => 'Visits')); $report = $this->insightReport->generateMoverAndShaker( - $reportMetadata, 'day', 'today', 'yesterday', 'nb_visits', $this->currentTable, $this->pastTable, + $reportMetadata, 'day', '2012-12-12', '2012-12-11', 'nb_visits', $this->currentTable, $this->pastTable, $totalValue, $lastTotalValue, $orderBy, $limitIncreaser, $limitDecreaser); return $report; @@ -330,7 +439,7 @@ private function generateInsight($minMoversPercent, $minNewPercent, $minDisappea $reportMetadata = array('name' => 'TestReport', 'metrics' => array('nb_visits' => 'Visits')); $report = $this->insightReport->generateInsight( - $reportMetadata, 'day', 'today', 'yesterday', 'nb_visits', $this->currentTable, $this->pastTable, + $reportMetadata, 'day', '2012-12-12', '2012-12-11', 'nb_visits', $this->currentTable, $this->pastTable, $totalValue = 200, $minMoversPercent, $minNewPercent, $minDisappearedPercent, $minGrowthPercentPositive, $minGrowthPercentNegative, $orderBy, $limitIncreaser, $limitDecreaser); diff --git a/plugins/Insights/tests/ModelTest.php b/plugins/Insights/tests/ModelTest.php index 36bbe180120..19020fc99a0 100644 --- a/plugins/Insights/tests/ModelTest.php +++ b/plugins/Insights/tests/ModelTest.php @@ -120,6 +120,14 @@ public function test_getLastDate_shouldThrowExceptionIfNotPossibleToGetLastDate( $this->model->getLastDate('last10', 'day', 1); } + /** + * @expectedException \Exception + */ + public function test_getLastDate_shouldThrowExceptionInCaseOfRangePeriod() + { + $this->model->getLastDate('2012-11-11,2012-12-12', 'range', 1); + } + public function test_getTotalValue_shouldCalculateTotals() { $total = $this->model->getTotalValue(self::$fixture->idSite, 'day', self::$fixture->date1, 'nb_visits'); @@ -140,25 +148,25 @@ public function test_getTotalValue_shouldReturnZero_IfColumnDoesNotExist() public function test_getRelevantTotalValue_shouldReturnTotalValue_IfMetricTotalIsHighEnough() { $table = $this->getTableWithTotal(25); - $total = $this->model->getRelevantTotalValue($table, self::$fixture->idSite, 'day', self::$fixture->date1, 'nb_visits'); + $total = $this->model->getRelevantTotalValue($table, 'nb_visits', 50); $this->assertEquals(50, $total); } public function test_getRelevantTotalValue_shouldReturnMetricTotal_IfMetricTotalIsHigherThanTotalValue() { $table = $this->getTableWithTotal(80); - $total = $this->model->getRelevantTotalValue($table, self::$fixture->idSite, 'day', self::$fixture->date1, 'nb_visits'); + $total = $this->model->getRelevantTotalValue($table, 'nb_visits', 50); $this->assertEquals(80, $total); } public function test_getRelevantTotalValue_shouldReturnMetricTotal_IfMetricTotalIsTooLow() { $table = $this->getTableWithTotal(24); - $total = $this->model->getRelevantTotalValue($table, self::$fixture->idSite, 'day', self::$fixture->date1, 'nb_visits'); + $total = $this->model->getRelevantTotalValue($table, 'nb_visits', 50); $this->assertEquals(24, $total); $table = $this->getTableWithTotal(0); - $total = $this->model->getRelevantTotalValue($table, self::$fixture->idSite, 'day', self::$fixture->date1, 'nb_visits'); + $total = $this->model->getRelevantTotalValue($table, 'nb_visits', 50); $this->assertEquals(0, $total); }