fix: set label on adhoc column should persist (#26154)

superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.test.tsx

@@ -0,0 +1,77 @@
+/**
+ * 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 React from 'react';
+import { render, fireEvent } from '@testing-library/react';
+import '@testing-library/jest-dom/extend-expect';
+import { Provider } from 'react-redux';
+import configureMockStore from 'redux-mock-store';
+import thunk from 'redux-thunk';
+import { supersetTheme, ThemeProvider } from '@superset-ui/core';
+import ColumnSelectPopover from 'src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover';
+
+const middlewares = [thunk];
+const mockStore = configureMockStore(middlewares);
+
+describe('ColumnSelectPopover - onTabChange function', () => {
+  it('updates adhocColumn when switching to sqlExpression tab with custom label', () => {
+    const mockColumns = [{ column_name: 'year' }];
+    const mockOnClose = jest.fn();
+    const mockOnChange = jest.fn();
+    const mockGetCurrentTab = jest.fn();
+    const mockSetDatasetModal = jest.fn();
+    const mockSetLabel = jest.fn();
+
+    const store = mockStore({ explore: { datasource: { type: 'table' } } });
+
+    const { container, getByText } = render(
+      <Provider store={store}>
+        <ThemeProvider theme={supersetTheme}>
+          <ColumnSelectPopover
+            columns={mockColumns}
+            editedColumn={mockColumns[0]}
+            getCurrentTab={mockGetCurrentTab}
+            hasCustomLabel
+            isTemporal
+            label="Custom Label"
+            onChange={mockOnChange}
+            onClose={mockOnClose}
+            setDatasetModal={mockSetDatasetModal}
+            setLabel={mockSetLabel}
+          />
+        </ThemeProvider>
+      </Provider>,
+    );
+
+    const sqlExpressionTab = container.querySelector(
+      '#adhoc-metric-edit-tabs-tab-sqlExpression',
+    );
+    expect(sqlExpressionTab).not.toBeNull();
+    fireEvent.click(sqlExpressionTab!);
+    expect(mockGetCurrentTab).toHaveBeenCalledWith('sqlExpression');
+
+    const saveButton = getByText('Save');
+    fireEvent.click(saveButton);
+    expect(mockOnChange).toHaveBeenCalledWith({
+      label: 'Custom Label',
+      sqlExpression: 'year',
+      expressionType: 'SQL',
+    });
+  });
+});

superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx

@@ -68,6 +68,7 @@ interface ColumnSelectPopoverProps {
   editedColumn?: ColumnMeta | AdhocColumn;
   onChange: (column: ColumnMeta | AdhocColumn) => void;
   onClose: () => void;
+  hasCustomLabel: boolean;
   setLabel: (title: string) => void;
   getCurrentTab: (tab: string) => void;
   label: string;
@@ -93,13 +94,14 @@ const getInitialColumnValues = (
 const ColumnSelectPopover = ({
   columns,
   editedColumn,
+  getCurrentTab,
+  hasCustomLabel,
+  isTemporal,
+  label,
   onChange,
   onClose,
   setDatasetModal,
   setLabel,
-  getCurrentTab,
-  label,
-  isTemporal,
 }: ColumnSelectPopoverProps) => {
   const datasourceType = useSelector<ExplorePageState, string | undefined>(
     state => state.explore.datasource.type,
@@ -117,6 +119,7 @@ const ColumnSelectPopover = ({
   const [selectedSimpleColumn, setSelectedSimpleColumn] = useState<
     ColumnMeta | undefined
   >(initialSimpleColumn);
+  const [selectedTab, setSelectedTab] = useState<string | null>(null);
 
   const [resizeButton, width, height] = useResizeButton(
     POPOVER_INITIAL_WIDTH,
@@ -188,7 +191,34 @@ const ColumnSelectPopover = ({
 
   useEffect(() => {
     getCurrentTab(defaultActiveTabKey);
-  }, [defaultActiveTabKey, getCurrentTab]);
+    setSelectedTab(defaultActiveTabKey);
+  }, [defaultActiveTabKey, getCurrentTab, setSelectedTab]);
+
+  useEffect(() => {
+    /* if the adhoc column is not set (because it was never edited) but the
+     * tab is selected and the label has changed, then we need to set the
+     * adhoc column manually */
+    if (
+      adhocColumn === undefined &&
+      selectedTab === 'sqlExpression' &&
+      hasCustomLabel
+    ) {
+      const sqlExpression =
+        selectedSimpleColumn?.column_name ||
+        selectedCalculatedColumn?.expression ||
+        '';
+      setAdhocColumn({ label, sqlExpression, expressionType: 'SQL' });
+    }
+  }, [
+    adhocColumn,
+    defaultActiveTabKey,
+    hasCustomLabel,
+    getCurrentTab,
+    label,
+    selectedCalculatedColumn,
+    selectedSimpleColumn,
+    selectedTab,
+  ]);
 
   const onSave = useCallback(() => {
     if (adhocColumn && adhocColumn.label !== label) {
@@ -225,6 +255,7 @@ const ColumnSelectPopover = ({
   const onTabChange = useCallback(
     tab => {
       getCurrentTab(tab);
+      setSelectedTab(tab);
       // @ts-ignore
       sqlEditorRef.current?.editor.focus();
     },

superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopoverTrigger.tsx

@@ -103,6 +103,7 @@ const ColumnSelectPopoverTrigger = ({
           setDatasetModal={setDatasetModal}
           onClose={handleClosePopover}
           onChange={onColumnEdit}
+          hasCustomLabel={hasCustomLabel}
           label={popoverLabel}
           setLabel={setPopoverLabel}
           getCurrentTab={getCurrentTab}
@@ -114,6 +115,7 @@ const ColumnSelectPopoverTrigger = ({
       columns,
       editedColumn,
       getCurrentTab,
+      hasCustomLabel,
       handleClosePopover,
       isTemporal,
       onColumnEdit,
@@ -121,10 +123,13 @@ const ColumnSelectPopoverTrigger = ({
     ],
   );
 
-  const onLabelChange = useCallback((e: any) => {
-    setPopoverLabel(e.target.value);
-    setHasCustomLabel(true);
-  }, []);
+  const onLabelChange = useCallback(
+    (e: any) => {
+      setPopoverLabel(e.target.value);
+      setHasCustomLabel(true);
+    },
+    [setPopoverLabel, setHasCustomLabel],
+  );
 
   const popoverTitle = useMemo(
     () => (