diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx index 50d40f94296..1277a8f7f38 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx @@ -683,16 +683,6 @@ const config: ControlPanelConfig = { type: 'ConditionalFormattingControl', renderTrigger: true, label: t('Custom conditional formatting'), - extraColorChoices: [ - { - value: ColorSchemeEnum.Green, - label: t('Green for increase, red for decrease'), - }, - { - value: ColorSchemeEnum.Red, - label: t('Red for increase, green for decrease'), - }, - ], description: t( 'Apply conditional color formatting to numeric columns', ), @@ -705,6 +695,22 @@ const config: ControlPanelConfig = { ) ? (explore?.datasource as Dataset)?.verbose_map : (explore?.datasource?.columns ?? {}); + const timeCompareValue = explore?.controls?.time_compare?.value; + const hasTimeComparison = !isEmpty(timeCompareValue); + + const extraColorChoices = hasTimeComparison + ? [ + { + value: ColorSchemeEnum.Green, + label: t('Green for increase, red for decrease'), + }, + { + value: ColorSchemeEnum.Red, + label: t('Red for increase, green for decrease'), + }, + ] + : []; + const chartStatus = chart?.chartStatus; const { colnames, coltypes } = chart?.queriesResponse?.[0] ?? {}; @@ -724,12 +730,10 @@ const config: ControlPanelConfig = { colnames && coltypes[colnames?.indexOf(colname)], })) : []; - const columnOptions = explore?.controls?.time_compare?.value + const columnOptions = hasTimeComparison ? processComparisonColumns( numericColumns || [], - ensureIsArray( - explore?.controls?.time_compare?.value, - )[0]?.toString() || '', + ensureIsArray(timeCompareValue)[0]?.toString() || '', ) : numericColumns; @@ -737,6 +741,7 @@ const config: ControlPanelConfig = { removeIrrelevantConditions: chartStatus === 'success', columnOptions, verboseMap, + extraColorChoices, }; }, }, diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/controlPanel.test.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/controlPanel.test.tsx new file mode 100644 index 00000000000..70501b17bdd --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/controlPanel.test.tsx @@ -0,0 +1,208 @@ +/** + * 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 } from '@apache-superset/core/api/core'; +import { QueryFormData } from '@superset-ui/core'; +import { + Dataset, + isCustomControlItem, + ControlConfig, + ControlPanelState, + ControlState, +} from '@superset-ui/chart-controls'; +import config from '../src/controlPanel'; +import { ColorSchemeEnum } from '../src/types'; + +const findConditionalFormattingControl = (): ControlConfig | null => { + for (const section of config.controlPanelSections) { + if (!section) continue; + for (const row of section.controlSetRows) { + for (const control of row) { + if ( + isCustomControlItem(control) && + control.name === 'conditional_formatting' + ) { + return control.config; + } + } + } + } + return null; +}; + +const createMockControlState = (value: string[] | undefined): ControlState => ({ + type: 'SelectControl', + value, + label: '', + default: undefined, + renderTrigger: false, +}); + +const createMockExplore = ( + timeCompareValue: string[] | undefined, +): ControlPanelState => ({ + slice: { slice_id: 123 }, + datasource: { + verbose_map: { col1: 'Column 1', col2: 'Column 2' }, + columns: [], + } as Partial as Dataset, + controls: { + time_compare: createMockControlState(timeCompareValue), + }, + form_data: { + time_compare: timeCompareValue, + datasource: 'test', + viz_type: 'table', + } as QueryFormData, + common: {}, + metadata: {}, +}); + +const createMockChart = () => ({ + chartStatus: 'success' as const, + queriesResponse: [ + { + colnames: ['col1', 'col2'], + coltypes: [GenericDataType.Numeric, GenericDataType.Numeric], + }, + ], +}); + +const createMockControlStateForConditionalFormatting = (): ControlState => ({ + type: 'CollectionControl', + value: [], + label: '', + default: undefined, + renderTrigger: false, +}); + +test('extraColorChoices not included when time comparison is disabled', () => { + const controlConfig = findConditionalFormattingControl(); + expect(controlConfig).toBeTruthy(); + expect(controlConfig?.mapStateToProps).toBeTruthy(); + + const explore = createMockExplore(undefined); + const chart = createMockChart(); + const result = controlConfig!.mapStateToProps!( + explore, + createMockControlStateForConditionalFormatting(), + chart, + ); + + expect(result.extraColorChoices).toEqual([]); + expect(result.columnOptions).toEqual( + expect.arrayContaining([ + expect.objectContaining({ value: 'col1' }), + expect.objectContaining({ value: 'col2' }), + ]), + ); +}); + +test('extraColorChoices included when time comparison is enabled', () => { + const controlConfig = findConditionalFormattingControl(); + expect(controlConfig).toBeTruthy(); + + const explore = createMockExplore(['P1D']); + const chart = createMockChart(); + const result = controlConfig!.mapStateToProps!( + explore, + createMockControlStateForConditionalFormatting(), + chart, + ); + + expect(result.extraColorChoices).toEqual([ + { + value: ColorSchemeEnum.Green, + label: expect.stringContaining('Green for increase'), + }, + { + value: ColorSchemeEnum.Red, + label: expect.stringContaining('Red for increase'), + }, + ]); + expect(result.columnOptions).not.toEqual( + expect.arrayContaining([expect.objectContaining({ value: 'col1' })]), + ); +}); + +test('extraColorChoices not included when time_compare is empty array', () => { + const controlConfig = findConditionalFormattingControl(); + expect(controlConfig).toBeTruthy(); + + const explore = createMockExplore([]); + const chart = createMockChart(); + const result = controlConfig!.mapStateToProps!( + explore, + createMockControlStateForConditionalFormatting(), + chart, + ); + + expect(result.extraColorChoices).toEqual([]); +}); + +test('consistency between extraColorChoices and columnOptions', () => { + const controlConfig = findConditionalFormattingControl(); + expect(controlConfig).toBeTruthy(); + + const explore = createMockExplore(['P1D']); + const chart = createMockChart(); + const result = controlConfig!.mapStateToProps!( + explore, + createMockControlStateForConditionalFormatting(), + chart, + ); + + const hasExtraColorChoices = result.extraColorChoices.length > 0; + const hasComparisonColumns = result.columnOptions.some( + (col: { value: string }) => + col.value.includes('Main') || + col.value.includes('#') || + col.value.includes('△'), + ); + + expect(hasExtraColorChoices).toBe(true); + expect(hasComparisonColumns).toBe(true); +}); + +test('uses controls.time_compare.value not form_data.time_compare', () => { + const controlConfig = findConditionalFormattingControl(); + expect(controlConfig).toBeTruthy(); + + const explore: ControlPanelState = { + ...createMockExplore(undefined), + form_data: { + ...createMockExplore(undefined).form_data, + time_compare: ['P1D'], + }, + }; + const chart = createMockChart(); + const result = controlConfig!.mapStateToProps!( + explore, + createMockControlStateForConditionalFormatting(), + chart, + ); + + expect(result.extraColorChoices).toEqual([]); +}); + +test('static extraColorChoices removed from config', () => { + const controlConfig = findConditionalFormattingControl(); + expect(controlConfig).toBeTruthy(); + + expect(controlConfig?.extraColorChoices).toBeUndefined(); +}); diff --git a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx index cdff93eb8e7..2d051cf97c9 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx @@ -746,16 +746,6 @@ const config: ControlPanelConfig = { type: 'ConditionalFormattingControl', renderTrigger: true, label: t('Custom conditional formatting'), - extraColorChoices: [ - { - value: ColorSchemeEnum.Green, - label: t('Green for increase, red for decrease'), - }, - { - value: ColorSchemeEnum.Red, - label: t('Red for increase, green for decrease'), - }, - ], description: t( 'Apply conditional color formatting to numeric columns', ), @@ -768,6 +758,22 @@ const config: ControlPanelConfig = { ) ? (explore?.datasource as Dataset)?.verbose_map : (explore?.datasource?.columns ?? {}); + const timeCompareValue = explore?.controls?.time_compare?.value; + const hasTimeComparison = !isEmpty(timeCompareValue); + + const extraColorChoices = hasTimeComparison + ? [ + { + value: ColorSchemeEnum.Green, + label: t('Green for increase, red for decrease'), + }, + { + value: ColorSchemeEnum.Red, + label: t('Red for increase, green for decrease'), + }, + ] + : []; + const chartStatus = chart?.chartStatus; const value = _?.value ?? []; if (value && Array.isArray(value)) { @@ -796,7 +802,7 @@ const config: ControlPanelConfig = { ? colnames.reduce((acc, colname, index) => { if ( coltypes[index] === GenericDataType.Numeric || - (!explore?.controls?.time_compare?.value && + (!hasTimeComparison && (coltypes[index] === GenericDataType.String || coltypes[index] === GenericDataType.Boolean)) ) { @@ -811,12 +817,10 @@ const config: ControlPanelConfig = { return acc; }, []) : []; - const columnOptions = explore?.controls?.time_compare?.value + const columnOptions = hasTimeComparison ? processComparisonColumns( numericColumns || [], - ensureIsArray( - explore?.controls?.time_compare?.value, - )[0]?.toString() || '', + ensureIsArray(timeCompareValue)[0]?.toString() || '', ) : numericColumns; @@ -828,6 +832,7 @@ const config: ControlPanelConfig = { toAllRowCheck: true, toColorTextCheck: true, }, + extraColorChoices, }; }, }, diff --git a/superset-frontend/plugins/plugin-chart-table/test/controlPanel.test.tsx b/superset-frontend/plugins/plugin-chart-table/test/controlPanel.test.tsx new file mode 100644 index 00000000000..70501b17bdd --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-table/test/controlPanel.test.tsx @@ -0,0 +1,208 @@ +/** + * 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 } from '@apache-superset/core/api/core'; +import { QueryFormData } from '@superset-ui/core'; +import { + Dataset, + isCustomControlItem, + ControlConfig, + ControlPanelState, + ControlState, +} from '@superset-ui/chart-controls'; +import config from '../src/controlPanel'; +import { ColorSchemeEnum } from '../src/types'; + +const findConditionalFormattingControl = (): ControlConfig | null => { + for (const section of config.controlPanelSections) { + if (!section) continue; + for (const row of section.controlSetRows) { + for (const control of row) { + if ( + isCustomControlItem(control) && + control.name === 'conditional_formatting' + ) { + return control.config; + } + } + } + } + return null; +}; + +const createMockControlState = (value: string[] | undefined): ControlState => ({ + type: 'SelectControl', + value, + label: '', + default: undefined, + renderTrigger: false, +}); + +const createMockExplore = ( + timeCompareValue: string[] | undefined, +): ControlPanelState => ({ + slice: { slice_id: 123 }, + datasource: { + verbose_map: { col1: 'Column 1', col2: 'Column 2' }, + columns: [], + } as Partial as Dataset, + controls: { + time_compare: createMockControlState(timeCompareValue), + }, + form_data: { + time_compare: timeCompareValue, + datasource: 'test', + viz_type: 'table', + } as QueryFormData, + common: {}, + metadata: {}, +}); + +const createMockChart = () => ({ + chartStatus: 'success' as const, + queriesResponse: [ + { + colnames: ['col1', 'col2'], + coltypes: [GenericDataType.Numeric, GenericDataType.Numeric], + }, + ], +}); + +const createMockControlStateForConditionalFormatting = (): ControlState => ({ + type: 'CollectionControl', + value: [], + label: '', + default: undefined, + renderTrigger: false, +}); + +test('extraColorChoices not included when time comparison is disabled', () => { + const controlConfig = findConditionalFormattingControl(); + expect(controlConfig).toBeTruthy(); + expect(controlConfig?.mapStateToProps).toBeTruthy(); + + const explore = createMockExplore(undefined); + const chart = createMockChart(); + const result = controlConfig!.mapStateToProps!( + explore, + createMockControlStateForConditionalFormatting(), + chart, + ); + + expect(result.extraColorChoices).toEqual([]); + expect(result.columnOptions).toEqual( + expect.arrayContaining([ + expect.objectContaining({ value: 'col1' }), + expect.objectContaining({ value: 'col2' }), + ]), + ); +}); + +test('extraColorChoices included when time comparison is enabled', () => { + const controlConfig = findConditionalFormattingControl(); + expect(controlConfig).toBeTruthy(); + + const explore = createMockExplore(['P1D']); + const chart = createMockChart(); + const result = controlConfig!.mapStateToProps!( + explore, + createMockControlStateForConditionalFormatting(), + chart, + ); + + expect(result.extraColorChoices).toEqual([ + { + value: ColorSchemeEnum.Green, + label: expect.stringContaining('Green for increase'), + }, + { + value: ColorSchemeEnum.Red, + label: expect.stringContaining('Red for increase'), + }, + ]); + expect(result.columnOptions).not.toEqual( + expect.arrayContaining([expect.objectContaining({ value: 'col1' })]), + ); +}); + +test('extraColorChoices not included when time_compare is empty array', () => { + const controlConfig = findConditionalFormattingControl(); + expect(controlConfig).toBeTruthy(); + + const explore = createMockExplore([]); + const chart = createMockChart(); + const result = controlConfig!.mapStateToProps!( + explore, + createMockControlStateForConditionalFormatting(), + chart, + ); + + expect(result.extraColorChoices).toEqual([]); +}); + +test('consistency between extraColorChoices and columnOptions', () => { + const controlConfig = findConditionalFormattingControl(); + expect(controlConfig).toBeTruthy(); + + const explore = createMockExplore(['P1D']); + const chart = createMockChart(); + const result = controlConfig!.mapStateToProps!( + explore, + createMockControlStateForConditionalFormatting(), + chart, + ); + + const hasExtraColorChoices = result.extraColorChoices.length > 0; + const hasComparisonColumns = result.columnOptions.some( + (col: { value: string }) => + col.value.includes('Main') || + col.value.includes('#') || + col.value.includes('△'), + ); + + expect(hasExtraColorChoices).toBe(true); + expect(hasComparisonColumns).toBe(true); +}); + +test('uses controls.time_compare.value not form_data.time_compare', () => { + const controlConfig = findConditionalFormattingControl(); + expect(controlConfig).toBeTruthy(); + + const explore: ControlPanelState = { + ...createMockExplore(undefined), + form_data: { + ...createMockExplore(undefined).form_data, + time_compare: ['P1D'], + }, + }; + const chart = createMockChart(); + const result = controlConfig!.mapStateToProps!( + explore, + createMockControlStateForConditionalFormatting(), + chart, + ); + + expect(result.extraColorChoices).toEqual([]); +}); + +test('static extraColorChoices removed from config', () => { + const controlConfig = findConditionalFormattingControl(); + expect(controlConfig).toBeTruthy(); + + expect(controlConfig?.extraColorChoices).toBeUndefined(); +});