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 @@