From b380a57c91f9918e7dbc8f858df840e3ccb8d24d Mon Sep 17 00:00:00 2001 From: Michelle Thomas Date: Tue, 5 Jun 2018 11:14:36 -0700 Subject: [PATCH] Fixing sortby adhoc metrics for table viz --- superset/assets/src/visualizations/table.js | 6 +++--- superset/connectors/druid/models.py | 24 ++++++++++++--------- superset/connectors/sqla/models.py | 2 ++ superset/data/__init__.py | 21 +++++++++++++++++- superset/viz.py | 3 ++- 5 files changed, 41 insertions(+), 15 deletions(-) diff --git a/superset/assets/src/visualizations/table.js b/superset/assets/src/visualizations/table.js index 6b8deec2d8..72a326ac65 100644 --- a/superset/assets/src/visualizations/table.js +++ b/superset/assets/src/visualizations/table.js @@ -17,7 +17,7 @@ function tableVis(slice, payload) { const data = payload.data; const fd = slice.formData; - let metrics = fd.metrics || []; + let metrics = fd.metrics.map(m => m.label || m); // Add percent metrics metrics = metrics.concat((fd.percent_metrics || []).map(m => '%' + m)); // Removing metrics (aggregates) that are strings @@ -187,7 +187,7 @@ function tableVis(slice, payload) { let sortBy; if (fd.timeseries_limit_metric) { // Sort by as specified - sortBy = fd.timeseries_limit_metric; + sortBy = fd.timeseries_limit_metric.label || fd.timeseries_limit_metric; } else if (metrics.length > 0) { // If not specified, use the first metric from the list sortBy = metrics[0]; @@ -195,7 +195,7 @@ function tableVis(slice, payload) { if (sortBy) { datatable.column(data.columns.indexOf(sortBy)).order(fd.order_desc ? 'desc' : 'asc'); } - if (fd.timeseries_limit_metric && metrics.indexOf(fd.timeseries_limit_metric) < 0) { + if (sortBy && metrics.indexOf(sortBy) < 0) { // Hiding the sortBy column if not in the metrics list datatable.column(data.columns.indexOf(sortBy)).visible(false); } diff --git a/superset/connectors/druid/models.py b/superset/connectors/druid/models.py index 480e8307ef..e23537fe60 100644 --- a/superset/connectors/druid/models.py +++ b/superset/connectors/druid/models.py @@ -1100,6 +1100,18 @@ class DruidDatasource(Model, BaseDatasource): return values + @staticmethod + def sanitize_metric_object(metric): + """ + Update a metric with the correct type if necessary. + :param dict metric: The metric to sanitize + """ + if ( + utils.is_adhoc_metric(metric) and + metric['column']['type'].upper() == 'FLOAT' + ): + metric['column']['type'] = 'DOUBLE' + def run_query( # noqa / druid self, groupby, metrics, @@ -1143,16 +1155,8 @@ class DruidDatasource(Model, BaseDatasource): LooseVersion(self.cluster.get_druid_version()) < LooseVersion('0.11.0') ): for metric in metrics: - if ( - utils.is_adhoc_metric(metric) and - metric['column']['type'].upper() == 'FLOAT' - ): - metric['column']['type'] = 'DOUBLE' - if ( - utils.is_adhoc_metric(timeseries_limit_metric) and - timeseries_limit_metric['column']['type'].upper() == 'FLOAT' - ): - timeseries_limit_metric['column']['type'] = 'DOUBLE' + self.sanitize_metric_object(metric) + self.sanitize_metric_object(timeseries_limit_metric) aggregations, post_aggs = DruidDatasource.metrics_and_post_aggs( metrics, diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 8b02442358..e08053ccd1 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -651,6 +651,8 @@ class SqlaTable(Model, BaseDatasource): for col, ascending in orderby: direction = asc if ascending else desc + if utils.is_adhoc_metric(col): + col = self.adhoc_metric_to_sa(col, cols) qry = qry.order_by(direction(col)) if row_limit: diff --git a/superset/data/__init__.py b/superset/data/__init__.py index 27b3166bb0..dc681f4f40 100644 --- a/superset/data/__init__.py +++ b/superset/data/__init__.py @@ -626,7 +626,8 @@ def load_birth_names(): 'op': 'in', 'val': ['girl'], }], - row_limit=50)), + row_limit=50, + timeseries_limit_metric='sum__num')), Slice( slice_name="Boys", viz_type='table', @@ -789,6 +790,24 @@ def load_birth_names(): 'label': 'SUM(num_california)', }, limit='10')), + Slice( + slice_name="Names Sorted by Num in California", + viz_type='table', + datasource_type='table', + datasource_id=tbl.id, + params=get_slice_json( + defaults, + groupby=['name'], + row_limit=50, + timeseries_limit_metric={ + 'expressionType': 'SIMPLE', + 'column': { + 'column_name': 'num_california', + 'expression': "CASE WHEN state = 'CA' THEN num ELSE 0 END", + }, + 'aggregate': 'SUM', + 'label': 'SUM(num_california)', + })), ] for slc in slices: merge_slice(slc) diff --git a/superset/viz.py b/superset/viz.py index f3db31c247..e7165d317d 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -510,7 +510,8 @@ class TableViz(BaseViz): order_by_cols = fd.get('order_by_cols') or [] d['orderby'] = [json.loads(t) for t in order_by_cols] elif sort_by: - if sort_by not in d['metrics']: + sort_by_label = utils.get_metric_name(sort_by) + if sort_by_label not in utils.get_metric_names(d['metrics']): d['metrics'] += [sort_by] d['orderby'] = [(sort_by, not fd.get('order_desc', True))]