From 68e7e96e4d072f8885fa7907c00633983098a638 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 17 Dec 2021 16:37:14 +0100 Subject: [PATCH 1/5] feat(plugin-chart-pivot-table): support series limit --- .../src/plugin/buildQuery.ts | 37 ++++--------------- .../src/plugin/controlPanel.tsx | 36 +++++++++++++++--- 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts index cd5db2aedb0b2..e3617a366d7ef 100644 --- a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts @@ -19,46 +19,23 @@ import { buildQueryContext, ensureIsArray, - getMetricLabel, normalizeOrderBy, QueryFormColumn, } from '@superset-ui/core'; import { PivotTableQueryFormData } from '../types'; export default function buildQuery(formData: PivotTableQueryFormData) { - const { - groupbyColumns = [], - groupbyRows = [], - order_desc = true, - legacy_order_by, - } = formData; + const { groupbyColumns = [], groupbyRows = [] } = formData; // TODO: add deduping of AdhocColumns const groupbySet = new Set([ ...ensureIsArray(groupbyColumns), ...ensureIsArray(groupbyRows), ]); - return buildQueryContext(formData, baseQueryObject => { - const queryObject = normalizeOrderBy({ + return buildQueryContext(formData, baseQueryObject => [ + { ...baseQueryObject, - order_desc, - legacy_order_by, - }); - const { metrics } = queryObject; - const orderBy = ensureIsArray(legacy_order_by); - if ( - orderBy.length && - !metrics?.find( - metric => getMetricLabel(metric) === getMetricLabel(orderBy[0]), - ) - ) { - metrics?.push(orderBy[0]); - } - return [ - { - ...queryObject, - columns: [...groupbySet], - metrics, - }, - ]; - }); + orderby: normalizeOrderBy(baseQueryObject).orderby, + columns: [...groupbySet], + }, + ]); } diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx index 989e32381d971..328e79f0375cc 100644 --- a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx @@ -30,7 +30,6 @@ import { sections, sharedControls, emitFilterControl, - legacySortBy, } from '@superset-ui/chart-controls'; import { MetricsLayoutEnum } from '../types'; @@ -90,15 +89,40 @@ const config: ControlPanelConfig = { ], ['adhoc_filters'], emitFilterControl, + ['limit'], [ { name: 'row_limit', config: { ...sharedControls.row_limit, + label: t('Cell limit'), + description: t('Limits the number of cells that get retrieved.'), + }, + }, + ], + [ + { + name: 'timeseries_limit_metric', + config: { + ...sharedControls.timeseries_limit_metric, + description: t( + 'Metric used to define how the top series are sorted if a series or cell limit is present. ' + + 'If undefined reverts to the first metric (where appropriate).', + ), + }, + }, + ], + [ + { + name: 'order_desc', + config: { + type: 'CheckboxControl', + label: t('Sort Descending'), + default: true, + description: t('Whether to sort descending or ascending'), }, }, ], - ...legacySortBy, ], }, { @@ -237,14 +261,14 @@ const config: ControlPanelConfig = { ], renderTrigger: true, description: ( - + <>
{t('Change order of rows.')}
{t('Available sorting modes:')}
  • {t('By key: use row names as sorting key')}
  • {t('By value: use metric values as sorting key')}
- + ), }, }, @@ -265,14 +289,14 @@ const config: ControlPanelConfig = { ], renderTrigger: true, description: ( - + <>
{t('Change order of columns.')}
{t('Available sorting modes:')}
  • {t('By key: use column names as sorting key')}
  • {t('By value: use metric values as sorting key')}
- + ), }, }, From 3d891affd320e242666f3538a50c513ea71bd888 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 17 Dec 2021 17:04:04 +0100 Subject: [PATCH 2/5] Add a migration --- ...move_pivot_table_v2_legacy_order_by_to_.py | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 superset/migrations/versions/31bb738bd1d2_move_pivot_table_v2_legacy_order_by_to_.py diff --git a/superset/migrations/versions/31bb738bd1d2_move_pivot_table_v2_legacy_order_by_to_.py b/superset/migrations/versions/31bb738bd1d2_move_pivot_table_v2_legacy_order_by_to_.py new file mode 100644 index 0000000000000..21fecee88ba69 --- /dev/null +++ b/superset/migrations/versions/31bb738bd1d2_move_pivot_table_v2_legacy_order_by_to_.py @@ -0,0 +1,95 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""move_pivot_table_v2_legacy_order_by_to_timeseries_limit_metric + +Revision ID: 31bb738bd1d2 +Revises: fe23025b9441 +Create Date: 2021-12-17 16:56:55.186285 + +""" + +# revision identifiers, used by Alembic. +revision = "31bb738bd1d2" +down_revision = "fe23025b9441" + + +import json +import logging + +from alembic import op +from sqlalchemy import Column, Integer, String, Text +from sqlalchemy.ext.declarative import declarative_base + +from superset import db + +Base = declarative_base() + +logger = logging.getLogger("alembic") + + +class Slice(Base): + __tablename__ = "slices" + + id = Column(Integer, primary_key=True) + params = Column(Text) + viz_type = Column(String(250)) + + +def upgrade(): + bind = op.get_bind() + session = db.Session(bind=bind) + + slices = session.query(Slice).filter(Slice.viz_type == "pivot_table_v2").all() + for slc in slices: + try: + params = json.loads(slc.params) + legacy_order_by = params.pop("legacy_order_by", None) + if legacy_order_by: + params["timeseries_limit_metric"] = legacy_order_by + slc.params = json.dumps(params, sort_keys=True) + except Exception as e: + logger.exception( + f"An error occurred: parsing params for slice {slc.id} failed." + f"You need to fix it before upgrading your DB." + ) + raise e + + session.commit() + session.close() + + +def downgrade(): + bind = op.get_bind() + session = db.Session(bind=bind) + + slices = session.query(Slice).filter(Slice.viz_type == "pivot_table_v2").all() + for slc in slices: + try: + params = json.loads(slc.params) + timeseries_limit_metric = params.pop("timeseries_limit_metric", None) + if timeseries_limit_metric: + params["legacy_order_by"] = timeseries_limit_metric + slc.params = json.dumps(params, sort_keys=True) + except Exception as e: + logger.exception( + f"An error occurred: parsing params for slice {slc.id} failed. " + "You need to fix it before downgrading your DB." + ) + raise e + + session.commit() + session.close() From a7373ecec5bffbd2518c7a53c458ea2fa68f0b57 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Mon, 20 Dec 2021 19:26:08 +0100 Subject: [PATCH 3/5] Use non-legacy series limit controls --- .../src/plugin/buildQuery.ts | 26 +++++++++++++------ .../src/plugin/controlPanel.tsx | 6 ++--- ...move_pivot_table_v2_legacy_order_by_to_.py | 8 +++--- 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts index e3617a366d7ef..6c601ff9dec95 100644 --- a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts @@ -19,8 +19,8 @@ import { buildQueryContext, ensureIsArray, - normalizeOrderBy, QueryFormColumn, + QueryFormOrderBy, } from '@superset-ui/core'; import { PivotTableQueryFormData } from '../types'; @@ -31,11 +31,21 @@ export default function buildQuery(formData: PivotTableQueryFormData) { ...ensureIsArray(groupbyColumns), ...ensureIsArray(groupbyRows), ]); - return buildQueryContext(formData, baseQueryObject => [ - { - ...baseQueryObject, - orderby: normalizeOrderBy(baseQueryObject).orderby, - columns: [...groupbySet], - }, - ]); + return buildQueryContext(formData, baseQueryObject => { + const { series_limit_metric, metrics, order_desc } = baseQueryObject; + let orderBy: QueryFormOrderBy[] | undefined; + if (series_limit_metric) { + orderBy = [[series_limit_metric, !order_desc]]; + } + if (Array.isArray(metrics) && metrics[0]) { + orderBy = [[metrics[0], !order_desc]]; + } + return [ + { + ...baseQueryObject, + orderby: orderBy, + columns: [...groupbySet], + }, + ]; + }); } diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx index 328e79f0375cc..ecc7e118d4629 100644 --- a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx @@ -89,7 +89,7 @@ const config: ControlPanelConfig = { ], ['adhoc_filters'], emitFilterControl, - ['limit'], + ['series_limit'], [ { name: 'row_limit', @@ -102,9 +102,9 @@ const config: ControlPanelConfig = { ], [ { - name: 'timeseries_limit_metric', + name: 'series_limit_metric', config: { - ...sharedControls.timeseries_limit_metric, + ...sharedControls.series_limit_metric, description: t( 'Metric used to define how the top series are sorted if a series or cell limit is present. ' + 'If undefined reverts to the first metric (where appropriate).', diff --git a/superset/migrations/versions/31bb738bd1d2_move_pivot_table_v2_legacy_order_by_to_.py b/superset/migrations/versions/31bb738bd1d2_move_pivot_table_v2_legacy_order_by_to_.py index 21fecee88ba69..0adaa0d4f9c06 100644 --- a/superset/migrations/versions/31bb738bd1d2_move_pivot_table_v2_legacy_order_by_to_.py +++ b/superset/migrations/versions/31bb738bd1d2_move_pivot_table_v2_legacy_order_by_to_.py @@ -59,7 +59,7 @@ def upgrade(): params = json.loads(slc.params) legacy_order_by = params.pop("legacy_order_by", None) if legacy_order_by: - params["timeseries_limit_metric"] = legacy_order_by + params["series_limit_metric"] = legacy_order_by slc.params = json.dumps(params, sort_keys=True) except Exception as e: logger.exception( @@ -80,9 +80,9 @@ def downgrade(): for slc in slices: try: params = json.loads(slc.params) - timeseries_limit_metric = params.pop("timeseries_limit_metric", None) - if timeseries_limit_metric: - params["legacy_order_by"] = timeseries_limit_metric + series_limit_metric = params.pop("series_limit_metric", None) + if series_limit_metric: + params["legacy_order_by"] = series_limit_metric slc.params = json.dumps(params, sort_keys=True) except Exception as e: logger.exception( From b0c6b2aa9de81adac62a535e5d16f7d37a457376 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Mon, 20 Dec 2021 19:28:26 +0100 Subject: [PATCH 4/5] Add a todo comment --- .../plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx index ecc7e118d4629..b546c08cc69db 100644 --- a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/controlPanel.tsx @@ -100,6 +100,7 @@ const config: ControlPanelConfig = { }, }, ], + // TODO(kgabryje): add series_columns control after control panel is redesigned to avoid clutter [ { name: 'series_limit_metric', From 2318cc18bd944ae63e3bfd1dab814572d8ba4dbc Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Tue, 21 Dec 2021 11:13:43 +0100 Subject: [PATCH 5/5] Bug fix --- .../plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts index 6c601ff9dec95..677902b796800 100644 --- a/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-pivot-table/src/plugin/buildQuery.ts @@ -36,8 +36,7 @@ export default function buildQuery(formData: PivotTableQueryFormData) { let orderBy: QueryFormOrderBy[] | undefined; if (series_limit_metric) { orderBy = [[series_limit_metric, !order_desc]]; - } - if (Array.isArray(metrics) && metrics[0]) { + } else if (Array.isArray(metrics) && metrics[0]) { orderBy = [[metrics[0], !order_desc]]; } return [