fix: Allows X-Axis Sort By for custom SQL (#30393)

superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx

@@ -39,6 +39,7 @@ import {
   SORT_SERIES_CHOICES,
 } from '../constants';
 import { checkColumnType } from '../utils/checkColumnType';
+import { isSortable } from '../utils/isSortable';
 
 export const contributionModeControl = {
   name: 'contributionMode',
@@ -55,27 +56,6 @@ export const contributionModeControl = {
   },
 };
 
-function isForcedCategorical(controls: ControlStateMapping): boolean {
-  return (
-    checkColumnType(
-      getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
-      controls?.datasource?.datasource,
-      [GenericDataType.Numeric],
-    ) && !!controls?.xAxisForceCategorical?.value
-  );
-}
-
-function isSortable(controls: ControlStateMapping): boolean {
-  return (
-    isForcedCategorical(controls) ||
-    checkColumnType(
-      getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
-      controls?.datasource?.datasource,
-      [GenericDataType.String, GenericDataType.Boolean],
-    )
-  );
-}
-
 const xAxisSortVisibility = ({ controls }: { controls: ControlStateMapping }) =>
   isSortable(controls) &&
   ensureIsArray(controls?.groupby?.value).length === 0 &&

superset-frontend/packages/superset-ui-chart-controls/src/utils/isSortable.ts

@@ -0,0 +1,51 @@
+/**
+ * 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 {
+  GenericDataType,
+  getColumnLabel,
+  isPhysicalColumn,
+  QueryFormColumn,
+} from '@superset-ui/core';
+import { checkColumnType, ControlStateMapping } from '..';
+
+export function isSortable(controls: ControlStateMapping): boolean {
+  const isForcedCategorical =
+    checkColumnType(
+      getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
+      controls?.datasource?.datasource,
+      [GenericDataType.Numeric],
+    ) && !!controls?.xAxisForceCategorical?.value;
+
+  const xAxisValue = controls?.x_axis?.value as QueryFormColumn;
+
+  // Given that we don't know the type of a custom SQL column,
+  // we treat it as sortable and give the responsibility to the
+  // user to provide a sortable result.
+  const isCustomSQL = !isPhysicalColumn(xAxisValue);
+
+  return (
+    isForcedCategorical ||
+    isCustomSQL ||
+    checkColumnType(
+      getColumnLabel(xAxisValue),
+      controls?.datasource?.datasource,
+      [GenericDataType.String, GenericDataType.Boolean],
+    )
+  );
+}

superset-frontend/packages/superset-ui-chart-controls/test/utils/isSortable.test.ts

@@ -0,0 +1,70 @@
+/**
+ * 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 { ControlStateMapping } from '@superset-ui/chart-controls';
+import { GenericDataType } from '@superset-ui/core';
+import { isSortable } from '../../src/utils/isSortable';
+
+const controls: ControlStateMapping = {
+  datasource: {
+    datasource: {
+      columns: [
+        { column_name: 'a', type_generic: GenericDataType.String },
+        { column_name: 'b', type_generic: GenericDataType.Numeric },
+        { column_name: 'c', type_generic: GenericDataType.Boolean },
+      ],
+    },
+    type: 'Select',
+  },
+};
+
+test('should return true if the column is forced to be categorical', () => {
+  const c: ControlStateMapping = {
+    ...controls,
+    x_axis: { value: 'b', type: 'Select' },
+    xAxisForceCategorical: { value: true, type: 'Checkbox' },
+  };
+  expect(isSortable(c)).toBe(true);
+});
+
+test('should return true if the column is a custom SQL column', () => {
+  const c: ControlStateMapping = {
+    ...controls,
+    x_axis: {
+      value: { label: 'custom_sql', sqlExpression: 'MAX(ID)' },
+      type: 'Select',
+    },
+  };
+  expect(isSortable(c)).toBe(true);
+});
+
+test('should return true if the column type is String or Boolean', () => {
+  const c: ControlStateMapping = {
+    ...controls,
+    x_axis: { value: 'c', type: 'Checkbox' },
+  };
+  expect(isSortable(c)).toBe(true);
+});
+
+test('should return false if none of the conditions are met', () => {
+  const c: ControlStateMapping = {
+    ...controls,
+    x_axis: { value: 'b', type: 'Input' },
+  };
+  expect(isSortable(c)).toBe(false);
+});