Skip to content

Commit

Permalink
Refs #3465, make sure labels are associated with correct rows in mult…
Browse files Browse the repository at this point in the history 
…i-row evolution after generic filters (ie, Sort) are applied.
  • Loading branch information
diosmosis committed Apr 16, 2013
1 parent f2f4cd1 commit 7a89c65
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 38 deletions.
25 changes: 10 additions & 15 deletions core/API/DataTableManipulator/LabelFilter.php
View file
Expand Up @@ -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.
Expand All @@ -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);
}

Expand Down Expand Up @@ -137,27 +137,22 @@ private function getLabelVariations($label)
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);
$labelVariation = array_map('urldecode', $labelVariation);

$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;
}
Expand Down
4 changes: 2 additions & 2 deletions core/API/ResponseBuilder.php
View file
Expand Up @@ -291,10 +291,10 @@ protected function handleDataTable($datatable)
$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);
}
Expand Down
68 changes: 47 additions & 21 deletions plugins/API/API.php
View file
Expand Up @@ -1175,6 +1175,15 @@ public function getRowEvolution($idSite, $period, $date, $apiModule, $apiAction,
$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) {
Expand Down Expand Up @@ -1219,7 +1228,7 @@ private function getSingleRowEvolution($dataTable, $idSite, $period, $date, $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) {
Expand Down Expand Up @@ -1306,19 +1315,20 @@ private function loadRowEvolutionDataFromAPI($idSite, $period, $date, $apiModule
$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
Expand All @@ -1330,7 +1340,7 @@ private function loadRowEvolutionDataFromAPI($idSite, $period, $date, $apiModule
$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';
}
Expand Down Expand Up @@ -1466,15 +1476,12 @@ private function getMultiRowEvolution($dataTable, $idSite, $period, $date, $apiM
$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(
Expand All @@ -1500,7 +1507,7 @@ private function getMultiRowEvolution($dataTable, $idSite, $period, $date, $apiM
$newRow = new Piwik_DataTable_Row();

foreach ($labels as $labelIdx => $label) {
$row = $table->getRowFromId($labelIdx);
$row = $this->getRowEvolutionRowFromLabelIdx($table, $labelIdx);

$value = 0;
if ($row) {
Expand Down Expand Up @@ -1547,6 +1554,25 @@ private function getMultiRowEvolution($dataTable, $idSite, $period, $date, $apiM
'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
Expand Down
1 change: 1 addition & 0 deletions ...nit/Integration/expected/test_RowEvolution_processedRowLabel__API.getRowEvolution_day.xml
View file
Expand Up @@ -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>
Expand Down

0 comments on commit 7a89c65

Please sign in to comment.