From c864e6cd2ba5c4b26e3d8eaf55349163331ad033 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com> Date: Wed, 25 Sep 2024 19:08:28 -0300 Subject: [PATCH] fix: Allows X-Axis Sort By for custom SQL (#30393) (cherry picked from commit abf2943e4d8910f486f738ada22cbd5da1f7487d) --- .../src/shared-controls/customControls.tsx | 22 +----- .../src/utils/isSortable.ts | 51 ++++++++++++++ .../test/utils/isSortable.test.ts | 70 +++++++++++++++++++ 3 files changed, 122 insertions(+), 21 deletions(-) create mode 100644 superset-frontend/packages/superset-ui-chart-controls/src/utils/isSortable.ts create mode 100644 superset-frontend/packages/superset-ui-chart-controls/test/utils/isSortable.test.ts 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 7fb4f9a8b9e4b..d25273c08e9be 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 @@ -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 && diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/utils/isSortable.ts b/superset-frontend/packages/superset-ui-chart-controls/src/utils/isSortable.ts new file mode 100644 index 0000000000000..65b07ec91dc5a --- /dev/null +++ b/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], + ) + ); +} diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/utils/isSortable.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/utils/isSortable.test.ts new file mode 100644 index 0000000000000..0ef9844f99507 --- /dev/null +++ b/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); +});