You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2023/12/04 21:05:54 UTC
(superset) 09/15: fix: set label on adhoc column should persist (#26154)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit ceac19fa2fb623756a938c069fabc184f78d9e91
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Fri Dec 1 08:40:52 2023 -0500
fix: set label on adhoc column should persist (#26154)
(cherry picked from commit b2ea97a98484e18eee760b7a2914926143918231)
---
.../ColumnSelectPopover.test.tsx | 77 ++++++++++++++++++++++
.../DndColumnSelectControl/ColumnSelectPopover.tsx | 39 +++++++++--
.../ColumnSelectPopoverTrigger.tsx | 13 ++--
3 files changed, 121 insertions(+), 8 deletions(-)
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.test.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.test.tsx
new file mode 100644
index 0000000000..e7ff7cd9a7
--- /dev/null
+++ b/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',
+ });
+ });
+});
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
index 4806e5394a..96abf36484 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
+++ b/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();
},
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopoverTrigger.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopoverTrigger.tsx
index 4340317f04..341d91e616 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopoverTrigger.tsx
+++ b/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(
() => (