From 9ddb5e3b60165cee6e394c243678e2723a46dc30 Mon Sep 17 00:00:00 2001 From: Justin Park Date: Fri, 23 Jun 2023 15:29:01 -0700 Subject: [PATCH 01/17] feat(sqllab): non-blocking persistence mode --- .../src/SqlLab/actions/sqlLab.js | 288 +++--------------- .../src/SqlLab/actions/sqlLab.test.js | 153 ++-------- .../EditorAutoSync/EditorAutoSync.test.tsx | 178 +++++++++++ .../components/EditorAutoSync/index.tsx | 120 ++++++++ .../SqlLab/components/QueryTable/index.tsx | 6 +- .../src/SqlLab/components/SqlEditor/index.tsx | 7 +- .../middlewares/persistSqlLabStateEnhancer.js | 38 ++- .../SqlLab/reducers/getInitialState.test.ts | 110 ++++++- .../src/SqlLab/reducers/getInitialState.ts | 70 +++-- .../src/SqlLab/reducers/sqlLab.js | 33 +- superset-frontend/src/SqlLab/types.ts | 1 + .../hooks/apiResources/sqlEditorTabs.test.ts | 94 ++++++ .../src/hooks/apiResources/sqlEditorTabs.ts | 69 +++++ .../src/hooks/apiResources/sqlLab.ts | 2 +- .../src/hooks/useDebounceValue.ts | 4 +- superset-frontend/src/pages/SqlLab/index.tsx | 6 +- superset-frontend/src/views/store.ts | 4 +- 17 files changed, 764 insertions(+), 419 deletions(-) create mode 100644 superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx create mode 100644 superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx create mode 100644 superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts create mode 100644 superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 44b4307a1906e..17dc92e0af619 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -99,6 +99,8 @@ export const CREATE_DATASOURCE_STARTED = 'CREATE_DATASOURCE_STARTED'; 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 addInfoToast = addInfoToastAction; export const addSuccessToast = addSuccessToastAction; export const addDangerToast = addDangerToastAction; @@ -160,6 +162,10 @@ export function updateQueryEditor(alterations) { return { type: UPDATE_QUERY_EDITOR, alterations }; } +export function setEditorTabLastUpdate(timestamp) { + return { type: SET_EDITOR_TAB_LAST_UPDATE, timestamp }; +} + export function scheduleQuery(query) { return dispatch => SupersetClient.post({ @@ -237,44 +243,11 @@ export function startQuery(query) { } export function querySuccess(query, results) { - return function (dispatch) { - const sqlEditorId = results?.query?.sqlEditorId; - const sync = - sqlEditorId && - !query.isDataPreview && - isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${sqlEditorId}`), - postPayload: { latest_query_id: query.id }, - }) - : Promise.resolve(); - - return sync - .then(() => dispatch({ type: QUERY_SUCCESS, query, results })) - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while storing the latest query id in the backend. ' + - 'Please contact your administrator if this problem persists.', - ), - ), - ), - ); - }; + return { type: QUERY_SUCCESS, query, results }; } export function queryFailed(query, msg, link, errors) { return function (dispatch) { - const sync = - !query.isDataPreview && - isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${query.sqlEditorId}`), - postPayload: { latest_query_id: query.id }, - }) - : Promise.resolve(); - const eventData = { has_err: true, start_offset: query.startDttm, @@ -295,22 +268,7 @@ export function queryFailed(query, msg, link, errors) { }); }); - return ( - sync - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while storing the latest query id in the backend. ' + - 'Please contact your administrator if this problem persists.', - ), - ), - ), - ) - // We should always show the error message, even if we couldn't sync the - // state to the backend - .then(() => dispatch({ type: QUERY_FAILED, query, msg, link, errors })) - ); + dispatch({ type: QUERY_FAILED, query, msg, link, errors }); }; } @@ -736,11 +694,6 @@ export function switchQueryEditor(queryEditor, displayLimit) { schema: json.schema, queryLimit: json.query_limit, remoteId: json.saved_query?.id, - validationResult: { - id: null, - errors: [], - completed: false, - }, hideLeftBar: json.hide_left_bar, }; dispatch(loadQueryEditor(loadedQueryEditor)); @@ -770,31 +723,10 @@ export function setActiveSouthPaneTab(tabId) { export function toggleLeftBar(queryEditor) { const hideLeftBar = !queryEditor.hideLeftBar; - return function (dispatch) { - const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), - postPayload: { hide_left_bar: hideLeftBar }, - }) - : Promise.resolve(); - - return sync - .then(() => - dispatch({ - type: QUERY_EDITOR_TOGGLE_LEFT_BAR, - queryEditor, - hideLeftBar, - }), - ) - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while hiding the left bar. Please contact your administrator.', - ), - ), - ), - ); + return { + type: QUERY_EDITOR_TOGGLE_LEFT_BAR, + queryEditor, + hideLeftBar, }; } @@ -856,110 +788,26 @@ export function removeQuery(query) { } export function queryEditorSetDb(queryEditor, dbId) { - return function (dispatch) { - const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), - postPayload: { database_id: dbId }, - }) - : Promise.resolve(); - - return sync - .then(() => dispatch({ type: QUERY_EDITOR_SETDB, queryEditor, dbId })) - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while setting the tab database ID. Please contact your administrator.', - ), - ), - ), - ); - }; + return { type: QUERY_EDITOR_SETDB, queryEditor, dbId }; } export function queryEditorSetSchema(queryEditor, schema) { - return function (dispatch) { - const sync = - isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) && - typeof queryEditor === 'object' - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), - postPayload: { schema }, - }) - : Promise.resolve(); - - return sync - .then(() => - dispatch({ - type: QUERY_EDITOR_SET_SCHEMA, - queryEditor: queryEditor || {}, - schema, - }), - ) - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while setting the tab schema. Please contact your administrator.', - ), - ), - ), - ); + return { + type: QUERY_EDITOR_SET_SCHEMA, + queryEditor: queryEditor || {}, + schema, }; } export function queryEditorSetAutorun(queryEditor, autorun) { - return function (dispatch) { - const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), - postPayload: { autorun }, - }) - : Promise.resolve(); - - return sync - .then(() => - dispatch({ type: QUERY_EDITOR_SET_AUTORUN, queryEditor, autorun }), - ) - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while setting the tab autorun. Please contact your administrator.', - ), - ), - ), - ); - }; + return { type: QUERY_EDITOR_SET_AUTORUN, queryEditor, autorun }; } export function queryEditorSetTitle(queryEditor, name, id) { - return function (dispatch) { - const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${id}`), - postPayload: { label: name }, - }) - : Promise.resolve(); - - return sync - .then(() => - dispatch({ - type: QUERY_EDITOR_SET_TITLE, - queryEditor: { ...queryEditor, id }, - name, - }), - ) - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while setting the tab name. Please contact your administrator.', - ), - ), - ), - ); + return { + type: QUERY_EDITOR_SET_TITLE, + queryEditor: { ...queryEditor, id }, + name, }; } @@ -1029,32 +877,19 @@ export function updateSavedQuery(query, clientId) { .then(() => dispatch(updateQueryEditor(query))); } -export function queryEditorSetSql(queryEditor, sql) { - return { type: QUERY_EDITOR_SET_SQL, queryEditor, sql }; +export function queryEditorSetSql(queryEditor, sql, queryId) { + return { type: QUERY_EDITOR_SET_SQL, queryEditor, sql, queryId }; } -export function formatQuery(queryEditor) { - return function (dispatch, getState) { - const { sql } = getUpToDateQuery(getState(), queryEditor); - return SupersetClient.post({ - endpoint: `/api/v1/sqllab/format_sql/`, - body: JSON.stringify({ sql }), - headers: { 'Content-Type': 'application/json' }, - }).then(({ json }) => { - dispatch(queryEditorSetSql(queryEditor, json.result)); - }); - }; -} - -export function queryEditorSetAndSaveSql(targetQueryEditor, sql) { +export function queryEditorSetAndSaveSql(targetQueryEditor, sql, queryId) { return function (dispatch, getState) { const queryEditor = getUpToDateQuery(getState(), targetQueryEditor); // saved query and set tab state use this action - dispatch(queryEditorSetSql(queryEditor, sql)); + dispatch(queryEditorSetSql(queryEditor, sql, queryId)); if (isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE)) { return SupersetClient.put({ endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), - postPayload: { sql, latest_query_id: queryEditor.latestQueryId }, + postPayload: { sql, latest_query_id: queryId }, }).catch(() => dispatch( addDangerToast( @@ -1071,59 +906,32 @@ export function queryEditorSetAndSaveSql(targetQueryEditor, sql) { }; } -export function queryEditorSetQueryLimit(queryEditor, queryLimit) { - return function (dispatch) { - const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), - postPayload: { query_limit: queryLimit }, - }) - : Promise.resolve(); +export function formatQuery(queryEditor) { + return function (dispatch, getState) { + const { sql } = getUpToDateQuery(getState(), queryEditor); + return SupersetClient.post({ + endpoint: `/api/v1/sqllab/format_sql/`, + body: JSON.stringify({ sql }), + headers: { 'Content-Type': 'application/json' }, + }).then(({ json }) => { + dispatch(queryEditorSetSql(queryEditor, json.result)); + }); + }; +} - return sync - .then(() => - dispatch({ - type: QUERY_EDITOR_SET_QUERY_LIMIT, - queryEditor, - queryLimit, - }), - ) - .catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while setting the tab name. Please contact your administrator.', - ), - ), - ), - ); +export function queryEditorSetQueryLimit(queryEditor, queryLimit) { + return { + type: QUERY_EDITOR_SET_QUERY_LIMIT, + queryEditor, + queryLimit, }; } export function queryEditorSetTemplateParams(queryEditor, templateParams) { - return function (dispatch) { - dispatch({ - type: QUERY_EDITOR_SET_TEMPLATE_PARAMS, - queryEditor, - templateParams, - }); - const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), - postPayload: { template_params: templateParams }, - }) - : Promise.resolve(); - - return sync.catch(() => - dispatch( - addDangerToast( - t( - 'An error occurred while setting the tab template parameters. ' + - 'Please contact your administrator.', - ), - ), - ), - ); + return { + type: QUERY_EDITOR_SET_TEMPLATE_PARAMS, + queryEditor, + templateParams, }; } diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js index dbf4e8a5c55f1..0ba039bc33db2 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -531,88 +531,6 @@ describe('async actions', () => { afterEach(fetchMock.resetHistory); - describe('querySuccess', () => { - it('updates the tab state in the backend', () => { - expect.assertions(2); - - const store = mockStore({}); - const results = { query: { sqlEditorId: 'abcd' } }; - const expectedActions = [ - { - type: actions.QUERY_SUCCESS, - query, - results, - }, - ]; - return store.dispatch(actions.querySuccess(query, results)).then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); - }); - }); - - describe('fetchQueryResults', () => { - it('updates the tab state in the backend', () => { - expect.assertions(2); - - const results = { - data: mockBigNumber, - query: { sqlEditorId: 'abcd' }, - status: QueryState.SUCCESS, - query_id: 'efgh', - }; - fetchMock.get(fetchQueryEndpoint, JSON.stringify(results), { - overwriteRoutes: true, - }); - const store = mockStore({}); - const expectedActions = [ - { - type: actions.REQUEST_QUERY_RESULTS, - query, - }, - // missing below - { - type: actions.QUERY_SUCCESS, - query, - results, - }, - ]; - return store.dispatch(actions.fetchQueryResults(query)).then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); - }); - - it("doesn't update the tab state in the backend on stoppped query", () => { - expect.assertions(2); - - const results = { - status: QueryState.STOPPED, - query_id: 'efgh', - }; - fetchMock.get(fetchQueryEndpoint, JSON.stringify(results), { - overwriteRoutes: true, - }); - const store = mockStore({}); - const expectedActions = [ - { - type: actions.REQUEST_QUERY_RESULTS, - query, - }, - // missing below - { - type: actions.QUERY_SUCCESS, - query, - results, - }, - ]; - return store.dispatch(actions.fetchQueryResults(query)).then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(0); - }); - }); - }); - describe('addQueryEditor', () => { it('updates the tab state in the backend', () => { expect.assertions(2); @@ -673,7 +591,7 @@ describe('async actions', () => { describe('queryEditorSetDb', () => { it('updates the tab state in the backend', () => { - expect.assertions(2); + expect.assertions(1); const dbId = 42; const store = mockStore({}); @@ -684,18 +602,14 @@ describe('async actions', () => { dbId, }, ]; - return store - .dispatch(actions.queryEditorSetDb(queryEditor, dbId)) - .then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); + store.dispatch(actions.queryEditorSetDb(queryEditor, dbId)); + expect(store.getActions()).toEqual(expectedActions); }); }); describe('queryEditorSetSchema', () => { it('updates the tab state in the backend', () => { - expect.assertions(2); + expect.assertions(1); const schema = 'schema'; const store = mockStore({}); @@ -706,18 +620,14 @@ describe('async actions', () => { schema, }, ]; - return store - .dispatch(actions.queryEditorSetSchema(queryEditor, schema)) - .then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); + store.dispatch(actions.queryEditorSetSchema(queryEditor, schema)); + expect(store.getActions()).toEqual(expectedActions); }); }); describe('queryEditorSetAutorun', () => { it('updates the tab state in the backend', () => { - expect.assertions(2); + expect.assertions(1); const autorun = true; const store = mockStore({}); @@ -728,18 +638,14 @@ describe('async actions', () => { autorun, }, ]; - return store - .dispatch(actions.queryEditorSetAutorun(queryEditor, autorun)) - .then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); + store.dispatch(actions.queryEditorSetAutorun(queryEditor, autorun)); + expect(store.getActions()).toEqual(expectedActions); }); }); describe('queryEditorSetTitle', () => { it('updates the tab state in the backend', () => { - expect.assertions(2); + expect.assertions(1); const name = 'name'; const store = mockStore({}); @@ -750,14 +656,10 @@ describe('async actions', () => { name, }, ]; - return store - .dispatch( - actions.queryEditorSetTitle(queryEditor, name, queryEditor.id), - ) - .then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); + store.dispatch( + actions.queryEditorSetTitle(queryEditor, name, queryEditor.id), + ); + expect(store.getActions()).toEqual(expectedActions); }); }); @@ -803,7 +705,7 @@ describe('async actions', () => { describe('queryEditorSetQueryLimit', () => { it('updates the tab state in the backend', () => { - expect.assertions(2); + expect.assertions(1); const queryLimit = 10; const store = mockStore({}); @@ -814,18 +716,16 @@ describe('async actions', () => { queryLimit, }, ]; - return store - .dispatch(actions.queryEditorSetQueryLimit(queryEditor, queryLimit)) - .then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); + store.dispatch( + actions.queryEditorSetQueryLimit(queryEditor, queryLimit), + ); + expect(store.getActions()).toEqual(expectedActions); }); }); describe('queryEditorSetTemplateParams', () => { it('updates the tab state in the backend', () => { - expect.assertions(2); + expect.assertions(1); const templateParams = '{"foo": "bar"}'; const store = mockStore({}); @@ -836,14 +736,11 @@ describe('async actions', () => { templateParams, }, ]; - return store - .dispatch( - actions.queryEditorSetTemplateParams(queryEditor, templateParams), - ) - .then(() => { - expect(store.getActions()).toEqual(expectedActions); - expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1); - }); + store.dispatch( + actions.queryEditorSetTemplateParams(queryEditor, templateParams), + ); + + 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 new file mode 100644 index 0000000000000..7e3cc264d6591 --- /dev/null +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx @@ -0,0 +1,178 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React from 'react'; +import fetchMock from 'fetch-mock'; +import * as featureFlags from 'src/featureFlags'; +import { FeatureFlag } from '@superset-ui/core'; +import { render, act } from 'spec/helpers/testing-library'; +import ToastContainer from 'src/components/MessageToasts/ToastContainer'; +import { initialState, defaultQueryEditor } from 'src/SqlLab/fixtures'; +import EditorAutoSync from '.'; + +const editorTabLastUpdatedAt = Date.now(); +const unsavedSqlLabState = { + ...initialState.sqlLab, + unsavedQueryEditor: { + id: defaultQueryEditor.id, + name: 'updated tab name', + updatedAt: editorTabLastUpdatedAt + 100, + }, + editorTabLastUpdatedAt, +}; +beforeAll(() => { + jest.useFakeTimers(); +}); + +afterAll(() => { + jest.useRealTimers(); +}); + +test('sync the unsaved editor tab state with api when unsaved has made since lastest updated', async () => { + const updateEditorTabState = `glob:*/tabstateview/${defaultQueryEditor.id}`; + fetchMock.put(updateEditorTabState, 200); + const isFeatureEnabledMock = jest + .spyOn(featureFlags, 'isFeatureEnabled') + .mockImplementation( + featureFlag => featureFlag === FeatureFlag.SQLLAB_BACKEND_PERSISTENCE, + ); + expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); + render(, { + useRedux: true, + initialState: { + ...initialState, + sqlLab: unsavedSqlLabState, + }, + }); + await act(async () => { + jest.runAllTimers(); + }); + expect(fetchMock.calls(updateEditorTabState)).toHaveLength(1); + isFeatureEnabledMock.mockClear(); + fetchMock.restore(); +}); + +test('skip syncing the unsaved editor tab state when the updates already synced', async () => { + const updateEditorTabState = `glob:*/tabstateview/${defaultQueryEditor.id}`; + fetchMock.put(updateEditorTabState, 200); + const isFeatureEnabledMock = jest + .spyOn(featureFlags, 'isFeatureEnabled') + .mockImplementation( + featureFlag => featureFlag === FeatureFlag.SQLLAB_BACKEND_PERSISTENCE, + ); + expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); + render(, { + useRedux: true, + initialState: { + ...initialState, + sqlLab: { + ...initialState.sqlLab, + unsavedQueryEditor: { + id: defaultQueryEditor.id, + name: 'updated tab name', + updatedAt: editorTabLastUpdatedAt - 100, + }, + editorTabLastUpdatedAt, + }, + }, + }); + await act(async () => { + jest.runAllTimers(); + }); + expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); + isFeatureEnabledMock.mockClear(); + fetchMock.restore(); +}); + +test('renders an error toast when api update failed', async () => { + const updateEditorTabState = `glob:*/tabstateview/${defaultQueryEditor.id}`; + fetchMock.put(updateEditorTabState, { + throws: new Error('errorMessage'), + }); + const isFeatureEnabledMock = jest + .spyOn(featureFlags, 'isFeatureEnabled') + .mockImplementation( + featureFlag => featureFlag === FeatureFlag.SQLLAB_BACKEND_PERSISTENCE, + ); + expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); + const { findByText } = render( + <> + + + , + { + useRedux: true, + initialState: { + ...initialState, + sqlLab: unsavedSqlLabState, + }, + }, + ); + await act(async () => { + jest.runAllTimers(); + }); + const errorToast = await findByText( + 'An error occurred while storing your query in the backend. ' + + 'Please contact your administrator if this problem persists.', + ); + expect(errorToast).toBeTruthy(); + isFeatureEnabledMock.mockClear(); + fetchMock.restore(); +}); + +test('skip syncing the unsaved editor tab state with api when SQLLAB_BACKEND_PERSISTENCE is off', async () => { + const updateEditorTabState = `glob:*/tabstateview/${defaultQueryEditor.id}`; + fetchMock.put(updateEditorTabState, 200); + const isFeatureEnabledMock = jest + .spyOn(featureFlags, 'isFeatureEnabled') + .mockImplementation( + featureFlag => featureFlag !== FeatureFlag.SQLLAB_BACKEND_PERSISTENCE, + ); + expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); + render(, { + useRedux: true, + initialState: { + ...initialState, + sqlLab: unsavedSqlLabState, + }, + }); + await act(async () => { + jest.runAllTimers(); + }); + expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); + isFeatureEnabledMock.mockClear(); + fetchMock.restore(); +}); diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx new file mode 100644 index 0000000000000..a883314aa9e9d --- /dev/null +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx @@ -0,0 +1,120 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React, { useRef, useEffect } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; +import { FeatureFlag, t } from '@superset-ui/core'; +import { + SqlLabRootState, + QueryEditor, + UnsavedQueryEditor, +} from 'src/SqlLab/types'; +import { isFeatureEnabled } from 'src/featureFlags'; +import { useUpdateSqlEditorTabMutation } from 'src/hooks/apiResources/sqlEditorTabs'; +import { useDebounceValue } from 'src/hooks/useDebounceValue'; +import { + addDangerToast, + setEditorTabLastUpdate, +} from 'src/SqlLab/actions/sqlLab'; + +const INTERVAL = 5000; + +function hasUnsavedChanges( + queryEditor: QueryEditor, + lastSavedDateTime: number, +) { + return ( + queryEditor.inLocalStorage || + (queryEditor.updatedAt && queryEditor.updatedAt > lastSavedDateTime) + ); +} + +export function filterUnsavedQueryEditorList( + queryEditors: QueryEditor[], + unsavedQueryEditor: UnsavedQueryEditor, + lastSavedDateTime: number, +) { + return queryEditors + .map(queryEditor => ({ + ...queryEditor, + ...(unsavedQueryEditor.id === queryEditor.id && unsavedQueryEditor), + })) + .filter(queryEditor => hasUnsavedChanges(queryEditor, lastSavedDateTime)); +} + +const EditorAutoSync: React.FC = () => { + const queryEditors = useSelector( + state => state.sqlLab.queryEditors, + ); + const unsavedQueryEditor = useSelector( + state => state.sqlLab.unsavedQueryEditor, + ); + const editorTabLastUpdatedAt = useSelector( + state => state.sqlLab.editorTabLastUpdatedAt, + ); + const dispatch = useDispatch(); + const lastSavedDateTimeRef = useRef(editorTabLastUpdatedAt); + const [updateSqlEditor, { isError }] = useUpdateSqlEditorTabMutation(); + + const debouncedUnsavedQueryEditor = useDebounceValue( + unsavedQueryEditor, + INTERVAL, + ); + + useEffect(() => { + if (!isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE)) { + return; + } + const unsaved = filterUnsavedQueryEditorList( + queryEditors, + debouncedUnsavedQueryEditor, + lastSavedDateTimeRef.current, + ); + + Promise.all( + unsaved + // TODO: Migrate migrateQueryEditorFromLocalStorage + // in TabbedSqlEditors logic by addSqlEditor mutation later + .filter(({ inLocalStorage }) => !inLocalStorage) + .map(queryEditor => updateSqlEditor({ queryEditor })), + ).then(resolvers => { + if (!resolvers.some(result => 'error' in result)) { + lastSavedDateTimeRef.current = Date.now(); + dispatch(setEditorTabLastUpdate(lastSavedDateTimeRef.current)); + } + }); + }, [debouncedUnsavedQueryEditor, dispatch, queryEditors, updateSqlEditor]); + + useEffect(() => { + if (isError) { + dispatch( + addDangerToast( + t( + 'An error occurred while storing your query in the backend. ' + + 'Please contact your administrator if this problem persists.', + ), + ), + ); + } + }, [dispatch, isError]); + + return null; +}; + +export default EditorAutoSync; diff --git a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx index 6ddae08e68520..5dc8a43c19310 100644 --- a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx @@ -25,7 +25,7 @@ import { t, useTheme, QueryResponse } from '@superset-ui/core'; import { useDispatch, useSelector } from 'react-redux'; import { - queryEditorSetAndSaveSql, + queryEditorSetSql, cloneQueryToNewTab, fetchQueryResults, clearQueryResults, @@ -109,7 +109,9 @@ const QueryTable = ({ const data = useMemo(() => { const restoreSql = (query: QueryResponse) => { - dispatch(queryEditorSetAndSaveSql({ id: query.sqlEditorId }, query.sql)); + dispatch( + queryEditorSetSql({ id: query.sqlEditorId }, query.sql, query.id), + ); }; const openQueryInNewTab = (query: QueryResponse) => { diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx index 609cb917b6f20..73941fbc79c6d 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx @@ -557,10 +557,9 @@ const SqlEditor: React.FC = ({ [setQueryEditorAndSaveSql], ); - const onSqlChanged = (sql: string) => { + const onSqlChanged = useEffectEvent((sql: string) => { dispatch(queryEditorSetSql(queryEditor, sql)); - setQueryEditorAndSaveSqlWithDebounce(sql); - }; + }); // Return the heights for the ace editor and the south pane as an object // given the height of the sql editor, north pane percent and south pane percent. @@ -785,7 +784,7 @@ const SqlEditor: React.FC = ({ )} 1.0 for typescript support import persistState from 'redux-localstorage'; +import { pickBy } from 'lodash'; +import { isFeatureEnabled, FeatureFlag } from '@superset-ui/core'; +import { filterUnsavedQueryEditorList } from 'src/SqlLab/components/EditorAutoSync'; import { emptyTablePersistData, emptyQueryResults, @@ -38,6 +41,37 @@ const sqlLabPersistStateConfig = { slicer: paths => state => { const subset = {}; paths.forEach(path => { + if (path === 'sqlLab.unsavedQueryEditor') { + const { + queryEditors, + editorTabLastUpdatedAt, + unsavedQueryEditor, + tables, + queries, + tabHistory, + } = state.sqlLab; + const unsavedQueryEditors = filterUnsavedQueryEditorList( + queryEditors, + unsavedQueryEditor, + editorTabLastUpdatedAt, + ); + const hasFinishedMigrationFromLocalStorage = + unsavedQueryEditors.every(({ inLocalStorage }) => !inLocalStorage); + if (unsavedQueryEditors.length > 0) { + subset.sqlLab = { + queryEditors: unsavedQueryEditors, + ...(!hasFinishedMigrationFromLocalStorage && { + tabHistory, + tables: tables.filter(table => table.inLocalStorage), + queries: pickBy( + queries, + query => query.inLocalStorage && !query.isDataPreview, + ), + }), + }; + } + return; + } // this line is used to remove old data from browser localStorage. // we used to persist all redux state into localStorage, but // it caused configurations passed from server-side got override. @@ -80,6 +114,8 @@ const sqlLabPersistStateConfig = { }; export const persistSqlLabStateEnhancer = persistState( - sqlLabPersistStateConfig.paths, + isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) + ? ['sqlLab.unsavedQueryEditor'] + : sqlLabPersistStateConfig.paths, sqlLabPersistStateConfig.config, ); diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts b/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts index aca11e2cca10f..009101ca1e7d5 100644 --- a/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts +++ b/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts @@ -54,6 +54,10 @@ const apiDataWithTabState = { }, }; describe('getInitialState', () => { + afterEach(() => { + localStorage.clear(); + }); + it('should output the user that is passed in', () => { expect(getInitialState(apiData).user?.userId).toEqual(1); }); @@ -134,10 +138,6 @@ describe('getInitialState', () => { }); describe('dedupe tables schema', () => { - afterEach(() => { - localStorage.clear(); - }); - it('should dedupe the table schema', () => { localStorage.setItem( 'redux', @@ -245,4 +245,106 @@ describe('getInitialState', () => { ); }); }); + + describe('restore unsaved changes for PERSISTENCE mode', () => { + const lastUpdatedTime = Date.now(); + const expectedValue = 'updated editor value'; + beforeEach(() => { + localStorage.setItem( + 'redux', + JSON.stringify({ + sqlLab: { + queryEditors: [ + { + // restore cached value since updates are after server update time + id: '1', + name: expectedValue, + updatedAt: lastUpdatedTime + 100, + }, + { + // out of update since last updated time is before server update time + id: '2', + name: expectedValue, + updatedAt: lastUpdatedTime - 100, + }, + { + // out of update since no updatedAt + id: '3', + name: expectedValue, + }, + ], + }, + }), + ); + }); + + it('restore unsaved changes for PERSISTENCE mode', () => { + const apiDataWithLocalStorage = { + ...apiData, + active_tab: { + id: 1, + label: 'persisted tab', + table_schemas: [], + extra_json: { + updatedAt: lastUpdatedTime, + }, + }, + tab_state_ids: [{ id: 1 }], + }; + expect( + getInitialState(apiDataWithLocalStorage).sqlLab.queryEditors[0], + ).toEqual( + expect.objectContaining({ + id: '1', + name: expectedValue, + }), + ); + }); + + it('skip unsaved changes for expired data', () => { + const apiDataWithLocalStorage = { + ...apiData, + active_tab: { + id: 2, + label: 'persisted tab', + table_schemas: [], + extra_json: { + updatedAt: lastUpdatedTime, + }, + }, + tab_state_ids: [{ id: 2 }], + }; + expect( + getInitialState(apiDataWithLocalStorage).sqlLab.queryEditors[1], + ).toEqual( + expect.objectContaining({ + id: '2', + name: apiDataWithLocalStorage.active_tab.label, + }), + ); + }); + + it('skip unsaved changes for legacy cache data', () => { + const apiDataWithLocalStorage = { + ...apiData, + active_tab: { + id: 3, + label: 'persisted tab', + table_schemas: [], + extra_json: { + updatedAt: lastUpdatedTime, + }, + }, + tab_state_ids: [{ id: 3 }], + }; + expect( + getInitialState(apiDataWithLocalStorage).sqlLab.queryEditors[2], + ).toEqual( + expect.objectContaining({ + id: '3', + name: apiDataWithLocalStorage.active_tab.label, + }), + ); + }); + }); }); diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.ts b/superset-frontend/src/SqlLab/reducers/getInitialState.ts index e2aa1d4688738..c55c9d1ceffc0 100644 --- a/superset-frontend/src/SqlLab/reducers/getInitialState.ts +++ b/superset-frontend/src/SqlLab/reducers/getInitialState.ts @@ -88,6 +88,7 @@ export default function getInitialState({ schema: activeTab.schema, queryLimit: activeTab.query_limit, hideLeftBar: activeTab.hide_left_bar, + updatedAt: activeTab.extra_json?.updatedAt, }; } else { // dummy state, actual state will be loaded on tab switch @@ -103,11 +104,12 @@ export default function getInitialState({ [queryEditor.id]: queryEditor, }; }); - const tabHistory = activeTab ? [activeTab.id.toString()] : []; let tables = {} as Record; - const editorTabLastUpdatedAt = Date.now(); + let editorTabLastUpdatedAt = Date.now(); if (activeTab) { + editorTabLastUpdatedAt = + activeTab.extra_json?.updatedAt || editorTabLastUpdatedAt; activeTab.table_schemas .filter(tableSchema => tableSchema.description !== null) .forEach(tableSchema => { @@ -153,37 +155,57 @@ export default function getInitialState({ // add query editors and tables to state with a special flag so they can // be migrated if the `SQLLAB_BACKEND_PERSISTENCE` feature flag is on sqlLab.queryEditors.forEach(qe => { + const hasConflictFromBackend = Boolean(queryEditors[qe.id]); + const unsavedUpdatedAt = queryEditors[qe.id]?.updatedAt; + const hasUnsavedUpdateSinceLastSave = + qe.updatedAt && + (!unsavedUpdatedAt || qe.updatedAt > unsavedUpdatedAt); + const cachedQueryEditor: UnsavedQueryEditor = + !hasConflictFromBackend || hasUnsavedUpdateSinceLastSave ? qe : {}; queryEditors = { ...queryEditors, [qe.id]: { ...queryEditors[qe.id], - ...qe, - name: qe.title || qe.name, - ...(unsavedQueryEditor.id === qe.id && unsavedQueryEditor), - inLocalStorage: true, + ...cachedQueryEditor, + name: + cachedQueryEditor.title || + cachedQueryEditor.name || + queryEditors[qe.id]?.name, + ...(cachedQueryEditor.id && + unsavedQueryEditor.id === qe.id && + unsavedQueryEditor), + inLocalStorage: !hasConflictFromBackend, loaded: true, }, }; }); const expandedTables = new Set(); - tables = sqlLab.tables.reduce((merged, table) => { - const expanded = !expandedTables.has(table.queryEditorId); - if (expanded) { - expandedTables.add(table.queryEditorId); - } - return { - ...merged, - [table.id]: { - ...tables[table.id], - ...table, - expanded, - }, - }; - }, tables); - Object.values(sqlLab.queries).forEach(query => { - queries[query.id] = { ...query, inLocalStorage: true }; - }); - tabHistory.push(...sqlLab.tabHistory); + + if (sqlLab.tables) { + tables = sqlLab.tables.reduce((merged, table) => { + const expanded = !expandedTables.has(table.queryEditorId); + if (expanded) { + expandedTables.add(table.queryEditorId); + } + return { + ...merged, + [table.id]: { + ...tables[table.id], + ...table, + expanded, + inLocalStorage: true, + }, + }; + }, tables); + } + if (sqlLab.queries) { + Object.values(sqlLab.queries).forEach(query => { + queries[query.id] = { ...query, inLocalStorage: true }; + }); + } + if (sqlLab.tabHistory) { + tabHistory.push(...sqlLab.tabHistory); + } } } } catch (error) { diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.js b/superset-frontend/src/SqlLab/reducers/sqlLab.js index 278109564f96a..59bd0558a1f1c 100644 --- a/superset-frontend/src/SqlLab/reducers/sqlLab.js +++ b/superset-frontend/src/SqlLab/reducers/sqlLab.js @@ -29,7 +29,7 @@ import { extendArr, } from '../../reduxUtils'; -function alterUnsavedQueryEditorState(state, updatedState, id) { +function alterUnsavedQueryEditorState(state, updatedState, id, silent = false) { if (state.tabHistory[state.tabHistory.length - 1] !== id) { const { queryEditors } = alterInArr( state, @@ -45,6 +45,7 @@ function alterUnsavedQueryEditorState(state, updatedState, id) { unsavedQueryEditor: { ...(state.unsavedQueryEditor.id === id && state.unsavedQueryEditor), ...(id ? { id, ...updatedState } : state.unsavedQueryEditor), + ...(!silent && { updatedAt: new Date().getTime() }), }, }; } @@ -64,7 +65,10 @@ export default function sqlLabReducer(state = {}, action) { ...mergeUnsavedState, tabHistory: [...state.tabHistory, action.queryEditor.id], }; - return addToArr(newState, 'queryEditors', action.queryEditor); + return addToArr(newState, 'queryEditors', { + ...action.queryEditor, + updatedAt: new Date().getTime(), + }); }, [actions.QUERY_EDITOR_SAVED]() { const { query, result, clientId } = action; @@ -308,6 +312,7 @@ export default function sqlLabReducer(state = {}, action) { latestQueryId: action.query.id, }, action.query.sqlEditorId, + action.query.isDataPreview, ), }; }, @@ -378,14 +383,12 @@ export default function sqlLabReducer(state = {}, action) { qeIds.indexOf(action.queryEditor?.id) > -1 && state.tabHistory[state.tabHistory.length - 1] !== action.queryEditor.id ) { - const mergeUnsavedState = alterInArr( - state, - 'queryEditors', - state.unsavedQueryEditor, - { + const mergeUnsavedState = { + ...alterInArr(state, 'queryEditors', state.unsavedQueryEditor, { ...state.unsavedQueryEditor, - }, - ); + }), + unsavedQueryEditor: {}, + }; return { ...(action.queryEditor.id === state.unsavedQueryEditor.id ? alterInArr( @@ -522,12 +525,20 @@ export default function sqlLabReducer(state = {}, action) { }; }, [actions.QUERY_EDITOR_SET_SQL]() { + const { unsavedQueryEditor } = state; + if ( + unsavedQueryEditor?.id === action.queryEditor.id && + unsavedQueryEditor.sql === action.sql + ) { + return state; + } return { ...state, ...alterUnsavedQueryEditorState( state, { sql: action.sql, + ...(action.queryId && { latestQueryId: action.queryId }), }, action.queryEditor.id, ), @@ -566,6 +577,7 @@ export default function sqlLabReducer(state = {}, action) { selectedText: action.sql, }, action.queryEditor.id, + true, ), }; }, @@ -708,6 +720,9 @@ export default function sqlLabReducer(state = {}, action) { [actions.CREATE_DATASOURCE_FAILED]() { return { ...state, isDatasourceLoading: false, errorMessage: action.err }; }, + [actions.SET_EDITOR_TAB_LAST_UPDATE]() { + return { ...state, editorTabLastUpdatedAt: action.timestamp }; + }, }; 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 5ecd69293ca8b..9b1548a87fff6 100644 --- a/superset-frontend/src/SqlLab/types.ts +++ b/superset-frontend/src/SqlLab/types.ts @@ -48,6 +48,7 @@ export interface QueryEditor { inLocalStorage?: boolean; northPercent?: number; southPercent?: number; + updatedAt?: number; } export type toastState = { diff --git a/superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts new file mode 100644 index 0000000000000..aad2191ca6855 --- /dev/null +++ b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts @@ -0,0 +1,94 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import fetchMock from 'fetch-mock'; +import { act, renderHook } from '@testing-library/react-hooks'; +import { + createWrapper, + defaultStore as store, +} from 'spec/helpers/testing-library'; +import { api } from 'src/hooks/apiResources/queryApi'; +import { useUpdateSqlEditorTabMutation } from './sqlEditorTabs'; + +const expectedQueryEditor = { + id: '123', + dbId: 456, + name: 'tab 1', + sql: 'SELECT * from example_table', + schema: 'my_schema', + templateParams: '{"a": 1, "v": "str"}', + queryLimit: 1000, + remoteId: null, + autorun: false, + hideLeftBar: false, + updatedAt: Date.now(), +}; + +afterEach(() => { + fetchMock.reset(); + act(() => { + store.dispatch(api.util.resetApiState()); + }); +}); + +test('puts api request with formData', async () => { + const tabStateMutationApiRoute = `glob:*/tabstateview/${expectedQueryEditor.id}`; + fetchMock.put(tabStateMutationApiRoute, 200); + const { result, waitFor } = renderHook( + () => useUpdateSqlEditorTabMutation(), + { + wrapper: createWrapper({ + useRedux: true, + store, + }), + }, + ); + act(() => { + result.current[0]({ + queryEditor: expectedQueryEditor, + }); + }); + await waitFor(() => + expect(fetchMock.calls(tabStateMutationApiRoute).length).toBe(1), + ); + const formData = fetchMock.calls(tabStateMutationApiRoute)[0][1] + ?.body as FormData; + expect(formData.get('database_id')).toBe(`${expectedQueryEditor.dbId}`); + expect(formData.get('schema')).toBe( + JSON.stringify(`${expectedQueryEditor.schema}`), + ); + expect(formData.get('sql')).toBe( + JSON.stringify(`${expectedQueryEditor.sql}`), + ); + expect(formData.get('label')).toBe( + JSON.stringify(`${expectedQueryEditor.name}`), + ); + expect(formData.get('query_limit')).toBe(`${expectedQueryEditor.queryLimit}`); + expect(formData.has('latest_query_id')).toBe(false); + expect(formData.get('template_params')).toBe( + JSON.stringify(`${expectedQueryEditor.templateParams}`), + ); + expect(formData.get('hide_left_bar')).toBe( + `${expectedQueryEditor.hideLeftBar}`, + ); + expect(formData.get('extra_json')).toBe( + JSON.stringify( + JSON.stringify({ updatedAt: expectedQueryEditor.updatedAt }), + ), + ); +}); diff --git a/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts new file mode 100644 index 0000000000000..ba2b85d358a9a --- /dev/null +++ b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts @@ -0,0 +1,69 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { pickBy } from 'lodash'; +import { QueryEditor } from 'src/SqlLab/types'; +import { api, JsonResponse } from './queryApi'; + +export type EditorMutationParams = { + queryEditor: QueryEditor; + extra?: Record; +}; + +const sqlEditorApi = api.injectEndpoints({ + endpoints: builder => ({ + updateSqlEditorTab: builder.mutation({ + query: ({ + queryEditor: { + id, + dbId, + schema, + queryLimit, + sql, + name, + latestQueryId, + hideLeftBar, + templateParams, + autorun, + updatedAt, + }, + extra, + }) => ({ + method: 'PUT', + endpoint: encodeURI(`/tabstateview/${id}`), + postPayload: pickBy( + { + database_id: dbId, + schema, + sql, + label: name, + query_limit: queryLimit, + latest_query_id: latestQueryId, + template_params: templateParams, + hide_left_bar: hideLeftBar, + autorun, + extra_json: JSON.stringify({ updatedAt, ...extra }), + }, + value => value !== undefined, + ), + }), + }), + }), +}); + +export const { useUpdateSqlEditorTabMutation } = sqlEditorApi; diff --git a/superset-frontend/src/hooks/apiResources/sqlLab.ts b/superset-frontend/src/hooks/apiResources/sqlLab.ts index 123db414e2681..16e8ffde6c609 100644 --- a/superset-frontend/src/hooks/apiResources/sqlLab.ts +++ b/superset-frontend/src/hooks/apiResources/sqlLab.ts @@ -50,7 +50,7 @@ export type InitialState = { template_params: string | null; hide_left_bar?: boolean; saved_query: { id: number } | null; - extra_json?: object; + extra_json?: Record; }; databases: object[]; queries: Record< diff --git a/superset-frontend/src/hooks/useDebounceValue.ts b/superset-frontend/src/hooks/useDebounceValue.ts index 711b2dbd5a98c..862c83770779d 100644 --- a/superset-frontend/src/hooks/useDebounceValue.ts +++ b/superset-frontend/src/hooks/useDebounceValue.ts @@ -19,8 +19,8 @@ import { useState, useEffect } from 'react'; import { FAST_DEBOUNCE } from 'src/constants'; -export function useDebounceValue(value: string, delay = FAST_DEBOUNCE) { - const [debouncedValue, setDebouncedValue] = useState(value); +export function useDebounceValue(value: T, delay = FAST_DEBOUNCE) { + const [debouncedValue, setDebouncedValue] = useState(value); useEffect(() => { const handler: NodeJS.Timeout = setTimeout(() => { diff --git a/superset-frontend/src/pages/SqlLab/index.tsx b/superset-frontend/src/pages/SqlLab/index.tsx index e9f84f1b1d646..16a0e917a5a53 100644 --- a/superset-frontend/src/pages/SqlLab/index.tsx +++ b/superset-frontend/src/pages/SqlLab/index.tsx @@ -18,7 +18,7 @@ */ import React, { useEffect } from 'react'; import { useDispatch, useSelector } from 'react-redux'; -import { css } from '@superset-ui/core'; +import { css, isFeatureEnabled, FeatureFlag } from '@superset-ui/core'; import { useSqlLabInitialState } from 'src/hooks/apiResources/sqlLab'; import type { InitialState } from 'src/hooks/apiResources/sqlLab'; import { resetState } from 'src/SqlLab/actions/sqlLab'; @@ -27,6 +27,7 @@ import type { SqlLabRootState } from 'src/SqlLab/types'; import { SqlLabGlobalStyles } from 'src/SqlLab//SqlLabGlobalStyles'; import App from 'src/SqlLab/components/App'; import Loading from 'src/components/Loading'; +import EditorAutoSync from 'src/SqlLab/components/EditorAutoSync'; import useEffectEvent from 'src/hooks/useEffectEvent'; import { LocationProvider } from './LocationContext'; @@ -72,6 +73,9 @@ export default function SqlLab() { > + {isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) && ( + + )} ); diff --git a/superset-frontend/src/views/store.ts b/superset-frontend/src/views/store.ts index 55df81c588b5f..65abca796ecd4 100644 --- a/superset-frontend/src/views/store.ts +++ b/superset-frontend/src/views/store.ts @@ -167,9 +167,7 @@ export function setupStore({ }, middleware: getMiddleware, devTools: process.env.WEBPACK_MODE === 'development' && !disableDebugger, - ...(!isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) && { - enhancers: [persistSqlLabStateEnhancer as StoreEnhancer], - }), + enhancers: [persistSqlLabStateEnhancer as StoreEnhancer], ...overrides, }); } From 957b7dde16ba51d9ea9a7fa6723f1ff15248c8a7 Mon Sep 17 00:00:00 2001 From: Justin Park Date: Wed, 28 Jun 2023 07:13:54 -0700 Subject: [PATCH 02/17] remove unused ref --- superset-frontend/src/SqlLab/actions/sqlLab.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js index 0ba039bc33db2..6065e7cce0500 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -32,7 +32,6 @@ import { initialState, queryId, } from 'src/SqlLab/fixtures'; -import { QueryState } from '@superset-ui/core'; const middlewares = [thunk]; const mockStore = configureMockStore(middlewares); From 6df0d29713f946833884ddae8014445f2ac359a4 Mon Sep 17 00:00:00 2001 From: justinpark Date: Thu, 13 Jul 2023 15:26:09 -0700 Subject: [PATCH 03/17] clean up constant and logic location --- .../SqlLab/middlewares/persistSqlLabStateEnhancer.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js b/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js index 944a2910c7ee4..574fd23c520b0 100644 --- a/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js +++ b/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js @@ -35,13 +35,15 @@ const CLEAR_ENTITY_HELPERS_MAP = { unsavedQueryEditor: qe => clearQueryEditors([qe])[0], }; +const PERSISTENCE_LOCAL_STORAGE_PATH = 'sqlLab.unsavedQueryEditor'; + const sqlLabPersistStateConfig = { paths: ['sqlLab'], config: { slicer: paths => state => { const subset = {}; paths.forEach(path => { - if (path === 'sqlLab.unsavedQueryEditor') { + if (path === PERSISTENCE_LOCAL_STORAGE_PATH) { const { queryEditors, editorTabLastUpdatedAt, @@ -55,9 +57,11 @@ const sqlLabPersistStateConfig = { unsavedQueryEditor, editorTabLastUpdatedAt, ); - const hasFinishedMigrationFromLocalStorage = - unsavedQueryEditors.every(({ inLocalStorage }) => !inLocalStorage); if (unsavedQueryEditors.length > 0) { + const hasFinishedMigrationFromLocalStorage = + unsavedQueryEditors.every( + ({ inLocalStorage }) => !inLocalStorage, + ); subset.sqlLab = { queryEditors: unsavedQueryEditors, ...(!hasFinishedMigrationFromLocalStorage && { @@ -115,7 +119,7 @@ const sqlLabPersistStateConfig = { export const persistSqlLabStateEnhancer = persistState( isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? ['sqlLab.unsavedQueryEditor'] + ? [PERSISTENCE_LOCAL_STORAGE_PATH] : sqlLabPersistStateConfig.paths, sqlLabPersistStateConfig.config, ); From ec881cd232ecdb8c8e5072d1064b1a61257be45d Mon Sep 17 00:00:00 2001 From: justinpark Date: Thu, 13 Jul 2023 15:28:27 -0700 Subject: [PATCH 04/17] rename datetime to timestamp --- .../SqlLab/components/EditorAutoSync/index.tsx | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx index a883314aa9e9d..e30cb801e2534 100644 --- a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx @@ -37,25 +37,25 @@ const INTERVAL = 5000; function hasUnsavedChanges( queryEditor: QueryEditor, - lastSavedDateTime: number, + lastSavedTimestamp: number, ) { return ( queryEditor.inLocalStorage || - (queryEditor.updatedAt && queryEditor.updatedAt > lastSavedDateTime) + (queryEditor.updatedAt && queryEditor.updatedAt > lastSavedTimestamp) ); } export function filterUnsavedQueryEditorList( queryEditors: QueryEditor[], unsavedQueryEditor: UnsavedQueryEditor, - lastSavedDateTime: number, + lastSavedTimestamp: number, ) { return queryEditors .map(queryEditor => ({ ...queryEditor, ...(unsavedQueryEditor.id === queryEditor.id && unsavedQueryEditor), })) - .filter(queryEditor => hasUnsavedChanges(queryEditor, lastSavedDateTime)); + .filter(queryEditor => hasUnsavedChanges(queryEditor, lastSavedTimestamp)); } const EditorAutoSync: React.FC = () => { @@ -69,7 +69,7 @@ const EditorAutoSync: React.FC = () => { state => state.sqlLab.editorTabLastUpdatedAt, ); const dispatch = useDispatch(); - const lastSavedDateTimeRef = useRef(editorTabLastUpdatedAt); + const lastSavedTimestampRef = useRef(editorTabLastUpdatedAt); const [updateSqlEditor, { isError }] = useUpdateSqlEditorTabMutation(); const debouncedUnsavedQueryEditor = useDebounceValue( @@ -84,7 +84,7 @@ const EditorAutoSync: React.FC = () => { const unsaved = filterUnsavedQueryEditorList( queryEditors, debouncedUnsavedQueryEditor, - lastSavedDateTimeRef.current, + lastSavedTimestampRef.current, ); Promise.all( @@ -95,8 +95,8 @@ const EditorAutoSync: React.FC = () => { .map(queryEditor => updateSqlEditor({ queryEditor })), ).then(resolvers => { if (!resolvers.some(result => 'error' in result)) { - lastSavedDateTimeRef.current = Date.now(); - dispatch(setEditorTabLastUpdate(lastSavedDateTimeRef.current)); + lastSavedTimestampRef.current = Date.now(); + dispatch(setEditorTabLastUpdate(lastSavedTimestampRef.current)); } }); }, [debouncedUnsavedQueryEditor, dispatch, queryEditors, updateSqlEditor]); From bb73656f2f6049e3b1e8a1f0109a65e22c536153 Mon Sep 17 00:00:00 2001 From: justinpark Date: Thu, 13 Jul 2023 15:35:20 -0700 Subject: [PATCH 05/17] typo --- .../components/EditorAutoSync/EditorAutoSync.test.tsx | 8 ++++---- .../src/SqlLab/components/EditorAutoSync/index.tsx | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx index 7e3cc264d6591..19e2a0c1bf680 100644 --- a/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx @@ -61,7 +61,7 @@ afterAll(() => { jest.useRealTimers(); }); -test('sync the unsaved editor tab state with api when unsaved has made since lastest updated', async () => { +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); const isFeatureEnabledMock = jest @@ -85,7 +85,7 @@ test('sync the unsaved editor tab state with api when unsaved has made since las fetchMock.restore(); }); -test('skip syncing the unsaved editor tab state when the updates already synced', async () => { +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); const isFeatureEnabledMock = jest @@ -117,7 +117,7 @@ test('skip syncing the unsaved editor tab state when the updates already synced' fetchMock.restore(); }); -test('renders an error toast when api update failed', async () => { +test('renders an error toast when the sync failed', async () => { const updateEditorTabState = `glob:*/tabstateview/${defaultQueryEditor.id}`; fetchMock.put(updateEditorTabState, { throws: new Error('errorMessage'), @@ -153,7 +153,7 @@ test('renders an error toast when api update failed', async () => { fetchMock.restore(); }); -test('skip syncing the unsaved editor tab state with api when SQLLAB_BACKEND_PERSISTENCE is off', async () => { +test('skip syncing the unsaved editor tab stat when SQLLAB_BACKEND_PERSISTENCE is off', async () => { const updateEditorTabState = `glob:*/tabstateview/${defaultQueryEditor.id}`; fetchMock.put(updateEditorTabState, 200); const isFeatureEnabledMock = jest diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx index e30cb801e2534..c8c077292bc48 100644 --- a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx @@ -106,7 +106,7 @@ const EditorAutoSync: React.FC = () => { dispatch( addDangerToast( t( - 'An error occurred while storing your query in the backend. ' + + 'An error occurred while saving your editor state. ' + 'Please contact your administrator if this problem persists.', ), ), From c28aee731252907a176e76d51069309946fc6180 Mon Sep 17 00:00:00 2001 From: justinpark Date: Thu, 13 Jul 2023 16:22:38 -0700 Subject: [PATCH 06/17] accidentally load_query_editor from new qe due to missing `loaded:true` flag --- superset-frontend/src/SqlLab/actions/sqlLab.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 17dc92e0af619..567d3383d752d 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -515,14 +515,15 @@ export function addQueryEditor(queryEditor) { ? SupersetClient.post({ endpoint: '/tabstateview/', postPayload: { queryEditor }, - }) - : Promise.resolve({ json: { id: shortid.generate() } }); + }).then(({ json }) => ({ ...json, loaded: true })) + : Promise.resolve({ id: shortid.generate() }); return sync - .then(({ json }) => { + .then(({ id, loaded }) => { const newQueryEditor = { ...queryEditor, - id: json.id.toString(), + id: id.toString(), + loaded, }; return dispatch({ type: ADD_QUERY_EDITOR, From fa48cc07c68616ad72c999b116c03472aafde9b3 Mon Sep 17 00:00:00 2001 From: justinpark Date: Thu, 13 Jul 2023 21:54:09 -0700 Subject: [PATCH 07/17] update message in the spec --- .../SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx index 19e2a0c1bf680..2c52eb71f1215 100644 --- a/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx @@ -145,7 +145,7 @@ test('renders an error toast when the sync failed', async () => { jest.runAllTimers(); }); const errorToast = await findByText( - 'An error occurred while storing your query in the backend. ' + + 'An error occurred while saving your editor state. ' + 'Please contact your administrator if this problem persists.', ); expect(errorToast).toBeTruthy(); From c5e04fea0c076ec852838a70659d2c5bc9e1b366 Mon Sep 17 00:00:00 2001 From: justinpark Date: Fri, 14 Jul 2023 09:03:02 -0700 Subject: [PATCH 08/17] fix test for loaded flag --- superset-frontend/src/SqlLab/actions/sqlLab.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js index 6065e7cce0500..175ea06ec3ecf 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -538,7 +538,7 @@ describe('async actions', () => { const expectedActions = [ { type: actions.ADD_QUERY_EDITOR, - queryEditor: { ...queryEditor, id: '1' }, + queryEditor: { ...queryEditor, id: '1', loaded: true }, }, ]; return store.dispatch(actions.addQueryEditor(queryEditor)).then(() => { From accb90f09f5028b1d83c0dcbfd0abe0aec0fa975 Mon Sep 17 00:00:00 2001 From: justinpark Date: Thu, 12 Oct 2023 17:09:35 -0400 Subject: [PATCH 09/17] update isFeatureEnabled path --- .../EditorAutoSync/EditorAutoSync.test.tsx | 23 +++++++++++-------- .../components/EditorAutoSync/index.tsx | 3 +-- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx index 2c52eb71f1215..0fd78f21e0875 100644 --- a/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx @@ -36,8 +36,7 @@ */ import React from 'react'; import fetchMock from 'fetch-mock'; -import * as featureFlags from 'src/featureFlags'; -import { FeatureFlag } from '@superset-ui/core'; +import * as uiCore from '@superset-ui/core'; import { render, act } from 'spec/helpers/testing-library'; import ToastContainer from 'src/components/MessageToasts/ToastContainer'; import { initialState, defaultQueryEditor } from 'src/SqlLab/fixtures'; @@ -65,9 +64,10 @@ test('sync the unsaved editor tab state when there are new changes since the las const updateEditorTabState = `glob:*/tabstateview/${defaultQueryEditor.id}`; fetchMock.put(updateEditorTabState, 200); const isFeatureEnabledMock = jest - .spyOn(featureFlags, 'isFeatureEnabled') + .spyOn(uiCore, 'isFeatureEnabled') .mockImplementation( - featureFlag => featureFlag === FeatureFlag.SQLLAB_BACKEND_PERSISTENCE, + featureFlag => + featureFlag === uiCore.FeatureFlag.SQLLAB_BACKEND_PERSISTENCE, ); expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); render(, { @@ -89,9 +89,10 @@ test('skip syncing the unsaved editor tab state when the updates are already syn const updateEditorTabState = `glob:*/tabstateview/${defaultQueryEditor.id}`; fetchMock.put(updateEditorTabState, 200); const isFeatureEnabledMock = jest - .spyOn(featureFlags, 'isFeatureEnabled') + .spyOn(uiCore, 'isFeatureEnabled') .mockImplementation( - featureFlag => featureFlag === FeatureFlag.SQLLAB_BACKEND_PERSISTENCE, + featureFlag => + featureFlag === uiCore.FeatureFlag.SQLLAB_BACKEND_PERSISTENCE, ); expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); render(, { @@ -123,9 +124,10 @@ test('renders an error toast when the sync failed', async () => { throws: new Error('errorMessage'), }); const isFeatureEnabledMock = jest - .spyOn(featureFlags, 'isFeatureEnabled') + .spyOn(uiCore, 'isFeatureEnabled') .mockImplementation( - featureFlag => featureFlag === FeatureFlag.SQLLAB_BACKEND_PERSISTENCE, + featureFlag => + featureFlag === uiCore.FeatureFlag.SQLLAB_BACKEND_PERSISTENCE, ); expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); const { findByText } = render( @@ -157,9 +159,10 @@ test('skip syncing the unsaved editor tab stat when SQLLAB_BACKEND_PERSISTENCE i const updateEditorTabState = `glob:*/tabstateview/${defaultQueryEditor.id}`; fetchMock.put(updateEditorTabState, 200); const isFeatureEnabledMock = jest - .spyOn(featureFlags, 'isFeatureEnabled') + .spyOn(uiCore, 'isFeatureEnabled') .mockImplementation( - featureFlag => featureFlag !== FeatureFlag.SQLLAB_BACKEND_PERSISTENCE, + featureFlag => + featureFlag !== uiCore.FeatureFlag.SQLLAB_BACKEND_PERSISTENCE, ); expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); render(, { diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx index c8c077292bc48..16788b2a0f78c 100644 --- a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx @@ -19,13 +19,12 @@ import React, { useRef, useEffect } from 'react'; import { useDispatch, useSelector } from 'react-redux'; -import { FeatureFlag, t } from '@superset-ui/core'; +import { FeatureFlag, isFeatureEnabled, t } from '@superset-ui/core'; import { SqlLabRootState, QueryEditor, UnsavedQueryEditor, } from 'src/SqlLab/types'; -import { isFeatureEnabled } from 'src/featureFlags'; import { useUpdateSqlEditorTabMutation } from 'src/hooks/apiResources/sqlEditorTabs'; import { useDebounceValue } from 'src/hooks/useDebounceValue'; import { From 4f105d68172ccc20c9c60dcb728b69f1345cd334 Mon Sep 17 00:00:00 2001 From: justinpark Date: Thu, 12 Oct 2023 18:13:04 -0400 Subject: [PATCH 10/17] ts lint --- .../src/SqlLab/reducers/getInitialState.test.ts | 9 ++++++--- superset-frontend/src/SqlLab/types.ts | 2 +- superset-frontend/src/views/store.ts | 1 - 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts b/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts index 009101ca1e7d5..d45f165813057 100644 --- a/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts +++ b/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts @@ -282,6 +282,7 @@ describe('getInitialState', () => { const apiDataWithLocalStorage = { ...apiData, active_tab: { + ...apiDataWithTabState.active_tab, id: 1, label: 'persisted tab', table_schemas: [], @@ -289,7 +290,7 @@ describe('getInitialState', () => { updatedAt: lastUpdatedTime, }, }, - tab_state_ids: [{ id: 1 }], + tab_state_ids: [{ id: 1, label: '' }], }; expect( getInitialState(apiDataWithLocalStorage).sqlLab.queryEditors[0], @@ -305,6 +306,7 @@ describe('getInitialState', () => { const apiDataWithLocalStorage = { ...apiData, active_tab: { + ...apiDataWithTabState.active_tab, id: 2, label: 'persisted tab', table_schemas: [], @@ -312,7 +314,7 @@ describe('getInitialState', () => { updatedAt: lastUpdatedTime, }, }, - tab_state_ids: [{ id: 2 }], + tab_state_ids: [{ id: 2, label: '' }], }; expect( getInitialState(apiDataWithLocalStorage).sqlLab.queryEditors[1], @@ -328,6 +330,7 @@ describe('getInitialState', () => { const apiDataWithLocalStorage = { ...apiData, active_tab: { + ...apiDataWithTabState.active_tab, id: 3, label: 'persisted tab', table_schemas: [], @@ -335,7 +338,7 @@ describe('getInitialState', () => { updatedAt: lastUpdatedTime, }, }, - tab_state_ids: [{ id: 3 }], + tab_state_ids: [{ id: 3, label: '' }], }; expect( getInitialState(apiDataWithLocalStorage).sqlLab.queryEditors[2], diff --git a/superset-frontend/src/SqlLab/types.ts b/superset-frontend/src/SqlLab/types.ts index 9b1548a87fff6..82e10a4b73d37 100644 --- a/superset-frontend/src/SqlLab/types.ts +++ b/superset-frontend/src/SqlLab/types.ts @@ -87,7 +87,7 @@ export type SqlLabRootState = { errorMessage: string | null; unsavedQueryEditor: UnsavedQueryEditor; queryCostEstimates?: Record; - editorTabLastUpdatedAt?: number; + editorTabLastUpdatedAt: number; }; localStorageUsageInKilobytes: number; messageToasts: toastState[]; diff --git a/superset-frontend/src/views/store.ts b/superset-frontend/src/views/store.ts index 65abca796ecd4..a9c3a9eb13d81 100644 --- a/superset-frontend/src/views/store.ts +++ b/superset-frontend/src/views/store.ts @@ -38,7 +38,6 @@ import logger from 'src/middleware/loggerMiddleware'; import saveModal from 'src/explore/reducers/saveModalReducer'; import explore from 'src/explore/reducers/exploreReducer'; import exploreDatasources from 'src/explore/reducers/datasourcesReducer'; -import { FeatureFlag, isFeatureEnabled } from '@superset-ui/core'; import { persistSqlLabStateEnhancer } from 'src/SqlLab/middlewares/persistSqlLabStateEnhancer'; import sqlLabReducer from 'src/SqlLab/reducers/sqlLab'; From 1315fc2a883dab42ee68f200c6df569cc5500941 Mon Sep 17 00:00:00 2001 From: justinpark Date: Tue, 17 Oct 2023 16:17:59 -0400 Subject: [PATCH 11/17] fix shouldInitialize condition --- superset-frontend/src/pages/SqlLab/index.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/pages/SqlLab/index.tsx b/superset-frontend/src/pages/SqlLab/index.tsx index 16a0e917a5a53..3f19b54c29511 100644 --- a/superset-frontend/src/pages/SqlLab/index.tsx +++ b/superset-frontend/src/pages/SqlLab/index.tsx @@ -32,12 +32,12 @@ import useEffectEvent from 'src/hooks/useEffectEvent'; import { LocationProvider } from './LocationContext'; export default function SqlLab() { - const editorTabLastUpdatedAt = useSelector( - state => state.sqlLab.editorTabLastUpdatedAt || 0, + const lastInitializedAt = useSelector( + state => state.sqlLab.queriesLastUpdate || 0, ); const { data, isLoading, isError, error, fulfilledTimeStamp } = useSqlLabInitialState(); - const shouldInitialize = editorTabLastUpdatedAt <= (fulfilledTimeStamp || 0); + const shouldInitialize = lastInitializedAt <= (fulfilledTimeStamp || 0); const dispatch = useDispatch(); const initBootstrapData = useEffectEvent( From d0a870c245a5ba31d86857b6266c3573e146e65d Mon Sep 17 00:00:00 2001 From: justinpark Date: Tue, 14 Nov 2023 14:50:54 -0800 Subject: [PATCH 12/17] remove FeatureFlag check since upper level check exists --- .../EditorAutoSync/EditorAutoSync.test.tsx | 61 ++----------------- .../components/EditorAutoSync/index.tsx | 5 +- 2 files changed, 5 insertions(+), 61 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx index 0fd78f21e0875..f459ef2b99294 100644 --- a/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx @@ -36,8 +36,7 @@ */ import React from 'react'; import fetchMock from 'fetch-mock'; -import * as uiCore from '@superset-ui/core'; -import { render, act } from 'spec/helpers/testing-library'; +import { render, waitFor } from 'spec/helpers/testing-library'; import ToastContainer from 'src/components/MessageToasts/ToastContainer'; import { initialState, defaultQueryEditor } from 'src/SqlLab/fixtures'; import EditorAutoSync from '.'; @@ -63,12 +62,6 @@ afterAll(() => { 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); - const isFeatureEnabledMock = jest - .spyOn(uiCore, 'isFeatureEnabled') - .mockImplementation( - featureFlag => - featureFlag === uiCore.FeatureFlag.SQLLAB_BACKEND_PERSISTENCE, - ); expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); render(, { useRedux: true, @@ -77,23 +70,14 @@ test('sync the unsaved editor tab state when there are new changes since the las sqlLab: unsavedSqlLabState, }, }); - await act(async () => { - jest.runAllTimers(); - }); + await waitFor(() => jest.runAllTimers()); expect(fetchMock.calls(updateEditorTabState)).toHaveLength(1); - isFeatureEnabledMock.mockClear(); 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); - const isFeatureEnabledMock = jest - .spyOn(uiCore, 'isFeatureEnabled') - .mockImplementation( - featureFlag => - featureFlag === uiCore.FeatureFlag.SQLLAB_BACKEND_PERSISTENCE, - ); expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); render(, { useRedux: true, @@ -110,11 +94,8 @@ test('skip syncing the unsaved editor tab state when the updates are already syn }, }, }); - await act(async () => { - jest.runAllTimers(); - }); + await waitFor(() => jest.runAllTimers()); expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); - isFeatureEnabledMock.mockClear(); fetchMock.restore(); }); @@ -123,12 +104,6 @@ test('renders an error toast when the sync failed', async () => { fetchMock.put(updateEditorTabState, { throws: new Error('errorMessage'), }); - const isFeatureEnabledMock = jest - .spyOn(uiCore, 'isFeatureEnabled') - .mockImplementation( - featureFlag => - featureFlag === uiCore.FeatureFlag.SQLLAB_BACKEND_PERSISTENCE, - ); expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); const { findByText } = render( <> @@ -143,39 +118,11 @@ test('renders an error toast when the sync failed', async () => { }, }, ); - await act(async () => { - jest.runAllTimers(); - }); + await waitFor(() => jest.runAllTimers()); const errorToast = await findByText( 'An error occurred while saving your editor state. ' + 'Please contact your administrator if this problem persists.', ); expect(errorToast).toBeTruthy(); - isFeatureEnabledMock.mockClear(); - fetchMock.restore(); -}); - -test('skip syncing the unsaved editor tab stat when SQLLAB_BACKEND_PERSISTENCE is off', async () => { - const updateEditorTabState = `glob:*/tabstateview/${defaultQueryEditor.id}`; - fetchMock.put(updateEditorTabState, 200); - const isFeatureEnabledMock = jest - .spyOn(uiCore, 'isFeatureEnabled') - .mockImplementation( - featureFlag => - featureFlag !== uiCore.FeatureFlag.SQLLAB_BACKEND_PERSISTENCE, - ); - expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); - render(, { - useRedux: true, - initialState: { - ...initialState, - sqlLab: unsavedSqlLabState, - }, - }); - await act(async () => { - jest.runAllTimers(); - }); - expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); - isFeatureEnabledMock.mockClear(); fetchMock.restore(); }); diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx index 16788b2a0f78c..cb90626ab24a1 100644 --- a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx @@ -19,7 +19,7 @@ import React, { useRef, useEffect } from 'react'; import { useDispatch, useSelector } from 'react-redux'; -import { FeatureFlag, isFeatureEnabled, t } from '@superset-ui/core'; +import { t } from '@superset-ui/core'; import { SqlLabRootState, QueryEditor, @@ -77,9 +77,6 @@ const EditorAutoSync: React.FC = () => { ); useEffect(() => { - if (!isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE)) { - return; - } const unsaved = filterUnsavedQueryEditorList( queryEditors, debouncedUnsavedQueryEditor, From 1245a8fc76b20426adb9281bd43bc4f3a9a10a6b Mon Sep 17 00:00:00 2001 From: justinpark Date: Tue, 14 Nov 2023 15:24:54 -0800 Subject: [PATCH 13/17] update comment --- superset-frontend/src/SqlLab/reducers/getInitialState.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts b/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts index d45f165813057..1dd3220fcc467 100644 --- a/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts +++ b/superset-frontend/src/SqlLab/reducers/getInitialState.test.ts @@ -262,13 +262,13 @@ describe('getInitialState', () => { updatedAt: lastUpdatedTime + 100, }, { - // out of update since last updated time is before server update time + // no update required given that last updated time comes before server update time id: '2', name: expectedValue, updatedAt: lastUpdatedTime - 100, }, { - // out of update since no updatedAt + // no update required given that there's no updatedAt id: '3', name: expectedValue, }, From 5b40a79bdc5927f016ecb96f0cc64d22d8b28938 Mon Sep 17 00:00:00 2001 From: justinpark Date: Tue, 14 Nov 2023 15:28:43 -0800 Subject: [PATCH 14/17] feature flag instead of custom key value --- .../src/SqlLab/middlewares/persistSqlLabStateEnhancer.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js b/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js index 574fd23c520b0..d1bec5e0c16a9 100644 --- a/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js +++ b/superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js @@ -35,15 +35,13 @@ const CLEAR_ENTITY_HELPERS_MAP = { unsavedQueryEditor: qe => clearQueryEditors([qe])[0], }; -const PERSISTENCE_LOCAL_STORAGE_PATH = 'sqlLab.unsavedQueryEditor'; - const sqlLabPersistStateConfig = { paths: ['sqlLab'], config: { slicer: paths => state => { const subset = {}; paths.forEach(path => { - if (path === PERSISTENCE_LOCAL_STORAGE_PATH) { + if (isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE)) { const { queryEditors, editorTabLastUpdatedAt, @@ -118,8 +116,6 @@ const sqlLabPersistStateConfig = { }; export const persistSqlLabStateEnhancer = persistState( - isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) - ? [PERSISTENCE_LOCAL_STORAGE_PATH] - : sqlLabPersistStateConfig.paths, + sqlLabPersistStateConfig.paths, sqlLabPersistStateConfig.config, ); From b240338f0affebbd7cb1700fee763d2d95778f9a Mon Sep 17 00:00:00 2001 From: justinpark Date: Tue, 14 Nov 2023 17:07:56 -0800 Subject: [PATCH 15/17] add version number in QueryEditor --- superset-frontend/src/SqlLab/fixtures.ts | 2 ++ superset-frontend/src/SqlLab/reducers/getInitialState.ts | 6 +++++- superset-frontend/src/SqlLab/types.ts | 7 +++++++ .../src/SqlLab/utils/emptyQueryResults.test.js | 7 ++++--- .../src/SqlLab/utils/reduxStateToLocalStorageHelper.js | 1 + .../src/hooks/apiResources/sqlEditorTabs.test.ts | 7 ++++++- superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts | 5 +++-- 7 files changed, 28 insertions(+), 7 deletions(-) diff --git a/superset-frontend/src/SqlLab/fixtures.ts b/superset-frontend/src/SqlLab/fixtures.ts index 4f6ad9ceb5dac..c578aac3fc010 100644 --- a/superset-frontend/src/SqlLab/fixtures.ts +++ b/superset-frontend/src/SqlLab/fixtures.ts @@ -25,6 +25,7 @@ import { QueryResponse, QueryState, } from '@superset-ui/core'; +import { LatestQueryEditorVersion } from 'src/SqlLab/types'; import { ISaveableDatasource } from 'src/SqlLab/components/SaveDatasetModal'; export const mockedActions = sinon.stub({ ...actions }); @@ -181,6 +182,7 @@ export const table = { }; export const defaultQueryEditor = { + version: LatestQueryEditorVersion, id: 'dfsadfs', autorun: false, dbId: undefined, diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.ts b/superset-frontend/src/SqlLab/reducers/getInitialState.ts index c55c9d1ceffc0..8d72a313b2716 100644 --- a/superset-frontend/src/SqlLab/reducers/getInitialState.ts +++ b/superset-frontend/src/SqlLab/reducers/getInitialState.ts @@ -20,11 +20,13 @@ import { t } from '@superset-ui/core'; import getToastsFromPyFlashMessages from 'src/components/MessageToasts/getToastsFromPyFlashMessages'; import type { BootstrapData } from 'src/types/bootstrapTypes'; import type { InitialState } from 'src/hooks/apiResources/sqlLab'; -import type { +import { QueryEditor, UnsavedQueryEditor, SqlLabRootState, Table, + LatestQueryEditorVersion, + QueryEditorVersion, } from 'src/SqlLab/types'; export function dedupeTabHistory(tabHistory: string[]) { @@ -53,6 +55,7 @@ export default function getInitialState({ */ let queryEditors: Record = {}; const defaultQueryEditor = { + version: LatestQueryEditorVersion, loaded: true, name: t('Untitled query'), sql: 'SELECT *\nFROM\nWHERE', @@ -73,6 +76,7 @@ export default function getInitialState({ let queryEditor: QueryEditor; if (activeTab && activeTab.id === id) { queryEditor = { + version: activeTab.extra_json?.version ?? QueryEditorVersion.v1, id: id.toString(), loaded: true, name: activeTab.label, diff --git a/superset-frontend/src/SqlLab/types.ts b/superset-frontend/src/SqlLab/types.ts index 82e10a4b73d37..6eb42718f0c70 100644 --- a/superset-frontend/src/SqlLab/types.ts +++ b/superset-frontend/src/SqlLab/types.ts @@ -29,7 +29,14 @@ export type QueryDictionary = { [id: string]: QueryResponse; }; +export enum QueryEditorVersion { + v1 = 1, +} + +export const LatestQueryEditorVersion = QueryEditorVersion.v1; + export interface QueryEditor { + version: QueryEditorVersion; id: string; dbId?: number; name: string; diff --git a/superset-frontend/src/SqlLab/utils/emptyQueryResults.test.js b/superset-frontend/src/SqlLab/utils/emptyQueryResults.test.js index 9984e1efcaf06..f08fccbef7a53 100644 --- a/superset-frontend/src/SqlLab/utils/emptyQueryResults.test.js +++ b/superset-frontend/src/SqlLab/utils/emptyQueryResults.test.js @@ -83,10 +83,11 @@ describe('reduxStateToLocalStorageHelper', () => { }); it('should only return selected keys for query editor', () => { - const queryEditors = [defaultQueryEditor]; - expect(Object.keys(queryEditors[0])).toContain('schema'); + const queryEditors = [{ ...defaultQueryEditor, dummy: 'value' }]; + expect(Object.keys(queryEditors[0])).toContain('dummy'); const clearedQueryEditors = clearQueryEditors(queryEditors); - expect(Object.keys(clearedQueryEditors)[0]).not.toContain('schema'); + expect(Object.keys(clearedQueryEditors[0])).toContain('version'); + expect(Object.keys(clearedQueryEditors[0])).not.toContain('dummy'); }); }); diff --git a/superset-frontend/src/SqlLab/utils/reduxStateToLocalStorageHelper.js b/superset-frontend/src/SqlLab/utils/reduxStateToLocalStorageHelper.js index 281f08bcb366f..f82711362ddbf 100644 --- a/superset-frontend/src/SqlLab/utils/reduxStateToLocalStorageHelper.js +++ b/superset-frontend/src/SqlLab/utils/reduxStateToLocalStorageHelper.js @@ -26,6 +26,7 @@ import { } from '../constants'; const PERSISTENT_QUERY_EDITOR_KEYS = new Set([ + 'version', 'remoteId', 'autorun', 'dbId', diff --git a/superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts index aad2191ca6855..d0f2230f13d90 100644 --- a/superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts +++ b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.test.ts @@ -23,9 +23,11 @@ import { defaultStore as store, } from 'spec/helpers/testing-library'; import { api } from 'src/hooks/apiResources/queryApi'; +import { LatestQueryEditorVersion } from 'src/SqlLab/types'; import { useUpdateSqlEditorTabMutation } from './sqlEditorTabs'; const expectedQueryEditor = { + version: LatestQueryEditorVersion, id: '123', dbId: 456, name: 'tab 1', @@ -88,7 +90,10 @@ test('puts api request with formData', async () => { ); expect(formData.get('extra_json')).toBe( JSON.stringify( - JSON.stringify({ updatedAt: expectedQueryEditor.updatedAt }), + JSON.stringify({ + updatedAt: expectedQueryEditor.updatedAt, + version: LatestQueryEditorVersion, + }), ), ); }); diff --git a/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts index ba2b85d358a9a..71e0cf2936e5a 100644 --- a/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts +++ b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts @@ -17,7 +17,7 @@ * under the License. */ import { pickBy } from 'lodash'; -import { QueryEditor } from 'src/SqlLab/types'; +import { QueryEditor, LatestQueryEditorVersion } from 'src/SqlLab/types'; import { api, JsonResponse } from './queryApi'; export type EditorMutationParams = { @@ -30,6 +30,7 @@ const sqlEditorApi = api.injectEndpoints({ updateSqlEditorTab: builder.mutation({ query: ({ queryEditor: { + version = LatestQueryEditorVersion, id, dbId, schema, @@ -57,7 +58,7 @@ const sqlEditorApi = api.injectEndpoints({ template_params: templateParams, hide_left_bar: hideLeftBar, autorun, - extra_json: JSON.stringify({ updatedAt, ...extra }), + extra_json: JSON.stringify({ updatedAt, version, ...extra }), }, value => value !== undefined, ), From 16444b6ea6154ba181543450af1e168ae4249eff Mon Sep 17 00:00:00 2001 From: justinpark Date: Tue, 14 Nov 2023 17:17:02 -0800 Subject: [PATCH 16/17] replace error toast by logger --- .../components/EditorAutoSync/index.tsx | 22 +++++-------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx index cb90626ab24a1..51399753e95b7 100644 --- a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx @@ -19,7 +19,7 @@ import React, { useRef, useEffect } from 'react'; import { useDispatch, useSelector } from 'react-redux'; -import { t } from '@superset-ui/core'; +import { logging } from '@superset-ui/core'; import { SqlLabRootState, QueryEditor, @@ -27,10 +27,7 @@ import { } from 'src/SqlLab/types'; import { useUpdateSqlEditorTabMutation } from 'src/hooks/apiResources/sqlEditorTabs'; import { useDebounceValue } from 'src/hooks/useDebounceValue'; -import { - addDangerToast, - setEditorTabLastUpdate, -} from 'src/SqlLab/actions/sqlLab'; +import { setEditorTabLastUpdate } from 'src/SqlLab/actions/sqlLab'; const INTERVAL = 5000; @@ -69,7 +66,7 @@ const EditorAutoSync: React.FC = () => { ); const dispatch = useDispatch(); const lastSavedTimestampRef = useRef(editorTabLastUpdatedAt); - const [updateSqlEditor, { isError }] = useUpdateSqlEditorTabMutation(); + const [updateSqlEditor, { error }] = useUpdateSqlEditorTabMutation(); const debouncedUnsavedQueryEditor = useDebounceValue( unsavedQueryEditor, @@ -98,17 +95,10 @@ const EditorAutoSync: React.FC = () => { }, [debouncedUnsavedQueryEditor, dispatch, queryEditors, updateSqlEditor]); useEffect(() => { - if (isError) { - dispatch( - addDangerToast( - t( - 'An error occurred while saving your editor state. ' + - 'Please contact your administrator if this problem persists.', - ), - ), - ); + if (error) { + logging.warn('An error occurred while saving your editor state.', error); } - }, [dispatch, isError]); + }, [dispatch, error]); return null; }; From defa032aab787da667563c4b612d7f513bab6706 Mon Sep 17 00:00:00 2001 From: justinpark Date: Wed, 15 Nov 2023 09:39:12 -0800 Subject: [PATCH 17/17] inspect console.warn --- .../EditorAutoSync/EditorAutoSync.test.tsx | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx index f459ef2b99294..52e1d44b247f8 100644 --- a/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx @@ -39,8 +39,16 @@ import fetchMock from 'fetch-mock'; 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 '.'; +jest.mock('@superset-ui/core', () => ({ + ...jest.requireActual('@superset-ui/core'), + logging: { + warn: jest.fn(), + }, +})); + const editorTabLastUpdatedAt = Date.now(); const unsavedSqlLabState = { ...initialState.sqlLab, @@ -105,7 +113,7 @@ test('renders an error toast when the sync failed', async () => { throws: new Error('errorMessage'), }); expect(fetchMock.calls(updateEditorTabState)).toHaveLength(0); - const { findByText } = render( + render( <> @@ -119,10 +127,11 @@ test('renders an error toast when the sync failed', async () => { }, ); await waitFor(() => jest.runAllTimers()); - const errorToast = await findByText( - 'An error occurred while saving your editor state. ' + - 'Please contact your administrator if this problem persists.', + + expect(logging.warn).toHaveBeenCalledTimes(1); + expect(logging.warn).toHaveBeenCalledWith( + 'An error occurred while saving your editor state.', + expect.anything(), ); - expect(errorToast).toBeTruthy(); fetchMock.restore(); });