diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index aa81f8429..18b74d2af 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -103,6 +103,7 @@ export const CREATE_DATASOURCE_FAILED = 'CREATE_DATASOURCE_FAILED'; export const SET_EDITOR_TAB_LAST_UPDATE = 'SET_EDITOR_TAB_LAST_UPDATE'; export const SET_LAST_UPDATED_ACTIVE_TAB = 'SET_LAST_UPDATED_ACTIVE_TAB'; +export const CLEAR_DESTROYED_QUERY_EDITOR = 'CLEAR_DESTROYED_QUERY_EDITOR'; export const addInfoToast = addInfoToastAction; export const addSuccessToast = addSuccessToastAction; @@ -715,29 +716,12 @@ export function toggleLeftBar(queryEditor) { }; } -export function removeQueryEditor(queryEditor) { - return function (dispatch) { - const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) - ? SupersetClient.delete({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), - }) - : Promise.resolve(); +export function clearDestoryedQueryEditor(queryEditorId) { + return { type: CLEAR_DESTROYED_QUERY_EDITOR, queryEditorId }; +} - return sync - .then(() => dispatch({ type: REMOVE_QUERY_EDITOR, queryEditor })) - .catch(({ status }) => { - if (status !== 404) { - return dispatch( - addDangerToast( - t( - 'An error occurred while removing tab. Please contact your administrator.', - ), - ), - ); - } - return dispatch({ type: REMOVE_QUERY_EDITOR, queryEditor }); - }); - }; +export function removeQueryEditor(queryEditor) { + return { type: REMOVE_QUERY_EDITOR, queryEditor }; } export function removeAllOtherQueryEditors(queryEditor) { diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js index db051da5e..11ab42451 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -610,7 +610,7 @@ describe('async actions', () => { describe('removeQueryEditor', () => { it('updates the tab state in the backend', () => { - expect.assertions(2); + expect.assertions(1); const store = mockStore({}); const expectedActions = [ @@ -619,12 +619,8 @@ describe('async actions', () => { queryEditor, }, ]; - return store - .dispatch(actions.removeQueryEditor(queryEditor)) - .then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); + store.dispatch(actions.removeQueryEditor(queryEditor)); + expect(store.getActions()).toEqual(expectedActions); }); }); diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx index 6ccae610a..121ca0752 100644 --- a/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx @@ -146,6 +146,29 @@ test('sync the active editor id when there are updates in tab history', async () expect(fetchMock.calls(updateActiveEditorTabState)).toHaveLength(1); }); +test('sync the destroyed editor id when there are updates in destroyed editors', async () => { + const removeId = 'removed-tab-id'; + const deleteEditorState = `glob:*/tabstateview/${removeId}`; + fetchMock.delete(deleteEditorState, { id: removeId }); + expect(fetchMock.calls(deleteEditorState)).toHaveLength(0); + render(, { + useRedux: true, + initialState: { + ...initialState, + sqlLab: { + ...initialState.sqlLab, + destroyedQueryEditors: { + [removeId]: 123, + }, + }, + }, + }); + await waitFor(() => jest.advanceTimersByTime(INTERVAL)); + expect(fetchMock.calls(deleteEditorState)).toHaveLength(1); + await waitFor(() => jest.advanceTimersByTime(INTERVAL)); + expect(fetchMock.calls(deleteEditorState)).toHaveLength(1); +}); + test('skip syncing the unsaved editor tab state when the updates are already synced', async () => { const updateEditorTabState = `glob:*/tabstateview/${defaultQueryEditor.id}`; fetchMock.put(updateEditorTabState, 200); diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx index 5477c4fa1..0b9173d36 100644 --- a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx @@ -29,12 +29,14 @@ import { import { useUpdateCurrentSqlEditorTabMutation, useUpdateSqlEditorTabMutation, + useDeleteSqlEditorTabMutation, } from 'src/hooks/apiResources/sqlEditorTabs'; import { useDebounceValue } from 'src/hooks/useDebounceValue'; import { syncQueryEditor, setEditorTabLastUpdate, setLastUpdatedActiveTab, + clearDestoryedQueryEditor, } from 'src/SqlLab/actions/sqlLab'; import useEffectEvent from 'src/hooks/useEffectEvent'; @@ -82,8 +84,13 @@ const EditorAutoSync: FC = () => { const lastUpdatedActiveTab = useSelector( ({ sqlLab }) => sqlLab.lastUpdatedActiveTab, ); + const destroyedQueryEditors = useSelector< + SqlLabRootState, + Record + >(({ sqlLab }) => sqlLab.destroyedQueryEditors); const [updateSqlEditor, { error }] = useUpdateSqlEditorTabMutation(); const [updateCurrentSqlEditor] = useUpdateCurrentSqlEditorTabMutation(); + const [deleteSqlEditor] = useDeleteSqlEditorTabMutation(); const debouncedUnsavedQueryEditor = useDebounceValue( unsavedQueryEditor, @@ -119,6 +126,22 @@ const EditorAutoSync: FC = () => { } }); + const syncDeletedQueryEditor = useEffectEvent(() => { + if (Object.keys(destroyedQueryEditors).length > 0) { + Object.keys(destroyedQueryEditors).forEach(id => { + deleteSqlEditor(id) + .then(() => { + dispatch(clearDestoryedQueryEditor(id)); + }) + .catch(({ status }) => { + if (status === 404) { + dispatch(clearDestoryedQueryEditor(id)); + } + }); + }); + } + }); + useEffect(() => { let saveTimer: NodeJS.Timeout; function saveUnsavedQueryEditor() { @@ -131,11 +154,18 @@ const EditorAutoSync: FC = () => { } const syncTimer = setInterval(syncCurrentQueryEditor, INTERVAL); saveTimer = setTimeout(saveUnsavedQueryEditor, INTERVAL); + const clearQueryEditorTimer = setInterval(syncDeletedQueryEditor, INTERVAL); return () => { clearTimeout(saveTimer); clearInterval(syncTimer); + clearInterval(clearQueryEditorTimer); }; - }, [getUnsavedNewQueryEditor, syncCurrentQueryEditor, dispatch]); + }, [ + getUnsavedNewQueryEditor, + syncCurrentQueryEditor, + syncDeletedQueryEditor, + dispatch, + ]); useEffect(() => { const unsaved = getUnsavedItems(debouncedUnsavedQueryEditor); diff --git a/superset-frontend/src/SqlLab/fixtures.ts b/superset-frontend/src/SqlLab/fixtures.ts index 54f1c278f..95d45ba76 100644 --- a/superset-frontend/src/SqlLab/fixtures.ts +++ b/superset-frontend/src/SqlLab/fixtures.ts @@ -681,6 +681,7 @@ export const initialState = { queriesLastUpdate: 0, activeSouthPaneTab: 'Results', unsavedQueryEditor: {}, + destroyedQueryEditors: {}, }, messageToasts: [], user, diff --git a/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js b/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js index d83e06d31..12c9c3485 100644 --- a/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js +++ b/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js @@ -50,6 +50,7 @@ const sqlLabPersistStateConfig = { queries, tabHistory, lastUpdatedActiveTab, + destroyedQueryEditors, } = state.sqlLab; const unsavedQueryEditors = filterUnsavedQueryEditorList( queryEditors, @@ -58,7 +59,13 @@ const sqlLabPersistStateConfig = { ); const hasUnsavedActiveTabState = tabHistory.slice(-1)[0] !== lastUpdatedActiveTab; - if (unsavedQueryEditors.length > 0 || hasUnsavedActiveTabState) { + const hasUnsavedDeletedQueryEditors = + Object.keys(destroyedQueryEditors).length > 0; + if ( + unsavedQueryEditors.length > 0 || + hasUnsavedActiveTabState || + hasUnsavedDeletedQueryEditors + ) { const hasFinishedMigrationFromLocalStorage = unsavedQueryEditors.every( ({ inLocalStorage }) => !inLocalStorage, @@ -76,6 +83,7 @@ const sqlLabPersistStateConfig = { ...(hasUnsavedActiveTabState && { tabHistory, }), + destroyedQueryEditors, }; } return; diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts b/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts index 1e7745f6d..f7e5897c4 100644 --- a/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts +++ b/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts @@ -274,6 +274,9 @@ describe('getInitialState', () => { name: expectedValue, }, ], + destroyedQueryEditors: { + 10: 12345, + }, }, }), ); @@ -291,7 +294,10 @@ describe('getInitialState', () => { updatedAt: lastUpdatedTime, }, }, - tab_state_ids: [{ id: 1, label: '' }], + tab_state_ids: [ + { id: 1, label: '' }, + { id: 10, label: 'removed' }, + ], }; expect( getInitialState(apiDataWithLocalStorage).sqlLab.queryEditors[0], @@ -301,6 +307,13 @@ describe('getInitialState', () => { name: expectedValue, }), ); + expect( + getInitialState(apiDataWithLocalStorage).sqlLab.queryEditors, + ).not.toContainEqual( + expect.objectContaining({ + id: '10', + }), + ); expect( getInitialState(apiDataWithLocalStorage).sqlLab.lastUpdatedActiveTab, ).toEqual(apiDataWithTabState.active_tab.id.toString()); diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.ts b/superset-frontend/src/SqlLab/reducers/getInitialState.ts index 6a156bacf..6052daedb 100644 --- a/superset-frontend/src/SqlLab/reducers/getInitialState.ts +++ b/superset-frontend/src/SqlLab/reducers/getInitialState.ts @@ -146,6 +146,8 @@ export default function getInitialState({ }), }; + const destroyedQueryEditors = {}; + /** * If the `SQLLAB_BACKEND_PERSISTENCE` feature flag is off, or if the user * hasn't used SQL Lab after it has been turned on, the state will be stored @@ -219,6 +221,15 @@ export default function getInitialState({ tabHistory.push(...sqlLab.tabHistory); } lastUpdatedActiveTab = tabHistory.slice(tabHistory.length - 1)[0] || ''; + + if (sqlLab.destroyedQueryEditors) { + Object.entries(sqlLab.destroyedQueryEditors).forEach(([id, ts]) => { + if (queryEditors[id]) { + destroyedQueryEditors[id] = ts; + delete queryEditors[id]; + } + }); + } } } } catch (error) { @@ -253,6 +264,7 @@ export default function getInitialState({ queryCostEstimates: {}, unsavedQueryEditor, lastUpdatedActiveTab, + destroyedQueryEditors, }, localStorageUsageInKilobytes: 0, common, diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.js b/superset-frontend/src/SqlLab/reducers/sqlLab.js index f290d29f6..5f0b27e97 100644 --- a/superset-frontend/src/SqlLab/reducers/sqlLab.js +++ b/superset-frontend/src/SqlLab/reducers/sqlLab.js @@ -163,9 +163,18 @@ export default function sqlLabReducer(state = {}, action) { ...(action.queryEditor.id !== state.unsavedQueryEditor.id && state.unsavedQueryEditor), }, + destroyedQueryEditors: { + ...newState.destroyedQueryEditors, + [queryEditor.id]: Date.now(), + }, }; return newState; }, + [actions.CLEAR_DESTROYED_QUERY_EDITOR]() { + const destroyedQueryEditors = { ...state.destroyedQueryEditors }; + delete destroyedQueryEditors[action.queryEditorId]; + return { ...state, destroyedQueryEditors }; + }, [actions.REMOVE_QUERY]() { const newQueries = { ...state.queries }; delete newQueries[action.query.id]; diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.test.js b/superset-frontend/src/SqlLab/reducers/sqlLab.test.js index 0cf6b1d48..c3b603667 100644 --- a/superset-frontend/src/SqlLab/reducers/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/reducers/sqlLab.test.js @@ -267,6 +267,23 @@ describe('sqlLabReducer', () => { expect(newState.tabHistory).toContain('updatedNewId'); expect(newState.tabHistory).not.toContain(qe.id); }); + it('should clear the destroyed query editors', () => { + const expectedQEId = '1233289'; + const action = { + type: actions.CLEAR_DESTROYED_QUERY_EDITOR, + queryEditorId: expectedQEId, + }; + newState = sqlLabReducer( + { + ...newState, + destroyedQueryEditors: { + [expectedQEId]: Date.now(), + }, + }, + action, + ); + expect(newState.destroyedQueryEditors).toEqual({}); + }); }); describe('Tables', () => { let newState; diff --git a/superset-frontend/src/SqlLab/types.ts b/superset-frontend/src/SqlLab/types.ts index 2dee58c22..eb49ca8ac 100644 --- a/superset-frontend/src/SqlLab/types.ts +++ b/superset-frontend/src/SqlLab/types.ts @@ -110,6 +110,7 @@ export type SqlLabRootState = { queryCostEstimates?: Record; editorTabLastUpdatedAt: number; lastUpdatedActiveTab: string; + destroyedQueryEditors: Record; }; localStorageUsageInKilobytes: number; messageToasts: toastState[]; diff --git a/superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts index e2bb7c41e..42f738f3c 100644 --- a/superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts +++ b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts @@ -25,6 +25,7 @@ import { import { api } from 'src/hooks/apiResources/queryApi'; import { LatestQueryEditorVersion } from 'src/SqlLab/types'; import { + useDeleteSqlEditorTabMutation, useUpdateCurrentSqlEditorTabMutation, useUpdateSqlEditorTabMutation, } from './sqlEditorTabs'; @@ -120,3 +121,23 @@ test('posts activate request with queryEditorId', async () => { expect(fetchMock.calls(tabStateMutationApiRoute).length).toBe(1), ); }); + +test('deletes destoryed query editors', async () => { + const tabStateMutationApiRoute = `glob:*/tabstateview/${expectedQueryEditor.id}`; + fetchMock.delete(tabStateMutationApiRoute, 200); + const { result, waitFor } = renderHook( + () => useDeleteSqlEditorTabMutation(), + { + wrapper: createWrapper({ + useRedux: true, + store, + }), + }, + ); + act(() => { + result.current[0](expectedQueryEditor.id); + }); + await waitFor(() => + expect(fetchMock.calls(tabStateMutationApiRoute).length).toBe(1), + ); +}); diff --git a/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts index 29fe10223..90f4a057c 100644 --- a/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts +++ b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts @@ -73,10 +73,18 @@ const sqlEditorApi = api.injectEndpoints({ transformResponse: () => queryEditorId, }), }), + deleteSqlEditorTab: builder.mutation({ + query: queryEditorId => ({ + method: 'DELETE', + endpoint: encodeURI(`/tabstateview/${queryEditorId}`), + transformResponse: () => queryEditorId, + }), + }), }), }); export const { useUpdateSqlEditorTabMutation, useUpdateCurrentSqlEditorTabMutation, + useDeleteSqlEditorTabMutation, } = sqlEditorApi;