diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index eb197f1a7..aa81f8429 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -102,6 +102,7 @@ export const CREATE_DATASOURCE_SUCCESS = 'CREATE_DATASOURCE_SUCCESS'; 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 addInfoToast = addInfoToastAction; export const addSuccessToast = addSuccessToastAction; @@ -611,29 +612,17 @@ export function cloneQueryToNewTab(query, autorun) { }; } -export function setActiveQueryEditor(queryEditor) { - return function (dispatch) { - const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) - ? SupersetClient.post({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}/activate`), - }) - : Promise.resolve(); +export function setLastUpdatedActiveTab(queryEditorId) { + return { + type: SET_LAST_UPDATED_ACTIVE_TAB, + queryEditorId, + }; +} - return sync - .then(() => dispatch({ type: SET_ACTIVE_QUERY_EDITOR, queryEditor })) - .catch(response => { - if (response.status !== 404) { - return dispatch( - addDangerToast( - t( - 'An error occurred while setting the active tab. Please contact ' + - 'your administrator.', - ), - ), - ); - } - return dispatch({ type: REMOVE_QUERY_EDITOR, queryEditor }); - }); +export function setActiveQueryEditor(queryEditor) { + return { + type: SET_ACTIVE_QUERY_EDITOR, + queryEditor, }; } @@ -674,51 +663,42 @@ export function setTables(tableSchemas) { return { type: SET_TABLES, tables }; } -export function switchQueryEditor(queryEditor, displayLimit) { +export function fetchQueryEditor(queryEditor, displayLimit) { return function (dispatch) { - if ( - isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) && - queryEditor && - !queryEditor.loaded - ) { - SupersetClient.get({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), + SupersetClient.get({ + endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), + }) + .then(({ json }) => { + const loadedQueryEditor = { + id: json.id.toString(), + loaded: true, + name: json.label, + sql: json.sql, + selectedText: null, + latestQueryId: json.latest_query?.id, + autorun: json.autorun, + dbId: json.database_id, + templateParams: json.template_params, + catalog: json.catalog, + schema: json.schema, + queryLimit: json.query_limit, + remoteId: json.saved_query?.id, + hideLeftBar: json.hide_left_bar, + }; + dispatch(loadQueryEditor(loadedQueryEditor)); + dispatch(setTables(json.table_schemas || [])); + if (json.latest_query && json.latest_query.resultsKey) { + dispatch(fetchQueryResults(json.latest_query, displayLimit)); + } }) - .then(({ json }) => { - const loadedQueryEditor = { - id: json.id.toString(), - loaded: true, - name: json.label, - sql: json.sql, - selectedText: null, - latestQueryId: json.latest_query?.id, - autorun: json.autorun, - dbId: json.database_id, - templateParams: json.template_params, - catalog: json.catalog, - schema: json.schema, - queryLimit: json.query_limit, - remoteId: json.saved_query?.id, - hideLeftBar: json.hide_left_bar, - }; - dispatch(loadQueryEditor(loadedQueryEditor)); - dispatch(setTables(json.table_schemas || [])); - dispatch(setActiveQueryEditor(loadedQueryEditor)); - if (json.latest_query && json.latest_query.resultsKey) { - dispatch(fetchQueryResults(json.latest_query, displayLimit)); - } - }) - .catch(response => { - if (response.status !== 404) { - return dispatch( - addDangerToast(t('An error occurred while fetching tab state')), - ); - } - return dispatch({ type: REMOVE_QUERY_EDITOR, queryEditor }); - }); - } else { - dispatch(setActiveQueryEditor(queryEditor)); - } + .catch(response => { + if (response.status !== 404) { + return dispatch( + addDangerToast(t('An error occurred while fetching tab state')), + ); + } + return dispatch({ type: REMOVE_QUERY_EDITOR, queryEditor }); + }); }; } diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js index 05b442306..db051da5e 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -528,6 +528,21 @@ describe('async actions', () => { }); }); + it('set current query editor', () => { + expect.assertions(1); + + const store = mockStore(initialState); + const expectedActions = [ + { + type: actions.SET_ACTIVE_QUERY_EDITOR, + queryEditor: defaultQueryEditor, + }, + ]; + store.dispatch(actions.setActiveQueryEditor(defaultQueryEditor)); + + expect(store.getActions()).toEqual(expectedActions); + }); + describe('backend sync', () => { const updateTabStateEndpoint = 'glob:*/tabstateview/*'; fetchMock.put(updateTabStateEndpoint, {}); @@ -593,26 +608,6 @@ describe('async actions', () => { }); }); - describe('setActiveQueryEditor', () => { - it('updates the tab state in the backend', () => { - expect.assertions(2); - - const store = mockStore({}); - const expectedActions = [ - { - type: actions.SET_ACTIVE_QUERY_EDITOR, - queryEditor, - }, - ]; - return store - .dispatch(actions.setActiveQueryEditor(queryEditor)) - .then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); - }); - }); - describe('removeQueryEditor', () => { it('updates the tab state in the backend', () => { expect.assertions(2); diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx index 197d90f38..6ccae610a 100644 --- a/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx @@ -58,6 +58,7 @@ const unsavedSqlLabState = { }, editorTabLastUpdatedAt, }; + beforeAll(() => { jest.useFakeTimers(); }); @@ -66,6 +67,16 @@ afterAll(() => { jest.useRealTimers(); }); +const updateActiveEditorTabState = `glob:*/tabstateview/*/activate`; + +beforeEach(() => { + fetchMock.post(updateActiveEditorTabState, {}); +}); + +afterEach(() => { + fetchMock.reset(); +}); + test('sync the unsaved editor tab state when there are new changes since the last update', async () => { const updateEditorTabState = `glob:*/tabstateview/${defaultQueryEditor.id}`; fetchMock.put(updateEditorTabState, 200); @@ -108,6 +119,33 @@ test('sync the unsaved NEW editor state when there are new in local storage', as fetchMock.restore(); }); +test('sync the active editor id when there are updates in tab history', async () => { + expect(fetchMock.calls(updateActiveEditorTabState)).toHaveLength(0); + render(, { + useRedux: true, + initialState: { + ...initialState, + sqlLab: { + ...initialState.sqlLab, + lastUpdatedActiveTab: 'old-tab-id', + queryEditors: [ + ...initialState.sqlLab.queryEditors, + { + id: 'rnd-new-id12', + name: 'new tab name', + inLocalStorage: false, + }, + ], + tabHistory: ['old-tab-id', 'rnd-new-id12'], + }, + }, + }); + await waitFor(() => jest.advanceTimersByTime(INTERVAL)); + expect(fetchMock.calls(updateActiveEditorTabState)).toHaveLength(1); + await waitFor(() => jest.advanceTimersByTime(INTERVAL)); + expect(fetchMock.calls(updateActiveEditorTabState)).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 ff204e100..5477c4fa1 100644 --- a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx @@ -26,11 +26,15 @@ import { QueryEditor, UnsavedQueryEditor, } from 'src/SqlLab/types'; -import { useUpdateSqlEditorTabMutation } from 'src/hooks/apiResources/sqlEditorTabs'; +import { + useUpdateCurrentSqlEditorTabMutation, + useUpdateSqlEditorTabMutation, +} from 'src/hooks/apiResources/sqlEditorTabs'; import { useDebounceValue } from 'src/hooks/useDebounceValue'; import { syncQueryEditor, setEditorTabLastUpdate, + setLastUpdatedActiveTab, } from 'src/SqlLab/actions/sqlLab'; import useEffectEvent from 'src/hooks/useEffectEvent'; @@ -71,7 +75,15 @@ const EditorAutoSync: FC = () => { ); const dispatch = useDispatch(); const lastSavedTimestampRef = useRef(editorTabLastUpdatedAt); + + const currentQueryEditorId = useSelector( + ({ sqlLab }) => sqlLab.tabHistory.slice(-1)[0] || '', + ); + const lastUpdatedActiveTab = useSelector( + ({ sqlLab }) => sqlLab.lastUpdatedActiveTab, + ); const [updateSqlEditor, { error }] = useUpdateSqlEditorTabMutation(); + const [updateCurrentSqlEditor] = useUpdateCurrentSqlEditorTabMutation(); const debouncedUnsavedQueryEditor = useDebounceValue( unsavedQueryEditor, @@ -94,21 +106,36 @@ const EditorAutoSync: FC = () => { ).find(({ inLocalStorage }) => Boolean(inLocalStorage)), ); + const syncCurrentQueryEditor = useEffectEvent(() => { + if ( + currentQueryEditorId && + currentQueryEditorId !== lastUpdatedActiveTab && + !queryEditors.find(({ id }) => id === currentQueryEditorId) + ?.inLocalStorage + ) { + updateCurrentSqlEditor(currentQueryEditorId).then(() => { + dispatch(setLastUpdatedActiveTab(currentQueryEditorId)); + }); + } + }); + useEffect(() => { - let timer: NodeJS.Timeout; + let saveTimer: NodeJS.Timeout; function saveUnsavedQueryEditor() { const firstUnsavedQueryEditor = getUnsavedNewQueryEditor(); if (firstUnsavedQueryEditor) { dispatch(syncQueryEditor(firstUnsavedQueryEditor)); } - timer = setTimeout(saveUnsavedQueryEditor, INTERVAL); + saveTimer = setTimeout(saveUnsavedQueryEditor, INTERVAL); } - timer = setTimeout(saveUnsavedQueryEditor, INTERVAL); + const syncTimer = setInterval(syncCurrentQueryEditor, INTERVAL); + saveTimer = setTimeout(saveUnsavedQueryEditor, INTERVAL); return () => { - clearTimeout(timer); + clearTimeout(saveTimer); + clearInterval(syncTimer); }; - }, [getUnsavedNewQueryEditor, dispatch]); + }, [getUnsavedNewQueryEditor, syncCurrentQueryEditor, dispatch]); useEffect(() => { const unsaved = getUnsavedItems(debouncedUnsavedQueryEditor); diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx index 6368f0b6e..9697d14a9 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx @@ -411,7 +411,8 @@ describe('SqlEditor', () => { await waitFor(() => expect(fetchMock.calls('glob:*/tabstateview/*').length).toBe(1), ); - expect(fetchMock.calls(switchTabApi).length).toBe(1); + // it will be called from EditorAutoSync + expect(fetchMock.calls(switchTabApi).length).toBe(0); }); }); }); diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx index a92c28116..1673220b3 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx @@ -79,7 +79,7 @@ import { setActiveSouthPaneTab, updateSavedQuery, formatQuery, - switchQueryEditor, + fetchQueryEditor, } from 'src/SqlLab/actions/sqlLab'; import { STATE_TYPE_MAP, @@ -506,7 +506,7 @@ const SqlEditor: FC = ({ const loadQueryEditor = useEffectEvent(() => { if (shouldLoadQueryEditor) { - dispatch(switchQueryEditor(queryEditor, displayLimit)); + dispatch(fetchQueryEditor(queryEditor, displayLimit)); } }); diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx index b26067556..d5ed29150 100644 --- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx +++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx @@ -181,10 +181,7 @@ class TabbedSqlEditors extends PureComponent { if (!queryEditor) { return; } - this.props.actions.switchQueryEditor( - queryEditor, - this.props.displayLimit, - ); + this.props.actions.setActiveQueryEditor(queryEditor); } } diff --git a/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js b/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js index 25dda5cd3..d83e06d31 100644 --- a/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js +++ b/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js @@ -49,13 +49,16 @@ const sqlLabPersistStateConfig = { tables, queries, tabHistory, + lastUpdatedActiveTab, } = state.sqlLab; const unsavedQueryEditors = filterUnsavedQueryEditorList( queryEditors, unsavedQueryEditor, editorTabLastUpdatedAt, ); - if (unsavedQueryEditors.length > 0) { + const hasUnsavedActiveTabState = + tabHistory.slice(-1)[0] !== lastUpdatedActiveTab; + if (unsavedQueryEditors.length > 0 || hasUnsavedActiveTabState) { const hasFinishedMigrationFromLocalStorage = unsavedQueryEditors.every( ({ inLocalStorage }) => !inLocalStorage, @@ -70,6 +73,9 @@ const sqlLabPersistStateConfig = { query => query.inLocalStorage && !query.isDataPreview, ), }), + ...(hasUnsavedActiveTabState && { + tabHistory, + }), }; } return; diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts b/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts index 6d6e65bad..1e7745f6d 100644 --- a/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts +++ b/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts @@ -301,6 +301,9 @@ describe('getInitialState', () => { name: expectedValue, }), ); + expect( + getInitialState(apiDataWithLocalStorage).sqlLab.lastUpdatedActiveTab, + ).toEqual(apiDataWithTabState.active_tab.id.toString()); }); it('skip unsaved changes for expired data', () => { diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.ts b/superset-frontend/src/SqlLab/reducers/getInitialState.ts index 52a977085..6a156bacf 100644 --- a/superset-frontend/src/SqlLab/reducers/getInitialState.ts +++ b/superset-frontend/src/SqlLab/reducers/getInitialState.ts @@ -110,6 +110,7 @@ export default function getInitialState({ }; }); const tabHistory = activeTab ? [activeTab.id.toString()] : []; + let lastUpdatedActiveTab = activeTab ? activeTab.id.toString() : ''; let tables = {} as Record; let editorTabLastUpdatedAt = Date.now(); if (activeTab) { @@ -217,6 +218,7 @@ export default function getInitialState({ if (sqlLab.tabHistory) { tabHistory.push(...sqlLab.tabHistory); } + lastUpdatedActiveTab = tabHistory.slice(tabHistory.length - 1)[0] || ''; } } } catch (error) { @@ -250,6 +252,7 @@ export default function getInitialState({ editorTabLastUpdatedAt, queryCostEstimates: {}, unsavedQueryEditor, + lastUpdatedActiveTab, }, localStorageUsageInKilobytes: 0, common, diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.js b/superset-frontend/src/SqlLab/reducers/sqlLab.js index 24acc721c..f290d29f6 100644 --- a/superset-frontend/src/SqlLab/reducers/sqlLab.js +++ b/superset-frontend/src/SqlLab/reducers/sqlLab.js @@ -742,6 +742,9 @@ export default function sqlLabReducer(state = {}, action) { [actions.SET_EDITOR_TAB_LAST_UPDATE]() { return { ...state, editorTabLastUpdatedAt: action.timestamp }; }, + [actions.SET_LAST_UPDATED_ACTIVE_TAB]() { + return { ...state, lastUpdatedActiveTab: action.queryEditorId }; + }, }; if (action.type in actionHandlers) { return actionHandlers[action.type](); diff --git a/superset-frontend/src/SqlLab/types.ts b/superset-frontend/src/SqlLab/types.ts index b1dea6f2e..2dee58c22 100644 --- a/superset-frontend/src/SqlLab/types.ts +++ b/superset-frontend/src/SqlLab/types.ts @@ -109,6 +109,7 @@ export type SqlLabRootState = { unsavedQueryEditor: UnsavedQueryEditor; queryCostEstimates?: Record; editorTabLastUpdatedAt: number; + lastUpdatedActiveTab: string; }; 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 d0f2230f1..e2bb7c41e 100644 --- a/superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts +++ b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts @@ -24,7 +24,10 @@ import { } from 'spec/helpers/testing-library'; import { api } from 'src/hooks/apiResources/queryApi'; import { LatestQueryEditorVersion } from 'src/SqlLab/types'; -import { useUpdateSqlEditorTabMutation } from './sqlEditorTabs'; +import { + useUpdateCurrentSqlEditorTabMutation, + useUpdateSqlEditorTabMutation, +} from './sqlEditorTabs'; const expectedQueryEditor = { version: LatestQueryEditorVersion, @@ -97,3 +100,23 @@ test('puts api request with formData', async () => { ), ); }); + +test('posts activate request with queryEditorId', async () => { + const tabStateMutationApiRoute = `glob:*/tabstateview/${expectedQueryEditor.id}/activate`; + fetchMock.post(tabStateMutationApiRoute, 200); + const { result, waitFor } = renderHook( + () => useUpdateCurrentSqlEditorTabMutation(), + { + 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 f25e9b402..29fe10223 100644 --- a/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts +++ b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts @@ -66,7 +66,17 @@ const sqlEditorApi = api.injectEndpoints({ ), }), }), + updateCurrentSqlEditorTab: builder.mutation({ + query: queryEditorId => ({ + method: 'POST', + endpoint: encodeURI(`/tabstateview/${queryEditorId}/activate`), + transformResponse: () => queryEditorId, + }), + }), }), }); -export const { useUpdateSqlEditorTabMutation } = sqlEditorApi; +export const { + useUpdateSqlEditorTabMutation, + useUpdateCurrentSqlEditorTabMutation, +} = sqlEditorApi;