(superset) branch fix/issue-32835-suggested-open-pr-test-first-v3 created (now 7cd415c4af4)

[ru...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a change to branch fix/issue-32835-suggested-open-pr-test-first-v3
in repository https://gitbox.apache.org/repos/asf/superset.git


      at 7cd415c4af4 fix(chart): stop duplicate metrics when switching between Waterfall and other chart types

This branch includes the following new commits:

     new 7cd415c4af4 fix(chart): stop duplicate metrics when switching between Waterfall and other chart types

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

(superset) 01/01: fix(chart): stop duplicate metrics when switching between Waterfall and other chart types

[ru...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a commit to branch fix/issue-32835-suggested-open-pr-test-first-v3
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 7cd415c4af4267fc40767254c1d9f0403434aca3
Author: rusackas <ev...@rusackas.com>
AuthorDate: Tue Jul 28 03:25:37 2026 -0700

    fix(chart): stop duplicate metrics when switching between Waterfall and other chart types
    
    The Waterfall control panel never drained the shared standardized-controls
    metric/column queue on viz-type switch (no formDataOverrides), so metrics
    picked up when entering Waterfall stayed queued and got re-applied on top
    of the original ones when switching back to a multi-metric chart (e.g.
    Line), producing duplicate measures. Add the same formDataOverrides
    pattern used by the other single-metric echarts charts (Pie, Gauge,
    Sunburst, etc.) so Waterfall shifts exactly one metric and pops all
    columns from the queue.
    
    Fixes #32835
    
    Co-Authored-By: Claude Sonnet 5 <no...@anthropic.com>
---
 .../src/Waterfall/controlPanel.tsx                 |  6 +++
 .../test/Waterfall/controlPanel.test.ts            | 62 ++++++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx
index 0826b9e1472..c73aaa794a9 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Waterfall/controlPanel.tsx
@@ -23,6 +23,7 @@ import {
   D3_TIME_FORMAT_DOCS,
   DEFAULT_TIME_FORMAT,
   formatSelectOptions,
+  getStandardizedControls,
   sharedControls,
 } from '@superset-ui/chart-controls';
 import { showValueControl } from '../controls';
@@ -245,6 +246,11 @@ const config: ControlPanelConfig = {
       multi: false,
     },
   },
+  formDataOverrides: formData => ({
+    ...formData,
+    metric: getStandardizedControls().shiftMetric(),
+    groupby: getStandardizedControls().popAllColumns(),
+  }),
 };
 
 export default config;
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Waterfall/controlPanel.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Waterfall/controlPanel.test.ts
new file mode 100644
index 00000000000..7f3f6b803cb
--- /dev/null
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/Waterfall/controlPanel.test.ts
@@ -0,0 +1,62 @@
+/**
+ * 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 { SqlaFormData } from '@superset-ui/core';
+
+// Mock getStandardizedControls so we can assert the Waterfall control panel
+// actually consumes (shifts/pops) the queued metrics and columns instead of
+// leaving them for the next viz-type switch to pick up again. Regression
+// test for https://github.com/apache/superset/issues/32835, where switching
+// away from and back to another chart type (e.g. Line) produced duplicate
+// metrics because Waterfall never drained the shared standardized-controls
+// queue.
+const mockShiftMetric = jest.fn(() => 'shiftedMetric');
+const mockPopAllColumns = jest.fn(() => ['poppedColumn']);
+
+jest.mock('@superset-ui/chart-controls', () => {
+  const actual = jest.requireActual('@superset-ui/chart-controls');
+  return {
+    ...actual,
+    getStandardizedControls: jest.fn(() => ({
+      shiftMetric: mockShiftMetric,
+      popAllColumns: mockPopAllColumns,
+    })),
+  };
+});
+
+// eslint-disable-next-line import/first
+import controlPanel from '../../src/Waterfall/controlPanel';
+
+test('formDataOverrides consumes a single metric and all columns from getStandardizedControls', () => {
+  expect(controlPanel.formDataOverrides).toBeDefined();
+
+  const dummyFormData = { someProp: 'test' } as unknown as SqlaFormData;
+  const newFormData = controlPanel.formDataOverrides!(dummyFormData);
+
+  // original properties are preserved
+  expect(newFormData.someProp).toBe('test');
+
+  // only a single metric is taken (Waterfall only supports one metric),
+  // leaving any remaining queued metrics for the next viz-type switch
+  expect(newFormData.metric).toBe('shiftedMetric');
+  expect(mockShiftMetric).toHaveBeenCalled();
+
+  // all queued columns are consumed for the (single) groupby control
+  expect(newFormData.groupby).toEqual(['poppedColumn']);
+  expect(mockPopAllColumns).toHaveBeenCalled();
+});