diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 82d103ed745..f0745695fbe 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -71,7 +71,6 @@ export const QUERY_EDITOR_SET_FUNCTION_NAMES = export const QUERY_EDITOR_PERSIST_HEIGHT = 'QUERY_EDITOR_PERSIST_HEIGHT'; export const QUERY_EDITOR_TOGGLE_LEFT_BAR = 'QUERY_EDITOR_TOGGLE_LEFT_BAR'; export const MIGRATE_QUERY_EDITOR = 'MIGRATE_QUERY_EDITOR'; -export const MIGRATE_TAB_HISTORY = 'MIGRATE_TAB_HISTORY'; export const MIGRATE_TABLE = 'MIGRATE_TABLE'; export const MIGRATE_QUERY = 'MIGRATE_QUERY'; @@ -391,7 +390,7 @@ export function runQueryFromSqlEditor( const query = { dbId: qe.dbId, sql: qe.selectedText || qe.sql, - sqlEditorId: qe.id, + sqlEditorId: qe.tabViewId ?? qe.id, tab: qe.name, catalog: qe.catalog, schema: qe.schema, @@ -499,26 +498,21 @@ export function syncQueryEditor(queryEditor) { .then(({ json }) => { const newQueryEditor = { ...queryEditor, - id: json.id.toString(), inLocalStorage: false, loaded: true, + tabViewId: json.id.toString(), }; dispatch({ type: MIGRATE_QUERY_EDITOR, oldQueryEditor: queryEditor, newQueryEditor, }); - dispatch({ - type: MIGRATE_TAB_HISTORY, - oldId: queryEditor.id, - newId: newQueryEditor.id, - }); return Promise.all([ ...localStorageTables.map(table => - migrateTable(table, newQueryEditor.id, dispatch), + migrateTable(table, newQueryEditor.tabViewId, dispatch), ), ...localStorageQueries.map(query => - migrateQuery(query.id, newQueryEditor.id, dispatch), + migrateQuery(query.id, newQueryEditor.tabViewId, dispatch), ), ]); }) @@ -685,8 +679,9 @@ export function setTables(tableSchemas) { export function fetchQueryEditor(queryEditor, displayLimit) { return function (dispatch) { + const queryEditorId = queryEditor.tabViewId ?? queryEditor.id; SupersetClient.get({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), + endpoint: encodeURI(`/tabstateview/${queryEditorId}`), }) .then(({ json }) => { const loadedQueryEditor = { @@ -756,10 +751,11 @@ export function removeAllOtherQueryEditors(queryEditor) { export function removeQuery(query) { return function (dispatch) { + const queryEditorId = query.sqlEditorId ?? query.id; const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) ? SupersetClient.delete({ endpoint: encodeURI( - `/tabstateview/${query.sqlEditorId}/query/${query.id}`, + `/tabstateview/${queryEditorId}/query/${query.id}`, ), }) : Promise.resolve(); @@ -839,9 +835,10 @@ export function saveQuery(query, clientId) { export const addSavedQueryToTabState = (queryEditor, savedQuery) => dispatch => { + const queryEditorId = queryEditor.tabViewId ?? queryEditor.id; const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) ? SupersetClient.put({ - endpoint: `/tabstateview/${queryEditor.id}`, + endpoint: `/tabstateview/${queryEditorId}`, postPayload: { saved_query_id: savedQuery.remoteId }, }) : Promise.resolve(); @@ -889,9 +886,10 @@ export function queryEditorSetAndSaveSql(targetQueryEditor, sql, queryId) { const queryEditor = getUpToDateQuery(getState(), targetQueryEditor); // saved query and set tab state use this action dispatch(queryEditorSetSql(queryEditor, sql, queryId)); + const queryEditorId = queryEditor.tabViewId ?? queryEditor.id; if (isFeatureEnabled(FeatureFlag.SqllabBackendPersistence)) { return SupersetClient.put({ - endpoint: encodeURI(`/tabstateview/${queryEditor.id}`), + endpoint: encodeURI(`/tabstateview/${queryEditorId}`), postPayload: { sql, latest_query_id: queryId }, }).catch(() => dispatch( diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js index fd0ea8b13f7..4f344c973b9 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -1252,16 +1252,11 @@ describe('async actions', () => { // new qe has a different id newQueryEditor: { ...oldQueryEditor, - id: '1', + tabViewId: '1', inLocalStorage: false, loaded: true, }, }, - { - type: actions.MIGRATE_TAB_HISTORY, - newId: '1', - oldId: 'abcd', - }, { type: actions.MIGRATE_TABLE, oldTable: tables[0], diff --git a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx index 0b9173d368e..e6ef2f4bad0 100644 --- a/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx +++ b/superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx @@ -17,7 +17,7 @@ * under the License. */ -import { useRef, useEffect, FC } from 'react'; +import { useRef, useEffect, FC, useMemo } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import { logging } from '@superset-ui/core'; @@ -69,6 +69,17 @@ const EditorAutoSync: FC = () => { const queryEditors = useSelector( state => state.sqlLab.queryEditors, ); + const queryEditorsById = useMemo( + () => + queryEditors.reduce( + (acc, queryEditor) => { + acc[queryEditor.id] = queryEditor; + return acc; + }, + {} as Record, + ), + [queryEditors], + ); const unsavedQueryEditor = useSelector( state => state.sqlLab.unsavedQueryEditor, ); @@ -120,7 +131,10 @@ const EditorAutoSync: FC = () => { !queryEditors.find(({ id }) => id === currentQueryEditorId) ?.inLocalStorage ) { - updateCurrentSqlEditor(currentQueryEditorId).then(() => { + const queryEditorId = + queryEditorsById[currentQueryEditorId]?.tabViewId ?? + currentQueryEditorId; + updateCurrentSqlEditor(queryEditorId).then(() => { dispatch(setLastUpdatedActiveTab(currentQueryEditorId)); }); } @@ -129,7 +143,8 @@ const EditorAutoSync: FC = () => { const syncDeletedQueryEditor = useEffectEvent(() => { if (Object.keys(destroyedQueryEditors).length > 0) { Object.keys(destroyedQueryEditors).forEach(id => { - deleteSqlEditor(id) + const queryEditorId = queryEditorsById[id]?.tabViewId ?? id; + deleteSqlEditor(queryEditorId) .then(() => { dispatch(clearDestoryedQueryEditor(id)); }) diff --git a/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx b/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx index c499a16ec37..a63f0cc7d99 100644 --- a/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx +++ b/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx @@ -20,10 +20,14 @@ import fetchMock from 'fetch-mock'; import { FeatureFlag, isFeatureEnabled, QueryState } from '@superset-ui/core'; import { render, screen, waitFor } from 'spec/helpers/testing-library'; import QueryHistory from 'src/SqlLab/components/QueryHistory'; -import { initialState } from 'src/SqlLab/fixtures'; +import { + initialState, + defaultQueryEditor, + extraQueryEditor3, +} from 'src/SqlLab/fixtures'; const mockedProps = { - queryEditorId: 123, + queryEditorId: defaultQueryEditor.id, displayLimit: 1000, latestQueryId: 'yhMUZCGb', }; @@ -77,6 +81,8 @@ const setup = (overrides = {}) => ( ); +afterEach(() => fetchMock.reset()); + test('Renders an empty state for query history', () => { render(setup(), { useRedux: true, initialState }); @@ -102,3 +108,28 @@ test('fetches the query history when the persistence mode is enabled', async () expect(queryResultText).toBeInTheDocument(); isFeatureEnabledMock.mockClear(); }); + +test('fetches the query history by the tabViewId', async () => { + const isFeatureEnabledMock = mockedIsFeatureEnabled.mockImplementation( + featureFlag => featureFlag === FeatureFlag.SqllabBackendPersistence, + ); + + const editorQueryApiRoute = `glob:*/api/v1/query/?q=*sql_editor_id*${extraQueryEditor3.tabViewId}*`; + fetchMock.get(editorQueryApiRoute, fakeApiResult); + render(setup({ queryEditorId: extraQueryEditor3.id }), { + useRedux: true, + initialState: { + ...initialState, + sqlLab: { + ...initialState.sqlLab, + queryEditors: [...initialState.sqlLab.queryEditors, extraQueryEditor3], + }, + }, + }); + await waitFor(() => + expect(fetchMock.calls(editorQueryApiRoute).length).toBe(1), + ); + const queryResultText = screen.getByText(fakeApiResult.result[0].rows); + expect(queryResultText).toBeInTheDocument(); + isFeatureEnabledMock.mockClear(); +}); diff --git a/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx b/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx index 85cf33ade30..0a311d48084 100644 --- a/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx @@ -32,6 +32,7 @@ import QueryTable from 'src/SqlLab/components/QueryTable'; import { SqlLabRootState } from 'src/SqlLab/types'; import { useEditorQueriesQuery } from 'src/hooks/apiResources/queries'; import useEffectEvent from 'src/hooks/useEffectEvent'; +import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor'; interface QueryHistoryProps { queryEditorId: string | number; @@ -63,6 +64,10 @@ const QueryHistory = ({ displayLimit, latestQueryId, }: QueryHistoryProps) => { + const { id, tabViewId } = useQueryEditor(String(queryEditorId), [ + 'tabViewId', + ]); + const editorId = tabViewId ?? id; const [ref, hasReachedBottom] = useInView({ threshold: 0 }); const [pageIndex, setPageIndex] = useState(0); const queries = useSelector( @@ -74,7 +79,7 @@ const QueryHistory = ({ isLoading, isFetching, } = useEditorQueriesQuery( - { editorId: `${queryEditorId}`, pageIndex }, + { editorId, pageIndex }, { skip: !isFeatureEnabled(FeatureFlag.SqllabBackendPersistence), }, @@ -87,12 +92,12 @@ const QueryHistory = ({ queries, data.result.map(({ id }) => id), ), - queryEditorId, + editorId, ) .concat(data.result) .reverse() - : getEditorQueries(queries, queryEditorId), - [queries, data, queryEditorId], + : getEditorQueries(queries, editorId), + [queries, data, editorId], ); const loadNext = useEffectEvent(() => { diff --git a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx index 7096f4d6328..15d8cfb7f71 100644 --- a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx +++ b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx @@ -27,6 +27,7 @@ import { removeTables, setActiveSouthPaneTab } from 'src/SqlLab/actions/sqlLab'; import { Label } from '@superset-ui/core/components'; import { Icons } from '@superset-ui/core/components/Icons'; import { SqlLabRootState } from 'src/SqlLab/types'; +import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor'; import QueryHistory from '../QueryHistory'; import { STATUS_OPTIONS, @@ -96,6 +97,8 @@ const SouthPane = ({ displayLimit, defaultQueryLimit, }: SouthPaneProps) => { + const { id, tabViewId } = useQueryEditor(queryEditorId, ['tabViewId']); + const editorId = tabViewId ?? id; const theme = useTheme(); const dispatch = useDispatch(); const { offline, tables } = useSelector( @@ -111,11 +114,8 @@ const SouthPane = ({ ) ?? 'Results'; const pinnedTables = useMemo( - () => - tables.filter( - ({ queryEditorId: qeId }) => String(queryEditorId) === qeId, - ), - [queryEditorId, tables], + () => tables.filter(({ queryEditorId: qeId }) => String(editorId) === qeId), + [editorId, tables], ); const pinnedTableKeys = useMemo( () => diff --git a/superset-frontend/src/SqlLab/fixtures.ts b/superset-frontend/src/SqlLab/fixtures.ts index 075ccc2e53d..9ac8bed6e78 100644 --- a/superset-frontend/src/SqlLab/fixtures.ts +++ b/superset-frontend/src/SqlLab/fixtures.ts @@ -216,6 +216,14 @@ export const extraQueryEditor2 = { name: 'Untitled Query 3', }; +export const extraQueryEditor3 = { + ...defaultQueryEditor, + id: 'kvk23', + sql: '', + name: 'Untitled Query 4', + tabViewId: 37, +}; + export const queries = [ { dbId: 1, diff --git a/superset-frontend/src/SqlLab/hooks/useQueryEditor/index.ts b/superset-frontend/src/SqlLab/hooks/useQueryEditor/index.ts index 4d0e0c984b4..237e5fc23fe 100644 --- a/superset-frontend/src/SqlLab/hooks/useQueryEditor/index.ts +++ b/superset-frontend/src/SqlLab/hooks/useQueryEditor/index.ts @@ -17,6 +17,7 @@ * under the License. */ import { pick } from 'lodash'; +import { useMemo } from 'react'; import { shallowEqual, useSelector } from 'react-redux'; import { SqlLabRootState, QueryEditor } from 'src/SqlLab/types'; @@ -24,11 +25,20 @@ export default function useQueryEditor( sqlEditorId: string, attributes: ReadonlyArray, ) { + const queryEditors = useSelector( + ({ sqlLab: { queryEditors } }) => queryEditors, + shallowEqual, + ); + const queryEditorsById = useMemo( + () => Object.fromEntries(queryEditors.map(editor => [editor.id, editor])), + [queryEditors.map(({ id }) => id).join(',')], + ); + return useSelector>( - ({ sqlLab: { unsavedQueryEditor, queryEditors } }) => + ({ sqlLab: { unsavedQueryEditor } }) => pick( { - ...queryEditors.find(({ id }) => id === sqlEditorId), + ...queryEditorsById[sqlEditorId], ...(sqlEditorId === unsavedQueryEditor?.id && unsavedQueryEditor), }, ['id'].concat(attributes), diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.ts b/superset-frontend/src/SqlLab/reducers/getInitialState.ts index 361cc5621dd..ce41095801f 100644 --- a/superset-frontend/src/SqlLab/reducers/getInitialState.ts +++ b/superset-frontend/src/SqlLab/reducers/getInitialState.ts @@ -171,8 +171,9 @@ 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 sqlEditorId = qe.tabViewId ?? qe.id; + const hasConflictFromBackend = Boolean(queryEditors[sqlEditorId]); + const unsavedUpdatedAt = queryEditors[sqlEditorId]?.updatedAt; const hasUnsavedUpdateSinceLastSave = qe.updatedAt && (!unsavedUpdatedAt || qe.updatedAt > unsavedUpdatedAt); @@ -180,13 +181,13 @@ export default function getInitialState({ !hasConflictFromBackend || hasUnsavedUpdateSinceLastSave ? qe : {}; queryEditors = { ...queryEditors, - [qe.id]: { - ...queryEditors[qe.id], + [sqlEditorId]: { + ...queryEditors[sqlEditorId], ...cachedQueryEditor, name: cachedQueryEditor.title || cachedQueryEditor.name || - queryEditors[qe.id]?.name, + queryEditors[sqlEditorId]?.name, ...(cachedQueryEditor.id && unsavedQueryEditor.id === qe.id && unsavedQueryEditor), diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.js b/superset-frontend/src/SqlLab/reducers/sqlLab.js index c77f708653f..d910068e5e1 100644 --- a/superset-frontend/src/SqlLab/reducers/sqlLab.js +++ b/superset-frontend/src/SqlLab/reducers/sqlLab.js @@ -167,7 +167,7 @@ export default function sqlLabReducer(state = {}, action) { }, destroyedQueryEditors: { ...newState.destroyedQueryEditors, - [queryEditor.id]: Date.now(), + ...(!queryEditor.inLocalStorage && { [queryEditor.id]: Date.now() }), }, }; return newState; @@ -317,10 +317,17 @@ export default function sqlLabReducer(state = {}, action) { }, [actions.START_QUERY]() { let newState = { ...state }; + let sqlEditorId; if (action.query.sqlEditorId) { + const queryEditorByTabId = getFromArr( + state.queryEditors, + action.query.sqlEditorId, + 'tabViewId', + ); + sqlEditorId = queryEditorByTabId?.id ?? action.query.sqlEditorId; const qe = { - ...getFromArr(state.queryEditors, action.query.sqlEditorId), - ...(action.query.sqlEditorId === state.unsavedQueryEditor.id && + ...getFromArr(state.queryEditors, sqlEditorId), + ...(sqlEditorId === state.unsavedQueryEditor.id && state.unsavedQueryEditor), }; if (qe.latestQueryId && state.queries[qe.latestQueryId]) { @@ -343,7 +350,7 @@ export default function sqlLabReducer(state = {}, action) { { latestQueryId: action.query.id, }, - action.query.sqlEditorId, + sqlEditorId, action.query.isDataPreview, ), }; @@ -495,12 +502,6 @@ export default function sqlLabReducer(state = {}, action) { action.newTable, ); }, - [actions.MIGRATE_TAB_HISTORY]() { - const tabHistory = state.tabHistory.map(tabId => - tabId === action.oldId ? action.newId : tabId, - ); - return { ...state, tabHistory }; - }, [actions.MIGRATE_QUERY]() { const query = { ...state.queries[action.queryId], diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.test.js b/superset-frontend/src/SqlLab/reducers/sqlLab.test.js index d805b2cf1ef..08ebacd50fe 100644 --- a/superset-frontend/src/SqlLab/reducers/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/reducers/sqlLab.test.js @@ -255,18 +255,6 @@ describe('sqlLabReducer', () => { 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); - }); it('should clear the destroyed query editors', () => { const expectedQEId = '1233289'; const action = { diff --git a/superset-frontend/src/SqlLab/types.ts b/superset-frontend/src/SqlLab/types.ts index 98ee87ff5f0..9665a88fae8 100644 --- a/superset-frontend/src/SqlLab/types.ts +++ b/superset-frontend/src/SqlLab/types.ts @@ -70,6 +70,7 @@ export interface QueryEditor { updatedAt?: number; cursorPosition?: CursorPosition; isDataset?: boolean; + tabViewId?: string; } export type toastState = { diff --git a/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts index 90f4a057c18..717a1cd1f25 100644 --- a/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts +++ b/superset-frontend/src/hooks/apiResources/sqlEditorTabs.ts @@ -43,11 +43,12 @@ const sqlEditorApi = api.injectEndpoints({ templateParams, autorun, updatedAt, + tabViewId, }, extra, }) => ({ method: 'PUT', - endpoint: encodeURI(`/tabstateview/${id}`), + endpoint: encodeURI(`/tabstateview/${tabViewId ?? id}`), postPayload: pickBy( { database_id: dbId, @@ -66,14 +67,14 @@ const sqlEditorApi = api.injectEndpoints({ ), }), }), - updateCurrentSqlEditorTab: builder.mutation({ + updateCurrentSqlEditorTab: builder.mutation({ query: queryEditorId => ({ method: 'POST', endpoint: encodeURI(`/tabstateview/${queryEditorId}/activate`), transformResponse: () => queryEditorId, }), }), - deleteSqlEditorTab: builder.mutation({ + deleteSqlEditorTab: builder.mutation({ query: queryEditorId => ({ method: 'DELETE', endpoint: encodeURI(`/tabstateview/${queryEditorId}`), diff --git a/superset-frontend/src/reduxUtils.ts b/superset-frontend/src/reduxUtils.ts index aeca9004546..9c3fa9b146f 100644 --- a/superset-frontend/src/reduxUtils.ts +++ b/superset-frontend/src/reduxUtils.ts @@ -83,10 +83,14 @@ export function removeFromArr( return { ...state, [arrKey]: newArr }; } -export function getFromArr(arr: Record[], id: string) { +export function getFromArr( + arr: Record[], + id: string, + idKey = 'id', +) { let obj; arr.forEach(o => { - if (o.id === id) { + if (o[idKey] === id) { obj = o; } });