diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js
index 03c185550..0a5a655a0 100644
--- a/superset-frontend/src/SqlLab/actions/sqlLab.js
+++ b/superset-frontend/src/SqlLab/actions/sqlLab.js
@@ -467,12 +467,24 @@ function migrateQuery(queryId, queryEditorId, dispatch) {
);
}
-export function migrateQueryEditorFromLocalStorage(
- queryEditor,
- tables,
- queries,
-) {
- return function (dispatch) {
+/**
+ * Persist QueryEditor from local storage to backend tab state.
+ * This ensures that when new tabs are created, query editors are
+ * asynchronously stored in local storage and periodically synchronized
+ * with the backend.
+ * When switching to persistence mode, the QueryEditors previously
+ * stored in local storage will also be synchronized to the backend
+ * through syncQueryEditor.
+ */
+export function syncQueryEditor(queryEditor) {
+ return function (dispatch, getState) {
+ const { tables, queries } = getState().sqlLab;
+ const localStorageTables = tables.filter(
+ table => table.inLocalStorage && table.queryEditorId === queryEditor.id,
+ );
+ const localStorageQueries = Object.values(queries).filter(
+ query => query.inLocalStorage && query.sqlEditorId === queryEditor.id,
+ );
return SupersetClient.post({
endpoint: '/tabstateview/',
postPayload: { queryEditor },
@@ -482,6 +494,7 @@ export function migrateQueryEditorFromLocalStorage(
...queryEditor,
id: json.id.toString(),
inLocalStorage: false,
+ loaded: true,
};
dispatch({
type: MIGRATE_QUERY_EDITOR,
@@ -494,10 +507,10 @@ export function migrateQueryEditorFromLocalStorage(
newId: newQueryEditor.id,
});
return Promise.all([
- ...tables.map(table =>
+ ...localStorageTables.map(table =>
migrateTable(table, newQueryEditor.id, dispatch),
),
- ...queries.map(query =>
+ ...localStorageQueries.map(query =>
migrateQuery(query.id, newQueryEditor.id, dispatch),
),
]);
@@ -516,35 +529,15 @@ export function migrateQueryEditorFromLocalStorage(
}
export function addQueryEditor(queryEditor) {
- return function (dispatch) {
- const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence)
- ? SupersetClient.post({
- endpoint: '/tabstateview/',
- postPayload: { queryEditor },
- }).then(({ json }) => ({ ...json, loaded: true }))
- : Promise.resolve({ id: shortid.generate() });
-
- return sync
- .then(({ id, loaded }) => {
- const newQueryEditor = {
- ...queryEditor,
- id: id.toString(),
- loaded,
- };
- return dispatch({
- type: ADD_QUERY_EDITOR,
- queryEditor: newQueryEditor,
- });
- })
- .catch(() =>
- dispatch(
- addDangerToast(
- t(
- 'Unable to add a new tab to the backend. Please contact your administrator.',
- ),
- ),
- ),
- );
+ const newQueryEditor = {
+ ...queryEditor,
+ id: shortid.generate().toString(),
+ loaded: true,
+ inLocalStorage: true,
+ };
+ return {
+ type: ADD_QUERY_EDITOR,
+ queryEditor: newQueryEditor,
};
}
diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js
index d6b70bf6a..9dca13a11 100644
--- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js
+++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js
@@ -427,13 +427,15 @@ describe('async actions', () => {
maxRow: undefined,
id: 'abcd',
templateParams: undefined,
+ inLocalStorage: true,
+ loaded: true,
},
},
];
const request = actions.cloneQueryToNewTab(query, true);
- return request(store.dispatch, store.getState).then(() => {
- expect(store.getActions()).toEqual(expectedActions);
- });
+ request(store.dispatch, store.getState);
+
+ expect(store.getActions()).toEqual(expectedActions);
});
});
@@ -453,13 +455,16 @@ describe('async actions', () => {
const expectedActions = [
{
type: actions.ADD_QUERY_EDITOR,
- queryEditor,
+ queryEditor: {
+ ...queryEditor,
+ inLocalStorage: true,
+ loaded: true,
+ },
},
];
- const request = actions.addQueryEditor(defaultQueryEditor);
- return request(store.dispatch, store.getState).then(() => {
- expect(store.getActions()).toEqual(expectedActions);
- });
+ store.dispatch(actions.addQueryEditor(defaultQueryEditor));
+
+ expect(store.getActions()).toEqual(expectedActions);
});
describe('addNewQueryEditor', () => {
@@ -488,13 +493,14 @@ describe('async actions', () => {
queryLimit:
defaultQueryEditor.queryLimit ||
initialState.common.conf.DEFAULT_SQLLAB_LIMIT,
+ inLocalStorage: true,
+ loaded: true,
},
},
];
const request = actions.addNewQueryEditor();
- return request(store.dispatch, store.getState).then(() => {
- expect(store.getActions()).toEqual(expectedActions);
- });
+ request(store.dispatch, store.getState);
+ expect(store.getActions()).toEqual(expectedActions);
});
});
});
@@ -534,20 +540,33 @@ describe('async actions', () => {
afterEach(fetchMock.resetHistory);
describe('addQueryEditor', () => {
- it('updates the tab state in the backend', () => {
+ let stub;
+ beforeEach(() => {
+ stub = sinon.stub(shortid, 'generate').returns('abcd');
+ });
+ afterEach(() => {
+ stub.restore();
+ });
+
+ it('creates the tab state in the local storage', () => {
expect.assertions(2);
const store = mockStore({});
const expectedActions = [
{
type: actions.ADD_QUERY_EDITOR,
- queryEditor: { ...queryEditor, id: '1', loaded: true },
+ queryEditor: {
+ ...queryEditor,
+ id: 'abcd',
+ loaded: true,
+ inLocalStorage: true,
+ },
},
];
- return store.dispatch(actions.addQueryEditor(queryEditor)).then(() => {
- expect(store.getActions()).toEqual(expectedActions);
- expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1);
- });
+ store.dispatch(actions.addQueryEditor(queryEditor));
+
+ expect(store.getActions()).toEqual(expectedActions);
+ expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(0);
});
});
@@ -965,7 +984,7 @@ describe('async actions', () => {
});
});
- describe('migrateQueryEditorFromLocalStorage', () => {
+ describe('syncQueryEditor', () => {
it('updates the tab state in the backend', () => {
expect.assertions(3);
@@ -978,16 +997,41 @@ describe('async actions', () => {
overwriteRoutes: true,
});
+ const oldQueryEditor = { ...queryEditor, inLocalStorage: true };
const tables = [
- { id: 'one', dataPreviewQueryId: 'previewOne' },
- { id: 'two', dataPreviewQueryId: 'previewTwo' },
+ {
+ id: 'one',
+ dataPreviewQueryId: 'previewOne',
+ queryEditorId: oldQueryEditor.id,
+ inLocalStorage: true,
+ },
+ {
+ id: 'two',
+ dataPreviewQueryId: 'previewTwo',
+ queryEditorId: oldQueryEditor.id,
+ inLocalStorage: true,
+ },
];
const queries = [
- { ...query, id: 'previewOne' },
- { ...query, id: 'previewTwo' },
+ {
+ ...query,
+ id: 'previewOne',
+ sqlEditorId: oldQueryEditor.id,
+ inLocalStorage: true,
+ },
+ {
+ ...query,
+ id: 'previewTwo',
+ sqlEditorId: oldQueryEditor.id,
+ inLocalStorage: true,
+ },
];
- const store = mockStore({});
- const oldQueryEditor = { ...queryEditor, inLocalStorage: true };
+ const store = mockStore({
+ sqlLab: {
+ queries,
+ tables,
+ },
+ });
const expectedActions = [
{
type: actions.MIGRATE_QUERY_EDITOR,
@@ -997,6 +1041,7 @@ describe('async actions', () => {
...oldQueryEditor,
id: '1',
inLocalStorage: false,
+ loaded: true,
},
},
{
@@ -1028,13 +1073,7 @@ describe('async actions', () => {
},
];
return store
- .dispatch(
- actions.migrateQueryEditorFromLocalStorage(
- oldQueryEditor,
- tables,
- queries,
- ),
- )
+ .dispatch(actions.syncQueryEditor(oldQueryEditor))
.then(() => {
expect(store.getActions()).toEqual(expectedActions);
expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(3);
diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx
index 52e1d44b2..82d0bf561 100644
--- a/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx
+++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx
@@ -40,7 +40,7 @@ import { render, waitFor } from 'spec/helpers/testing-library';
import ToastContainer from 'src/components/MessageToasts/ToastContainer';
import { initialState, defaultQueryEditor } from 'src/SqlLab/fixtures';
import { logging } from '@superset-ui/core';
-import EditorAutoSync from '.';
+import EditorAutoSync, { INTERVAL } from '.';
jest.mock('@superset-ui/core', () => ({
...jest.requireActual('@superset-ui/core'),
@@ -78,11 +78,37 @@ test('sync the unsaved editor tab state when there are new changes since the las
sqlLab: unsavedSqlLabState,
},
});
- await waitFor(() => jest.runAllTimers());
+ await waitFor(() => jest.advanceTimersByTime(INTERVAL));
expect(fetchMock.calls(updateEditorTabState)).toHaveLength(1);
fetchMock.restore();
});
+test('sync the unsaved NEW editor state when there are new in local storage', async () => {
+ const createEditorTabState = `glob:*/tabstateview/`;
+ fetchMock.post(createEditorTabState, { id: 123 });
+ expect(fetchMock.calls(createEditorTabState)).toHaveLength(0);
+ render(, {
+ useRedux: true,
+ initialState: {
+ ...initialState,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queryEditors: [
+ ...initialState.sqlLab.queryEditors,
+ {
+ id: 'rnd-new-id',
+ name: 'new tab name',
+ inLocalStorage: true,
+ },
+ ],
+ },
+ },
+ });
+ await waitFor(() => jest.advanceTimersByTime(INTERVAL));
+ expect(fetchMock.calls(createEditorTabState)).toHaveLength(1);
+ fetchMock.restore();
+});
+
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);
@@ -102,7 +128,7 @@ test('skip syncing the unsaved editor tab state when the updates are already syn
},
},
});
- await waitFor(() => jest.runAllTimers());
+ await waitFor(() => jest.advanceTimersByTime(INTERVAL));
expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0);
fetchMock.restore();
});
@@ -126,7 +152,7 @@ test('renders an error toast when the sync failed', async () => {
},
},
);
- await waitFor(() => jest.runAllTimers());
+ await waitFor(() => jest.advanceTimersByTime(INTERVAL));
expect(logging.warn).toHaveBeenCalledTimes(1);
expect(logging.warn).toHaveBeenCalledWith(
diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx
index 51399753e..ab68d9286 100644
--- a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx
+++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx
@@ -27,9 +27,13 @@ import {
} from 'src/SqlLab/types';
import { useUpdateSqlEditorTabMutation } from 'src/hooks/apiResources/sqlEditorTabs';
import { useDebounceValue } from 'src/hooks/useDebounceValue';
-import { setEditorTabLastUpdate } from 'src/SqlLab/actions/sqlLab';
+import {
+ syncQueryEditor,
+ setEditorTabLastUpdate,
+} from 'src/SqlLab/actions/sqlLab';
+import useEffectEvent from 'src/hooks/useEffectEvent';
-const INTERVAL = 5000;
+export const INTERVAL = 5000;
function hasUnsavedChanges(
queryEditor: QueryEditor,
@@ -73,17 +77,43 @@ const EditorAutoSync: React.FC = () => {
INTERVAL,
);
- useEffect(() => {
- const unsaved = filterUnsavedQueryEditorList(
+ const getUnsavedItems = useEffectEvent(unsavedQE =>
+ filterUnsavedQueryEditorList(
queryEditors,
- debouncedUnsavedQueryEditor,
+ unsavedQE,
lastSavedTimestampRef.current,
- );
+ ),
+ );
+
+ const getUnsavedNewQueryEditor = useEffectEvent(() =>
+ filterUnsavedQueryEditorList(
+ queryEditors,
+ unsavedQueryEditor,
+ lastSavedTimestampRef.current,
+ ).find(({ inLocalStorage }) => Boolean(inLocalStorage)),
+ );
+
+ useEffect(() => {
+ let timer: NodeJS.Timeout;
+ function saveUnsavedQueryEditor() {
+ const firstUnsavedQueryEditor = getUnsavedNewQueryEditor();
+
+ if (firstUnsavedQueryEditor) {
+ dispatch(syncQueryEditor(firstUnsavedQueryEditor));
+ }
+ timer = setTimeout(saveUnsavedQueryEditor, INTERVAL);
+ }
+ timer = setTimeout(saveUnsavedQueryEditor, INTERVAL);
+ return () => {
+ clearTimeout(timer);
+ };
+ }, [getUnsavedNewQueryEditor, dispatch]);
+
+ useEffect(() => {
+ const unsaved = getUnsavedItems(debouncedUnsavedQueryEditor);
Promise.all(
unsaved
- // TODO: Migrate migrateQueryEditorFromLocalStorage
- // in TabbedSqlEditors logic by addSqlEditor mutation later
.filter(({ inLocalStorage }) => !inLocalStorage)
.map(queryEditor => updateSqlEditor({ queryEditor })),
).then(resolvers => {
@@ -92,7 +122,7 @@ const EditorAutoSync: React.FC = () => {
dispatch(setEditorTabLastUpdate(lastSavedTimestampRef.current));
}
});
- }, [debouncedUnsavedQueryEditor, dispatch, queryEditors, updateSqlEditor]);
+ }, [debouncedUnsavedQueryEditor, getUnsavedItems, dispatch, updateSqlEditor]);
useEffect(() => {
if (error) {
diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.tsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.tsx
index 6b048830e..577b13c99 100644
--- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.tsx
+++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.tsx
@@ -165,8 +165,8 @@ test('should disable new tab when offline', () => {
});
expect(queryAllByLabelText('Add tab').length).toEqual(0);
});
-test('should have an empty state when query editors is empty', () => {
- const { getByText } = setup(undefined, {
+test('should have an empty state when query editors is empty', async () => {
+ const { getByText, getByRole } = setup(undefined, {
...initialState,
sqlLab: {
...initialState.sqlLab,
@@ -174,5 +174,12 @@ test('should have an empty state when query editors is empty', () => {
tabHistory: [],
},
});
- expect(getByText('Add a new tab to create SQL Query')).toBeInTheDocument();
+
+ // Clear the new tab applied in componentDidMount and check the state of the empty tab
+ const removeTabButton = getByRole('button', { name: 'remove' });
+ fireEvent.click(removeTabButton);
+
+ await waitFor(() =>
+ expect(getByText('Add a new tab to create SQL Query')).toBeInTheDocument(),
+ );
});
diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx
index 7b4db1cbe..632cadc17 100644
--- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx
+++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx
@@ -70,32 +70,6 @@ class TabbedSqlEditors extends React.PureComponent {
}
componentDidMount() {
- // migrate query editor and associated tables state to server
- if (isFeatureEnabled(FeatureFlag.SqllabBackendPersistence)) {
- const localStorageTables = this.props.tables.filter(
- table => table.inLocalStorage,
- );
- const localStorageQueries = Object.values(this.props.queries).filter(
- query => query.inLocalStorage,
- );
- this.props.queryEditors
- .filter(qe => qe.inLocalStorage)
- .forEach(qe => {
- // get all queries associated with the query editor
- const queries = localStorageQueries.filter(
- query => query.sqlEditorId === qe.id,
- );
- const tables = localStorageTables.filter(
- table => table.queryEditorId === qe.id,
- );
- this.props.actions.migrateQueryEditorFromLocalStorage(
- qe,
- tables,
- queries,
- );
- });
- }
-
// merge post form data with GET search params
// Hack: this data should be coming from getInitialState
// but for some reason this data isn't being passed properly through
@@ -322,7 +296,6 @@ export function mapStateToProps({ sqlLab, common }: SqlLabRootState) {
queryEditors: sqlLab.queryEditors ?? DEFAULT_PROPS.queryEditors,
queries: sqlLab.queries,
tabHistory: sqlLab.tabHistory,
- tables: sqlLab.tables,
defaultDbId: common.conf.SQLLAB_DEFAULT_DBID,
displayLimit: common.conf.DISPLAY_MAX_ROW,
offline: sqlLab.offline ?? DEFAULT_PROPS.offline,
diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.js b/superset-frontend/src/SqlLab/reducers/sqlLab.js
index ecc0f090a..24acc721c 100644
--- a/superset-frontend/src/SqlLab/reducers/sqlLab.js
+++ b/superset-frontend/src/SqlLab/reducers/sqlLab.js
@@ -442,9 +442,10 @@ export default function sqlLabReducer(state = {}, action) {
// continue regardless of error
}
// replace localStorage query editor with the server backed one
- return addToArr(
- removeFromArr(state, 'queryEditors', action.oldQueryEditor),
+ return alterInArr(
+ state,
'queryEditors',
+ action.oldQueryEditor,
action.newQueryEditor,
);
},
@@ -468,20 +469,9 @@ export default function sqlLabReducer(state = {}, action) {
);
},
[actions.MIGRATE_TAB_HISTORY]() {
- try {
- // remove migrated tab from localStorage tabHistory
- const { sqlLab } = JSON.parse(localStorage.getItem('redux'));
- sqlLab.tabHistory = sqlLab.tabHistory.filter(
- tabId => tabId !== action.oldId,
- );
- localStorage.setItem('redux', JSON.stringify({ sqlLab }));
- } catch (error) {
- // continue regardless of error
- }
- const tabHistory = state.tabHistory.filter(
- tabId => tabId !== action.oldId,
+ const tabHistory = state.tabHistory.map(tabId =>
+ tabId === action.oldId ? action.newId : tabId,
);
- tabHistory.push(action.newId);
return { ...state, tabHistory };
},
[actions.MIGRATE_QUERY]() {
diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.test.js b/superset-frontend/src/SqlLab/reducers/sqlLab.test.js
index 87e0212b1..0cf6b1d48 100644
--- a/superset-frontend/src/SqlLab/reducers/sqlLab.test.js
+++ b/superset-frontend/src/SqlLab/reducers/sqlLab.test.js
@@ -239,6 +239,34 @@ describe('sqlLabReducer', () => {
interceptedAction.northPercent,
);
});
+ it('should migrate query editor by new query editor id', () => {
+ const index = newState.queryEditors.findIndex(({ id }) => id === qe.id);
+ const newQueryEditor = {
+ ...qe,
+ id: 'updatedNewId',
+ schema: 'updatedSchema',
+ };
+ const action = {
+ type: actions.MIGRATE_QUERY_EDITOR,
+ oldQueryEditor: qe,
+ newQueryEditor,
+ };
+ newState = sqlLabReducer(newState, action);
+ expect(newState.queryEditors[index].id).toEqual('updatedNewId');
+ expect(newState.queryEditors[index]).toEqual(newQueryEditor);
+ });
+ it('should migrate tab history by new query editor id', () => {
+ expect(newState.tabHistory).toContain(qe.id);
+ const action = {
+ type: actions.MIGRATE_TAB_HISTORY,
+ oldId: qe.id,
+ newId: 'updatedNewId',
+ };
+ newState = sqlLabReducer(newState, action);
+
+ expect(newState.tabHistory).toContain('updatedNewId');
+ expect(newState.tabHistory).not.toContain(qe.id);
+ });
});
describe('Tables', () => {
let newState;