diff --git a/.gitignore b/.gitignore index b3ef8b6db115..575f3672daf8 100644 --- a/.gitignore +++ b/.gitignore @@ -126,3 +126,4 @@ docker/*local* test-report.html superset/static/stats/statistics.html .aider* +.cursor/ \ No newline at end of file diff --git a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts index 7068ab119304..82e82c48c99d 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts @@ -123,9 +123,8 @@ const buildQuery: BuildQuery = ( if (sortByMetric) { orderby = [[sortByMetric, !orderDesc]]; } else if (metrics?.length > 0) { - // default to ordering by first metric in descending order - // when no "sort by" metric is set (regardless if "SORT DESC" is set to true) - orderby = [[metrics[0], false]]; + // default to ordering by first metric, respecting the "Sort descending" toggle + orderby = [[metrics[0], !orderDesc]]; } // add postprocessing for percent metrics only when in aggregation mode if (percentMetrics && percentMetrics.length > 0) { @@ -134,9 +133,9 @@ const buildQuery: BuildQuery = ( baseQueryObject, ) ? addComparisonPercentMetrics( - percentMetrics.map(getMetricLabel), - timeOffsets, - ) + percentMetrics.map(getMetricLabel), + timeOffsets, + ) : percentMetrics.map(getMetricLabel); const percentMetricLabels = removeDuplicates( percentMetricsLabelsWithTimeComparison, @@ -218,7 +217,7 @@ const buildQuery: BuildQuery = ( formData.server_pagination && options?.extras?.cachedChanges?.[formData.slice_id] && JSON.stringify(options?.extras?.cachedChanges?.[formData.slice_id]) !== - JSON.stringify(queryObject.filters) + JSON.stringify(queryObject.filters) ) { queryObject = { ...queryObject, row_offset: 0 }; updateExternalFormData( @@ -291,7 +290,7 @@ export const cachedBuildQuery = (): BuildQuery => { ownState: options?.ownState ?? {}, hooks: { ...options?.hooks, - setDataMask: () => {}, + setDataMask: () => { }, setCachedChanges, }, }, diff --git a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx index 933ee6a0c1d0..1453ae4eb6d3 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx @@ -122,8 +122,8 @@ const percentMetricsControl: typeof sharedControls.metrics = { label: t('Percentage metrics'), description: t( 'Select one or many metrics to display, that will be displayed in the percentages of total. ' + - 'Percentage metrics will be calculated only from data within the row limit. ' + - 'You can use an aggregation function on a column or write custom SQL to create a percentage metric.', + 'Percentage metrics will be calculated only from data within the row limit. ' + + 'You can use an aggregation function on a column or write custom SQL to create a percentage metric.', ), visibility: isAggMode, resetOnHide: false, @@ -270,8 +270,8 @@ const config: ControlPanelConfig = { ) => ({ columns: datasource?.columns[0]?.hasOwnProperty('filterable') ? (datasource as Dataset)?.columns?.filter( - (c: ColumnMeta) => c.filterable, - ) + (c: ColumnMeta) => c.filterable, + ) : datasource?.columns, savedMetrics: defineSavedMetrics(datasource), // current active adhoc metrics @@ -308,6 +308,28 @@ const config: ControlPanelConfig = { resetOnHide: false, }, }, + ], + [ + { + name: 'order_desc', + config: { + type: 'CheckboxControl', + label: t('Sort descending'), + default: true, + description: t( + 'If enabled, this control sorts the results/values descending, otherwise it sorts the results ascending.', + ), + visibility: ({ controls }: ControlPanelsContainerProps) => { + const sortMetric = controls?.timeseries_limit_metric?.value; + return Boolean( + isAggMode({ controls }) && sortMetric && !isEmpty(sortMetric), + ); + }, + resetOnHide: false, + }, + }, + ], + [ { name: 'order_by_cols', config: { @@ -362,21 +384,6 @@ const config: ControlPanelConfig = { }, }, ], - [ - { - name: 'order_desc', - config: { - type: 'CheckboxControl', - label: t('Sort descending'), - default: true, - description: t( - 'If enabled, this control sorts the results/values descending, otherwise it sorts the results ascending.', - ), - visibility: isAggMode, - resetOnHide: false, - }, - }, - ], [ { name: 'show_totals', @@ -582,8 +589,8 @@ const config: ControlPanelConfig = { default: false, description: t( 'This will be applied to the whole table. Arrows (↑ and ↓) will be added to ' + - 'main columns for increase and decrease. Basic conditional formatting can be ' + - 'overwritten by conditional formatting below.', + 'main columns for increase and decrease. Basic conditional formatting can be ' + + 'overwritten by conditional formatting below.', ), }, }, @@ -605,7 +612,7 @@ const config: ControlPanelConfig = { Boolean(controls?.comparison_color_enabled?.value), description: t( 'Adds color to the chart symbols based on the positive or ' + - 'negative change from the comparison value.', + 'negative change from the comparison value.', ), }, }, @@ -645,24 +652,24 @@ const config: ControlPanelConfig = { const numericColumns = Array.isArray(colnames) && Array.isArray(coltypes) ? colnames - .filter( - (colname: string, index: number) => - coltypes[index] === GenericDataType.Numeric, - ) - .map((colname: string) => ({ - value: colname, - label: Array.isArray(verboseMap) - ? colname - : (verboseMap[colname] ?? colname), - })) + .filter( + (colname: string, index: number) => + coltypes[index] === GenericDataType.Numeric, + ) + .map((colname: string) => ({ + value: colname, + label: Array.isArray(verboseMap) + ? colname + : (verboseMap[colname] ?? colname), + })) : []; const columnOptions = explore?.controls?.time_compare?.value ? processComparisonColumns( - numericColumns || [], - ensureIsArray( - explore?.controls?.time_compare?.value, - )[0]?.toString() || '', - ) + numericColumns || [], + ensureIsArray( + explore?.controls?.time_compare?.value, + )[0]?.toString() || '', + ) : numericColumns; return { diff --git a/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts b/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts index f110b424c9bb..6133dc1a4691 100644 --- a/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts +++ b/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts @@ -157,5 +157,23 @@ describe('plugin-chart-table', () => { expect(queries[1].extras?.time_grain_sqla).toBeUndefined(); expect(queries[1].extras?.where).toEqual("(status IN ('In Process'))"); }); + it('should respect order_desc when defaulting to first metric', () => { + const query = buildQuery({ + ...basicFormData, + query_mode: QueryMode.Aggregate, + metrics: ['aaa', 'bbb'], + order_desc: false, + }).queries[0]; + expect(query.orderby).toEqual([['aaa', true]]); + }); + it('should fall back to descending when order_desc is true', () => { + const query = buildQuery({ + ...basicFormData, + query_mode: QueryMode.Aggregate, + metrics: ['aaa', 'bbb'], + order_desc: true, + }).queries[0]; + expect(query.orderby).toEqual([['aaa', false]]); + }); }); }); diff --git a/superset-frontend/plugins/plugin-chart-table/test/controlPanel.test.ts b/superset-frontend/plugins/plugin-chart-table/test/controlPanel.test.ts new file mode 100644 index 000000000000..6e471cd201d7 --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-table/test/controlPanel.test.ts @@ -0,0 +1,72 @@ +/** + * 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 { QueryMode } from '@superset-ui/core'; +import config from '../src/controlPanel'; + +function getOrderDescVisibility() { + const [querySection] = config.controlPanelSections; + if (!querySection) { + throw new Error('Query section missing from table control panel'); + } + const orderDescControl = querySection.controlSetRows + .flat() + .find(control => control && control.name === 'order_desc'); + if (!orderDescControl) { + throw new Error('order_desc control not found'); + } + return orderDescControl.config.visibility!; +} + +describe('plugin-chart-table control panel', () => { + const visibility = getOrderDescVisibility(); + + it('hides Sort Descending when no metric is selected', () => { + expect( + visibility({ + controls: { + query_mode: { value: QueryMode.Aggregate }, + timeseries_limit_metric: { value: null }, + }, + } as any), + ).toBe(false); + }); + + it('shows Sort Descending when a metric is selected in aggregate mode', () => { + expect( + visibility({ + controls: { + query_mode: { value: QueryMode.Aggregate }, + timeseries_limit_metric: { value: 'metric_1' }, + }, + } as any), + ).toBe(true); + }); + + it('keeps Sort Descending hidden in raw mode even when a metric exists', () => { + expect( + visibility({ + controls: { + query_mode: { value: QueryMode.Raw }, + timeseries_limit_metric: { value: 'metric_1' }, + }, + } as any), + ).toBe(false); + }); +}); +