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/27 18:02:51 UTC
(superset) 04/04: fix(sqllab): keep a newer save request pending when an older one settles
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a commit to branch fix-saved-query-duplicate-name
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 71d1a07ade7c3aa7997c4ce6fda0ea08f657633b
Author: Joe Li <jo...@preset.io>
AuthorDate: Tue Oct 27 17:57:05 2026 +0000
fix(sqllab): keep a newer save request pending when an older one settles
The Save query modal re-enabled its buttons whenever any request settled, so
an older request finishing after the modal was closed and reopened released
the buttons while a newer request was still in flight. Track the request that
owns the pending state with a token that closing the modal revokes, and only
clear the pending state from the request that still holds it.
Co-Authored-By: Claude Sonnet 5.5 <no...@anthropic.com>
---
.../SqlLab/components/SaveQuery/SaveQuery.test.tsx | 106 +++++++++++++++++++++
.../src/SqlLab/components/SaveQuery/index.tsx | 11 ++-
2 files changed, 116 insertions(+), 1 deletion(-)
diff --git a/superset-frontend/src/SqlLab/components/SaveQuery/SaveQuery.test.tsx b/superset-frontend/src/SqlLab/components/SaveQuery/SaveQuery.test.tsx
index 42ec195a4e1..6db51dff148 100644
--- a/superset-frontend/src/SqlLab/components/SaveQuery/SaveQuery.test.tsx
+++ b/superset-frontend/src/SqlLab/components/SaveQuery/SaveQuery.test.tsx
@@ -19,6 +19,7 @@
import configureStore from 'redux-mock-store';
import thunk from 'redux-thunk';
import {
+ act,
render,
screen,
userEvent,
@@ -749,4 +750,109 @@ describe('SavedQuery', () => {
);
expect(screen.queryByText(DUPLICATE_ERROR)).not.toBeInTheDocument();
});
+
+ test('re-enables the buttons when a request settles after the name was edited', async () => {
+ let resolveSave: () => void = () => {};
+ const mockOnSave = jest.fn().mockImplementation(
+ () =>
+ new Promise<void>(resolve => {
+ resolveSave = resolve;
+ }),
+ );
+
+ render(<SaveQuery {...mockedProps} onSave={mockOnSave} />, {
+ useRedux: true,
+ store: mockStore(mockState),
+ });
+
+ await userEvent.click(screen.getByRole('button', { name: /save/i }));
+ const modalSaveBtn = () =>
+ screen.getAllByRole('button', { name: /save/i })[1];
+ await userEvent.click(modalSaveBtn());
+ await waitFor(() => expect(modalSaveBtn()).toBeDisabled());
+
+ await userEvent.type(screen.getAllByRole('textbox')[0], ' 2');
+ expect(modalSaveBtn()).toBeDisabled();
+
+ resolveSave();
+ await waitFor(() => expect(modalSaveBtn()).toBeEnabled());
+ expect(mockOnSave).toHaveBeenCalledTimes(1);
+ });
+
+ const openAndSubmitTwice = async () => {
+ const settlers: {
+ resolve: () => void;
+ reject: (error: Error) => void;
+ }[] = [];
+ const mockOnSave = jest.fn().mockImplementation(
+ () =>
+ new Promise<void>((resolve, reject) => {
+ settlers.push({ resolve, reject });
+ }),
+ );
+
+ render(<SaveQuery {...mockedProps} onSave={mockOnSave} />, {
+ useRedux: true,
+ store: mockStore(mockState),
+ });
+
+ const modalSaveBtn = () =>
+ screen.getAllByRole('button', { name: /save/i })[1];
+ await userEvent.click(screen.getByRole('button', { name: /save/i }));
+ await userEvent.click(modalSaveBtn());
+ await waitFor(() => expect(mockOnSave).toHaveBeenCalledTimes(1));
+
+ await userEvent.click(screen.getByTestId('cancel-query'));
+ await waitFor(() =>
+ expect(
+ screen.queryByRole('heading', { name: /save query/i }),
+ ).not.toBeInTheDocument(),
+ );
+ await userEvent.click(screen.getByRole('button', { name: /save/i }));
+ await userEvent.click(modalSaveBtn());
+ await waitFor(() => expect(mockOnSave).toHaveBeenCalledTimes(2));
+ await waitFor(() => expect(modalSaveBtn()).toBeDisabled());
+
+ return { settlers, mockOnSave, modalSaveBtn };
+ };
+
+ test('keeps a newer request pending when an earlier one is rejected, then closes when it succeeds', async () => {
+ const { settlers, mockOnSave, modalSaveBtn } = await openAndSubmitTwice();
+
+ await act(async () => {
+ settlers[0].reject(new Error(DUPLICATE_ERROR));
+ });
+ expect(modalSaveBtn()).toBeDisabled();
+ expect(screen.queryByText(DUPLICATE_ERROR)).not.toBeInTheDocument();
+ await userEvent.click(modalSaveBtn());
+ expect(mockOnSave).toHaveBeenCalledTimes(2);
+
+ await act(async () => {
+ settlers[1].resolve();
+ });
+ await waitFor(() =>
+ expect(
+ screen.queryByRole('heading', { name: /save query/i }),
+ ).not.toBeInTheDocument(),
+ );
+ expect(mockOnSave).toHaveBeenCalledTimes(2);
+ });
+
+ test('keeps a newer request pending when an earlier one succeeds, then shows its error when it is rejected', async () => {
+ const { settlers, modalSaveBtn } = await openAndSubmitTwice();
+
+ await act(async () => {
+ settlers[0].resolve();
+ });
+ expect(modalSaveBtn()).toBeDisabled();
+ expect(
+ screen.getByRole('heading', { name: /save query/i }),
+ ).toBeInTheDocument();
+
+ await act(async () => {
+ settlers[1].reject(new Error(DUPLICATE_ERROR));
+ });
+ expect(await screen.findByText(DUPLICATE_ERROR)).toBeInTheDocument();
+ expect(modalSaveBtn()).toBeEnabled();
+ });
});
diff --git a/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx b/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx
index bf1caf9291a..2bd0befb3da 100644
--- a/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx
+++ b/superset-frontend/src/SqlLab/components/SaveQuery/index.tsx
@@ -117,6 +117,10 @@ const SaveQuery = ({
// that settles afterwards cannot restore an error or close a modal the user
// has since moved on from.
const generation = useRef(0);
+ // Identifies the request that owns the `submitting` state. Closing the modal
+ // revokes it, so an obsolete request settling later cannot re-enable the
+ // buttons while a newer request is still pending.
+ const submissionToken = useRef(0);
const [showSave, setShowSave] = useState<boolean>(false);
const [showSaveDatasetModal, setShowSaveDatasetModal] = useState(false);
// Saving a dataset runs the SQL to introspect columns, so it needs a
@@ -158,6 +162,7 @@ const SaveQuery = ({
const close = () => {
generation.current += 1;
+ submissionToken.current += 1;
setShowSave(false);
setLabelError('');
setSubmitting(false);
@@ -167,6 +172,8 @@ const SaveQuery = ({
// can be changed and the save retried.
const submit = async (action: SaveQueryProps['onSave']) => {
const submittedGeneration = generation.current;
+ submissionToken.current += 1;
+ const token = submissionToken.current;
setLabelError('');
setSubmitting(true);
try {
@@ -177,7 +184,9 @@ const SaveQuery = ({
}
return;
} finally {
- setSubmitting(false);
+ if (submissionToken.current === token) {
+ setSubmitting(false);
+ }
}
if (generation.current === submittedGeneration) {
close();