From 33b5eded9832df24dce6aac70f24d29ea4f6c852 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Tue, 1 Nov 2022 22:44:20 +0800 Subject: [PATCH 01/14] feat: xaxis sortby --- .../src/operators/sortOperator.ts | 34 +++-- .../src/sections/echartsTimeSeriesQuery.tsx | 131 ++++++++++++---- .../test/operators/sortOperator.test.ts | 143 +++++++++++------- .../src/query/getColumnLabel.ts | 4 +- .../superset-ui-core/src/query/getXAxis.ts | 17 +++ .../superset-ui-core/src/query/index.ts | 7 +- .../src/query/types/PostProcessing.ts | 4 +- .../test/query/types/PostProcessing.test.ts | 2 +- .../Timeseries/Regular/Bar/controlPanel.tsx | 37 +---- .../src/Timeseries/buildQuery.ts | 2 + .../XAxisSortControl/XAxisSortControl.tsx | 101 +++++++++++++ .../controls/XAxisSortControl/index.ts | 19 +++ .../controls/XAxisSortControl/types.ts | 32 ++++ .../src/explore/components/controls/index.js | 2 + superset/utils/pandas_postprocessing/sort.py | 24 ++- superset/utils/pandas_postprocessing/utils.py | 12 +- tests/common/query_context_generator.py | 4 +- .../pandas_postprocessing/test_sort.py | 29 +++- 18 files changed, 450 insertions(+), 154 deletions(-) create mode 100644 superset-frontend/src/explore/components/controls/XAxisSortControl/XAxisSortControl.tsx create mode 100644 superset-frontend/src/explore/components/controls/XAxisSortControl/index.ts create mode 100644 superset-frontend/src/explore/components/controls/XAxisSortControl/types.ts diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts index 277d2df559ce..2357469a12d3 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts @@ -1,4 +1,3 @@ -/* eslint-disable camelcase */ /** * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -17,25 +16,42 @@ * specific language governing permissions and limitationsxw * under the License. */ -import { DTTM_ALIAS, PostProcessingSort, RollingType } from '@superset-ui/core'; +import { isEmpty } from 'lodash'; +import { + getXAxisLabel, + hasGenericChartAxes, + PostProcessingSort, + UnsortedXAxis, + isAxisSortValue, +} from '@superset-ui/core'; import { PostProcessingFactory } from './types'; export const sortOperator: PostProcessingFactory = ( formData, queryObject, ) => { - const { x_axis: xAxis } = formData; if ( - (xAxis || queryObject.is_timeseries) && - Object.values(RollingType).includes(formData.rolling_type) + hasGenericChartAxes && + isAxisSortValue(formData?.x_axis_sort) && + formData.x_axis_sort.sortByLabel !== UnsortedXAxis && + // the sort operator doesn't support sort-by multiple series. + isEmpty(formData.groupby) ) { - const index = xAxis || DTTM_ALIAS; + if (formData.x_axis_sort.sortByLabel === getXAxisLabel(formData)) { + return { + operation: 'sort', + options: { + is_sort_index: true, + ascending: formData.x_axis_sort.isAsc, + }, + }; + } + return { operation: 'sort', options: { - columns: { - [index]: true, - }, + by: formData.x_axis_sort.sortByLabel, + ascending: formData.x_axis_sort.isAsc, }, }; } diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx index 4fbf9ee764e2..d81239ce96dd 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx @@ -16,9 +16,96 @@ * specific language governing permissions and limitations * under the License. */ -import { ContributionType, hasGenericChartAxes, t } from '@superset-ui/core'; -import { ControlPanelSectionConfig } from '../types'; -import { emitFilterControl } from '../shared-controls'; +import { + ContributionType, + ensureIsArray, + getColumnLabel, + getMetricLabel, + hasGenericChartAxes, + UnsortedXAxis, + isEqualArray, + t, + defaultAxisSortValue, +} from '@superset-ui/core'; +import { + ControlPanelSectionConfig, + ControlSetItem, + ControlSetRow, +} from '../types'; +import { emitFilterControl } from '../shared-controls/emitFilterControl'; + +const controlsWithoutXAxis: ControlSetRow[] = [ + ['metrics'], + ['groupby'], + [ + { + name: 'contributionMode', + config: { + type: 'SelectControl', + label: t('Contribution Mode'), + default: null, + choices: [ + [null, t('None')], + [ContributionType.Row, t('Row')], + [ContributionType.Column, t('Series')], + ], + description: t('Calculate contribution per series or row'), + }, + }, + ], + ['adhoc_filters'], + emitFilterControl, + ['limit'], + ['timeseries_limit_metric'], + ['order_desc'], + ['row_limit'], + ['truncate_metric'], + ['show_empty_columns'], +]; + +const xAxisSort: ControlSetItem = { + name: 'x_axis_sort', + config: { + type: 'XAxisSortControl', + label: t('X-Axis Sort By'), + description: '', + shouldMapStateToProps: (prevState, state) => { + const prevOptions = [ + getColumnLabel(prevState.form_data.x_axis), + ...ensureIsArray(prevState.form_data.metrics).map(metric => + getMetricLabel(metric), + ), + ]; + const currOptions = [ + getColumnLabel(state.form_data.x_axis), + ...ensureIsArray(state.form_data.metrics).map(metric => + getMetricLabel(metric), + ), + ]; + return !isEqualArray(prevOptions, currOptions); + }, + mapStateToProps: state => { + const choices = [ + getColumnLabel(state.form_data.x_axis), + ...ensureIsArray(state.form_data.metrics).map(metric => + getMetricLabel(metric), + ), + ].filter(Boolean); + + const xAxisSortByOptions = [UnsortedXAxis, ...choices].map(entry => ({ + value: entry, + label: entry, + })); + return { + xAxisSortByOptions, + }; + }, + visibility: ({ controls }) => + Array.isArray(controls?.groupby?.value) && + controls?.groupby?.value.length === 0, + default: defaultAxisSortValue, + }, +}; export const echartsTimeSeriesQuery: ControlPanelSectionConfig = { label: t('Query'), @@ -26,31 +113,17 @@ export const echartsTimeSeriesQuery: ControlPanelSectionConfig = { controlSetRows: [ [hasGenericChartAxes ? 'x_axis' : null], [hasGenericChartAxes ? 'time_grain_sqla' : null], - ['metrics'], - ['groupby'], - [ - { - name: 'contributionMode', - config: { - type: 'SelectControl', - label: t('Contribution Mode'), - default: null, - choices: [ - [null, t('None')], - [ContributionType.Row, t('Row')], - [ContributionType.Column, t('Series')], - ], - description: t('Calculate contribution per series or row'), - }, - }, - ], - ['adhoc_filters'], - emitFilterControl, - ['limit'], - ['timeseries_limit_metric'], - ['order_desc'], - ['row_limit'], - ['truncate_metric'], - ['show_empty_columns'], + ...controlsWithoutXAxis, + ], +}; + +export const echartsTimeSeriesQueryWithXAxisSort: ControlPanelSectionConfig = { + label: t('Query'), + expanded: true, + controlSetRows: [ + [hasGenericChartAxes ? 'x_axis' : null], + [hasGenericChartAxes ? 'time_grain_sqla' : null], + [hasGenericChartAxes ? xAxisSort : null], + ...controlsWithoutXAxis, ], }; diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/operators/sortOperator.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/sortOperator.test.ts index 6f0267d91305..5c261dad1f30 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/operators/sortOperator.test.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/test/operators/sortOperator.test.ts @@ -16,8 +16,9 @@ * specific language governing permissions and limitations * under the License. */ -import { QueryObject, SqlaFormData } from '@superset-ui/core'; +import { QueryObject, SqlaFormData, UnsortedXAxis } from '@superset-ui/core'; import { sortOperator } from '@superset-ui/chart-controls'; +import * as supersetCoreModule from '@superset-ui/core'; const formData: SqlaFormData = { metrics: [ @@ -52,92 +53,122 @@ const queryObject: QueryObject = { ], }; -test('skip sort', () => { +test('should ignore the sortOperator', () => { + // FF is disabled + Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', { + value: false, + }); expect(sortOperator(formData, queryObject)).toEqual(undefined); - expect( - sortOperator(formData, { ...queryObject, is_timeseries: false }), - ).toEqual(undefined); + + // FF is enabled + Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', { + value: true, + }); expect( sortOperator( - { ...formData, rolling_type: 'xxxx' }, - { ...queryObject, is_timeseries: true }, + { + ...formData, + ...{ + x_axis_sort: { + sortByLabel: undefined, + isAsc: true, + }, + }, + }, + queryObject, ), ).toEqual(undefined); - expect( - sortOperator(formData, { ...queryObject, is_timeseries: true }), - ).toEqual(undefined); -}); -test('sort by __timestamp', () => { - expect( - sortOperator( - { ...formData, rolling_type: 'cumsum' }, - { ...queryObject, is_timeseries: true }, - ), - ).toEqual({ - operation: 'sort', - options: { - columns: { - __timestamp: true, - }, - }, + // sortOperator will be ignored when sortByLabel is unsorted + Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', { + value: true, }); - expect( sortOperator( - { ...formData, rolling_type: 'sum' }, - { ...queryObject, is_timeseries: true }, - ), - ).toEqual({ - operation: 'sort', - options: { - columns: { - __timestamp: true, + { + ...formData, + ...{ + x_axis_sort: { + sortByLabel: UnsortedXAxis, + isAsc: true, + }, + }, }, - }, - }); + queryObject, + ), + ).toEqual(undefined); + // sortOperator doesn't support multiple series + Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', { + value: true, + }); expect( sortOperator( - { ...formData, rolling_type: 'mean' }, - { ...queryObject, is_timeseries: true }, - ), - ).toEqual({ - operation: 'sort', - options: { - columns: { - __timestamp: true, + { + ...formData, + ...{ + x_axis_sort: { + sortByLabel: 'metric label', + isAsc: true, + }, + groupby: ['col1'], + x_axis: 'axis column', + }, }, - }, - }); + queryObject, + ), + ).toEqual(undefined); +}); +test('should sort by metric', () => { + Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', { + value: true, + }); expect( sortOperator( - { ...formData, rolling_type: 'std' }, - { ...queryObject, is_timeseries: true }, + { + ...formData, + ...{ + x_axis_sort: { + sortByLabel: 'a metric label', + isAsc: true, + }, + }, + }, + queryObject, ), ).toEqual({ operation: 'sort', options: { - columns: { - __timestamp: true, - }, + by: 'a metric label', + ascending: true, }, }); }); -test('sort by named x-axis', () => { +test('should sort by axis', () => { + Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', { + value: true, + }); expect( sortOperator( - { ...formData, x_axis: 'ds', rolling_type: 'cumsum' }, - { ...queryObject }, + { + ...formData, + ...{ + x_axis_sort: { + sortByLabel: 'Categorical Column', + isAsc: true, + }, + x_axis: 'Categorical Column', + }, + }, + queryObject, ), ).toEqual({ operation: 'sort', options: { - columns: { - ds: true, - }, + is_sort_index: true, + ascending: true, }, }); }); diff --git a/superset-frontend/packages/superset-ui-core/src/query/getColumnLabel.ts b/superset-frontend/packages/superset-ui-core/src/query/getColumnLabel.ts index f449a44cb270..f1b4e6ffa730 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/getColumnLabel.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/getColumnLabel.ts @@ -23,8 +23,8 @@ export default function getColumnLabel(column: QueryFormColumn): string { if (isPhysicalColumn(column)) { return column; } - if (column.label) { + if (column?.label) { return column.label; } - return column.sqlExpression; + return column?.sqlExpression; } diff --git a/superset-frontend/packages/superset-ui-core/src/query/getXAxis.ts b/superset-frontend/packages/superset-ui-core/src/query/getXAxis.ts index 7c329c2a8bdf..9e4a92ec28eb 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/getXAxis.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/getXAxis.ts @@ -25,6 +25,7 @@ import { QueryFormData, QueryFormColumn, Optional, + isDefined, } from '@superset-ui/core'; export const isXAxisSet = (formData: QueryFormData) => @@ -55,3 +56,19 @@ export const getXAxisLabel = (formData: QueryFormData): Optional => { } return undefined; }; + +export interface AxisSortValue { + sortByLabel: string; + isAsc: boolean; +} + +export function isAxisSortValue(value: any): value is AxisSortValue { + return isDefined(value?.sortByLabel) && isDefined(value.isAsc); +} + +export const UnsortedXAxis = 'unsorted'; + +export const defaultAxisSortValue = { + sortByLabel: UnsortedXAxis, + isAsc: true, +}; diff --git a/superset-frontend/packages/superset-ui-core/src/query/index.ts b/superset-frontend/packages/superset-ui-core/src/query/index.ts index 3ea6dad75f58..bb83e3d340fd 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/index.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/index.ts @@ -29,13 +29,8 @@ export { default as getMetricLabel } from './getMetricLabel'; export { default as DatasourceKey } from './DatasourceKey'; export { default as normalizeOrderBy } from './normalizeOrderBy'; export { normalizeTimeColumn } from './normalizeTimeColumn'; -export { - getXAxisLabel, - getXAxisColumn, - isXAxisSet, - hasGenericChartAxes, -} from './getXAxis'; export { default as extractQueryFields } from './extractQueryFields'; +export * from './getXAxis'; export * from './types/AnnotationLayer'; export * from './types/QueryFormData'; diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/PostProcessing.ts b/superset-frontend/packages/superset-ui-core/src/query/types/PostProcessing.ts index 7b63ea056a7f..b88dafb5c793 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/types/PostProcessing.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/types/PostProcessing.ts @@ -182,7 +182,9 @@ export type PostProcessingCompare = interface _PostProcessingSort { operation: 'sort'; options: { - columns: Record; + is_sort_index?: boolean; + by?: string[] | string; + ascending?: boolean[] | boolean; }; } export type PostProcessingSort = _PostProcessingSort | DefaultPostProcessing; diff --git a/superset-frontend/packages/superset-ui-core/test/query/types/PostProcessing.test.ts b/superset-frontend/packages/superset-ui-core/test/query/types/PostProcessing.test.ts index 9cdec11ec8cb..db95137409a2 100644 --- a/superset-frontend/packages/superset-ui-core/test/query/types/PostProcessing.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/query/types/PostProcessing.test.ts @@ -147,7 +147,7 @@ const ROLLING_RULE: PostProcessingRolling = { const SORT_RULE: PostProcessingSort = { operation: 'sort', options: { - columns: { foo: true }, + by: 'foo', }, }; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Bar/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Bar/controlPanel.tsx index 91b0b3ca60e2..bb6284406415 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Bar/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Bar/controlPanel.tsx @@ -41,7 +41,6 @@ import { const { logAxis, minorSplitLine, - rowLimit, truncateYAxis, yAxisBounds, zoomable, @@ -260,7 +259,7 @@ function createAxisControl(axis: 'x' | 'y'): ControlSetRow[] { const config: ControlPanelConfig = { controlPanelSections: [ sections.genericTime, - sections.echartsTimeSeriesQuery, + sections.echartsTimeSeriesQueryWithXAxisSort, sections.advancedAnalyticsControls, sections.annotationsAndLayersControls, sections.forecastIntervalControls, @@ -324,40 +323,6 @@ const config: ControlPanelConfig = { ], }, ], - controlOverrides: { - row_limit: { - default: rowLimit, - }, - limit: { - rerender: ['timeseries_limit_metric', 'order_desc'], - }, - timeseries_limit_metric: { - label: t('Series Limit Sort By'), - description: t( - 'Metric used to order the limit if a series limit is present. ' + - 'If undefined reverts to the first metric (where appropriate).', - ), - visibility: ({ controls }) => Boolean(controls?.limit.value), - mapStateToProps: (state, controlState) => { - const timeserieslimitProps = - sharedControls.timeseries_limit_metric.mapStateToProps?.( - state, - controlState, - ) || {}; - timeserieslimitProps.value = state.controls?.limit?.value - ? controlState?.value - : []; - return timeserieslimitProps; - }, - }, - order_desc: { - label: t('Series Limit Sort Descending'), - default: false, - description: t( - 'Whether to sort descending or ascending if a series limit is present', - ), - }, - }, formDataOverrides: formData => ({ ...formData, metrics: getStandardizedControls().popAllMetrics(), diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts index 0ddb1f53fe8d..ad021f92b918 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts @@ -36,6 +36,7 @@ import { prophetOperator, timeComparePivotOperator, flattenOperator, + sortOperator, } from '@superset-ui/chart-controls'; export default function buildQuery(formData: QueryFormData) { @@ -95,6 +96,7 @@ export default function buildQuery(formData: QueryFormData) { resampleOperator(formData, baseQueryObject), renameOperator(formData, baseQueryObject), contributionOperator(formData, baseQueryObject), + sortOperator(formData, baseQueryObject), flattenOperator(formData, baseQueryObject), // todo: move prophet before flatten prophetOperator(formData, baseQueryObject), diff --git a/superset-frontend/src/explore/components/controls/XAxisSortControl/XAxisSortControl.tsx b/superset-frontend/src/explore/components/controls/XAxisSortControl/XAxisSortControl.tsx new file mode 100644 index 000000000000..7bd51423b800 --- /dev/null +++ b/superset-frontend/src/explore/components/controls/XAxisSortControl/XAxisSortControl.tsx @@ -0,0 +1,101 @@ +/** + * 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. + */ +import React, { useState, useEffect, useMemo } from 'react'; +import { t, UnsortedXAxis } from '@superset-ui/core'; +import ControlHeader from 'src/explore/components/ControlHeader'; +import Label from 'src/components/Label'; +import ControlPopover from 'src/explore/components/controls/ControlPopover/ControlPopover'; +import { AntdCheckbox, Select } from 'src/components'; +import { xAxisSortControlProps } from './types'; + +export default function XAxisSortControl({ + xAxisSortByOptions, + value, + onChange, + label, + description, +}: xAxisSortControlProps) { + const [labelOnControl, setLabelOnControl] = useState( + value.sortByLabel === UnsortedXAxis + ? value.sortByLabel + : `${value.sortByLabel} (${value.isAsc ? 'Asc' : 'Desc'})`, + ); + const [sortByLabel, setSortByLabel] = useState(value.sortByLabel); + const [isAsc, setIsAsc] = useState(value.isAsc); + + useEffect(() => { + onChange({ + sortByLabel, + isAsc, + }); + setLabelOnControl( + sortByLabel === UnsortedXAxis + ? UnsortedXAxis + : `${sortByLabel} (${isAsc ? 'Asc' : 'Desc'})`, + ); + }, [sortByLabel, isAsc]); + + useEffect(() => { + if ( + !xAxisSortByOptions + .map(option => option.value) + .includes(value.sortByLabel) + ) { + setSortByLabel(UnsortedXAxis); + setLabelOnControl(UnsortedXAxis); + } + }, [xAxisSortByOptions]); + + const popoverContent = useMemo( + () => ( + <> +
{t('SORT BY')}
+ setSortByLabel(val)} - /> - - setIsAsc(e.target.checked)} - > - {t('Ascending')} - - - ), - [isAsc, sortByLabel, xAxisSortByOptions], - ); - - return ( - <> - - - - - - ); -} diff --git a/superset-frontend/src/explore/components/controls/XAxisSortControl/index.ts b/superset-frontend/src/explore/components/controls/XAxisSortControl/index.ts deleted file mode 100644 index c92a2a3179dd..000000000000 --- a/superset-frontend/src/explore/components/controls/XAxisSortControl/index.ts +++ /dev/null @@ -1,19 +0,0 @@ -/** - * 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. - */ -export { default } from './XAxisSortControl'; diff --git a/superset-frontend/src/explore/components/controls/XAxisSortControl/types.ts b/superset-frontend/src/explore/components/controls/XAxisSortControl/types.ts deleted file mode 100644 index 33ee6cd0e9de..000000000000 --- a/superset-frontend/src/explore/components/controls/XAxisSortControl/types.ts +++ /dev/null @@ -1,32 +0,0 @@ -/** - * 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. - */ -import { AxisSortValue } from '@superset-ui/core'; - -export type SelectOptionType = { - value: string; - label: string; -}; - -export interface xAxisSortControlProps { - value: AxisSortValue; - onChange: (value: AxisSortValue) => void; - xAxisSortByOptions: SelectOptionType[]; - label: string; - description: string; -} diff --git a/superset-frontend/src/explore/components/controls/index.js b/superset-frontend/src/explore/components/controls/index.js index 21e3bd4cf285..23cd3a73d85b 100644 --- a/superset-frontend/src/explore/components/controls/index.js +++ b/superset-frontend/src/explore/components/controls/index.js @@ -45,7 +45,6 @@ import DndColumnSelectControl, { DndFilterSelect, DndMetricSelect, } from './DndColumnSelectControl'; -import XAxisSortControl from './XAxisSortControl'; const controlMap = { AnnotationLayerControl, @@ -75,7 +74,6 @@ const controlMap = { AdhocFilterControl, FilterBoxItemControl, ConditionalFormattingControl, - XAxisSortControl, ...sharedControlComponents, }; export default controlMap; From dfadcc54c65a54cef6c3d4b80304f9ff6fd60a83 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 9 Nov 2022 21:05:18 +0800 Subject: [PATCH 05/14] fix initial control bug --- .../src/sections/echartsTimeSeriesQuery.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx index 1846f3a381fe..f3a289d8af7e 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx @@ -22,6 +22,7 @@ import { getColumnLabel, getMetricLabel, hasGenericChartAxes, + isDefined, isEqualArray, QueryFormColumn, t, @@ -111,6 +112,7 @@ const xAxisSort = { }; }, visibility: ({ controls }: { controls: ControlStateMapping }) => + isDefined(controls?.x_axis?.value) && !isTemporalColumn( getColumnLabel(controls?.x_axis?.value as QueryFormColumn), controls?.datasource?.datasource, @@ -130,6 +132,7 @@ const xAxisSortAsc = { 'Whether to sort descending or ascending. Takes effect only when "Sort by" is set', ), visibility: ({ controls }: { controls: ControlStateMapping }) => + isDefined(controls?.x_axis?.value) && !isTemporalColumn( getColumnLabel(controls?.x_axis?.value as QueryFormColumn), controls?.datasource?.datasource, From 258c8148d7a8942c484f0c86c5c24fa2e6d7e714 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 9 Nov 2022 22:51:22 +0800 Subject: [PATCH 06/14] fix updates --- .../src/sections/echartsTimeSeriesQuery.tsx | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx index f3a289d8af7e..ccd72f59b2e4 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx @@ -25,6 +25,7 @@ import { isDefined, isEqualArray, QueryFormColumn, + QueryFormMetric, t, } from '@superset-ui/core'; import { @@ -70,32 +71,30 @@ const xAxisSort = { config: { type: 'SelectControl', label: t('X-Axis Sort By'), - description: t( - 'Whether to sort descending or ascending. Takes effect only when "Sort by" is set', - ), + description: t('Whether to sort descending or ascending on the X-Axis.'), shouldMapStateToProps: ( prevState: ControlPanelState, state: ControlPanelState, ) => { const prevOptions = [ - getColumnLabel(prevState.form_data.x_axis), - ...ensureIsArray(prevState.form_data.metrics).map(metric => - getMetricLabel(metric), + getColumnLabel(prevState?.controls?.x_axis?.value as QueryFormColumn), + ...ensureIsArray(prevState?.controls?.metrics?.value).map(metric => + getMetricLabel(metric as QueryFormMetric), ), ]; const currOptions = [ - getColumnLabel(state.form_data.x_axis), - ...ensureIsArray(state.form_data.metrics).map(metric => - getMetricLabel(metric), + getColumnLabel(state?.controls?.x_axis?.value as QueryFormColumn), + ...ensureIsArray(state?.controls?.metrics?.value).map(metric => + getMetricLabel(metric as QueryFormMetric), ), ]; return !isEqualArray(prevOptions, currOptions); }, mapStateToProps: (state: ControlPanelState, controlState: ControlState) => { const choices = [ - getColumnLabel(state.form_data.x_axis), - ...ensureIsArray(state.form_data.metrics).map(metric => - getMetricLabel(metric), + getColumnLabel(state?.controls?.x_axis?.value as QueryFormColumn), + ...ensureIsArray(state?.controls?.metrics?.value).map(metric => + getMetricLabel(metric as QueryFormMetric), ), ].filter(Boolean); const value = @@ -128,9 +127,7 @@ const xAxisSortAsc = { type: 'CheckboxControl', label: t('X-Axis Sort Ascending'), default: true, - description: t( - 'Whether to sort descending or ascending. Takes effect only when "Sort by" is set', - ), + description: t('Whether to sort descending or ascending on the X-Axis.'), visibility: ({ controls }: { controls: ControlStateMapping }) => isDefined(controls?.x_axis?.value) && !isTemporalColumn( From 7c3c4ec5b6f8933645e14c8cf7df50990641f1c5 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Thu, 10 Nov 2022 16:01:12 +0800 Subject: [PATCH 07/14] escape to change select component --- .../src/sections/echartsTimeSeriesQuery.tsx | 2 +- .../components/controls/XAxisSortControl.tsx | 31 +++++++++++++++++++ .../src/explore/components/controls/index.js | 2 ++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 superset-frontend/src/explore/components/controls/XAxisSortControl.tsx diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx index ccd72f59b2e4..dff8386f86f5 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx @@ -69,7 +69,7 @@ const controlsWithoutXAxis: ControlSetRow[] = [ const xAxisSort = { name: 'x_axis_sort', config: { - type: 'SelectControl', + type: 'XAxisSortControl', label: t('X-Axis Sort By'), description: t('Whether to sort descending or ascending on the X-Axis.'), shouldMapStateToProps: ( diff --git a/superset-frontend/src/explore/components/controls/XAxisSortControl.tsx b/superset-frontend/src/explore/components/controls/XAxisSortControl.tsx new file mode 100644 index 000000000000..5c725bc5afdd --- /dev/null +++ b/superset-frontend/src/explore/components/controls/XAxisSortControl.tsx @@ -0,0 +1,31 @@ +/** + * 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. + */ +import React, { useEffect } from 'react'; +import SelectControl from './SelectControl'; + +export default function XAxisSortControl(props: { + onChange: (val: string) => void; + value: string; +}) { + useEffect(() => { + props.onChange(props.value); + }, [props.value]); + + return ; +} diff --git a/superset-frontend/src/explore/components/controls/index.js b/superset-frontend/src/explore/components/controls/index.js index 23cd3a73d85b..21e3bd4cf285 100644 --- a/superset-frontend/src/explore/components/controls/index.js +++ b/superset-frontend/src/explore/components/controls/index.js @@ -45,6 +45,7 @@ import DndColumnSelectControl, { DndFilterSelect, DndMetricSelect, } from './DndColumnSelectControl'; +import XAxisSortControl from './XAxisSortControl'; const controlMap = { AnnotationLayerControl, @@ -74,6 +75,7 @@ const controlMap = { AdhocFilterControl, FilterBoxItemControl, ConditionalFormattingControl, + XAxisSortControl, ...sharedControlComponents, }; export default controlMap; From eb2c61c307490b9d0282ac6e9fde8fc87fbdc844 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Thu, 10 Nov 2022 17:23:21 +0800 Subject: [PATCH 08/14] fix temporal -> categorical columns --- .../src/sections/echartsTimeSeriesQuery.tsx | 23 ++++++++++++------- .../components/controls/XAxisSortControl.tsx | 17 +++++++++----- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx index dff8386f86f5..b6ea77cade04 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx @@ -90,20 +90,27 @@ const xAxisSort = { ]; return !isEqualArray(prevOptions, currOptions); }, - mapStateToProps: (state: ControlPanelState, controlState: ControlState) => { + mapStateToProps: ( + { controls }: { controls: ControlStateMapping }, + controlState: ControlState, + ) => { const choices = [ - getColumnLabel(state?.controls?.x_axis?.value as QueryFormColumn), - ...ensureIsArray(state?.controls?.metrics?.value).map(metric => + getColumnLabel(controls?.x_axis?.value as QueryFormColumn), + ...ensureIsArray(controls?.metrics?.value).map(metric => getMetricLabel(metric as QueryFormMetric), ), ].filter(Boolean); - const value = + const shouldReset = !( typeof controlState.value === 'string' && - choices.includes(controlState.value) - ? controlState.value - : undefined; + choices.includes(controlState.value) && + !isTemporalColumn( + getColumnLabel(controls?.x_axis?.value as QueryFormColumn), + controls?.datasource?.datasource, + ) + ); + return { - value, + shouldReset, options: choices.map(entry => ({ value: entry, label: entry, diff --git a/superset-frontend/src/explore/components/controls/XAxisSortControl.tsx b/superset-frontend/src/explore/components/controls/XAxisSortControl.tsx index 5c725bc5afdd..96afb3db98c8 100644 --- a/superset-frontend/src/explore/components/controls/XAxisSortControl.tsx +++ b/superset-frontend/src/explore/components/controls/XAxisSortControl.tsx @@ -16,16 +16,21 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useEffect } from 'react'; +import React, { useEffect, useState } from 'react'; import SelectControl from './SelectControl'; export default function XAxisSortControl(props: { - onChange: (val: string) => void; - value: string; + onChange: (val: string | undefined) => void; + value: string | undefined; + shouldReset: boolean; }) { + const [value, setValue] = useState(props.value); useEffect(() => { - props.onChange(props.value); - }, [props.value]); + if (props.shouldReset) { + props.onChange(undefined); + setValue(undefined); + } + }, [props.shouldReset, props.value]); - return ; + return ; } From 535018e254880b9d03687750e935ba7436df0c98 Mon Sep 17 00:00:00 2001 From: geido Date: Tue, 22 Nov 2022 19:12:25 +0200 Subject: [PATCH 09/14] Nullify select value --- .../src/explore/components/controls/XAxisSortControl.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/explore/components/controls/XAxisSortControl.tsx b/superset-frontend/src/explore/components/controls/XAxisSortControl.tsx index 96afb3db98c8..05d27dc0d436 100644 --- a/superset-frontend/src/explore/components/controls/XAxisSortControl.tsx +++ b/superset-frontend/src/explore/components/controls/XAxisSortControl.tsx @@ -21,14 +21,14 @@ import SelectControl from './SelectControl'; export default function XAxisSortControl(props: { onChange: (val: string | undefined) => void; - value: string | undefined; + value: string | null; shouldReset: boolean; }) { const [value, setValue] = useState(props.value); useEffect(() => { if (props.shouldReset) { props.onChange(undefined); - setValue(undefined); + setValue(null); } }, [props.shouldReset, props.value]); From 100ff1120a2cdc1123c4867274e64e92603f08e0 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Fri, 25 Nov 2022 17:54:48 +0800 Subject: [PATCH 10/14] fix conflict --- .../src/sections/echartsTimeSeriesQuery.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx index b6ea77cade04..28b0aeec48f6 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx @@ -35,7 +35,7 @@ import { isTemporalColumn, } from '@superset-ui/chart-controls'; import { ControlPanelSectionConfig, ControlSetRow } from '../types'; -import { emitFilterControl } from '../shared-controls/emitFilterControl'; +import { emitFilterControl } from '../shared-controls'; const controlsWithoutXAxis: ControlSetRow[] = [ ['metrics'], From e0566a19a1f97ac5c0ccb17964a6e477b790d906 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Sat, 26 Nov 2022 10:20:21 +0800 Subject: [PATCH 11/14] restructure controls --- .../src/sections/echartsTimeSeriesQuery.tsx | 101 +----------------- .../src/shared-controls/customControls.tsx | 90 +++++++++++++++- 2 files changed, 91 insertions(+), 100 deletions(-) diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx index 28b0aeec48f6..0a5d08bd8731 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx @@ -16,26 +16,9 @@ * specific language governing permissions and limitations * under the License. */ -import { - ContributionType, - ensureIsArray, - getColumnLabel, - getMetricLabel, - hasGenericChartAxes, - isDefined, - isEqualArray, - QueryFormColumn, - QueryFormMetric, - t, -} from '@superset-ui/core'; -import { - ControlPanelState, - ControlState, - ControlStateMapping, - isTemporalColumn, -} from '@superset-ui/chart-controls'; +import { ContributionType, hasGenericChartAxes, t } from '@superset-ui/core'; import { ControlPanelSectionConfig, ControlSetRow } from '../types'; -import { emitFilterControl } from '../shared-controls'; +import { emitFilterControl, xAxisSort, xAxisSortAsc } from '../shared-controls'; const controlsWithoutXAxis: ControlSetRow[] = [ ['metrics'], @@ -66,86 +49,6 @@ const controlsWithoutXAxis: ControlSetRow[] = [ ['show_empty_columns'], ]; -const xAxisSort = { - name: 'x_axis_sort', - config: { - type: 'XAxisSortControl', - label: t('X-Axis Sort By'), - description: t('Whether to sort descending or ascending on the X-Axis.'), - shouldMapStateToProps: ( - prevState: ControlPanelState, - state: ControlPanelState, - ) => { - const prevOptions = [ - getColumnLabel(prevState?.controls?.x_axis?.value as QueryFormColumn), - ...ensureIsArray(prevState?.controls?.metrics?.value).map(metric => - getMetricLabel(metric as QueryFormMetric), - ), - ]; - const currOptions = [ - getColumnLabel(state?.controls?.x_axis?.value as QueryFormColumn), - ...ensureIsArray(state?.controls?.metrics?.value).map(metric => - getMetricLabel(metric as QueryFormMetric), - ), - ]; - return !isEqualArray(prevOptions, currOptions); - }, - mapStateToProps: ( - { controls }: { controls: ControlStateMapping }, - controlState: ControlState, - ) => { - const choices = [ - getColumnLabel(controls?.x_axis?.value as QueryFormColumn), - ...ensureIsArray(controls?.metrics?.value).map(metric => - getMetricLabel(metric as QueryFormMetric), - ), - ].filter(Boolean); - const shouldReset = !( - typeof controlState.value === 'string' && - choices.includes(controlState.value) && - !isTemporalColumn( - getColumnLabel(controls?.x_axis?.value as QueryFormColumn), - controls?.datasource?.datasource, - ) - ); - - return { - shouldReset, - options: choices.map(entry => ({ - value: entry, - label: entry, - })), - }; - }, - visibility: ({ controls }: { controls: ControlStateMapping }) => - isDefined(controls?.x_axis?.value) && - !isTemporalColumn( - getColumnLabel(controls?.x_axis?.value as QueryFormColumn), - controls?.datasource?.datasource, - ) && - Array.isArray(controls?.groupby?.value) && - controls.groupby.value.length === 0, - }, -}; - -const xAxisSortAsc = { - name: 'x_axis_sort_asc', - config: { - type: 'CheckboxControl', - label: t('X-Axis Sort Ascending'), - default: true, - description: t('Whether to sort descending or ascending on the X-Axis.'), - visibility: ({ controls }: { controls: ControlStateMapping }) => - isDefined(controls?.x_axis?.value) && - !isTemporalColumn( - getColumnLabel(controls?.x_axis?.value as QueryFormColumn), - controls?.datasource?.datasource, - ) && - Array.isArray(controls?.groupby?.value) && - controls.groupby.value.length === 0, - }, -}; - export const echartsTimeSeriesQuery: ControlPanelSectionConfig = { label: t('Query'), expanded: true, diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx index 44773cf5d1cf..b88a8841a14c 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx @@ -17,7 +17,20 @@ * under the License. */ -import { FeatureFlag, isFeatureEnabled, t } from '@superset-ui/core'; +import { + ensureIsArray, + FeatureFlag, + getColumnLabel, + getMetricLabel, + isDefined, + isEqualArray, + isFeatureEnabled, + QueryFormColumn, + QueryFormMetric, + t, +} from '@superset-ui/core'; +import { ControlPanelState, ControlState, ControlStateMapping } from '../types'; +import { isTemporalColumn } from '../utils'; export const emitFilterControl = isFeatureEnabled( FeatureFlag.DASHBOARD_CROSS_FILTERS, @@ -35,3 +48,78 @@ export const emitFilterControl = isFeatureEnabled( }, ] : []; + +const xAxisSortVisibility = ({ controls }: { controls: ControlStateMapping }) => + isDefined(controls?.x_axis?.value) && + !isTemporalColumn( + getColumnLabel(controls?.x_axis?.value as QueryFormColumn), + controls?.datasource?.datasource, + ) && + Array.isArray(controls?.groupby?.value) && + controls.groupby.value.length === 0; + +export const xAxisSort = { + name: 'x_axis_sort', + config: { + type: 'XAxisSortControl', + label: t('X-Axis Sort By'), + description: t('Whether to sort descending or ascending on the X-Axis.'), + shouldMapStateToProps: ( + prevState: ControlPanelState, + state: ControlPanelState, + ) => { + const prevOptions = [ + getColumnLabel(prevState?.controls?.x_axis?.value as QueryFormColumn), + ...ensureIsArray(prevState?.controls?.metrics?.value).map(metric => + getMetricLabel(metric as QueryFormMetric), + ), + ]; + const currOptions = [ + getColumnLabel(state?.controls?.x_axis?.value as QueryFormColumn), + ...ensureIsArray(state?.controls?.metrics?.value).map(metric => + getMetricLabel(metric as QueryFormMetric), + ), + ]; + return !isEqualArray(prevOptions, currOptions); + }, + mapStateToProps: ( + { controls }: { controls: ControlStateMapping }, + controlState: ControlState, + ) => { + const choices = [ + getColumnLabel(controls?.x_axis?.value as QueryFormColumn), + ...ensureIsArray(controls?.metrics?.value).map(metric => + getMetricLabel(metric as QueryFormMetric), + ), + ].filter(Boolean); + const shouldReset = !( + typeof controlState.value === 'string' && + choices.includes(controlState.value) && + !isTemporalColumn( + getColumnLabel(controls?.x_axis?.value as QueryFormColumn), + controls?.datasource?.datasource, + ) + ); + + return { + shouldReset, + options: choices.map(entry => ({ + value: entry, + label: entry, + })), + }; + }, + visibility: xAxisSortVisibility, + }, +}; + +export const xAxisSortAsc = { + name: 'x_axis_sort_asc', + config: { + type: 'CheckboxControl', + label: t('X-Axis Sort Ascending'), + default: true, + description: t('Whether to sort descending or ascending on the X-Axis.'), + visibility: xAxisSortVisibility, + }, +}; From bbe5c3618fbc63e6ce83c5655392cbe2a33cd4b6 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Sat, 26 Nov 2022 13:29:57 +0800 Subject: [PATCH 12/14] fix sort operator fail query --- .../src/operators/sortOperator.ts | 9 ++++++ .../src/sections/echartsTimeSeriesQuery.tsx | 30 +++++++------------ .../src/shared-controls/customControls.tsx | 20 +++++++++++-- .../src/shared-controls/sharedControls.tsx | 4 +-- .../src/BoxPlot/buildQuery.ts | 2 +- .../src/BoxPlot/controlPanel.ts | 2 +- .../src/plugin/buildQuery.ts | 2 +- .../src/plugin/controlPanel.tsx | 2 +- .../plugin-chart-table/src/buildQuery.ts | 2 +- .../plugin-chart-table/src/controlPanel.tsx | 2 +- 10 files changed, 45 insertions(+), 30 deletions(-) diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts index 3844de88f4e1..b00f077e7b24 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts @@ -18,6 +18,8 @@ */ import { isEmpty } from 'lodash'; import { + ensureIsArray, + getMetricLabel, getXAxisLabel, hasGenericChartAxes, isDefined, @@ -29,10 +31,17 @@ export const sortOperator: PostProcessingFactory = ( formData, queryObject, ) => { + // the sortOperator only used in the barchart v2 + const labelsInMetricsAndXAxis = [ + getXAxisLabel(formData), + ...ensureIsArray(formData.metrics).map(metric => getMetricLabel(metric)), + ].filter(Boolean); + if ( hasGenericChartAxes && isDefined(formData?.x_axis_sort) && isDefined(formData?.x_axis_sort_asc) && + labelsInMetricsAndXAxis.includes(formData.x_axis_sort) && // the sort operator doesn't support sort-by multiple series. isEmpty(formData.groupby) ) { diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx index 0a5d08bd8731..296f8d9da820 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/sections/echartsTimeSeriesQuery.tsx @@ -16,29 +16,19 @@ * specific language governing permissions and limitations * under the License. */ -import { ContributionType, hasGenericChartAxes, t } from '@superset-ui/core'; +import { hasGenericChartAxes, t } from '@superset-ui/core'; import { ControlPanelSectionConfig, ControlSetRow } from '../types'; -import { emitFilterControl, xAxisSort, xAxisSortAsc } from '../shared-controls'; +import { + contributionModeControl, + emitFilterControl, + xAxisSortControl, + xAxisSortAscControl, +} from '../shared-controls'; const controlsWithoutXAxis: ControlSetRow[] = [ ['metrics'], ['groupby'], - [ - { - name: 'contributionMode', - config: { - type: 'SelectControl', - label: t('Contribution Mode'), - default: null, - choices: [ - [null, t('None')], - [ContributionType.Row, t('Row')], - [ContributionType.Column, t('Series')], - ], - description: t('Calculate contribution per series or row'), - }, - }, - ], + [contributionModeControl], ['adhoc_filters'], emitFilterControl, ['limit'], @@ -65,8 +55,8 @@ export const echartsTimeSeriesQueryWithXAxisSort: ControlPanelSectionConfig = { controlSetRows: [ [hasGenericChartAxes ? 'x_axis' : null], [hasGenericChartAxes ? 'time_grain_sqla' : null], - [hasGenericChartAxes ? xAxisSort : null], - [hasGenericChartAxes ? xAxisSortAsc : null], + [hasGenericChartAxes ? xAxisSortControl : null], + [hasGenericChartAxes ? xAxisSortAscControl : null], ...controlsWithoutXAxis, ], }; diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx index b88a8841a14c..5ece20ac0600 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx @@ -18,6 +18,7 @@ */ import { + ContributionType, ensureIsArray, FeatureFlag, getColumnLabel, @@ -49,6 +50,21 @@ export const emitFilterControl = isFeatureEnabled( ] : []; +export const contributionModeControl = { + name: 'contributionMode', + config: { + type: 'SelectControl', + label: t('Contribution Mode'), + default: null, + choices: [ + [null, t('None')], + [ContributionType.Row, t('Row')], + [ContributionType.Column, t('Series')], + ], + description: t('Calculate contribution per series or row'), + }, +}; + const xAxisSortVisibility = ({ controls }: { controls: ControlStateMapping }) => isDefined(controls?.x_axis?.value) && !isTemporalColumn( @@ -58,7 +74,7 @@ const xAxisSortVisibility = ({ controls }: { controls: ControlStateMapping }) => Array.isArray(controls?.groupby?.value) && controls.groupby.value.length === 0; -export const xAxisSort = { +export const xAxisSortControl = { name: 'x_axis_sort', config: { type: 'XAxisSortControl', @@ -113,7 +129,7 @@ export const xAxisSort = { }, }; -export const xAxisSortAsc = { +export const xAxisSortAscControl = { name: 'x_axis_sort_asc', config: { type: 'CheckboxControl', diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx index de94c4e16c92..11c4a4849039 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx @@ -354,7 +354,7 @@ const show_empty_columns: SharedControlConfig<'CheckboxControl'> = { description: t('Show empty columns'), }; -const datetime_columns_lookup: SharedControlConfig<'HiddenControl'> = { +const temporal_columns_lookup: SharedControlConfig<'HiddenControl'> = { type: 'HiddenControl', initialValue: (control: ControlState, state: ControlPanelState | null) => Object.fromEntries( @@ -400,5 +400,5 @@ export default { truncate_metric, x_axis: dndXAxisControl, show_empty_columns, - datetime_columns_lookup, + temporal_columns_lookup, }; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts index a897eea156ac..03b9058c8a3b 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts @@ -38,7 +38,7 @@ export default function buildQuery(formData: BoxPlotQueryFormData) { if ( isPhysicalColumn(col) && formData.time_grain_sqla && - formData?.datetime_columns_lookup?.[col] + formData?.temporal_columns_lookup?.[col] ) { return { timeGrain: formData.time_grain_sqla, diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts b/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts index fa9307ee042c..9e97459066e0 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts @@ -73,7 +73,7 @@ const config: ControlPanelConfig = { }, }, }, - 'datetime_columns_lookup', + 'temporal_columns_lookup', ], ['groupby'], ['metrics'], 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 b4bff7b45c05..6e33b41cc790 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 @@ -42,7 +42,7 @@ export default function buildQuery(formData: PivotTableQueryFormData) { isPhysicalColumn(col) && formData.time_grain_sqla && hasGenericChartAxes && - formData?.datetime_columns_lookup?.[col] + formData?.temporal_columns_lookup?.[col] ) { return { timeGrain: formData.time_grain_sqla, 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 7287d2b9b25e..a577fb6058d8 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 @@ -97,7 +97,7 @@ const config: ControlPanelConfig = { }, } : null, - hasGenericChartAxes ? 'datetime_columns_lookup' : null, + hasGenericChartAxes ? 'temporal_columns_lookup' : null, ], [ { diff --git a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts index 6e12123dce12..2c0f3385bd4e 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts @@ -104,7 +104,7 @@ const buildQuery: BuildQuery = ( isPhysicalColumn(col) && formData.time_grain_sqla && hasGenericChartAxes && - formData?.datetime_columns_lookup?.[col] + formData?.temporal_columns_lookup?.[col] ) { return { timeGrain: formData.time_grain_sqla, diff --git a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx index f2d3740a782a..e7e6b3d9852f 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx @@ -218,7 +218,7 @@ const config: ControlPanelConfig = { }, } : null, - hasGenericChartAxes && isAggMode ? 'datetime_columns_lookup' : null, + hasGenericChartAxes && isAggMode ? 'temporal_columns_lookup' : null, ], [ { From 9296c87e840bdf35369b11aa126826febca4ba0d Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Sat, 26 Nov 2022 13:41:52 +0800 Subject: [PATCH 13/14] fix ut in FE --- .../test/operators/sortOperator.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/operators/sortOperator.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/sortOperator.test.ts index 8046ab4a1a7e..750d726c902a 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/operators/sortOperator.test.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/test/operators/sortOperator.test.ts @@ -106,6 +106,7 @@ test('should sort by metric', () => { { ...formData, ...{ + metrics: ['a metric label'], x_axis_sort: 'a metric label', x_axis_sort_asc: true, }, From c225baaf5a1bc0884f2f64726e602f41ba74c1d4 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Sat, 26 Nov 2022 21:20:47 +0800 Subject: [PATCH 14/14] address comments --- .../superset-ui-chart-controls/src/operators/sortOperator.ts | 4 ++-- superset/utils/pandas_postprocessing/utils.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts index b00f077e7b24..0650c8b57785 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts @@ -32,7 +32,7 @@ export const sortOperator: PostProcessingFactory = ( queryObject, ) => { // the sortOperator only used in the barchart v2 - const labelsInMetricsAndXAxis = [ + const sortableLabels = [ getXAxisLabel(formData), ...ensureIsArray(formData.metrics).map(metric => getMetricLabel(metric)), ].filter(Boolean); @@ -41,7 +41,7 @@ export const sortOperator: PostProcessingFactory = ( hasGenericChartAxes && isDefined(formData?.x_axis_sort) && isDefined(formData?.x_axis_sort_asc) && - labelsInMetricsAndXAxis.includes(formData.x_axis_sort) && + sortableLabels.includes(formData.x_axis_sort) && // the sort operator doesn't support sort-by multiple series. isEmpty(formData.groupby) ) { diff --git a/superset/utils/pandas_postprocessing/utils.py b/superset/utils/pandas_postprocessing/utils.py index 3accf2f22977..c35af8b1a233 100644 --- a/superset/utils/pandas_postprocessing/utils.py +++ b/superset/utils/pandas_postprocessing/utils.py @@ -101,7 +101,7 @@ def _is_multi_index_on_columns(df: DataFrame) -> bool: return isinstance(df.columns, pd.MultiIndex) -def scalar_to_list(val: Any) -> Sequence[str]: +def scalar_to_sequence(val: Any) -> Sequence[str]: if val is None: return [] if isinstance(val, str): @@ -119,7 +119,7 @@ def wrapped(df: DataFrame, **options: Any) -> Any: columns = df.columns.tolist() for name in argnames: if name in options and not all( - elem in columns for elem in scalar_to_list(options.get(name)) + elem in columns for elem in scalar_to_sequence(options.get(name)) ): raise InvalidPostProcessingError( _("Referenced columns not available in DataFrame.")