You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2026/10/20 17:46:27 UTC
(superset) branch master updated: fix(dataset): warn immediately when normalize_columns is toggled on (#44617)
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 3988a771f37 fix(dataset): warn immediately when normalize_columns is toggled on (#44617)
3988a771f37 is described below
commit 3988a771f37b067432a5252cd95f278aec47a3d9
Author: Joe Li <jo...@preset.io>
AuthorDate: Tue Oct 20 10:46:17 2026 -0700
fix(dataset): warn immediately when normalize_columns is toggled on (#44617)
Co-authored-by: Claude Sonnet 5 <no...@anthropic.com>
---
.../DatasourceEditor/DatasourceEditor.tsx | 45 +++++--
...atasourceEditorNormalizeColumnsWarning.test.tsx | 146 +++++++++++++++++++++
superset/translations/messages.pot | 6 +
3 files changed, 188 insertions(+), 9 deletions(-)
diff --git a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
index 546cde8f1fa..34e88c1b14b 100644
--- a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
+++ b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
@@ -1031,6 +1031,13 @@ function DatasourceEditor({
propsDatasource.datasource_type === 'table' ||
propsDatasource.type === 'table',
);
+ // Captured once when the editor opens: onChange round-trips through the
+ // parent (e.g. DatasourceModal) and echoes the edit back down as a new
+ // `datasource` prop, so comparing against the live prop would immediately
+ // erase the "just toggled" signal this baseline is used to detect.
+ const [normalizeColumnsBaseline] = useState(
+ propsDatasource.normalize_columns,
+ );
const [isEditMode, setIsEditMode] = useState(false);
const [databaseColumns, setDatabaseColumns] = useState<Column[]>(
propsDatasource.columns.filter(col => !col.expression),
@@ -1934,15 +1941,29 @@ function DatasourceEditor({
/>
)}
{datasourceType === DATASOURCE_TYPES.physical.key && (
- <Field
- inline
- fieldKey="normalize_columns"
- label={t('Normalize column names')}
- description={t(
- 'Allow column names to be changed to case insensitive format, if supported (e.g. Oracle, Snowflake).',
+ <>
+ <Field
+ inline
+ fieldKey="normalize_columns"
+ label={t('Normalize column names')}
+ description={t(
+ 'Allow column names to be changed to case insensitive format, if supported (e.g. Oracle, Snowflake).',
+ )}
+ control={<CheckboxControl />}
+ />
+ {datasource.normalize_columns && !normalizeColumnsBaseline && (
+ <Alert
+ css={themeParam => ({ marginBottom: themeParam.sizeUnit * 4 })}
+ type="warning"
+ showIcon
+ message={t(
+ 'Changing this setting may change the casing for all columns in this dataset, ' +
+ 'which may break any existing charts and dashboard filters that reference the ' +
+ 'current column names.',
+ )}
+ />
)}
- control={<CheckboxControl />}
- />
+ </>
)}
<Field
inline
@@ -1955,7 +1976,13 @@ function DatasourceEditor({
/>
</Fieldset>
),
- [datasource, onDatasourcePropChange, isSqla, datasourceType],
+ [
+ datasource,
+ onDatasourcePropChange,
+ isSqla,
+ normalizeColumnsBaseline,
+ datasourceType,
+ ],
);
const renderSourceFieldset = useCallback(
diff --git a/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorNormalizeColumnsWarning.test.tsx b/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorNormalizeColumnsWarning.test.tsx
new file mode 100644
index 00000000000..d82d70ef972
--- /dev/null
+++ b/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorNormalizeColumnsWarning.test.tsx
@@ -0,0 +1,146 @@
+/**
+ * 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 { useState } from 'react';
+import fetchMock from 'fetch-mock';
+import {
+ render,
+ screen,
+ userEvent,
+ waitFor,
+ within,
+} from 'spec/helpers/testing-library';
+import type { DatasetObject } from 'src/features/datasets/types';
+import DatasourceEditor from '..';
+import {
+ createProps,
+ DATASOURCE_ENDPOINT,
+ setupDatasourceEditorMocks,
+ cleanupAsyncOperations,
+ fastRender,
+ dismissDatasourceWarning,
+ DatasourceEditorProps,
+} from './DatasourceEditor.test.utils';
+
+beforeEach(() => {
+ fetchMock.get(DATASOURCE_ENDPOINT, [], { name: DATASOURCE_ENDPOINT });
+ setupDatasourceEditorMocks();
+});
+
+afterEach(async () => {
+ await cleanupAsyncOperations();
+ fetchMock.clearHistory().removeRoutes();
+});
+
+const WARNING_TEXT = /may break/i;
+
+const goToNormalizeColumnsCheckbox = async () => {
+ await userEvent.click(await screen.findByRole('tab', { name: 'Settings' }));
+ const label = await screen.findByText('Normalize column names');
+ const controlHeader = label.closest('.ControlHeader');
+ if (!controlHeader) {
+ throw new Error('Could not find the Normalize column names control');
+ }
+ return within(controlHeader as HTMLElement).getByRole('checkbox');
+};
+
+// DatasourceModal keeps a `currentDatasource` state that it updates on every
+// `onChange` and passes back down as a new `datasource` prop object, echoing
+// the in-progress edit back through the same prop DatasourceEditor was
+// initialized from. Mirror that round-trip here rather than using a static
+// prop, since a static prop can't catch a fix that (incorrectly) compares
+// against the live prop instead of a baseline captured once on mount.
+function DatasourceEditorWithParentEcho(props: DatasourceEditorProps) {
+ const [datasource, setDatasource] = useState(props.datasource);
+ return (
+ <DatasourceEditor
+ {...props}
+ datasource={datasource}
+ onChange={(data: DatasetObject, err?: unknown) => {
+ setDatasource({ ...data });
+ props.onChange(data, err);
+ }}
+ />
+ );
+}
+
+const renderWithParentEcho = (renderProps: DatasourceEditorProps) =>
+ render(<DatasourceEditorWithParentEcho {...renderProps} />, {
+ useRedux: true,
+ initialState: { common: { currencies: ['USD', 'GBP', 'EUR'] } },
+ useRouter: true,
+ });
+
+test('warns immediately when normalize_columns is toggled on, before saving', async () => {
+ const testProps = createProps();
+ testProps.datasource.normalize_columns = false;
+ fastRender(testProps);
+ await dismissDatasourceWarning();
+
+ const checkbox = await goToNormalizeColumnsCheckbox();
+ expect(screen.queryByText(WARNING_TEXT)).not.toBeInTheDocument();
+
+ await userEvent.click(checkbox);
+
+ expect(await screen.findByText(WARNING_TEXT)).toBeInTheDocument();
+});
+
+test('hides the warning again when normalize_columns is toggled back off', async () => {
+ const testProps = createProps();
+ testProps.datasource.normalize_columns = false;
+ fastRender(testProps);
+ await dismissDatasourceWarning();
+
+ const checkbox = await goToNormalizeColumnsCheckbox();
+ await userEvent.click(checkbox);
+ expect(await screen.findByText(WARNING_TEXT)).toBeInTheDocument();
+
+ await userEvent.click(checkbox);
+
+ expect(screen.queryByText(WARNING_TEXT)).not.toBeInTheDocument();
+});
+
+test('does not warn when the dataset already has normalize_columns enabled', async () => {
+ const testProps = createProps();
+ testProps.datasource.normalize_columns = true;
+ fastRender(testProps);
+ await dismissDatasourceWarning();
+
+ await goToNormalizeColumnsCheckbox();
+
+ expect(screen.queryByText(WARNING_TEXT)).not.toBeInTheDocument();
+});
+
+test('keeps the warning visible after the parent echoes the toggle back down as a new datasource prop', async () => {
+ const testProps = createProps();
+ testProps.datasource.normalize_columns = false;
+ renderWithParentEcho(testProps);
+ await dismissDatasourceWarning();
+
+ const checkbox = await goToNormalizeColumnsCheckbox();
+ await userEvent.click(checkbox);
+
+ // Wait until the echo has actually happened (the parent's onChange fired
+ // with the toggled-on value) before asserting the warning survived it.
+ await waitFor(() => {
+ const [datasource] = testProps.onChange.mock.calls.at(-1) ?? [];
+ expect(datasource?.normalize_columns).toBe(true);
+ });
+
+ expect(screen.getByText(WARNING_TEXT)).toBeInTheDocument();
+});
diff --git a/superset/translations/messages.pot b/superset/translations/messages.pot
index 99f89528d4b..d965c7039ab 100644
--- a/superset/translations/messages.pot
+++ b/superset/translations/messages.pot
@@ -3081,6 +3081,12 @@ msgstr ""
msgid "Changing this semantic view is forbidden"
msgstr ""
+msgid ""
+"Changing this setting may change the casing for all columns in this dataset, "
+"which may break any existing charts and dashboard filters that reference the "
+"current column names."
+msgstr ""
+
msgid "Changing this task is forbidden"
msgstr ""