This is an automated email from the ASF dual-hosted git repository.
rusackas 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 d203f0de337 chore(sql-lab): finish SqlLab typed-dispatch migration for
SaveDatasetModal (#40040)
d203f0de337 is described below
commit d203f0de3375fd1ddb962d0596395e6d12488827
Author: Evan Rusackas <[email protected]>
AuthorDate: Wed May 20 16:04:38 2026 -0700
chore(sql-lab): finish SqlLab typed-dispatch migration for SaveDatasetModal
(#40040)
Co-authored-by: Claude Code <[email protected]>
Co-authored-by: Claude <[email protected]>
---
superset-frontend/src/SqlLab/actions/sqlLab.ts | 7 +-
.../SaveDatasetModal/SaveDatasetModal.test.tsx | 105 ++++++++-------------
.../SqlLab/components/SaveDatasetModal/index.tsx | 11 +--
3 files changed, 46 insertions(+), 77 deletions(-)
diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.ts
b/superset-frontend/src/SqlLab/actions/sqlLab.ts
index 302f0b82cb0..7058471a55b 100644
--- a/superset-frontend/src/SqlLab/actions/sqlLab.ts
+++ b/superset-frontend/src/SqlLab/actions/sqlLab.ts
@@ -1671,7 +1671,7 @@ export interface VizOptions {
export function createDatasource(
vizOptions: VizOptions,
-): SqlLabThunkAction<Promise<unknown>> {
+): SqlLabThunkAction<Promise<{ id: number }>> {
return (dispatch: AppDispatch) => {
dispatch(createDatasourceStarted());
const { dbId, catalog, schema, datasourceName, sql, templateParams } =
@@ -1691,9 +1691,10 @@ export function createDatasource(
}),
})
.then(({ json }) => {
- dispatch(createDatasourceSuccess(json as { id: number }));
+ const result = json as { id: number };
+ dispatch(createDatasourceSuccess(result));
- return Promise.resolve(json);
+ return result;
})
.catch(error => {
getClientErrorObject(error).then(e => {
diff --git
a/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx
b/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx
index b89f1ef9525..861a4772858 100644
---
a/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx
+++
b/superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx
@@ -16,8 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-import * as reactRedux from 'react-redux';
-import { act } from 'react';
+import { act, type ComponentProps } from 'react';
import {
cleanup,
fireEvent,
@@ -40,6 +39,19 @@ const mockedProps = {
datasource: testQuery,
};
+// Render with the SqlLab user fixture preloaded into the mock store so the
+// component's useSelector(state => state.user) returns a useful value.
+// Previously this test used jest.spyOn(reactRedux, 'useSelector') to inject
+// the user directly, which can't intercept calls routed through the typed
+// useAppSelector hook.
+const renderModal = (
+ props: Partial<ComponentProps<typeof SaveDatasetModal>> = {},
+) =>
+ render(<SaveDatasetModal {...mockedProps} {...props} />, {
+ useRedux: true,
+ initialState: { user },
+ });
+
fetchMock.get('glob:*/api/v1/dataset/?*', {
result: mockdatasets,
dataset_count: 3,
@@ -47,17 +59,17 @@ fetchMock.get('glob:*/api/v1/dataset/?*', {
jest.useFakeTimers({ advanceTimers: true });
-// Mock the user
-const useSelectorMock = jest.spyOn(reactRedux, 'useSelector');
beforeEach(() => {
- useSelectorMock.mockClear();
cleanup();
});
-// Mock the createDatasource action
-const useDispatchMock = jest.spyOn(reactRedux, 'useDispatch');
+// Mock createDatasource to return a thunk that resolves with the dataset's
+// new id. The test's mock store includes redux-thunk middleware (from RTK's
+// getDefaultMiddleware), so dispatch(createDatasource(...)) properly unwraps
+// the thunk and the production code's .then((data) =>
clearDatasetCache(data.id))
+// chain receives `{ id: 123 }`. Individual tests can override per-call as
needed.
jest.mock('src/SqlLab/actions/sqlLab', () => ({
- createDatasource: jest.fn(),
+ createDatasource: jest.fn(() => () => Promise.resolve({ id: 123 })),
}));
jest.mock('src/explore/exploreUtils/formData', () => ({
postFormData: jest.fn(),
@@ -70,7 +82,7 @@ jest.mock('src/utils/cachedSupersetGet', () => ({
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
describe('SaveDatasetModal', () => {
test('renders a "Save as new" field', () => {
- render(<SaveDatasetModal {...mockedProps} />, { useRedux: true });
+ renderModal();
const saveRadioBtn = screen.getByRole('radio', {
name: /save as new/i,
@@ -87,7 +99,7 @@ describe('SaveDatasetModal', () => {
});
test('renders an "Overwrite existing" field', () => {
- render(<SaveDatasetModal {...mockedProps} />, { useRedux: true });
+ renderModal();
const overwriteRadioBtn = screen.getByRole('radio', {
name: /overwrite existing/i,
@@ -103,20 +115,20 @@ describe('SaveDatasetModal', () => {
});
test('renders a close button', () => {
- render(<SaveDatasetModal {...mockedProps} />, { useRedux: true });
+ renderModal();
expect(screen.getByRole('button', { name: /close/i })).toBeInTheDocument();
});
test('renders a save button when "Save as new" is selected', () => {
- render(<SaveDatasetModal {...mockedProps} />, { useRedux: true });
+ renderModal();
// "Save as new" is selected when the modal opens by default
expect(screen.getByRole('button', { name: /save/i })).toBeInTheDocument();
});
test('renders an overwrite button when "Overwrite existing" is selected', ()
=> {
- render(<SaveDatasetModal {...mockedProps} />, { useRedux: true });
+ renderModal();
// Click the overwrite radio button to reveal the overwrite confirmation
and back buttons
const overwriteRadioBtn = screen.getByRole('radio', {
@@ -130,8 +142,7 @@ describe('SaveDatasetModal', () => {
});
test('renders the overwrite button as disabled until an existing dataset is
selected', async () => {
- useSelectorMock.mockReturnValue({ ...user });
- render(<SaveDatasetModal {...mockedProps} />, { useRedux: true });
+ renderModal();
// Click the overwrite radio button
const overwriteRadioBtn = screen.getByRole('radio', {
@@ -168,8 +179,7 @@ describe('SaveDatasetModal', () => {
});
test('renders a confirm overwrite screen when overwrite is clicked', async
() => {
- useSelectorMock.mockReturnValue({ ...user });
- render(<SaveDatasetModal {...mockedProps} />, { useRedux: true });
+ renderModal();
// Click the overwrite radio button
const overwriteRadioBtn = screen.getByRole('radio', {
@@ -215,11 +225,7 @@ describe('SaveDatasetModal', () => {
});
test('sends the schema when creating the dataset', async () => {
- const dummyDispatch = jest.fn().mockResolvedValue({});
- useDispatchMock.mockReturnValue(dummyDispatch);
- useSelectorMock.mockReturnValue({ ...user });
-
- render(<SaveDatasetModal {...mockedProps} />, { useRedux: true });
+ renderModal();
const inputFieldText = screen.getByDisplayValue(/unimportant/i);
fireEvent.change(inputFieldText, { target: { value: 'my dataset' } });
@@ -240,17 +246,9 @@ describe('SaveDatasetModal', () => {
});
test('sends the catalog when creating the dataset', async () => {
- const dummyDispatch = jest.fn().mockResolvedValue({});
- useDispatchMock.mockReturnValue(dummyDispatch);
- useSelectorMock.mockReturnValue({ ...user });
-
- render(
- <SaveDatasetModal
- {...mockedProps}
- datasource={{ ...mockedProps.datasource, catalog: 'public' }}
- />,
- { useRedux: true },
- );
+ renderModal({
+ datasource: { ...mockedProps.datasource, catalog: 'public' },
+ });
const inputFieldText = screen.getByDisplayValue(/unimportant/i);
fireEvent.change(inputFieldText, { target: { value: 'my dataset' } });
@@ -271,7 +269,7 @@ describe('SaveDatasetModal', () => {
});
test('does not renders a checkbox button when template processing is
disabled', () => {
- render(<SaveDatasetModal {...mockedProps} />, { useRedux: true });
+ renderModal();
expect(screen.queryByRole('checkbox')).not.toBeInTheDocument();
});
@@ -280,7 +278,7 @@ describe('SaveDatasetModal', () => {
global.featureFlags = {
[FeatureFlag.EnableTemplateProcessing]: true,
};
- render(<SaveDatasetModal {...mockedProps} />, { useRedux: true });
+ renderModal();
expect(screen.getByRole('checkbox')).toBeInTheDocument();
});
@@ -289,15 +287,11 @@ describe('SaveDatasetModal', () => {
global.featureFlags = {
[FeatureFlag.EnableTemplateProcessing]: true,
};
- const propsWithTemplateParam = {
- ...mockedProps,
+ renderModal({
datasource: {
...testQuery,
templateParams: JSON.stringify({ my_param: 12 }),
},
- };
- render(<SaveDatasetModal {...propsWithTemplateParam} />, {
- useRedux: true,
});
const inputFieldText = screen.getByDisplayValue(/unimportant/i);
fireEvent.change(inputFieldText, { target: { value: 'my dataset' } });
@@ -324,15 +318,11 @@ describe('SaveDatasetModal', () => {
global.featureFlags = {
[FeatureFlag.EnableTemplateProcessing]: true,
};
- const propsWithTemplateParam = {
- ...mockedProps,
+ renderModal({
datasource: {
...testQuery,
templateParams: JSON.stringify({ my_param: 12 }),
},
- };
- render(<SaveDatasetModal {...propsWithTemplateParam} />, {
- useRedux: true,
});
const inputFieldText = screen.getByDisplayValue(/unimportant/i);
fireEvent.change(inputFieldText, { target: { value: 'my dataset' } });
@@ -393,19 +383,11 @@ describe('SaveDatasetModal', () => {
.spyOn(SupersetClient, 'put')
.mockResolvedValue({ json: { result: { id: 0 } } } as any);
- const dummyDispatch = jest.fn().mockResolvedValue({});
- useDispatchMock.mockReturnValue(dummyDispatch);
- useSelectorMock.mockReturnValue({ ...user });
-
- const propsWithTemplateParam = {
- ...mockedProps,
+ renderModal({
datasource: {
...testQuery,
templateParams: JSON.stringify({ my_param: 12, _filters: 'foo' }),
},
- };
- render(<SaveDatasetModal {...propsWithTemplateParam} />, {
- useRedux: true,
});
// Check the "Include Template Parameters" checkbox
@@ -443,19 +425,11 @@ describe('SaveDatasetModal', () => {
.spyOn(SupersetClient, 'put')
.mockResolvedValue({ json: { result: { id: 0 } } } as any);
- const dummyDispatch = jest.fn().mockResolvedValue({});
- useDispatchMock.mockReturnValue(dummyDispatch);
- useSelectorMock.mockReturnValue({ ...user });
-
- const propsWithTemplateParam = {
- ...mockedProps,
+ renderModal({
datasource: {
...testQuery,
templateParams: JSON.stringify({ my_param: 12 }),
},
- };
- render(<SaveDatasetModal {...propsWithTemplateParam} />, {
- useRedux: true,
});
// Do NOT check the "Include Template Parameters" checkbox
@@ -489,12 +463,9 @@ describe('SaveDatasetModal', () => {
'postFormData',
);
- const dummyDispatch = jest.fn().mockResolvedValue({ id: 123 });
- useDispatchMock.mockReturnValue(dummyDispatch);
- useSelectorMock.mockReturnValue({ ...user });
postFormData.mockResolvedValue('chart_key_123');
- render(<SaveDatasetModal {...mockedProps} />, { useRedux: true });
+ renderModal();
const inputFieldText = screen.getByDisplayValue(/unimportant/i);
fireEvent.change(inputFieldText, { target: { value: 'my dataset' } });
diff --git a/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
b/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
index c3b4ddf4991..281a2edcae2 100644
--- a/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
+++ b/superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
@@ -34,7 +34,6 @@ import { t } from '@apache-superset/core/translation';
import {
SupersetClient,
JsonResponse,
- JsonObject,
QueryResponse,
QueryFormData,
VizType,
@@ -44,16 +43,14 @@ import {
} from '@superset-ui/core';
import { styled } from '@apache-superset/core/theme';
import { extendedDayjs as dayjs } from '@superset-ui/core/utils/dates';
-import { useSelector, useDispatch } from 'react-redux';
+import { useAppDispatch, useAppSelector } from 'src/views/store';
import rison from 'rison';
import { createDatasource } from 'src/SqlLab/actions/sqlLab';
import { addDangerToast } from 'src/components/MessageToasts/actions';
-import { UserWithPermissionsAndRoles as User } from 'src/types/bootstrapTypes';
import {
DatasetRadioState,
EXPLORE_CHART_DEFAULT,
DatasetOwner,
- SqlLabRootState,
} from 'src/SqlLab/types';
import { mountExploreUrl } from 'src/explore/exploreUtils';
import { postFormData } from 'src/explore/exploreUtils/formData';
@@ -221,7 +218,7 @@ export const SaveDatasetModal = ({
openWindow = true,
formData = {},
}: SaveDatasetModalProps) => {
- const defaultVizType = useSelector<SqlLabRootState, string>(
+ const defaultVizType = useAppSelector(
state => state.common?.conf?.DEFAULT_VIZ_TYPE || VizType.Table,
);
@@ -240,8 +237,8 @@ export const SaveDatasetModal = ({
>(undefined);
const [loading, setLoading] = useState<boolean>(false);
- const user = useSelector<SqlLabRootState, User>(state => state.user);
- const dispatch = useDispatch<(dispatch: any) => Promise<JsonObject>>();
+ const user = useAppSelector(state => state.user);
+ const dispatch = useAppDispatch();
const [includeTemplateParameters, setIncludeTemplateParameters] =
useState(false);
