From b1205a5db1a884e2e02a8d6845cd2566d05cc355 Mon Sep 17 00:00:00 2001 From: Benaka Moorthi Date: Sat, 14 Sep 2013 15:31:40 -0400 Subject: [PATCH] Refs #4041, force DataTable visualizations to specify which properties should be overridable and move some view properties to different visualizations. --- core/DataTableVisualization.php | 70 ++++++++------- core/ViewDataTable.php | 72 +++++++++------- core/ViewDataTable/Properties.php | 85 ++++++++++++++----- core/Visualization/Graph.php | 5 ++ .../Actions/javascripts/actionsDataTable.js | 2 +- .../Visualizations/Cloud.php | 16 +++- .../Visualizations/HtmlTable.php | 27 ++++++ .../Visualizations/HtmlTable/AllColumns.php | 2 +- .../Visualizations/HtmlTable/Goals.php | 2 +- .../Visualizations/JqplotGraph.php | 2 + .../Visualizations/JqplotGraph/Evolution.php | 2 + plugins/ExampleUI/Controller.php | 6 +- tests/PHPUnit/UI | 2 +- 13 files changed, 203 insertions(+), 90 deletions(-) diff --git a/core/DataTableVisualization.php b/core/DataTableVisualization.php index 082a072315d..04760b04625 100644 --- a/core/DataTableVisualization.php +++ b/core/DataTableVisualization.php @@ -55,26 +55,14 @@ public static function getDefaultPropertyValues() * in every AJAX request. * * Derived DataTableVisualizations can specify client side parameters by declaring - * a static $clientSideParameters field. + * a static $clientSideParameters field that contains a list of view property + * names. * * @return array */ public static function getClientSideParameters() { - if (isset(static::$clientSideParameters)) { - $result = array(); - - $lineage = static::getVisualizationClassLineage(get_called_class()); - foreach ($lineage as $klass) { - if (isset($klass::$clientSideParameters)) { - $result = array_merge($result, $klass::$clientSideParameters); - } - } - - return array_unique($result); - } else { - return array(); - } + return self::getPropertyNameListWithMetaProperty('clientSideParameters'); } /** @@ -83,26 +71,28 @@ public static function getClientSideParameters() * these will not be passed with AJAX requests as query parameters. * * Derived DataTableVisualizations can specify client side properties by declaring - * a static $clientSideProperties field. + * a static $clientSideProperties field that contains a list of view property + * names. * * @return array */ public static function getClientSideProperties() { - if (isset(static::$clientSideProperties)) { - $result = array(); + return self::getPropertyNameListWithMetaProperty('clientSideProperties'); + } - $lineage = static::getVisualizationClassLineage(get_called_class()); - foreach ($lineage as $klass) { - if (isset($klass::$clientSideProperties)) { - $result = array_merge($result, $klass::$clientSideProperties); - } - } - - return array_unique($result); - } else { - return array(); - } + /** + * Returns an array of view property names that can be overriden by query parameters. + * If a query parameter is sent with the same name as a view property, the view + * property will be set to the value of the query parameter. + * + * Derived DataTableVisualizations can specify overridable properties by declaring + * a static $overridableProperties field that contains a list of view property + * names. + */ + public static function getOverridableProperties() + { + return self::getPropertyNameListWithMetaProperty('overridableProperties'); } /** @@ -240,4 +230,26 @@ public static function getClassFromId($id) } return $visualizationClasses[$id]; } + + /** + * Helper function that merges the static field values of every class in this + * classes inheritance hierarchy. Uses late-static binding. + */ + private static function getPropertyNameListWithMetaProperty($staticFieldName) + { + if (isset(static::$$staticFieldName)) { + $result = array(); + + $lineage = static::getVisualizationClassLineage(get_called_class()); + foreach ($lineage as $klass) { + if (isset($klass::$$staticFieldName)) { + $result = array_merge($result, $klass::$$staticFieldName); + } + } + + return array_unique($result); + } else { + return array(); + } + } } \ No newline at end of file diff --git a/core/ViewDataTable.php b/core/ViewDataTable.php index 5fdd36c32a4..8f2b0a88c80 100644 --- a/core/ViewDataTable.php +++ b/core/ViewDataTable.php @@ -145,7 +145,6 @@ public function __construct($currentControllerAction, $this->setDefaultProperties(); $this->setViewProperties($viewProperties); - $this->overrideViewPropertiesWithQueryParams(); $this->idSubtable = Common::getRequestVar('idSubtable', false, 'int'); $this->viewProperties['show_footer_icons'] = ($this->idSubtable == false); @@ -154,6 +153,8 @@ public function __construct($currentControllerAction, $this->viewProperties['report_id'] = $currentControllerName . '.' . $currentControllerAction; $this->viewProperties['self_url'] = $this->getBaseReportUrl($currentControllerName, $currentControllerAction); + $this->overrideViewPropertiesWithQueryParams(); + // the exclude low population threshold value is sometimes obtained by requesting data. // to avoid issuing unecessary requests when display properties are determined by metadata, // we allow it to be a closure. @@ -282,14 +283,7 @@ static public function factory($defaultType = null, $apiAction = false, $control */ public function getClientSideProperties() { - $result = array(); - - if ($this->visualizationClass) { - $klass = $this->visualizationClass; - $result = array_merge($result, $klass::getClientSideProperties()); - } - - return $result; + return $this->getPropertyNameListWithMetaProperty(Properties::$clientSideProperties, __FUNCTION__); } /** @@ -300,19 +294,17 @@ public function getClientSideProperties() */ public function getClientSideParameters() { - $result = array( - 'filter_excludelowpop', - 'filter_excludelowpop_value', - 'filter_pattern', - 'filter_column', - ); - - if ($this->visualizationClass) { - $klass = $this->visualizationClass; - $result = array_merge($result, $klass::getClientSideParameters()); - } + return $this->getPropertyNameListWithMetaProperty(Properties::$clientSideParameters, __FUNCTION__); + } - return $result; + /** + * Returns the list of view properties that can be overriden by query parameters. + * + * @return array + */ + public function getOverridableProperties() + { + return $this->getPropertyNameListWithMetaProperty(Properties::$overridableProperties, __FUNCTION__); } public function getCurrentControllerAction() @@ -1244,20 +1236,36 @@ private function convertForJson($value) private function overrideViewPropertiesWithQueryParams() { - // TODO: should mark properties that are overridable so not all properties can be overidden this way - $queryParams = $_GET + $_POST; - foreach ($queryParams as $name => $value) { - if (empty($value)) { - continue; - } - - $value = Common::getRequestVar($name, $default = null, $type = null, $queryParams); - + $properties = $this->getOverridableProperties(); + foreach ($properties as $name) { if (Properties::isCoreViewProperty($name)) { - $this->viewProperties[$name] = $value; + $default = $this->viewProperties[$name]; + + $this->viewProperties[$name] = $this->getPropertyFromQueryParam($name, $default); } else if (Properties::isValidVisualizationProperty($this->visualizationClass, $name)) { - $this->viewProperties['visualization_properties']->$name = $value; + $default = $this->viewProperties['visualization_properties']->$name; + + $this->viewProperties['visualization_properties']->$name = + $this->getPropertyFromQueryParam($name, $default); } } } + + private function getPropertyFromQueryParam($name, $defaultValue) + { + $type = is_numeric($defaultValue) ? 'int' : null; + return Common::getRequestVar($name, $defaultValue, $type); + } + + /** + * Helper function for getCliendSiteProperties/getClientSideParameters/etc. + */ + private function getPropertyNameListWithMetaProperty($propertyNames, $getPropertiesFunctionName) + { + if ($this->visualizationClass) { + $klass = $this->visualizationClass; + $propertyNames = array_merge($propertyNames, $klass::$getPropertiesFunctionName()); + } + return $propertyNames; + } } \ No newline at end of file diff --git a/core/ViewDataTable/Properties.php b/core/ViewDataTable/Properties.php index 5c5d0630fa3..884bbf1ba15 100644 --- a/core/ViewDataTable/Properties.php +++ b/core/ViewDataTable/Properties.php @@ -113,12 +113,6 @@ class Properties */ const COLUMNS_TO_DISPLAY = 'columns_to_display'; - /** - * Whether to display the logo assocatied with a DataTable row (stored as 'logo' row metadata) - * isntead of the label in Tag Clouds. - */ - const DISPLAY_LOGO_INSTEAD_OF_LABEL = 'display_logo_instead_of_label'; - /** * Controls whether the footer icons that change the ViewDataTable type of a view are shown * or not. @@ -244,13 +238,6 @@ class Properties */ const SHOW_ECOMMERCE_FOOTER_ICONS = 'show_ecommerce'; - /** - * Controls whether the summary row is displayed on every page of the datatable view or not. - * If false, the summary row will be treated as the last row of the dataset and will only visible - * when viewing the last rows. - */ - const KEEP_SUMMARY_ROW = 'keep_summary_row'; - /** * Stores the column name to filter when filtering out rows with low values. * @@ -279,13 +266,6 @@ class Properties */ const METRIC_DOCUMENTATION = 'metrics_documentation'; - /** - * If true, the summary row will be colored differently than all other DataTable rows. - * - * @see also self::KEEP_SUMMARY_ROW - */ - const HIGHLIGHT_SUMMARY_ROW = 'highlight_summary_row'; - /** * Row metadata name that contains the tooltip for the specific row. */ @@ -399,6 +379,67 @@ class Properties */ const SHOW_NON_CORE_VISUALIZATIONS = 'show_non_core_visualizations'; + /** + * The list of ViewDataTable properties that are 'Client Side Parameters'. + * + * @see Piwik\DataTableVisualization::getClientSideParameters + */ + public static $clientSideParameters = array( + 'filter_excludelowpop', + 'filter_excludelowpop_value', + 'filter_pattern', + 'filter_column', + ); + + /** + * The list of ViewDataTable properties that are 'Client Side Properties'. + * + * @see Piwik\DataTableVisualization::getClientSideProperties + */ + public static $clientSideProperties = array(); + + /** + * The list of ViewDataTable properties that can be overriden by query parameters. + * + * @see Piwik\DataTableVisualization::getOverridableProperties + */ + public static $overridableProperties = array( + 'show_goals', + 'filter_sort_column', + 'filter_sort_order', + 'filter_limit', + 'filter_offset', + 'filter_pattern', + 'filter_column', + 'disable_generic_filters', + 'disable_queued_filters', + 'show_exclude_low_population', + 'show_table', + 'show_table_all_columns', + 'show_footer', + 'show_footer_icons', + 'show_all_views_icons', + 'show_active_view_icon', + 'show_related_reports', + 'show_limit_control', + 'show_search', + 'enable_sort', + 'show_bar_chart', + 'show_pie_chart', + 'show_tag_cloud', + 'show_export_as_rss_feed', + 'show_ecommerce', + 'filter_excludelowpop', + 'filter_excludelowpop_value', + 'search_recursive', + 'show_export_as_image_icon', + 'show_pagination_control', + 'show_offset_information', + 'hide_annotations_view', + 'export_limit', + 'show_non_core_visualizations' + ); + /** * Returns the set of all valid ViewDataTable properties. The result is an array with property * names as keys. Values of the array are undefined. @@ -521,17 +562,17 @@ public static function getDefaultPropertyValues() 'show_pagination_control' => true, 'show_limit_control' => false, 'show_footer' => true, + 'show_footer_icons' => true, 'show_related_reports' => true, 'show_non_core_visualizations' => true, 'export_limit' => Config::getInstance()->General['API_datatable_default_limit'], - 'highlight_summary_row' => false, 'related_reports' => array(), 'title' => '', 'tooltip_metadata_name' => false, 'enable_sort' => true, 'disable_generic_filters' => false, 'disable_queued_filters' => false, - 'keep_summary_row' => false, + 'search_recursive' => false, 'filter_excludelowpop' => false, 'filter_excludelowpop_value' => false, 'filter_pattern' => false, diff --git a/core/Visualization/Graph.php b/core/Visualization/Graph.php index fb06484bf18..adc3cebbf10 100644 --- a/core/Visualization/Graph.php +++ b/core/Visualization/Graph.php @@ -100,6 +100,11 @@ abstract class Graph extends DataTableVisualization 'columns' ); + public static $overridableProperties = array( + 'show_all_ticks', + 'show_series_picker' + ); + /** * Constructor. * diff --git a/plugins/Actions/javascripts/actionsDataTable.js b/plugins/Actions/javascripts/actionsDataTable.js index 74fcb9d095f..eb9d4dcc769 100644 --- a/plugins/Actions/javascripts/actionsDataTable.js +++ b/plugins/Actions/javascripts/actionsDataTable.js @@ -34,7 +34,7 @@ } /** - * TODO + * UI control that handles extra functionality for Actions datatables. * * @constructor */ diff --git a/plugins/CoreVisualizations/Visualizations/Cloud.php b/plugins/CoreVisualizations/Visualizations/Cloud.php index 31f0f291efb..d2a45349bcd 100644 --- a/plugins/CoreVisualizations/Visualizations/Cloud.php +++ b/plugins/CoreVisualizations/Visualizations/Cloud.php @@ -28,9 +28,17 @@ class Cloud extends DataTableVisualization { const ID = 'cloud'; + /** + * Whether to display the logo assocatied with a DataTable row (stored as 'logo' row metadata) + * instead of the label in Tag Clouds. + */ + const DISPLAY_LOGO_INSTEAD_OF_LABEL = 'display_logo_instead_of_label'; + /** Used by integration tests to make sure output is consistent. */ public static $debugDisableShuffle = false; + public static $overridableProperties = array('display_logo_instead_of_label'); + protected $wordsArray = array(); public $truncatingLimit = 50; @@ -39,7 +47,11 @@ public static function getDefaultPropertyValues() return array( 'show_offset_information' => false, 'show_exclude_low_population' => false, - 'display_logo_instead_of_label' => false, + 'visualization_properties' => array( + 'cloud' => array( + 'display_logo_instead_of_label' => false, + ) + ) ); } @@ -75,7 +87,7 @@ public function render($dataTable, $properties) $labelMetadata = array(); foreach ($dataTable->getRows() as $row) { $logo = false; - if ($properties['display_logo_instead_of_label']) { + if ($properties['visualization_properties']->display_logo_instead_of_label) { $logo = $row->getMetadata('logo'); } diff --git a/plugins/CoreVisualizations/Visualizations/HtmlTable.php b/plugins/CoreVisualizations/Visualizations/HtmlTable.php index d5e1f32775a..0dd35715a36 100644 --- a/plugins/CoreVisualizations/Visualizations/HtmlTable.php +++ b/plugins/CoreVisualizations/Visualizations/HtmlTable.php @@ -86,6 +86,20 @@ class HtmlTable extends DataTableVisualization */ const DISABLE_SUBTABLE_IN_GOALS_VIEW = 'disable_subtable_when_show_goals'; + /** + * Controls whether the summary row is displayed on every page of the datatable view or not. + * If false, the summary row will be treated as the last row of the dataset and will only visible + * when viewing the last rows. + */ + const KEEP_SUMMARY_ROW = 'keep_summary_row'; + + /** + * If true, the summary row will be colored differently than all other DataTable rows. + * + * @see also self::KEEP_SUMMARY_ROW + */ + const HIGHLIGHT_SUMMARY_ROW = 'highlight_summary_row'; + static public $clientSideParameters = array( 'search_recursive', 'filter_limit', @@ -104,6 +118,17 @@ class HtmlTable extends DataTableVisualization 'subtable_controller_action', ); + public static $overridableProperties = array( + 'show_expanded', + 'disable_row_actions', + 'disable_row_evolution', + 'show_extra_columns', + 'show_goals_columns', + 'disable_subtable_when_show_goals', + 'keep_summary_row', + 'highlight_summary_row', + ); + /** * Constructor. */ @@ -153,6 +178,8 @@ public static function getDefaultPropertyValues() 'show_extra_columns' => false, 'show_goals_columns' => false, 'disable_subtable_when_show_goals' => false, + 'keep_summary_row' => false, + 'highlight_summary_row' => false, ), ), ); diff --git a/plugins/CoreVisualizations/Visualizations/HtmlTable/AllColumns.php b/plugins/CoreVisualizations/Visualizations/HtmlTable/AllColumns.php index 7f51275c609..02eee0b12df 100644 --- a/plugins/CoreVisualizations/Visualizations/HtmlTable/AllColumns.php +++ b/plugins/CoreVisualizations/Visualizations/HtmlTable/AllColumns.php @@ -14,7 +14,7 @@ use Piwik\Plugins\CoreVisualizations\Visualizations\HtmlTable; /** - * TODO + * DataTableVisualization that derives from HtmlTable and sets show_extra_columns to true. */ class AllColumns extends HtmlTable { diff --git a/plugins/CoreVisualizations/Visualizations/HtmlTable/Goals.php b/plugins/CoreVisualizations/Visualizations/HtmlTable/Goals.php index e2823fb84df..d42ea06ef26 100644 --- a/plugins/CoreVisualizations/Visualizations/HtmlTable/Goals.php +++ b/plugins/CoreVisualizations/Visualizations/HtmlTable/Goals.php @@ -14,7 +14,7 @@ use Piwik\Plugins\CoreVisualizations\Visualizations\HtmlTable; /** - * TODO + * DataTableVisualization that derives from HtmlTable and sets show_goals_columns to true. */ class Goals extends HtmlTable { diff --git a/plugins/CoreVisualizations/Visualizations/JqplotGraph.php b/plugins/CoreVisualizations/Visualizations/JqplotGraph.php index 2b844a9fc95..11e1e403004 100644 --- a/plugins/CoreVisualizations/Visualizations/JqplotGraph.php +++ b/plugins/CoreVisualizations/Visualizations/JqplotGraph.php @@ -49,6 +49,8 @@ class JqplotGraph extends Graph 'external_series_toggle_show_all' ); + public static $overridableProperties = array('x_axis_step_size'); + /** * Constructor. * diff --git a/plugins/CoreVisualizations/Visualizations/JqplotGraph/Evolution.php b/plugins/CoreVisualizations/Visualizations/JqplotGraph/Evolution.php index 1d9d69a74b4..40892f09c03 100644 --- a/plugins/CoreVisualizations/Visualizations/JqplotGraph/Evolution.php +++ b/plugins/CoreVisualizations/Visualizations/JqplotGraph/Evolution.php @@ -33,6 +33,8 @@ class Evolution extends JqplotGraph public static $clientSideProperties = array('show_line_graph'); + public static $overridableProperties = array('show_line_graph'); + public function __construct($view) { parent::__construct($view); diff --git a/plugins/ExampleUI/Controller.php b/plugins/ExampleUI/Controller.php index dc1415b411d..284266ed70a 100644 --- a/plugins/ExampleUI/Controller.php +++ b/plugins/ExampleUI/Controller.php @@ -108,7 +108,11 @@ public function echoAdvancedTagClouds() { $view = ViewDataTable::factory( 'cloud', 'ExampleUI.getPlanetRatiosWithLogos', $controllerAction = 'ExampleUI.echoAdvancedTagClouds'); - $view->display_logo_instead_of_label = true; + $view->visualization_properties->setForVisualization( + 'Piwik\\Plugins\\CoreVisualizations\\Visualizations\\Cloud', + 'display_logo_instead_of_label', + true + ); $view->columns_to_display = array('label', 'value'); $view->translations['value'] = "times the diameter of Earth"; echo $view->render(); diff --git a/tests/PHPUnit/UI b/tests/PHPUnit/UI index 05d16405b09..1de0575fb39 160000 --- a/tests/PHPUnit/UI +++ b/tests/PHPUnit/UI @@ -1 +1 @@ -Subproject commit 05d16405b097e56b6a68e6f4efa1c42f72471cec +Subproject commit 1de0575fb399cf2cc4b15c5d2f9f2e92d031c8bd