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..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 @@ -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,50 @@ * specific language governing permissions and limitationsxw * under the License. */ -import { DTTM_ALIAS, PostProcessingSort, RollingType } from '@superset-ui/core'; +import { isEmpty } from 'lodash'; +import { + ensureIsArray, + getMetricLabel, + getXAxisLabel, + hasGenericChartAxes, + isDefined, + PostProcessingSort, +} from '@superset-ui/core'; import { PostProcessingFactory } from './types'; export const sortOperator: PostProcessingFactory = ( formData, queryObject, ) => { - const { x_axis: xAxis } = formData; + // the sortOperator only used in the barchart v2 + const sortableLabels = [ + getXAxisLabel(formData), + ...ensureIsArray(formData.metrics).map(metric => getMetricLabel(metric)), + ].filter(Boolean); + if ( - (xAxis || queryObject.is_timeseries) && - Object.values(RollingType).includes(formData.rolling_type) + hasGenericChartAxes && + isDefined(formData?.x_axis_sort) && + isDefined(formData?.x_axis_sort_asc) && + sortableLabels.includes(formData.x_axis_sort) && + // the sort operator doesn't support sort-by multiple series. + isEmpty(formData.groupby) ) { - const index = xAxis || DTTM_ALIAS; + if (formData.x_axis_sort === getXAxisLabel(formData)) { + return { + operation: 'sort', + options: { + is_sort_index: true, + ascending: formData.x_axis_sort_asc, + }, + }; + } + return { operation: 'sort', options: { - columns: { - [index]: true, - }, + by: formData.x_axis_sort, + ascending: formData.x_axis_sort_asc, }, }; } 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..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,9 +16,28 @@ * 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 { hasGenericChartAxes, t } from '@superset-ui/core'; +import { ControlPanelSectionConfig, ControlSetRow } from '../types'; +import { + contributionModeControl, + emitFilterControl, + xAxisSortControl, + xAxisSortAscControl, +} from '../shared-controls'; + +const controlsWithoutXAxis: ControlSetRow[] = [ + ['metrics'], + ['groupby'], + [contributionModeControl], + ['adhoc_filters'], + emitFilterControl, + ['limit'], + ['timeseries_limit_metric'], + ['order_desc'], + ['row_limit'], + ['truncate_metric'], + ['show_empty_columns'], +]; export const echartsTimeSeriesQuery: ControlPanelSectionConfig = { label: t('Query'), @@ -26,31 +45,18 @@ 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 ? 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 44773cf5d1cf..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 @@ -17,7 +17,21 @@ * under the License. */ -import { FeatureFlag, isFeatureEnabled, t } from '@superset-ui/core'; +import { + ContributionType, + 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 +49,93 @@ 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( + getColumnLabel(controls?.x_axis?.value as QueryFormColumn), + controls?.datasource?.datasource, + ) && + Array.isArray(controls?.groupby?.value) && + controls.groupby.value.length === 0; + +export const xAxisSortControl = { + 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 xAxisSortAscControl = { + 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, + }, +}; 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/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..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 @@ -18,6 +18,7 @@ */ import { QueryObject, SqlaFormData } 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,96 @@ 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: undefined, + x_axis_sort_asc: 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 doesn't support multiple series + 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: 'metric label', + x_axis_sort_asc: true, + groupby: ['col1'], + x_axis: 'axis column', + }, }, - }, - }); - - expect( - sortOperator( - { ...formData, rolling_type: 'mean' }, - { ...queryObject, is_timeseries: true }, + queryObject, ), - ).toEqual({ - operation: 'sort', - options: { - columns: { - __timestamp: true, - }, - }, - }); + ).toEqual(undefined); +}); +test('should sort by metric', () => { + Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', { + value: true, + }); expect( sortOperator( - { ...formData, rolling_type: 'std' }, - { ...queryObject, is_timeseries: true }, + { + ...formData, + ...{ + metrics: ['a metric label'], + x_axis_sort: 'a metric label', + x_axis_sort_asc: 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: 'Categorical Column', + x_axis_sort_asc: 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/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/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-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/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, ], [ { 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..05d27dc0d436 --- /dev/null +++ b/superset-frontend/src/explore/components/controls/XAxisSortControl.tsx @@ -0,0 +1,36 @@ +/** + * 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, useState } from 'react'; +import SelectControl from './SelectControl'; + +export default function XAxisSortControl(props: { + onChange: (val: string | undefined) => void; + value: string | null; + shouldReset: boolean; +}) { + const [value, setValue] = useState(props.value); + useEffect(() => { + if (props.shouldReset) { + props.onChange(undefined); + setValue(null); + } + }, [props.shouldReset, 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; diff --git a/superset/utils/pandas_postprocessing/sort.py b/superset/utils/pandas_postprocessing/sort.py index feacfb68ea1d..66041a7166b9 100644 --- a/superset/utils/pandas_postprocessing/sort.py +++ b/superset/utils/pandas_postprocessing/sort.py @@ -14,22 +14,34 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from typing import Dict +from typing import List, Optional, Union from pandas import DataFrame from superset.utils.pandas_postprocessing.utils import validate_column_args -@validate_column_args("columns") -def sort(df: DataFrame, columns: Dict[str, bool]) -> DataFrame: +# pylint: disable=invalid-name +@validate_column_args("by") +def sort( + df: DataFrame, + is_sort_index: bool = False, + by: Optional[Union[List[str], str]] = None, + ascending: Union[List[bool], bool] = True, +) -> DataFrame: """ Sort a DataFrame. :param df: DataFrame to sort. - :param columns: columns by by which to sort. The key specifies the column name, - value specifies if sorting in ascending order. + :param is_sort_index: Whether by index or value to sort + :param by: Name or list of names to sort by. + :param ascending: Sort ascending or descending. :return: Sorted DataFrame :raises InvalidPostProcessingError: If the request in incorrect """ - return df.sort_values(by=list(columns.keys()), ascending=list(columns.values())) + if not is_sort_index and not by: + return df + + if is_sort_index: + return df.sort_index(ascending=ascending) + return df.sort_values(by=by, ascending=ascending) diff --git a/superset/utils/pandas_postprocessing/utils.py b/superset/utils/pandas_postprocessing/utils.py index bff62dcb64b8..c35af8b1a233 100644 --- a/superset/utils/pandas_postprocessing/utils.py +++ b/superset/utils/pandas_postprocessing/utils.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. from functools import partial -from typing import Any, Callable, Dict +from typing import Any, Callable, Dict, Sequence import numpy as np import pandas as pd @@ -101,6 +101,14 @@ def _is_multi_index_on_columns(df: DataFrame) -> bool: return isinstance(df.columns, pd.MultiIndex) +def scalar_to_sequence(val: Any) -> Sequence[str]: + if val is None: + return [] + if isinstance(val, str): + return [val] + return val + + def validate_column_args(*argnames: str) -> Callable[..., Any]: def wrapper(func: Callable[..., Any]) -> Callable[..., Any]: def wrapped(df: DataFrame, **options: Any) -> Any: @@ -111,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 options.get(name) or [] + elem in columns for elem in scalar_to_sequence(options.get(name)) ): raise InvalidPostProcessingError( _("Referenced columns not available in DataFrame.") diff --git a/tests/common/query_context_generator.py b/tests/common/query_context_generator.py index a22646983bdd..15b013dc845c 100644 --- a/tests/common/query_context_generator.py +++ b/tests/common/query_context_generator.py @@ -195,9 +195,7 @@ }, { "operation": "sort", - "options": { - "columns": {"q1": False, "name": True}, - }, + "options": {"by": ["q1", "name"], "ascending": [False, True]}, }, ] } diff --git a/tests/unit_tests/pandas_postprocessing/test_sort.py b/tests/unit_tests/pandas_postprocessing/test_sort.py index c489381ff16f..e19da38efc1e 100644 --- a/tests/unit_tests/pandas_postprocessing/test_sort.py +++ b/tests/unit_tests/pandas_postprocessing/test_sort.py @@ -15,16 +15,39 @@ # specific language governing permissions and limitations # under the License. import pytest +from dateutil.parser import parse from superset.exceptions import InvalidPostProcessingError from superset.utils.pandas_postprocessing import sort -from tests.unit_tests.fixtures.dataframes import categories_df +from tests.unit_tests.fixtures.dataframes import categories_df, timeseries_df from tests.unit_tests.pandas_postprocessing.utils import series_to_list def test_sort(): - df = sort(df=categories_df, columns={"category": True, "asc_idx": False}) + df = sort(df=categories_df, by=["category", "asc_idx"], ascending=[True, False]) assert series_to_list(df["asc_idx"])[1] == 96 + df = sort(df=categories_df.set_index("name"), is_sort_index=True) + assert df.index[0] == "person0" + + df = sort(df=categories_df.set_index("name"), is_sort_index=True, ascending=False) + assert df.index[0] == "person99" + + df = sort(df=categories_df.set_index("name"), by="asc_idx") + assert df["asc_idx"][0] == 0 + + df = sort(df=categories_df.set_index("name"), by="asc_idx", ascending=False) + assert df["asc_idx"][0] == 100 + + df = sort(df=timeseries_df, is_sort_index=True) + assert df.index[0] == parse("2019-01-01") + + df = sort(df=timeseries_df, is_sort_index=True, ascending=False) + assert df.index[0] == parse("2019-01-07") + + df = sort(df=timeseries_df) + assert df.equals(timeseries_df) + with pytest.raises(InvalidPostProcessingError): - sort(df=df, columns={"abc": True}) + sort(df=df, by="abc", ascending=False) + sort(df=df, by=["abc", "def"])