diff options
author | Kate Butler <kate@innocraft.com> | 2019-05-02 06:54:39 +0300 |
---|---|---|
committer | Thomas Steur <tsteur@users.noreply.github.com> | 2019-05-02 06:54:39 +0300 |
commit | 2b4eaf16b81b166f2af308e7ce822db5c0a00716 (patch) | |
tree | 73b0ef31dd9b9f59ec6543c7079ffc4f082322a2 /plugins/CoreVisualizations | |
parent | 754f61a128ce3d47a1c567b81365028c3107bab5 (diff) |
Bar graph should only have Users and Unique Visitors options when they are valid (#14332)
* Hide the nb_uniq_visitors metric from user when date range doesn't support it
* Refactoring
* Fix logic for choosing columns to display in bar graphs
* Further improvements/bug fixes when column not available in pie or bar graph
* Refactoring
* update custom dimensions submodule
* Prevent valid columns being removed from columns_to_display, add a sprinkling of unit tets
* minor tweak cause label might not always be the first key
Diffstat (limited to 'plugins/CoreVisualizations')
4 files changed, 233 insertions, 19 deletions
diff --git a/plugins/CoreVisualizations/Visualizations/Graph.php b/plugins/CoreVisualizations/Visualizations/Graph.php index 86cfd15787..2efecda529 100644 --- a/plugins/CoreVisualizations/Visualizations/Graph.php +++ b/plugins/CoreVisualizations/Visualizations/Graph.php @@ -117,8 +117,22 @@ abstract class Graph extends Visualization { $this->determineWhichRowsAreSelectable(); + // set default selectable columns, if none specified + $selectableColumns = $this->config->selectable_columns; + if (false === $selectableColumns) { + $this->generateSelectableColumns(); + } + + $this->ensureValidColumnsToDisplay(); + + $this->addTranslations(); + $this->config->selectable_rows = array_values($this->selectableRows); + } + + protected function addTranslations() + { if ($this->config->add_total_row) { $totalTranslation = Piwik::translate('General_Total'); $this->config->selectable_rows[] = array( @@ -135,26 +149,64 @@ abstract class Graph extends Visualization )); } - // set default selectable columns, if none specified - $selectableColumns = $this->config->selectable_columns; - if (false === $selectableColumns) { - $selectableColumns = array('nb_visits', 'nb_actions', 'nb_uniq_visitors', 'nb_users'); - - if ($this->config->show_goals) { - $goalMetrics = array('nb_conversions', 'revenue'); - $selectableColumns = array_merge($selectableColumns, $goalMetrics); - } - } - $transformed = array(); - foreach ($selectableColumns as $column) { + foreach ($this->config->selectable_columns as $column) { $transformed[] = array( 'column' => $column, 'translation' => @$this->config->translations[$column], 'displayed' => in_array($column, $this->config->columns_to_display) ); } - $this->config->selectable_columns = $transformed; } + + protected function generateSelectableColumns() + { + $defaultColumns = array( + 'nb_visits', + 'nb_actions', + 'nb_uniq_visitors', + 'nb_users' + ); + if ($this->config->show_goals) { + $goalMetrics = array('nb_conversions', 'revenue'); + $defaultColumns = array_merge($defaultColumns, $goalMetrics); + } + + // Use the subset of default columns that are actually present in the datatable + $allColumns = $this->getDataTable()->getColumns(); + $selectableColumns = array_intersect($defaultColumns, $allColumns); + + // If there are no default columns, just strip out the 'label' column and use all the others + if (empty($selectableColumns)) { + $selectableColumns = $this->removeLabelFromArray($allColumns); + } + + $this->config->selectable_columns = $selectableColumns; + } + + private function removeLabelFromArray($theArray) + { + if (!empty($theArray) && is_array($theArray)) { + $key = array_search('label', $theArray); + if ($key !== false) { + unset($theArray[$key]); + $theArray = array_values($theArray); + } + } + + return $theArray; + } + + protected function ensureValidColumnsToDisplay() + { + $columnsToDisplay = $this->config->columns_to_display; + + // Remove 'label' from columns to display if present + $columnsToDisplay = $this->removeLabelFromArray($columnsToDisplay); + + // Strip out any columns_to_display that are not in the dataset + $allColumns = $this->getDataTable()->getColumns(); + $this->config->columns_to_display = array_intersect($columnsToDisplay, $allColumns); + } } diff --git a/plugins/CoreVisualizations/Visualizations/JqplotGraph/Bar.php b/plugins/CoreVisualizations/Visualizations/JqplotGraph/Bar.php index a31a88d190..1204ae8a09 100644 --- a/plugins/CoreVisualizations/Visualizations/JqplotGraph/Bar.php +++ b/plugins/CoreVisualizations/Visualizations/JqplotGraph/Bar.php @@ -36,6 +36,16 @@ class Bar extends JqplotGraph return $config; } + protected function ensureValidColumnsToDisplay() + { + parent::ensureValidColumnsToDisplay(); + + $columnsToDisplay = $this->config->columns_to_display; + + // Use a sensible default if the columns_to_display is empty + $this->config->columns_to_display = $columnsToDisplay ? : array('nb_visits'); + } + protected function makeDataGenerator($properties) { return JqplotDataGenerator::factory('bar', $properties); diff --git a/plugins/CoreVisualizations/Visualizations/JqplotGraph/Pie.php b/plugins/CoreVisualizations/Visualizations/JqplotGraph/Pie.php index 3c43bb31a1..42ba69ef3f 100644 --- a/plugins/CoreVisualizations/Visualizations/JqplotGraph/Pie.php +++ b/plugins/CoreVisualizations/Visualizations/JqplotGraph/Pie.php @@ -38,16 +38,16 @@ class Pie extends JqplotGraph $this->config->datatable_js_type = 'JqplotPieGraphDataTable'; } - public function afterAllFiltersAreApplied() + protected function ensureValidColumnsToDisplay() { - parent::afterAllFiltersAreApplied(); + parent::ensureValidColumnsToDisplay(); - $metricColumn = reset($this->config->columns_to_display); + $columnsToDisplay = $this->config->columns_to_display; - if ($metricColumn == 'label') { - $metricColumn = next($this->config->columns_to_display); - } + // Ensure only one column_to_display - it is a pie graph after all! + $metricColumn = reset($columnsToDisplay); + // Set to a sensible default if no suitable value was found $this->config->columns_to_display = array($metricColumn ? : 'nb_visits'); } diff --git a/plugins/CoreVisualizations/tests/Unit/GraphTest.php b/plugins/CoreVisualizations/tests/Unit/GraphTest.php new file mode 100644 index 0000000000..285960ae61 --- /dev/null +++ b/plugins/CoreVisualizations/tests/Unit/GraphTest.php @@ -0,0 +1,152 @@ +<?php +/** + * Matomo - free/libre analytics platform + * + * @link http://matomo.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + */ + +namespace Piwik\Plugins\CoreVisualizations\tests\Unit; + +use Piwik\DataTable; + +/** + * @group CoreVisualizations + * @group SparklinesConfigTest + * @group Sparklines + * @group Plugins + */ +class GraphTest extends \PHPUnit_Framework_TestCase +{ + public function testSelectableColumnsAlreadySet() + { + $bar = $this->getMockGraph(array()); + $bar->config->selectable_columns = array('nb_abc', 'nb_xyz'); + $bar->afterAllFiltersAreApplied(); + + $columns = array_column($bar->config->selectable_columns, 'column'); + $this->assertEquals(array('nb_abc', 'nb_xyz'), $columns); + } + + public function testSelectableColumnsNbVisitsOnlyPresent() + { + $bar = $this->getMockGraph(array( + 'label' => 'abc', + 'nb_visits' => 0, + )); + + $bar->afterAllFiltersAreApplied(); + + $columns = array_column($bar->config->selectable_columns, 'column'); + $this->assertEquals(array('nb_visits'), $columns); + } + + public function testSelectableColumnsUniqueVisitorsPresent() + { + $bar = $this->getMockGraph(array( + 'label' => 'abc', + 'nb_visits' => 5, + 'nb_uniq_visitors' => 2, + 'nb_something_else' => 1 + )); + + $bar->afterAllFiltersAreApplied(); + + $columns = array_column($bar->config->selectable_columns, 'column'); + $this->assertEquals(array('nb_visits', 'nb_uniq_visitors'), $columns); + } + + public function testSelectableColumnsNoDefaultColumnsPresent() + { + $bar = $this->getMockGraph(array( + 'label' => 'abc', + 'foo' => 5, + 'bar' => 2 + )); + + $bar->afterAllFiltersAreApplied(); + + $columns = array_column($bar->config->selectable_columns, 'column'); + $this->assertEquals(array('foo', 'bar'), $columns); + } + + public function testSelectableColumnsNoDefaultColumnsPresentLabelNotPresent() + { + $bar = $this->getMockGraph(array( + 'foo' => 5, + 'bar' => 2 + )); + + $bar->afterAllFiltersAreApplied(); + + $columns = array_column($bar->config->selectable_columns, 'column'); + $this->assertEquals(array('foo', 'bar'), $columns); + } + + public function testSelectableColumnsAlreadySetToEmptyArray() + { + $bar = $this->getMockGraph(array( + 'label' => 'abc', + 'foo' => 5, + 'bar' => 2 + )); + $bar->config->selectable_columns = array(); + + $bar->afterAllFiltersAreApplied(); + + $this->assertEmpty($bar->config->selectable_columns); + } + + public function testColumnsToDisplayNotInDataset() + { + $bar = $this->getMockGraph(array( + 'label' => 'abc', + 'nb_visits' => 25, + 'nb_actions' => 80 + )); + // Not present in the dataset = should be thrown out and replaced with first value from dataset + $bar->config->columns_to_display = array('nb_uniq_visitors'); + + $bar->afterAllFiltersAreApplied(); + + $this->assertEquals(array('nb_visits'), $bar->config->columns_to_display); + } + + public function testColumnsToDisplayNotInSelectableColumns() + { + $bar = $this->getMockGraph(array( + 'label' => 'abc', + 'nb_visits' => 25, + 'nb_actions' => 80 + )); + // columns_to_display is not present in selectable_columbs, but as long as it's in the data set that's OK + $bar->config->selectable_columns = array('nb_visits'); + $bar->config->columns_to_display = array('nb_actions'); + + $bar->afterAllFiltersAreApplied(); + + $this->assertEquals(array('nb_actions'), $bar->config->columns_to_display); + } + + /** + * @return \Piwik\Plugins\CoreVisualizations\Visualizations\JqplotGraph\Bar + */ + private function getMockGraph(array $firstDataRow) + { + $row = new DataTable\Row(array( + DataTable\Row::COLUMNS => $firstDataRow + )); + $dataTable = new DataTable(); + $dataTable->setRows(array($row)); + + $bar = $this->getMock('Piwik\Plugins\CoreVisualizations\Visualizations\JqplotGraph\Bar', + array('getDataTable'), + array('', '') + ); + $bar->expects($this->any()) + ->method('getDataTable') + ->will($this->returnValue($dataTable)); + + return $bar; + } +} |