fix(plugin-chart-table): improve orderby logic and add tests for orde…

.gitignore

@@ -126,3 +126,4 @@ docker/*local*
 test-report.html
 superset/static/stats/statistics.html
 .aider*
+.cursor/

superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts

@@ -123,9 +123,8 @@ const buildQuery: BuildQuery<TableChartFormData> = (
       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<TableChartFormData> = (
           baseQueryObject,
         )
           ? addComparisonPercentMetrics(
-              percentMetrics.map(getMetricLabel),
-              timeOffsets,
-            )
+            percentMetrics.map(getMetricLabel),
+            timeOffsets,
+          )
           : percentMetrics.map(getMetricLabel);
         const percentMetricLabels = removeDuplicates(
           percentMetricsLabelsWithTimeComparison,
@@ -218,7 +217,7 @@ const buildQuery: BuildQuery<TableChartFormData> = (
       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<TableChartFormData> => {
         ownState: options?.ownState ?? {},
         hooks: {
           ...options?.hooks,
-          setDataMask: () => {},
+          setDataMask: () => { },
           setCachedChanges,
         },
       },

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 {

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]]);
+    });
   });
 });

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);
+    });
+});
+