You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2024/02/29 19:09:10 UTC
(superset) 05/06: fix(sqllab): invalid dump sql shown after closing tab (#27295)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit da6a25780ef157bfb1e94d647f2e1c734b95ac94
Author: JUST.in DO IT <ju...@airbnb.com>
AuthorDate: Thu Feb 29 09:57:30 2024 -0800
fix(sqllab): invalid dump sql shown after closing tab (#27295)
---
.../SqlLab/components/SqlEditor/SqlEditor.test.tsx | 43 +++++++++++++++++++++-
.../src/SqlLab/components/SqlEditor/index.tsx | 28 ++++++++++++--
.../src/SqlLab/reducers/getInitialState.ts | 2 +-
superset-frontend/src/SqlLab/reducers/sqlLab.js | 5 ++-
.../src/SqlLab/reducers/sqlLab.test.js | 19 ++++++++++
5 files changed, 91 insertions(+), 6 deletions(-)
diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx
index 63f67170d0..d491330443 100644
--- a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx
+++ b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx
@@ -17,6 +17,7 @@
* under the License.
*/
import React from 'react';
+import * as uiCore from '@superset-ui/core';
import { act } from 'react-dom/test-utils';
import { fireEvent, render, waitFor } from 'spec/helpers/testing-library';
import fetchMock from 'fetch-mock';
@@ -31,7 +32,7 @@ import {
import SqlEditorLeftBar from 'src/SqlLab/components/SqlEditorLeftBar';
import ResultSet from 'src/SqlLab/components/ResultSet';
import { api } from 'src/hooks/apiResources/queryApi';
-import { getExtensionsRegistry } from '@superset-ui/core';
+import { getExtensionsRegistry, FeatureFlag } from '@superset-ui/core';
import setupExtensions from 'src/setup/setupExtensions';
import type { Action, Middleware, Store } from 'redux';
import SqlEditor, { Props } from '.';
@@ -63,6 +64,7 @@ fetchMock.get('glob:*/api/v1/database/*/function_names/', {
});
fetchMock.get('glob:*/api/v1/database/*', { result: [] });
fetchMock.get('glob:*/api/v1/database/*/tables/*', { options: [] });
+fetchMock.get('glob:*/tabstateview/*', defaultQueryEditor);
fetchMock.post('glob:*/sqllab/execute/*', { result: [] });
let store: Store;
@@ -290,4 +292,43 @@ describe('SqlEditor', () => {
await findByText('sqleditor.extension.form extension component'),
).toBeInTheDocument();
});
+
+ describe('with SqllabBackendPersistence enabled', () => {
+ let isFeatureEnabledMock: jest.MockInstance<
+ boolean,
+ [feature: FeatureFlag]
+ >;
+ beforeEach(() => {
+ isFeatureEnabledMock = jest
+ .spyOn(uiCore, 'isFeatureEnabled')
+ .mockImplementation(
+ featureFlag =>
+ featureFlag === uiCore.FeatureFlag.SqllabBackendPersistence,
+ );
+ });
+ afterEach(() => {
+ isFeatureEnabledMock.mockClear();
+ });
+
+ it('should render loading state when its Editor is not loaded', async () => {
+ const switchTabApi = `glob:*/tabstateview/${defaultQueryEditor.id}/activate`;
+ fetchMock.post(switchTabApi, {});
+ const { getByTestId } = setup(
+ {
+ ...mockedProps,
+ queryEditor: {
+ ...mockedProps.queryEditor,
+ loaded: false,
+ },
+ },
+ store,
+ );
+ const indicator = getByTestId('sqlEditor-loading');
+ expect(indicator).toBeInTheDocument();
+ await waitFor(() =>
+ expect(fetchMock.calls('glob:*/tabstateview/*').length).toBe(1),
+ );
+ expect(fetchMock.calls(switchTabApi).length).toBe(1);
+ });
+ });
});
diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx
index 0881434487..3970eae513 100644
--- a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx
+++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx
@@ -51,7 +51,7 @@ import Mousetrap from 'mousetrap';
import Button from 'src/components/Button';
import Timer from 'src/components/Timer';
import ResizableSidebar from 'src/components/ResizableSidebar';
-import { AntdDropdown, AntdSwitch } from 'src/components';
+import { AntdDropdown, AntdSwitch, Skeleton } from 'src/components';
import { Input } from 'src/components/Input';
import { Menu } from 'src/components/Menu';
import Icons from 'src/components/Icons';
@@ -73,6 +73,7 @@ import {
setActiveSouthPaneTab,
updateSavedQuery,
formatQuery,
+ switchQueryEditor,
} from 'src/SqlLab/actions/sqlLab';
import {
STATE_TYPE_MAP,
@@ -489,6 +490,16 @@ const SqlEditor: React.FC<Props> = ({
}
});
+ const shouldLoadQueryEditor =
+ isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) &&
+ !queryEditor.loaded;
+
+ const loadQueryEditor = useEffectEvent(() => {
+ if (shouldLoadQueryEditor) {
+ dispatch(switchQueryEditor(queryEditor, displayLimit));
+ }
+ });
+
useEffect(() => {
// We need to measure the height of the sql editor post render to figure the height of
// the south pane so it gets rendered properly
@@ -498,6 +509,7 @@ const SqlEditor: React.FC<Props> = ({
WINDOW_RESIZE_THROTTLE_MS,
);
+ loadQueryEditor();
window.addEventListener('resize', handleWindowResizeWithThrottle);
window.addEventListener('beforeunload', onBeforeUnload);
@@ -506,7 +518,7 @@ const SqlEditor: React.FC<Props> = ({
window.removeEventListener('beforeunload', onBeforeUnload);
};
// TODO: Remove useEffectEvent deps once https://github.com/facebook/react/pull/25881 is released
- }, [onBeforeUnload]);
+ }, [onBeforeUnload, loadQueryEditor]);
useEffect(() => {
if (!database || isEmpty(database)) {
@@ -841,7 +853,17 @@ const SqlEditor: React.FC<Props> = ({
)}
</ResizableSidebar>
</CSSTransition>
- {showEmptyState ? (
+ {shouldLoadQueryEditor ? (
+ <div
+ data-test="sqlEditor-loading"
+ css={css`
+ flex: 1;
+ padding: ${theme.gridUnit * 4}px;
+ `}
+ >
+ <Skeleton active />
+ </div>
+ ) : showEmptyState ? (
<EmptyStateBig
image="vector.svg"
title={t('Select a database to write a query')}
diff --git a/superset-frontend/src/SqlLab/reducers/getInitialState.ts b/superset-frontend/src/SqlLab/reducers/getInitialState.ts
index 02bb5b45fc..e7c6a52c35 100644
--- a/superset-frontend/src/SqlLab/reducers/getInitialState.ts
+++ b/superset-frontend/src/SqlLab/reducers/getInitialState.ts
@@ -57,7 +57,7 @@ export default function getInitialState({
version: LatestQueryEditorVersion,
loaded: true,
name: t('Untitled query'),
- sql: 'SELECT *\nFROM\nWHERE',
+ sql: '',
latestQueryId: null,
autorun: false,
dbId: common.conf.SQLLAB_DEFAULT_DBID,
diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.js b/superset-frontend/src/SqlLab/reducers/sqlLab.js
index 59bd0558a1..d74c2c815b 100644
--- a/superset-frontend/src/SqlLab/reducers/sqlLab.js
+++ b/superset-frontend/src/SqlLab/reducers/sqlLab.js
@@ -152,7 +152,10 @@ export default function sqlLabReducer(state = {}, action) {
newState = {
...newState,
- tabHistory,
+ tabHistory:
+ tabHistory.length === 0 && newState.queryEditors.length > 0
+ ? newState.queryEditors.slice(-1).map(qe => qe.id)
+ : tabHistory,
tables,
queries,
unsavedQueryEditor: {
diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.test.js b/superset-frontend/src/SqlLab/reducers/sqlLab.test.js
index e1a234734b..041835d04b 100644
--- a/superset-frontend/src/SqlLab/reducers/sqlLab.test.js
+++ b/superset-frontend/src/SqlLab/reducers/sqlLab.test.js
@@ -75,6 +75,25 @@ describe('sqlLabReducer', () => {
initialState.queryEditors.length,
);
});
+ it('should select the latest query editor when tabHistory is empty', () => {
+ const currentQE = newState.queryEditors[0];
+ newState = {
+ ...initialState,
+ tabHistory: [initialState.queryEditors[0]],
+ };
+ const action = {
+ type: actions.REMOVE_QUERY_EDITOR,
+ queryEditor: currentQE,
+ };
+ newState = sqlLabReducer(newState, action);
+ expect(newState.queryEditors).toHaveLength(
+ initialState.queryEditors.length - 1,
+ );
+ expect(newState.queryEditors.map(qe => qe.id)).not.toContainEqual(
+ currentQE.id,
+ );
+ expect(newState.tabHistory).toEqual([initialState.queryEditors[2].id]);
+ });
it('should remove a query editor including unsaved changes', () => {
expect(newState.queryEditors).toHaveLength(
initialState.queryEditors.length + 1,