fix(chart): Resolve incorrect column customization when switching met…

…rics in table chart (apache#26393) Co-authored-by: Sonia <sonia.gautam@agoda.com>
sfirke · Mar 22, 2024 · 14b19ff · 14b19ff
1 parent 1a51cc6
commit 14b19ff
Show file tree

Hide file tree

Showing 2 changed files with 185 additions and 0 deletions.
diff --git 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
@@ -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]
           ) {