From 19772ecadf44bfa380d8e72d9384617cbda5e528 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Tue, 21 Dec 2021 16:58:54 +0100 Subject: [PATCH] feat(plugin-chart-pivot-table): support series limit (#17803) * feat(plugin-chart-pivot-table): support series limit * Add a migration * Use non-legacy series limit controls * Add a todo comment * Bug fix --- .../src/plugin/buildQuery.ts | 34 ++----- .../src/plugin/controlPanel.tsx | 29 +++++- ...move_pivot_table_v2_legacy_order_by_to_.py | 95 +++++++++++++++++++ 3 files changed, 132 insertions(+), 26 deletions(-) create mode 100644 superset/migrations/versions/31bb738bd1d2_move_pivot_table_v2_legacy_order_by_to_.py 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..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 @@ -19,45 +19,31 @@ import { buildQueryContext, ensureIsArray, - getMetricLabel, - normalizeOrderBy, QueryFormColumn, + QueryFormOrderBy, } 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({ - ...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]); + const { series_limit_metric, metrics, order_desc } = baseQueryObject; + let orderBy: QueryFormOrderBy[] | undefined; + if (series_limit_metric) { + orderBy = [[series_limit_metric, !order_desc]]; + } else if (Array.isArray(metrics) && metrics[0]) { + orderBy = [[metrics[0], !order_desc]]; } return [ { - ...queryObject, + ...baseQueryObject, + orderby: orderBy, columns: [...groupbySet], - metrics, }, ]; }); 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 ccb74e45bb4b9..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 @@ -30,7 +30,6 @@ import { sections, sharedControls, emitFilterControl, - legacySortBy, } from '@superset-ui/chart-controls'; import { MetricsLayoutEnum } from '../types'; @@ -90,15 +89,41 @@ const config: ControlPanelConfig = { ], ['adhoc_filters'], emitFilterControl, + ['series_limit'], [ { name: 'row_limit', config: { ...sharedControls.row_limit, + label: t('Cell limit'), + description: t('Limits the number of cells that get retrieved.'), + }, + }, + ], + // TODO(kgabryje): add series_columns control after control panel is redesigned to avoid clutter + [ + { + name: 'series_limit_metric', + config: { + ...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).', + ), + }, + }, + ], + [ + { + name: 'order_desc', + config: { + type: 'CheckboxControl', + label: t('Sort Descending'), + default: true, + description: t('Whether to sort descending or ascending'), }, }, ], - ...legacySortBy, ], }, { 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..0adaa0d4f9c06 --- /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["series_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) + 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( + 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()