From e881675f58b600e5c0023ebcc63a0d0a03fee759 Mon Sep 17 00:00:00 2001 From: "JUST.in DO IT" Date: Thu, 29 Feb 2024 09:57:30 -0800 Subject: [PATCH] fix(sqllab): invalid dump sql shown after closing tab (#27295) (cherry picked from commit 8d245704ef94ffb7fff250855230e3c2c461e6c3) --- .../components/SqlEditor/SqlEditor.test.tsx | 43 ++++++++++++++++++- .../src/SqlLab/components/SqlEditor/index.tsx | 28 ++++++++++-- .../src/SqlLab/reducers/getInitialState.ts | 2 +- .../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 6a25492ce5..f174110a2b 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; @@ -291,4 +293,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 8e9d84bd74..1590726953 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx @@ -54,7 +54,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'; @@ -77,6 +77,7 @@ import { setActiveSouthPaneTab, updateSavedQuery, formatQuery, + switchQueryEditor, } from 'src/SqlLab/actions/sqlLab'; import { STATE_TYPE_MAP, @@ -494,6 +495,16 @@ const SqlEditor: React.FC = ({ } }); + const shouldLoadQueryEditor = + isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) && + !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 @@ -503,6 +514,7 @@ const SqlEditor: React.FC = ({ WINDOW_RESIZE_THROTTLE_MS, ); if (isActive) { + loadQueryEditor(); window.addEventListener('resize', handleWindowResizeWithThrottle); window.addEventListener('beforeunload', onBeforeUnload); } @@ -512,7 +524,7 @@ const SqlEditor: React.FC = ({ window.removeEventListener('beforeunload', onBeforeUnload); }; // TODO: Remove useEffectEvent deps once https://github.com/facebook/react/pull/25881 is released - }, [onBeforeUnload, isActive]); + }, [onBeforeUnload, loadQueryEditor, isActive]); useEffect(() => { if (!database || isEmpty(database)) { @@ -847,7 +859,17 @@ const SqlEditor: React.FC = ({ )} - {showEmptyState ? ( + {shouldLoadQueryEditor ? ( +
+ +
+ ) : showEmptyState ? ( 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 485094adaf..87e0212b16 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,