Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(sqllab): nonblocking switch query editor #29108

Merged
merged 3 commits into from
Jun 12, 2024
Merged

refactor(sqllab): nonblocking switch query editor #29108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
80c7b7d
refactor(sqllab): nonblocking switch query editor
justinpark Jun 6, 2024
6a4fef2
update spec
justinpark Jun 6, 2024
3f3accb
rename update action
justinpark Jun 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 45 additions & 65 deletions superset-frontend/src/SqlLab/actions/sqlLab.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ export const CREATE_DATASOURCE_SUCCESS = 'CREATE_DATASOURCE_SUCCESS';
export const CREATE_DATASOURCE_FAILED = 'CREATE_DATASOURCE_FAILED';

export const SET_EDITOR_TAB_LAST_UPDATE = 'SET_EDITOR_TAB_LAST_UPDATE';
export const SET_LAST_UPDATED_ACTIVE_TAB = 'SET_LAST_UPDATED_ACTIVE_TAB';

export const addInfoToast = addInfoToastAction;
export const addSuccessToast = addSuccessToastAction;
Expand Down Expand Up @@ -610,29 +611,17 @@ export function cloneQueryToNewTab(query, autorun) {
};
}

export function setActiveQueryEditor(queryEditor) {
return function (dispatch) {
const sync = isFeatureEnabled(FeatureFlag.SqllabBackendPersistence)
? SupersetClient.post({
endpoint: encodeURI(`/tabstateview/${queryEditor.id}/activate`),
})
: Promise.resolve();
export function setLastUpdatedActiveTab(queryEditorId) {
return {
type: SET_LAST_UPDATED_ACTIVE_TAB,
queryEditorId,
};
}

return sync
.then(() => dispatch({ type: SET_ACTIVE_QUERY_EDITOR, queryEditor }))
.catch(response => {
if (response.status !== 404) {
return dispatch(
addDangerToast(
t(
'An error occurred while setting the active tab. Please contact ' +
'your administrator.',
),
),
);
}
return dispatch({ type: REMOVE_QUERY_EDITOR, queryEditor });
});
export function setActiveQueryEditor(queryEditor) {
return {
type: SET_ACTIVE_QUERY_EDITOR,
queryEditor,
};
}

Expand Down Expand Up @@ -673,51 +662,42 @@ export function setTables(tableSchemas) {
return { type: SET_TABLES, tables };
}

export function switchQueryEditor(queryEditor, displayLimit) {
export function fetchQueryEditor(queryEditor, displayLimit) {
return function (dispatch) {
if (
isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) &&
queryEditor &&
!queryEditor.loaded
) {
SupersetClient.get({
endpoint: encodeURI(`/tabstateview/${queryEditor.id}`),
SupersetClient.get({
endpoint: encodeURI(`/tabstateview/${queryEditor.id}`),
})
.then(({ json }) => {
const loadedQueryEditor = {
id: json.id.toString(),
loaded: true,
name: json.label,
sql: json.sql,
selectedText: null,
latestQueryId: json.latest_query?.id,
autorun: json.autorun,
dbId: json.database_id,
templateParams: json.template_params,
catalog: json.catalog,
schema: json.schema,
queryLimit: json.query_limit,
remoteId: json.saved_query?.id,
hideLeftBar: json.hide_left_bar,
};
dispatch(loadQueryEditor(loadedQueryEditor));
dispatch(setTables(json.table_schemas || []));
if (json.latest_query && json.latest_query.resultsKey) {
dispatch(fetchQueryResults(json.latest_query, displayLimit));
}
})
.then(({ json }) => {
const loadedQueryEditor = {
id: json.id.toString(),
loaded: true,
name: json.label,
sql: json.sql,
selectedText: null,
latestQueryId: json.latest_query?.id,
autorun: json.autorun,
dbId: json.database_id,
templateParams: json.template_params,
catalog: json.catalog,
schema: json.schema,
queryLimit: json.query_limit,
remoteId: json.saved_query?.id,
hideLeftBar: json.hide_left_bar,
};
dispatch(loadQueryEditor(loadedQueryEditor));
dispatch(setTables(json.table_schemas || []));
dispatch(setActiveQueryEditor(loadedQueryEditor));
if (json.latest_query && json.latest_query.resultsKey) {
dispatch(fetchQueryResults(json.latest_query, displayLimit));
}
})
.catch(response => {
if (response.status !== 404) {
return dispatch(
addDangerToast(t('An error occurred while fetching tab state')),
);
}
return dispatch({ type: REMOVE_QUERY_EDITOR, queryEditor });
});
} else {
dispatch(setActiveQueryEditor(queryEditor));
}
.catch(response => {
if (response.status !== 404) {
return dispatch(
addDangerToast(t('An error occurred while fetching tab state')),
);
}
return dispatch({ type: REMOVE_QUERY_EDITOR, queryEditor });
});
};
}

Expand Down
35 changes: 15 additions & 20 deletions superset-frontend/src/SqlLab/actions/sqlLab.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,21 @@ describe('async actions', () => {
});
});

it('set current query editor', () => {
expect.assertions(1);

const store = mockStore(initialState);
const expectedActions = [
{
type: actions.SET_ACTIVE_QUERY_EDITOR,
queryEditor: defaultQueryEditor,
},
];
store.dispatch(actions.setActiveQueryEditor(defaultQueryEditor));

expect(store.getActions()).toEqual(expectedActions);
});

describe('backend sync', () => {
const updateTabStateEndpoint = 'glob:*/tabstateview/*';
fetchMock.put(updateTabStateEndpoint, {});
Expand Down Expand Up @@ -570,26 +585,6 @@ describe('async actions', () => {
});
});

describe('setActiveQueryEditor', () => {
it('updates the tab state in the backend', () => {
expect.assertions(2);

const store = mockStore({});
const expectedActions = [
{
type: actions.SET_ACTIVE_QUERY_EDITOR,
queryEditor,
},
];
return store
.dispatch(actions.setActiveQueryEditor(queryEditor))
.then(() => {
expect(store.getActions()).toEqual(expectedActions);
expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(1);
});
});
});

describe('removeQueryEditor', () => {
it('updates the tab state in the backend', () => {
expect.assertions(2);
Expand Down
38 changes: 38 additions & 0 deletions superset-frontend/src/SqlLab/components/EditorAutoSync/EditorAutoSync.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ const unsavedSqlLabState = {
},
editorTabLastUpdatedAt,
};

beforeAll(() => {
jest.useFakeTimers();
});
Expand All @@ -66,6 +67,16 @@ afterAll(() => {
jest.useRealTimers();
});

const updateActiveEditorTabState = `glob:*/tabstateview/*/activate`;

beforeEach(() => {
fetchMock.post(updateActiveEditorTabState, {});
});

afterEach(() => {
fetchMock.reset();
});

test('sync the unsaved editor tab state when there are new changes since the last update', async () => {
const updateEditorTabState = `glob:*/tabstateview/${defaultQueryEditor.id}`;
fetchMock.put(updateEditorTabState, 200);
Expand Down Expand Up @@ -108,6 +119,33 @@ test('sync the unsaved NEW editor state when there are new in local storage', as
fetchMock.restore();
});

test('sync the active editor id when there are updates in tab history', async () => {
expect(fetchMock.calls(updateActiveEditorTabState)).toHaveLength(0);
render(<EditorAutoSync />, {
useRedux: true,
initialState: {
...initialState,
sqlLab: {
...initialState.sqlLab,
lastUpdatedActiveTab: 'old-tab-id',
queryEditors: [
...initialState.sqlLab.queryEditors,
{
id: 'rnd-new-id12',
name: 'new tab name',
inLocalStorage: false,
},
],
tabHistory: ['old-tab-id', 'rnd-new-id12'],
},
},
});
await waitFor(() => jest.advanceTimersByTime(INTERVAL));
expect(fetchMock.calls(updateActiveEditorTabState)).toHaveLength(1);
await waitFor(() => jest.advanceTimersByTime(INTERVAL));
expect(fetchMock.calls(updateActiveEditorTabState)).toHaveLength(1);
});

test('skip syncing the unsaved editor tab state when the updates are already synced', async () => {
const updateEditorTabState = `glob:*/tabstateview/${defaultQueryEditor.id}`;
fetchMock.put(updateEditorTabState, 200);
Expand Down
39 changes: 33 additions & 6 deletions superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,15 @@ import {
QueryEditor,
UnsavedQueryEditor,
} from 'src/SqlLab/types';
import { useUpdateSqlEditorTabMutation } from 'src/hooks/apiResources/sqlEditorTabs';
import {
useUpdateCurrentSqlEditorTabMutation,
useUpdateSqlEditorTabMutation,
} from 'src/hooks/apiResources/sqlEditorTabs';
import { useDebounceValue } from 'src/hooks/useDebounceValue';
import {
syncQueryEditor,
setEditorTabLastUpdate,
setLastUpdatedActiveTab,
} from 'src/SqlLab/actions/sqlLab';
import useEffectEvent from 'src/hooks/useEffectEvent';

Expand Down Expand Up @@ -71,7 +75,15 @@ const EditorAutoSync: FC = () => {
);
const dispatch = useDispatch();
const lastSavedTimestampRef = useRef<number>(editorTabLastUpdatedAt);

const currentQueryEditorId = useSelector<SqlLabRootState, string>(
({ sqlLab }) => sqlLab.tabHistory.slice(-1)[0] || '',
);
const lastUpdatedActiveTab = useSelector<SqlLabRootState, string>(
({ sqlLab }) => sqlLab.lastUpdatedActiveTab,
);
const [updateSqlEditor, { error }] = useUpdateSqlEditorTabMutation();
const [updateCurrentSqlEditor] = useUpdateCurrentSqlEditorTabMutation();

const debouncedUnsavedQueryEditor = useDebounceValue(
unsavedQueryEditor,
Expand All @@ -94,21 +106,36 @@ const EditorAutoSync: FC = () => {
).find(({ inLocalStorage }) => Boolean(inLocalStorage)),
);

const syncCurrentQueryEditor = useEffectEvent(() => {
if (
currentQueryEditorId &&
currentQueryEditorId !== lastUpdatedActiveTab &&
!queryEditors.find(({ id }) => id === currentQueryEditorId)
?.inLocalStorage
) {
updateCurrentSqlEditor(currentQueryEditorId).then(() => {
dispatch(setLastUpdatedActiveTab(currentQueryEditorId));
});
}
});

useEffect(() => {
let timer: NodeJS.Timeout;
let saveTimer: NodeJS.Timeout;
function saveUnsavedQueryEditor() {
const firstUnsavedQueryEditor = getUnsavedNewQueryEditor();

if (firstUnsavedQueryEditor) {
dispatch(syncQueryEditor(firstUnsavedQueryEditor));
}
timer = setTimeout(saveUnsavedQueryEditor, INTERVAL);
saveTimer = setTimeout(saveUnsavedQueryEditor, INTERVAL);
}
timer = setTimeout(saveUnsavedQueryEditor, INTERVAL);
const syncTimer = setInterval(syncCurrentQueryEditor, INTERVAL);
saveTimer = setTimeout(saveUnsavedQueryEditor, INTERVAL);
return () => {
clearTimeout(timer);
clearTimeout(saveTimer);
clearInterval(syncTimer);
};
}, [getUnsavedNewQueryEditor, dispatch]);
}, [getUnsavedNewQueryEditor, syncCurrentQueryEditor, dispatch]);

useEffect(() => {
const unsaved = getUnsavedItems(debouncedUnsavedQueryEditor);
Expand Down
3 changes: 2 additions & 1 deletion superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,8 @@ describe('SqlEditor', () => {
await waitFor(() =>
expect(fetchMock.calls('glob:*/tabstateview/*').length).toBe(1),
);
expect(fetchMock.calls(switchTabApi).length).toBe(1);
// it will be called from EditorAutoSync
expect(fetchMock.calls(switchTabApi).length).toBe(0);
});
});
});
4 changes: 2 additions & 2 deletions superset-frontend/src/SqlLab/components/SqlEditor/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ import {
setActiveSouthPaneTab,
updateSavedQuery,
formatQuery,
switchQueryEditor,
fetchQueryEditor,
} from 'src/SqlLab/actions/sqlLab';
import {
STATE_TYPE_MAP,
Expand Down Expand Up @@ -506,7 +506,7 @@ const SqlEditor: FC<Props> = ({

const loadQueryEditor = useEffectEvent(() => {
if (shouldLoadQueryEditor) {
dispatch(switchQueryEditor(queryEditor, displayLimit));
dispatch(fetchQueryEditor(queryEditor, displayLimit));
}
});

Expand Down
5 changes: 1 addition & 4 deletions superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,7 @@ class TabbedSqlEditors extends PureComponent<TabbedSqlEditorsProps> {
if (!queryEditor) {
return;
}
this.props.actions.switchQueryEditor(
queryEditor,
this.props.displayLimit,
);
this.props.actions.setActiveQueryEditor(queryEditor);
}
}

Expand Down
8 changes: 7 additions & 1 deletion superset-frontend/src/SqlLab/middlewares/persistSqlLabStateEnhancer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,16 @@ const sqlLabPersistStateConfig = {
tables,
queries,
tabHistory,
lastUpdatedActiveTab,
} = state.sqlLab;
const unsavedQueryEditors = filterUnsavedQueryEditorList(
queryEditors,
unsavedQueryEditor,
editorTabLastUpdatedAt,
);
if (unsavedQueryEditors.length > 0) {
const hasUnsavedActiveTabState =
tabHistory.slice(-1)[0] !== lastUpdatedActiveTab;
if (unsavedQueryEditors.length > 0 || hasUnsavedActiveTabState) {
const hasFinishedMigrationFromLocalStorage =
unsavedQueryEditors.every(
({ inLocalStorage }) => !inLocalStorage,
Expand All @@ -70,6 +73,9 @@ const sqlLabPersistStateConfig = {
query => query.inLocalStorage && !query.isDataPreview,
),
}),
...(hasUnsavedActiveTabState && {
tabHistory,
}),
};
}
return;
Expand Down
3 changes: 3 additions & 0 deletions superset-frontend/src/SqlLab/reducers/getInitialState.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,9 @@ describe('getInitialState', () => {
name: expectedValue,
}),
);
expect(
getInitialState(apiDataWithLocalStorage).sqlLab.lastUpdatedActiveTab,
).toEqual(apiDataWithTabState.active_tab.id.toString());
});

it('skip unsaved changes for expired data', () => {
Expand Down
Loading
Loading