From 26b26e46e477265b146da4538e66702423b95be1 Mon Sep 17 00:00:00 2001 From: Sonia Gautam <96282005+soniagtm@users.noreply.github.com> Date: Fri, 5 Jan 2024 02:31:24 +0700 Subject: [PATCH] fix(chart): Resolve incorrect column customization when switching metrics in table chart (#26393) Co-authored-by: Sonia (cherry picked from commit dfde2adf27faef3e73c44e7280e77d9d3928dc97) --- .../explore/actions/exploreActions.test.js | 180 ++++++++++++++++++ .../src/explore/reducers/exploreReducer.js | 5 + 2 files changed, 185 insertions(+) diff --git a/superset-frontend/src/explore/actions/exploreActions.test.js b/superset-frontend/src/explore/actions/exploreActions.test.js index 9dd53756800d1..54cf8f16c5c35 100644 --- a/superset-frontend/src/explore/actions/exploreActions.test.js +++ b/superset-frontend/src/explore/actions/exploreActions.test.js @@ -21,6 +21,63 @@ import { defaultState } from 'src/explore/store'; import exploreReducer from 'src/explore/reducers/exploreReducer'; import * as actions from 'src/explore/actions/exploreActions'; +const METRICS = [ + { + expressionType: 'SIMPLE', + column: { + advanced_data_type: null, + certification_details: null, + certified_by: null, + column_name: 'a', + description: null, + expression: null, + filterable: true, + groupby: true, + id: 1, + is_certified: false, + is_dttm: false, + python_date_format: null, + type: 'DOUBLE PRECISION', + type_generic: 0, + verbose_name: null, + warning_markdown: null, + }, + aggregate: 'SUM', + sqlExpression: null, + datasourceWarning: false, + hasCustomLabel: false, + label: 'SUM(a)', + optionName: 'metric_1a2b3c4d5f_1a2b3c4d5f', + }, + { + expressionType: 'SIMPLE', + column: { + advanced_data_type: null, + certification_details: null, + certified_by: null, + column_name: 'b', + description: null, + expression: null, + filterable: true, + groupby: true, + id: 2, + is_certified: false, + is_dttm: false, + python_date_format: null, + type: 'BIGINT', + type_generic: 0, + verbose_name: null, + warning_markdown: null, + }, + aggregate: 'AVG', + sqlExpression: null, + datasourceWarning: false, + hasCustomLabel: false, + label: 'AVG(b)', + optionName: 'metric_6g7h8i9j0k_6g7h8i9j0k', + }, +]; + describe('reducers', () => { it('Does not set a control value if control does not exist', () => { const newState = exploreReducer( @@ -37,4 +94,127 @@ describe('reducers', () => { expect(newState.controls.y_axis_format.value).toBe('$,.2f'); expect(newState.form_data.y_axis_format).toBe('$,.2f'); }); + it('Keeps the column config when metric column positions are swapped', () => { + const mockedState = { + ...defaultState, + controls: { + ...defaultState.controls, + metrics: { + ...defaultState.controls.metrics, + value: METRICS, + }, + column_config: { + ...defaultState.controls.column_config, + value: { + 'AVG(b)': { + currencyFormat: { + symbolPosition: 'prefix', + symbol: 'USD', + }, + }, + }, + }, + }, + form_data: { + ...defaultState.form_data, + metrics: METRICS, + column_config: { + 'AVG(b)': { + currencyFormat: { + symbolPosition: 'prefix', + symbol: 'USD', + }, + }, + }, + }, + }; + + const swappedMetrics = [METRICS[1], METRICS[0]]; + const newState = exploreReducer( + mockedState, + actions.setControlValue('metrics', swappedMetrics, []), + ); + + const expectedColumnConfig = { + 'AVG(b)': { + currencyFormat: { + symbolPosition: 'prefix', + symbol: 'USD', + }, + }, + }; + + expect(newState.controls.metrics.value).toStrictEqual(swappedMetrics); + expect(newState.form_data.metrics).toStrictEqual(swappedMetrics); + expect(newState.controls.column_config.value).toStrictEqual( + expectedColumnConfig, + ); + expect(newState.form_data.column_config).toStrictEqual( + expectedColumnConfig, + ); + }); + + it('Keeps the column config when metric column name is updated', () => { + const mockedState = { + ...defaultState, + controls: { + ...defaultState.controls, + metrics: { + ...defaultState.controls.metrics, + value: METRICS, + }, + column_config: { + ...defaultState.controls.column_config, + value: { + 'AVG(b)': { + currencyFormat: { + symbolPosition: 'prefix', + symbol: 'USD', + }, + }, + }, + }, + }, + form_data: { + ...defaultState.form_data, + metrics: METRICS, + column_config: { + 'AVG(b)': { + currencyFormat: { + symbolPosition: 'prefix', + symbol: 'USD', + }, + }, + }, + }, + }; + + const updatedMetrics = [ + METRICS[0], + { + ...METRICS[1], + hasCustomLabel: true, + label: 'AVG of b', + }, + ]; + + const newState = exploreReducer( + mockedState, + actions.setControlValue('metrics', updatedMetrics, []), + ); + + const expectedColumnConfig = { + 'AVG of b': { + currencyFormat: { + symbolPosition: 'prefix', + symbol: 'USD', + }, + }, + }; + expect(newState.controls.metrics.value).toStrictEqual(updatedMetrics); + expect(newState.form_data.metrics).toStrictEqual(updatedMetrics); + expect(newState.form_data.column_config).toStrictEqual( + expectedColumnConfig, + ); + }); }); diff --git a/superset-frontend/src/explore/reducers/exploreReducer.js b/superset-frontend/src/explore/reducers/exploreReducer.js index d5565a0dad5eb..1797c57637d2d 100644 --- a/superset-frontend/src/explore/reducers/exploreReducer.js +++ b/superset-frontend/src/explore/reducers/exploreReducer.js @@ -115,7 +115,12 @@ export default function exploreReducer(state = {}, action) { // need to update column config as well to keep the previous config. if (controlName === 'metrics' && old_metrics_data && new_column_config) { value.forEach((item, index) => { + const itemExist = old_metrics_data.some( + oldItem => oldItem?.label === item?.label, + ); + if ( + !itemExist && item?.label !== old_metrics_data[index]?.label && !!new_column_config[old_metrics_data[index]?.label] ) {