From 7a89c65378fd7893d5c0b0fdec925b870adb4ed9 Mon Sep 17 00:00:00 2001
From: diosmosis <benaka.moorthi@gmail.com>
Date: Tue, 16 Apr 2013 08:22:33 +0000
Subject: [PATCH] Refs #3465, make sure labels are associated with correct rows
in multi-row evolution after generic filters (ie, Sort) are applied.
---
core/API/DataTableManipulator/LabelFilter.php | 25 +++----
core/API/ResponseBuilder.php | 4 +-
plugins/API/API.php | 68 +++++++++++++------
...essedRowLabel__API.getRowEvolution_day.xml | 1 +
4 files changed, 60 insertions(+), 38 deletions(-)
diff --git a/core/API/DataTableManipulator/LabelFilter.php b/core/API/DataTableManipulator/LabelFilter.php
index 550781bd18..6d5a4aa4b3 100644
--- a/core/API/DataTableManipulator/LabelFilter.php
+++ b/core/API/DataTableManipulator/LabelFilter.php
@@ -25,7 +25,7 @@ class Piwik_API_DataTableManipulator_LabelFilter extends Piwik_API_DataTableMani
const SEPARATOR_RECURSIVE_LABEL = '>';
private $labels;
- private $addEmptyRows;
+ private $addLabelIndex;
/**
* Filter a data table by label.
@@ -37,18 +37,18 @@ class Piwik_API_DataTableManipulator_LabelFilter extends Piwik_API_DataTableMani
*
* @param string $labels the labels to search for
* @param Piwik_DataTable $dataTable the data table to be filtered
- * @param bool $addEmptyRows Whether to add empty rows when a row isn't found
- * for a label, or not.
+ * @param bool $addLabelIndex Whether to add label_index metadata describing which
+ * label a row corresponds to.
* @return Piwik_DataTable
*/
- public function filter($labels, $dataTable, $addEmptyRows = false)
+ public function filter($labels, $dataTable, $addLabelIndex = false)
{
if (!is_array($labels)) {
$labels = array($labels);
}
$this->labels = $labels;
- $this->addEmptyRows = (bool)$addEmptyRows;
+ $this->addLabelIndex = (bool)$addLabelIndex;
return $this->manipulate($dataTable);
}
@@ -137,7 +137,7 @@ class Piwik_API_DataTableManipulator_LabelFilter extends Piwik_API_DataTableMani
protected function manipulateDataTable($dataTable)
{
$result = $dataTable->getEmptyClone();
- foreach ($this->labels as $label) {
+ foreach ($this->labels as $labelIndex => $label) {
$row = null;
foreach ($this->getLabelVariations($label) as $labelVariation) {
$labelVariation = explode(self::SEPARATOR_RECURSIVE_LABEL, $labelVariation);
@@ -145,19 +145,14 @@ class Piwik_API_DataTableManipulator_LabelFilter extends Piwik_API_DataTableMani
$row = $this->doFilterRecursiveDescend($labelVariation, $dataTable);
if ($row) {
+ if ($this->addLabelIndex) {
+ $row->setMetadata('label_index', $labelIndex);
+ }
+
$result->addRow($row);
break;
}
}
-
- if (empty($row)
- && $this->addEmptyRows
- ) // if no row has been found, add an empty one
- {
- $row = new Piwik_DataTable_Row();
- $row->setColumn('label', $label);
- $result->addRow($row);
- }
}
return $result;
}
diff --git a/core/API/ResponseBuilder.php b/core/API/ResponseBuilder.php
index 6830e34c0e..cfd50981f1 100644
--- a/core/API/ResponseBuilder.php
+++ b/core/API/ResponseBuilder.php
@@ -291,10 +291,10 @@ class Piwik_API_ResponseBuilder
$label = $this->getLabelQueryParam();
if (!empty($label)) {
$label = Piwik_Common::unsanitizeInputValues($label);
- $addEmptyRows = Piwik_Common::getRequestVar('labelFilterAddEmptyRows', 0, 'int', $this->request) == 1;
+ $addLabelIndex = Piwik_Common::getRequestVar('labelFilterAddLabelIndex', 0, 'int', $this->request) == 1;
$filter = new Piwik_API_DataTableManipulator_LabelFilter($this->apiModule, $this->apiMethod, $this->request);
- $datatable = $filter->filter($label, $datatable, $addEmptyRows);
+ $datatable = $filter->filter($label, $datatable, $addLabelIndex);
}
return $this->getRenderedDataTable($datatable);
}
diff --git a/plugins/API/API.php b/plugins/API/API.php
index ec60ad2d24..31f5d3274c 100644
--- a/plugins/API/API.php
+++ b/plugins/API/API.php
@@ -1175,6 +1175,15 @@ class Piwik_API_API
$labels = array_merge($labels, $table->getColumn('label'));
}
$labels = array_values(array_unique($labels));
+
+ // set label index metadata
+ $labelsToIndex = array_flip($labels);
+ foreach ($dataTable->getArray() as $table) {
+ foreach ($table->getRows() as $row) {
+ $label = $row->getColumn('label');
+ $row->setMetadata('label_index', $labelsToIndex[$label]);
+ }
+ }
}
if (count($labels) > 1) {
@@ -1219,7 +1228,7 @@ class Piwik_API_API
{
$metadata = $this->getRowEvolutionMetaData($idSite, $period, $date, $apiModule, $apiAction, $language, $idGoal);
$metricNames = array_keys($metadata['metrics']);
-
+
$logo = $actualLabel = false;
$urlFound = false;
foreach ($dataTable->getArray() as $date => $subTable) {
@@ -1306,19 +1315,20 @@ class Piwik_API_API
$label = array_map('rawurlencode', $label);
$parameters = array(
- 'method' => $apiModule . '.' . $apiAction,
- 'label' => $label,
- 'idSite' => $idSite,
- 'period' => $period,
- 'date' => $date,
- 'format' => 'original',
- 'serialize' => '0',
- 'segment' => $segment,
- 'idGoal' => $idGoal,
-
- // if more than one label is used, we add empty rows for labels we can't
- // find to ensure we know the order of the rows in the returned data table
- 'labelFilterAddEmptyRows' => count($label) > 1 ? 1 : 0,
+ 'method' => $apiModule . '.' . $apiAction,
+ 'label' => $label,
+ 'idSite' => $idSite,
+ 'period' => $period,
+ 'date' => $date,
+ 'format' => 'original',
+ 'serialize' => '0',
+ 'segment' => $segment,
+ 'idGoal' => $idGoal,
+
+ // if more than one label is used, we add metadata to ensure we know which
+ // row corresponds with which label (since the labels can change, and rows
+ // can be sorted in a different order)
+ 'labelFilterAddLabelIndex' => count($label) > 1 ? 1 : 0,
);
// add "processed metrics" like actions per visit or bounce rate
@@ -1330,7 +1340,7 @@ class Piwik_API_API
$apiModule != 'Actions'
&&
($apiModule != 'Goals' || ($apiAction != 'getVisitsUntilConversion' && $apiAction != 'getDaysToConversion'))
- && $label // do not request processed metrics when retrieving top n labels
+ && !empty($label)
) {
$parameters['filter_add_columns_when_show_all_columns'] = '1';
}
@@ -1466,15 +1476,12 @@ class Piwik_API_API
$metrics = array_keys($metadata['metrics']);
$column = reset($metrics);
}
-
+
// get the processed label and logo (if any) for every requested label
$actualLabels = $logos = array();
foreach ($labels as $labelIdx => $label) {
foreach ($dataTable->getArray() as $table) {
- // find row for this label. LabelFilter will add empty rows and
- // keep them ordered in the same way the labels array is, so we
- // assume the $labelIdx is also the row Id
- $labelRow = $table->getRowFromId($labelIdx);
+ $labelRow = $this->getRowEvolutionRowFromLabelIdx($table, $labelIdx);
if ($labelRow) {
$actualLabels[$labelIdx] = $this->getRowUrlForEvolutionLabel(
@@ -1500,7 +1507,7 @@ class Piwik_API_API
$newRow = new Piwik_DataTable_Row();
foreach ($labels as $labelIdx => $label) {
- $row = $table->getRowFromId($labelIdx);
+ $row = $this->getRowEvolutionRowFromLabelIdx($table, $labelIdx);
$value = 0;
if ($row) {
@@ -1547,6 +1554,25 @@ class Piwik_API_API
'metadata' => $metadata
);
}
+
+ /**
+ * Returns the row in a datatable by its label_index metadata.
+ *
+ * @param Piwik_DataTable $table
+ * @param int $labelIdx
+ * @return Piwik_DataTable_Row|false
+ */
+ private function getRowEvolutionRowFromLabelIdx($table, $labelIdx)
+ {
+ foreach ($table->getRows() as $row)
+ {
+ if ($row->getMetadata('label_index') == $labelIdx)
+ {
+ return $row;
+ }
+ }
+ return false;
+ }
/**
* Returns a prettier, more comprehensible version of a row evolution label
diff --git a/tests/PHPUnit/Integration/expected/test_RowEvolution_processedRowLabel__API.getRowEvolution_day.xml b/tests/PHPUnit/Integration/expected/test_RowEvolution_processedRowLabel__API.getRowEvolution_day.xml
index a372c32668..7b26cb0421 100644
--- a/tests/PHPUnit/Integration/expected/test_RowEvolution_processedRowLabel__API.getRowEvolution_day.xml
+++ b/tests/PHPUnit/Integration/expected/test_RowEvolution_processedRowLabel__API.getRowEvolution_day.xml
@@ -58,6 +58,7 @@
</nb_visits_1>
<nb_visits_2>
<name>Opera (Visits)</name>
+ <logo>plugins/UserSettings/images/browsers/OP.gif</logo>
<min>0</min>
<max>1</max>
<change>-100%</change>
--
GitLab