Fixing goals UI tests.

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'));
     }

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;
         }

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)

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);
     }

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

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

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

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);
     }

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) {

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;

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>