From 983535f3d749e1b95b3a4b22305c2627902c1de7 Mon Sep 17 00:00:00 2001
From: diosmosis <benaka@piwik.pro>
Date: Mon, 10 Nov 2014 23:24:27 -0800
Subject: [PATCH] Fixing goals UI tests.
---
core/API/DataTablePostProcessor.php | 40 +++++++++----------
core/Plugin/Metric.php | 7 ++--
core/Plugin/Visualization.php | 17 ++++----
.../Metrics/GoalSpecific/ConversionRate.php | 5 ++-
.../Metrics/GoalSpecific/Conversions.php | 5 ++-
.../Goals/Metrics/GoalSpecific/ItemsCount.php | 5 ++-
.../Goals/Metrics/GoalSpecific/Revenue.php | 5 ++-
.../Metrics/GoalSpecific/RevenuePerVisit.php | 7 +++-
plugins/Goals/Metrics/RevenuePerVisit.php | 2 +-
plugins/Goals/Reports/BaseEcommerceItem.php | 9 ++++-
...test_apiGetReportMetadata__API.get_day.xml | 1 +
11 files changed, 62 insertions(+), 41 deletions(-)
diff --git a/core/API/DataTablePostProcessor.php b/core/API/DataTablePostProcessor.php
index 3e6deae6b3..fd761dad15 100644
--- a/core/API/DataTablePostProcessor.php
+++ b/core/API/DataTablePostProcessor.php
@@ -72,21 +72,17 @@ class DataTablePostProcessor
*/
public function process(DataTableInterface $dataTable, $applyFormatting = true)
{
- $label = self::getLabelFromRequest($this->request);
-
$dataTable = $this->applyPivotByFilter($dataTable);
$dataTable = $this->applyFlattener($dataTable);
$dataTable = $this->applyTotalsCalculator($dataTable);
- $dataTable = $this->applyGenericFilters($label, $dataTable);
-
- $this->applyComputeProcessedMetrics($dataTable);
+ $dataTable = $this->applyGenericFilters($dataTable);
// we automatically safe decode all datatable labels (against xss)
$dataTable->queueFilter('SafeDecodeLabel');
$dataTable = $this->applyQueuedFilters($dataTable);
$dataTable = $this->applyRequestedColumnDeletion($dataTable);
- $dataTable = $this->applyLabelFilter($label, $dataTable);
+ $dataTable = $this->applyLabelFilter($dataTable);
$dataTable = $this->applyProcessedMetricsFormatting($dataTable, $applyFormatting);
@@ -97,7 +93,7 @@ class DataTablePostProcessor
* @param DataTableInterface $dataTable
* @return DataTableInterface
*/
- private function applyPivotByFilter(DataTableInterface $dataTable)
+ public function applyPivotByFilter(DataTableInterface $dataTable)
{
$pivotBy = Common::getRequestVar('pivotBy', false, 'string', $this->request);
if (!empty($pivotBy)) {
@@ -117,7 +113,7 @@ class DataTablePostProcessor
* @param DataTableInterface $dataTable
* @return DataTable|DataTableInterface|DataTable\Map
*/
- private function applyFlattener($dataTable)
+ public function applyFlattener($dataTable)
{
if (Common::getRequestVar('flat', '0', 'string', $this->request) == '1') {
$flattener = new Flattener($this->apiModule, $this->apiMethod, $this->request);
@@ -133,7 +129,7 @@ class DataTablePostProcessor
* @param DataTableInterface $dataTable
* @return DataTableInterface
*/
- private function applyTotalsCalculator($dataTable)
+ public function applyTotalsCalculator($dataTable)
{
if (1 == Common::getRequestVar('totals', '1', 'integer', $this->request)) {
$reportTotalsCalculator = new ReportTotalsCalculator($this->apiModule, $this->apiMethod, $this->request);
@@ -143,14 +139,15 @@ class DataTablePostProcessor
}
/**
- * @param string $label
* @param DataTableInterface $dataTable
* @return DataTableInterface
*/
- private function applyGenericFilters($label, $dataTable)
+ public function applyGenericFilters($dataTable)
{
// if the flag disable_generic_filters is defined we skip the generic filters
if (0 == Common::getRequestVar('disable_generic_filters', '0', 'string', $this->request)) {
+ $label = self::getLabelFromRequest($this->request);
+
$genericFilter = new DataTableGenericFilter($this->request);
$self = $this;
@@ -176,7 +173,7 @@ class DataTablePostProcessor
* @param DataTableInterface $dataTable
* @return DataTableInterface
*/
- private function applyQueuedFilters($dataTable)
+ public function applyQueuedFilters($dataTable)
{
// if the flag disable_queued_filters is defined we skip the filters that were queued
if (Common::getRequestVar('disable_queued_filters', 0, 'int', $this->request) == 0) {
@@ -189,7 +186,7 @@ class DataTablePostProcessor
* @param DataTableInterface $dataTable
* @return DataTableInterface
*/
- private function applyRequestedColumnDeletion($dataTable)
+ public function applyRequestedColumnDeletion($dataTable)
{
// use the ColumnDelete filter if hideColumns/showColumns is provided (must be done
// after queued filters are run so processed metrics can be removed, too)
@@ -205,12 +202,13 @@ class DataTablePostProcessor
}
/**
- * @param string $label
* @param DataTableInterface $dataTable
* @return DataTableInterface
*/
- private function applyLabelFilter($label, $dataTable)
+ public function applyLabelFilter($dataTable)
{
+ $label = self::getLabelFromRequest($this->request);
+
// apply label filter: only return rows matching the label parameter (more than one if more than one label)
if (!empty($label)) {
$addLabelIndex = Common::getRequestVar('labelFilterAddLabelIndex', 0, 'int', $this->request) == 1;
@@ -225,7 +223,7 @@ class DataTablePostProcessor
* @param DataTableInterface $dataTable
* @return DataTableInterface
*/
- private function applyProcessedMetricsFormatting($dataTable, $applyFormatting)
+ public function applyProcessedMetricsFormatting($dataTable, $applyFormatting)
{
if ($applyFormatting) {
$dataTable->filter(array($this, 'formatProcessedMetrics'));
@@ -271,13 +269,13 @@ class DataTablePostProcessor
return;
}
- $dataTable->setMetadata(self::PROCESSED_METRICS_COMPUTED_FLAG, true);
-
$processedMetrics = $this->getProcessedMetricsFor($dataTable, $this->report);
if (empty($processedMetrics)) {
return;
}
+ $dataTable->setMetadata(self::PROCESSED_METRICS_COMPUTED_FLAG, true);
+
foreach ($processedMetrics as $name => $processedMetric) {
if (!$processedMetric->beforeCompute($this->report, $dataTable)) {
continue;
@@ -305,13 +303,13 @@ class DataTablePostProcessor
return;
}
- $dataTable->setMetadata(self::PROCESSED_METRICS_FORMATTED_FLAG, true);
-
$processedMetrics = $this->getProcessedMetricsFor($dataTable, $this->report);
if (empty($processedMetrics)) {
return;
}
+ $dataTable->setMetadata(self::PROCESSED_METRICS_FORMATTED_FLAG, true);
+
foreach ($dataTable->getRows() as $row) {
foreach ($processedMetrics as $name => $processedMetric) {
$columnValue = $row->getColumn($name);
@@ -351,7 +349,7 @@ class DataTablePostProcessor
return $result;
}
- private function applyComputeProcessedMetrics(DataTableInterface $dataTable)
+ public function applyComputeProcessedMetrics(DataTableInterface $dataTable)
{
$dataTable->filter(array($this, 'computeProcessedMetrics'));
}
diff --git a/core/Plugin/Metric.php b/core/Plugin/Metric.php
index 06068df7e5..9200c0a607 100644
--- a/core/Plugin/Metric.php
+++ b/core/Plugin/Metric.php
@@ -91,7 +91,7 @@ abstract class Metric
*/
public static function getMetric($row, $columnName, $mappingNameToId = null)
{
- if (empty($mappingIdToName)) {
+ if (empty($mappingNameToId)) {
$mappingNameToId = Metrics::getMappingFromNameToId();
}
@@ -104,10 +104,11 @@ abstract class Metric
}
} else {
$value = @$row[$columnName];
- if ($value === false
+ if ($value === null
&& isset($mappingNameToId[$columnName])
) {
- $value = $row[$mappingNameToId[$columnName]];
+ $columnName = $mappingNameToId[$columnName];
+ $value = @$row[$columnName];
}
return $value;
}
diff --git a/core/Plugin/Visualization.php b/core/Plugin/Visualization.php
index e7e79c62e2..cb92727d55 100644
--- a/core/Plugin/Visualization.php
+++ b/core/Plugin/Visualization.php
@@ -9,6 +9,7 @@
namespace Piwik\Plugin;
+use Piwik\API\DataTablePostProcessor;
use Piwik\Common;
use Piwik\DataTable;
use Piwik\Date;
@@ -163,7 +164,7 @@ class Visualization extends ViewDataTable
$this->beforeLoadDataTable();
- $this->loadDataTableFromAPI(array('disable_generic_filters' => 1));
+ $this->loadDataTableFromAPI(array('disable_generic_filters' => 1, 'disable_queued_filters' => 1));
$this->postDataTableLoadedFromAPI();
$requestPropertiesAfterLoadDataTable = $this->requestConfig->getProperties();
@@ -308,6 +309,8 @@ class Visualization extends ViewDataTable
private function applyFilters()
{
+ $postProcessor = $this->makeDataTablePostProcessor();
+
list($priorityFilters, $otherFilters) = $this->config->getFiltersToRun();
// First, filters that delete rows
@@ -323,9 +326,11 @@ class Visualization extends ViewDataTable
}
if (!$this->requestConfig->areGenericFiltersDisabled()) {
- $this->applyGenericFilters();
+ $this->dataTable = $postProcessor->applyGenericFilters($this->dataTable);
}
+ $postProcessor->applyComputeProcessedMetrics($this->dataTable);
+
$this->afterGenericFiltersAreAppliedToLoadedDataTable();
// queue other filters so they can be applied later if queued filters are disabled
@@ -567,10 +572,7 @@ class Visualization extends ViewDataTable
// eg $this->config->showFooterColumns = true;
}
- /**
- * Second, generic filters (Sort, Limit, Replace Column Names, etc.)
- */
- private function applyGenericFilters()
+ private function makeDataTablePostProcessor()
{
$requestArray = $this->request->getRequestArray();
$request = \Piwik\API\Request::getRequestArrayFromString($requestArray);
@@ -580,8 +582,7 @@ class Visualization extends ViewDataTable
$request['filter_sort_order'] = '';
}
- $genericFilter = new \Piwik\API\DataTableGenericFilter($request);
- $genericFilter->filter($this->dataTable);
+ return new DataTablePostProcessor($this->requestConfig->getApiModuleToRequest(), $this->requestConfig->getApiMethodToRequest(), $request);
}
private function logMessageIfRequestPropertiesHaveChanged(array $requestPropertiesBefore)
diff --git a/plugins/Goals/Metrics/GoalSpecific/ConversionRate.php b/plugins/Goals/Metrics/GoalSpecific/ConversionRate.php
index 93ce90c3da..48e3a4a4c1 100644
--- a/plugins/Goals/Metrics/GoalSpecific/ConversionRate.php
+++ b/plugins/Goals/Metrics/GoalSpecific/ConversionRate.php
@@ -8,6 +8,7 @@
namespace Piwik\Plugins\Goals\Metrics\GoalSpecific;
use Piwik\DataTable\Row;
+use Piwik\Metrics;
use Piwik\Piwik;
use Piwik\Plugins\Goals\Metrics\GoalSpecificProcessedMetric;
use Piwik\Tracker\GoalManager;
@@ -44,10 +45,12 @@ class ConversionRate extends GoalSpecificProcessedMetric
public function compute(Row $row)
{
+ $mappingFromNameToIdGoal = Metrics::getMappingFromNameToIdGoal();
+
$goalMetrics = $this->getGoalMetrics($row);
$nbVisits = $this->getMetric($row, 'nb_visits');
- $conversions = $this->getMetric($goalMetrics, 'nb_conversions');
+ $conversions = $this->getMetric($goalMetrics, 'nb_conversions', $mappingFromNameToIdGoal);
return Piwik::getQuotientSafe($conversions, $nbVisits, GoalManager::REVENUE_PRECISION + 2);
}
diff --git a/plugins/Goals/Metrics/GoalSpecific/Conversions.php b/plugins/Goals/Metrics/GoalSpecific/Conversions.php
index eb8c09e3f0..a9545cf3c9 100644
--- a/plugins/Goals/Metrics/GoalSpecific/Conversions.php
+++ b/plugins/Goals/Metrics/GoalSpecific/Conversions.php
@@ -8,6 +8,7 @@
namespace Piwik\Plugins\Goals\Metrics\GoalSpecific;
use Piwik\DataTable\Row;
+use Piwik\Metrics;
use Piwik\Plugins\Goals\Metrics\GoalSpecificProcessedMetric;
/**
@@ -33,7 +34,9 @@ class Conversions extends GoalSpecificProcessedMetric
public function compute(Row $row)
{
+ $mappingFromNameToIdGoal = Metrics::getMappingFromNameToIdGoal();
+
$goalMetrics = $this->getGoalMetrics($row);
- return (int) $this->getMetric($goalMetrics, 'nb_conversions');
+ return (int) $this->getMetric($goalMetrics, 'nb_conversions', $mappingFromNameToIdGoal);
}
}
\ No newline at end of file
diff --git a/plugins/Goals/Metrics/GoalSpecific/ItemsCount.php b/plugins/Goals/Metrics/GoalSpecific/ItemsCount.php
index 0bafcd6e43..c6d93497b3 100644
--- a/plugins/Goals/Metrics/GoalSpecific/ItemsCount.php
+++ b/plugins/Goals/Metrics/GoalSpecific/ItemsCount.php
@@ -8,6 +8,7 @@
namespace Piwik\Plugins\Goals\Metrics\GoalSpecific;
use Piwik\DataTable\Row;
+use Piwik\Metrics;
use Piwik\Plugins\Goals\Metrics\GoalSpecificProcessedMetric;
/**
@@ -33,7 +34,9 @@ class ItemsCount extends GoalSpecificProcessedMetric
public function compute(Row $row)
{
+ $mappingFromNameToIdGoal = Metrics::getMappingFromNameToIdGoal();
+
$goalMetrics = $this->getGoalMetrics($row);
- return (int) $this->getMetric($goalMetrics, 'items');
+ return (int) $this->getMetric($goalMetrics, 'items', $mappingFromNameToIdGoal);
}
}
\ No newline at end of file
diff --git a/plugins/Goals/Metrics/GoalSpecific/Revenue.php b/plugins/Goals/Metrics/GoalSpecific/Revenue.php
index e3089c20f6..876a53494c 100644
--- a/plugins/Goals/Metrics/GoalSpecific/Revenue.php
+++ b/plugins/Goals/Metrics/GoalSpecific/Revenue.php
@@ -8,6 +8,7 @@
namespace Piwik\Plugins\Goals\Metrics\GoalSpecific;
use Piwik\DataTable\Row;
+use Piwik\Metrics;
use Piwik\Plugins\Goals\Metrics\GoalSpecificProcessedMetric;
/**
@@ -32,7 +33,9 @@ class Revenue extends GoalSpecificProcessedMetric
public function compute(Row $row)
{
+ $mappingFromNameToIdGoal = Metrics::getMappingFromNameToIdGoal();
+
$goalMetrics = $this->getGoalMetrics($row);
- return (float) $this->getMetric($goalMetrics, 'revenue');
+ return (float) $this->getMetric($goalMetrics, 'revenue', $mappingFromNameToIdGoal);
}
}
\ No newline at end of file
diff --git a/plugins/Goals/Metrics/GoalSpecific/RevenuePerVisit.php b/plugins/Goals/Metrics/GoalSpecific/RevenuePerVisit.php
index 338ab01748..814649b007 100644
--- a/plugins/Goals/Metrics/GoalSpecific/RevenuePerVisit.php
+++ b/plugins/Goals/Metrics/GoalSpecific/RevenuePerVisit.php
@@ -8,6 +8,7 @@
namespace Piwik\Plugins\Goals\Metrics\GoalSpecific;
use Piwik\DataTable\Row;
+use Piwik\Metrics;
use Piwik\Piwik;
use Piwik\Plugins\Goals\Metrics\GoalSpecificProcessedMetric;
use Piwik\Tracker\GoalManager;
@@ -38,12 +39,14 @@ class RevenuePerVisit extends GoalSpecificProcessedMetric
public function compute(Row $row)
{
+ $mappingFromNameToIdGoal = Metrics::getMappingFromNameToIdGoal();
+
$goalMetrics = $this->getGoalMetrics($row);
$nbVisits = $this->getMetric($row, 'nb_visits');
- $conversions = $this->getMetric($goalMetrics, 'nb_conversions');
+ $conversions = $this->getMetric($goalMetrics, 'nb_conversions', $mappingFromNameToIdGoal);
- $goalRevenue = (float) $this->getMetric($goalMetrics, 'revenue');
+ $goalRevenue = (float) $this->getMetric($goalMetrics, 'revenue', $mappingFromNameToIdGoal);
return Piwik::getQuotientSafe($goalRevenue, $nbVisits == 0 ? $conversions : $nbVisits, GoalManager::REVENUE_PRECISION);
}
diff --git a/plugins/Goals/Metrics/RevenuePerVisit.php b/plugins/Goals/Metrics/RevenuePerVisit.php
index a35402cd84..3e2367fed9 100644
--- a/plugins/Goals/Metrics/RevenuePerVisit.php
+++ b/plugins/Goals/Metrics/RevenuePerVisit.php
@@ -40,7 +40,7 @@ class RevenuePerVisit extends ProcessedMetric
public function compute(Row $row)
{
$mappingFromNameToIdGoal = Metrics::getMappingFromNameToIdGoal();
- $goals = $this->getMetric($row, 'goals');
+ $goals = $this->getMetric($row, 'goals') ?: array();
$revenue = 0;
foreach ($goals as $goalId => $goalMetrics) {
diff --git a/plugins/Goals/Reports/BaseEcommerceItem.php b/plugins/Goals/Reports/BaseEcommerceItem.php
index 5181d6fcc3..ca9ab605c7 100644
--- a/plugins/Goals/Reports/BaseEcommerceItem.php
+++ b/plugins/Goals/Reports/BaseEcommerceItem.php
@@ -80,7 +80,9 @@ abstract class BaseEcommerceItem extends BaseEcommerce
// set columns/translations which differ based on viewDataTable TODO: shouldn't have to do this check...
// amount of reports should be dynamic, but metadata should be static
- $columns = $this->getMetrics();
+ $columns = array_merge($this->getMetrics(), $this->getProcessedMetrics());
+ $columnsOrdered = array('label', 'revenue', 'quantity', 'orders', 'avg_price', 'avg_quantity',
+ 'nb_visits', 'conversion_rate');
$abandonedCart = $this->isAbandonedCart();
if ($abandonedCart) {
@@ -92,12 +94,15 @@ abstract class BaseEcommerceItem extends BaseEcommerce
unset($columns['conversion_rate']);
$view->requestConfig->request_parameters_to_modify['abandonedCarts'] = '1';
+
+ $columnsOrdered = array('label', 'revenue', 'quantity', 'avg_price', 'avg_quantity', 'nb_visits',
+ 'abandoned_carts');
}
$translations = array_merge(array('label' => $this->name), $columns);
$view->config->addTranslations($translations);
- $view->config->columns_to_display = array_keys($translations);
+ $view->config->columns_to_display = $columnsOrdered;
$view->config->custom_parameters['viewDataTable'] =
$abandonedCart ? Piwik::LABEL_ID_GOAL_IS_ECOMMERCE_CART : Piwik::LABEL_ID_GOAL_IS_ECOMMERCE_ORDER;
diff --git a/tests/PHPUnit/System/expected/test_apiGetReportMetadata__API.get_day.xml b/tests/PHPUnit/System/expected/test_apiGetReportMetadata__API.get_day.xml
index 59ad705757..06d17abbd9 100644
--- a/tests/PHPUnit/System/expected/test_apiGetReportMetadata__API.get_day.xml
+++ b/tests/PHPUnit/System/expected/test_apiGetReportMetadata__API.get_day.xml
@@ -14,6 +14,7 @@
<nb_users_returning>0</nb_users_returning>
<nb_visits_converted_returning>0</nb_visits_converted_returning>
<max_actions_returning>0</max_actions_returning>
+ <bounce_count_returning>0</bounce_count_returning>
<bounce_rate_returning>0%</bounce_rate_returning>
<nb_actions_per_visit_returning>0</nb_actions_per_visit_returning>
<avg_time_on_site_returning>0</avg_time_on_site_returning>
--
GitLab