You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2024/02/14 21:05:06 UTC
(superset) branch master updated: fix(sqllab): flaky json explore modal due to over-rendering (#26791)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 7b59c94827 fix(sqllab): flaky json explore modal due to over-rendering (#26791)
7b59c94827 is described below
commit 7b59c94827c6e0c2c5be6130869d70a168e9630f
Author: JUST.in DO IT <ju...@airbnb.com>
AuthorDate: Wed Feb 14 13:04:59 2024 -0800
fix(sqllab): flaky json explore modal due to over-rendering (#26791)
---
.../src/SqlLab/components/App/App.test.tsx | 20 +-
.../src/SqlLab/components/App/index.tsx | 3 +
.../components/QueryHistory/QueryHistory.test.tsx | 5 +-
.../src/SqlLab/components/QueryHistory/index.tsx | 29 +-
.../src/SqlLab/components/QueryTable/index.tsx | 3 +-
.../SqlLab/components/ResultSet/ResultSet.test.tsx | 333 ++++++++++++++++-----
.../src/SqlLab/components/ResultSet/index.tsx | 52 +++-
.../{SouthPane.test.tsx => Results.test.tsx} | 87 +++---
.../src/SqlLab/components/SouthPane/Results.tsx | 106 +++++++
.../SqlLab/components/SouthPane/SouthPane.test.tsx | 81 +++--
.../src/SqlLab/components/SouthPane/index.tsx | 174 ++++-------
.../SqlLab/components/SqlEditor/SqlEditor.test.tsx | 7 +-
.../src/SqlLab/components/SqlEditor/index.tsx | 72 ++---
.../SqlLab/components/TabbedSqlEditors/index.tsx | 1 -
14 files changed, 621 insertions(+), 352 deletions(-)
diff --git a/superset-frontend/src/SqlLab/components/App/App.test.tsx b/superset-frontend/src/SqlLab/components/App/App.test.tsx
index b609419cb1..027347daac 100644
--- a/superset-frontend/src/SqlLab/components/App/App.test.tsx
+++ b/superset-frontend/src/SqlLab/components/App/App.test.tsx
@@ -18,6 +18,7 @@
*/
import React from 'react';
import { AnyAction, combineReducers } from 'redux';
+import Mousetrap from 'mousetrap';
import configureStore from 'redux-mock-store';
import thunk from 'redux-thunk';
import { render } from 'spec/helpers/testing-library';
@@ -37,6 +38,9 @@ jest.mock('src/SqlLab/components/TabbedSqlEditors', () => () => (
jest.mock('src/SqlLab/components/QueryAutoRefresh', () => () => (
<div data-test="mock-query-auto-refresh" />
));
+jest.mock('mousetrap', () => ({
+ reset: jest.fn(),
+}));
const sqlLabReducer = combineReducers({
localStorageUsageInKilobytes: reducers.localStorageUsageInKilobytes,
@@ -48,6 +52,14 @@ describe('SqlLab App', () => {
const mockStore = configureStore(middlewares);
const store = mockStore(sqlLabReducer(undefined, mockAction));
+ beforeEach(() => {
+ jest.clearAllMocks();
+ jest.useFakeTimers();
+ });
+ afterEach(() => {
+ jest.useRealTimers();
+ });
+
it('is valid', () => {
expect(React.isValidElement(<App />)).toBe(true);
});
@@ -58,7 +70,13 @@ describe('SqlLab App', () => {
expect(getByTestId('mock-tabbed-sql-editors')).toBeInTheDocument();
});
- it('logs current usage warning', async () => {
+ it('reset hotkey events on unmount', () => {
+ const { unmount } = render(<App />, { useRedux: true, store });
+ unmount();
+ expect(Mousetrap.reset).toHaveBeenCalled();
+ });
+
+ it('logs current usage warning', () => {
const localStorageUsageInKilobytes = LOCALSTORAGE_MAX_USAGE_KB + 10;
const initialState = {
localStorageUsageInKilobytes,
diff --git a/superset-frontend/src/SqlLab/components/App/index.tsx b/superset-frontend/src/SqlLab/components/App/index.tsx
index 4d2e1d222c..973fc79dcb 100644
--- a/superset-frontend/src/SqlLab/components/App/index.tsx
+++ b/superset-frontend/src/SqlLab/components/App/index.tsx
@@ -19,6 +19,7 @@
import React from 'react';
import { connect } from 'react-redux';
import { Redirect } from 'react-router-dom';
+import Mousetrap from 'mousetrap';
import { css, styled, t } from '@superset-ui/core';
import { throttle } from 'lodash';
import {
@@ -165,6 +166,8 @@ class App extends React.PureComponent<AppProps, AppState> {
// And now we need to reset the overscroll behavior back to the default.
document.body.style.overscrollBehaviorX = 'auto';
+
+ Mousetrap.reset();
}
onHashChanged() {
diff --git a/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx b/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx
index 6fd84a0d2a..ad1881b5d9 100644
--- a/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx
+++ b/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx
@@ -19,9 +19,10 @@
import React from 'react';
import { render, screen } from 'spec/helpers/testing-library';
import QueryHistory from 'src/SqlLab/components/QueryHistory';
+import { initialState } from 'src/SqlLab/fixtures';
const mockedProps = {
- queries: [],
+ queryEditorId: 123,
displayLimit: 1000,
latestQueryId: 'yhMUZCGb',
};
@@ -32,7 +33,7 @@ const setup = (overrides = {}) => (
describe('QueryHistory', () => {
it('Renders an empty state for query history', () => {
- render(setup());
+ render(setup(), { useRedux: true, initialState });
const emptyStateText = screen.getByText(
/run a query to display query history/i,
diff --git a/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx b/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx
index cab1160144..311a125d55 100644
--- a/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx
+++ b/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx
@@ -16,13 +16,15 @@
* specific language governing permissions and limitations
* under the License.
*/
-import React from 'react';
+import React, { useMemo } from 'react';
+import { shallowEqual, useSelector } from 'react-redux';
import { EmptyStateMedium } from 'src/components/EmptyState';
-import { t, styled, QueryResponse } from '@superset-ui/core';
+import { t, styled } from '@superset-ui/core';
import QueryTable from 'src/SqlLab/components/QueryTable';
+import { SqlLabRootState } from 'src/SqlLab/types';
interface QueryHistoryProps {
- queries: QueryResponse[];
+ queryEditorId: string | number;
displayLimit: number;
latestQueryId: string | undefined;
}
@@ -39,11 +41,23 @@ const StyledEmptyStateWrapper = styled.div`
`;
const QueryHistory = ({
- queries,
+ queryEditorId,
displayLimit,
latestQueryId,
-}: QueryHistoryProps) =>
- queries.length > 0 ? (
+}: QueryHistoryProps) => {
+ const queries = useSelector(
+ ({ sqlLab: { queries } }: SqlLabRootState) => queries,
+ shallowEqual,
+ );
+ const editorQueries = useMemo(
+ () =>
+ Object.values(queries).filter(
+ ({ sqlEditorId }) => String(sqlEditorId) === String(queryEditorId),
+ ),
+ [queries, queryEditorId],
+ );
+
+ return editorQueries.length > 0 ? (
<QueryTable
columns={[
'state',
@@ -55,7 +69,7 @@ const QueryHistory = ({
'results',
'actions',
]}
- queries={queries}
+ queries={editorQueries}
displayLimit={displayLimit}
latestQueryId={latestQueryId}
/>
@@ -67,5 +81,6 @@ const QueryHistory = ({
/>
</StyledEmptyStateWrapper>
);
+};
export default QueryHistory;
diff --git a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx
index 5dc8a43c19..3282a939ef 100644
--- a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx
+++ b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx
@@ -251,8 +251,7 @@ const QueryTable = ({
modalBody={
<ResultSet
showSql
- user={user}
- query={query}
+ queryId={query.id}
height={400}
displayLimit={displayLimit}
defaultQueryLimit={1000}
diff --git a/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.tsx b/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.tsx
index d823c586f7..e5844fed5c 100644
--- a/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.tsx
+++ b/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.tsx
@@ -37,65 +37,91 @@ import {
const mockedProps = {
cache: true,
- query: queries[0],
+ queryId: queries[0].id,
height: 140,
database: { allows_virtual_table_explore: true },
- user,
+ displayLimit: 1000,
defaultQueryLimit: 1000,
};
-const stoppedQueryProps = { ...mockedProps, query: stoppedQuery };
-const runningQueryProps = { ...mockedProps, query: runningQuery };
-const fetchingQueryProps = {
- ...mockedProps,
- query: {
- dbId: 1,
- cached: false,
- ctas: false,
- id: 'ryhHUZCGb',
- progress: 100,
- state: 'fetching',
- startDttm: Date.now() - 500,
+const stoppedQueryState = {
+ ...initialState,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queries: {
+ [stoppedQuery.id]: stoppedQuery,
+ },
},
};
-const cachedQueryProps = { ...mockedProps, query: cachedQuery };
-const failedQueryWithErrorMessageProps = {
- ...mockedProps,
- query: failedQueryWithErrorMessage,
-};
-const failedQueryWithErrorsProps = {
- ...mockedProps,
- query: failedQueryWithErrors,
+const runningQueryState = {
+ ...initialState,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queries: {
+ [runningQuery.id]: runningQuery,
+ },
+ },
};
-const newProps = {
- query: {
- cached: false,
- resultsKey: 'new key',
- results: {
- data: [{ a: 1 }],
+const fetchingQueryState = {
+ ...initialState,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queries: {
+ [mockedProps.queryId]: {
+ dbId: 1,
+ cached: false,
+ ctas: false,
+ id: 'ryhHUZCGb',
+ progress: 100,
+ state: 'fetching',
+ startDttm: Date.now() - 500,
+ },
},
},
};
-const asyncQueryProps = {
- ...mockedProps,
- database: { allow_run_async: true },
+const cachedQueryState = {
+ ...initialState,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queries: {
+ [cachedQuery.id]: cachedQuery,
+ },
+ },
};
-const asyncRefetchDataPreviewProps = {
- ...asyncQueryProps,
- query: {
- state: 'success',
- results: undefined,
- isDataPreview: true,
+const failedQueryWithErrorMessageState = {
+ ...initialState,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queries: {
+ [failedQueryWithErrorMessage.id]: failedQueryWithErrorMessage,
+ },
},
};
-const asyncRefetchResultsTableProps = {
- ...asyncQueryProps,
- query: {
- state: 'success',
- results: undefined,
- resultsKey: 'async results key',
+const failedQueryWithErrorsState = {
+ ...initialState,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queries: {
+ [failedQueryWithErrors.id]: failedQueryWithErrors,
+ },
},
};
+
+const newProps = {
+ displayLimit: 1001,
+};
+const asyncQueryProps = {
+ ...mockedProps,
+ database: { allow_run_async: true },
+};
+
+const reRunQueryEndpoint = 'glob:*/api/v1/sqllab/execute/';
fetchMock.get('glob:*/api/v1/dataset/?*', { result: [] });
+fetchMock.post(reRunQueryEndpoint, { result: [] });
+fetchMock.get('glob:*/api/v1/sqllab/results/*', { result: [] });
+
+beforeEach(() => {
+ fetchMock.resetHistory();
+});
const middlewares = [thunk];
const mockStore = configureStore(middlewares);
@@ -107,25 +133,47 @@ const setup = (props?: any, store?: Store) =>
describe('ResultSet', () => {
test('renders a Table', async () => {
- const { getByTestId } = setup(mockedProps, mockStore(initialState));
+ const { getByTestId } = setup(
+ mockedProps,
+ mockStore({
+ ...initialState,
+ user,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queries: {
+ [queries[0].id]: queries[0],
+ },
+ },
+ }),
+ );
const table = getByTestId('table-container');
expect(table).toBeInTheDocument();
});
test('should render success query', async () => {
+ const query = queries[0];
const { queryAllByText, getByTestId } = setup(
mockedProps,
- mockStore(initialState),
+ mockStore({
+ ...initialState,
+ user,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queries: {
+ [query.id]: query,
+ },
+ },
+ }),
);
const table = getByTestId('table-container');
expect(table).toBeInTheDocument();
const firstColumn = queryAllByText(
- mockedProps.query.results?.columns[0].column_name ?? '',
+ query.results?.columns[0].column_name ?? '',
)[0];
const secondColumn = queryAllByText(
- mockedProps.query.results?.columns[1].column_name ?? '',
+ query.results?.columns[1].column_name ?? '',
)[0];
expect(firstColumn).toBeInTheDocument();
expect(secondColumn).toBeInTheDocument();
@@ -135,12 +183,24 @@ describe('ResultSet', () => {
});
test('should render empty results', async () => {
- const props = {
- ...mockedProps,
- query: { ...mockedProps.query, results: { data: [] } },
+ const query = {
+ ...queries[0],
+ results: { data: [] },
};
await waitFor(() => {
- setup(props, mockStore(initialState));
+ setup(
+ mockedProps,
+ mockStore({
+ ...initialState,
+ user,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queries: {
+ [query.id]: query,
+ },
+ },
+ }),
+ );
});
const alert = screen.getByRole('alert');
@@ -149,42 +209,70 @@ describe('ResultSet', () => {
});
test('should call reRunQuery if timed out', async () => {
- const store = mockStore(initialState);
- const propsWithError = {
- ...mockedProps,
- query: { ...queries[0], errorMessage: 'Your session timed out' },
+ const query = {
+ ...queries[0],
+ errorMessage: 'Your session timed out',
};
+ const store = mockStore({
+ ...initialState,
+ user,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queries: {
+ [query.id]: query,
+ },
+ },
+ });
- setup(propsWithError, store);
+ expect(fetchMock.calls(reRunQueryEndpoint)).toHaveLength(0);
+ setup(mockedProps, store);
expect(store.getActions()).toHaveLength(1);
expect(store.getActions()[0].query.errorMessage).toEqual(
'Your session timed out',
);
expect(store.getActions()[0].type).toEqual('START_QUERY');
+ await waitFor(() =>
+ expect(fetchMock.calls(reRunQueryEndpoint)).toHaveLength(1),
+ );
});
test('should not call reRunQuery if no error', async () => {
- const store = mockStore(initialState);
+ const query = queries[0];
+ const store = mockStore({
+ ...initialState,
+ user,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queries: {
+ [query.id]: query,
+ },
+ },
+ });
setup(mockedProps, store);
expect(store.getActions()).toEqual([]);
+ expect(fetchMock.calls(reRunQueryEndpoint)).toHaveLength(0);
});
test('should render cached query', async () => {
- const store = mockStore(initialState);
- const { rerender } = setup(cachedQueryProps, store);
+ const store = mockStore(cachedQueryState);
+ const { rerender } = setup(
+ { ...mockedProps, queryId: cachedQuery.id },
+ store,
+ );
// @ts-ignore
- rerender(<ResultSet {...newProps} />);
- expect(store.getActions()).toHaveLength(2);
- expect(store.getActions()[0].query.results).toEqual(
- cachedQueryProps.query.results,
- );
+ rerender(<ResultSet {...mockedProps} {...newProps} />);
+ expect(store.getActions()).toHaveLength(1);
+ expect(store.getActions()[0].query.results).toEqual(cachedQuery.results);
expect(store.getActions()[0].type).toEqual('CLEAR_QUERY_RESULTS');
});
test('should render stopped query', async () => {
await waitFor(() => {
- setup(stoppedQueryProps, mockStore(initialState));
+ setup(
+ { ...mockedProps, queryId: stoppedQuery.id },
+ mockStore(stoppedQueryState),
+ );
});
const alert = screen.getByRole('alert');
@@ -192,15 +280,18 @@ describe('ResultSet', () => {
});
test('should render running/pending/fetching query', async () => {
- const { getByTestId } = setup(runningQueryProps, mockStore(initialState));
+ const { getByTestId } = setup(
+ { ...mockedProps, queryId: runningQuery.id },
+ mockStore(runningQueryState),
+ );
const progressBar = getByTestId('progress-bar');
expect(progressBar).toBeInTheDocument();
});
test('should render fetching w/ 100 progress query', async () => {
const { getByRole, getByText } = setup(
- fetchingQueryProps,
- mockStore(initialState),
+ mockedProps,
+ mockStore(fetchingQueryState),
);
const loading = getByRole('status');
expect(loading).toBeInTheDocument();
@@ -209,7 +300,10 @@ describe('ResultSet', () => {
test('should render a failed query with an error message', async () => {
await waitFor(() => {
- setup(failedQueryWithErrorMessageProps, mockStore(initialState));
+ setup(
+ { ...mockedProps, queryId: failedQueryWithErrorMessage.id },
+ mockStore(failedQueryWithErrorMessageState),
+ );
});
expect(screen.getByText('Database error')).toBeInTheDocument();
@@ -218,44 +312,129 @@ describe('ResultSet', () => {
test('should render a failed query with an errors object', async () => {
await waitFor(() => {
- setup(failedQueryWithErrorsProps, mockStore(initialState));
+ setup(
+ { ...mockedProps, queryId: failedQueryWithErrors.id },
+ mockStore(failedQueryWithErrorsState),
+ );
});
expect(screen.getByText('Database error')).toBeInTheDocument();
});
test('renders if there is no limit in query.results but has queryLimit', async () => {
+ const query = {
+ ...queries[0],
+ };
+ await waitFor(() => {
+ setup(
+ mockedProps,
+ mockStore({
+ ...initialState,
+ user,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queries: {
+ [query.id]: query,
+ },
+ },
+ }),
+ );
+ });
const { getByRole } = setup(mockedProps, mockStore(initialState));
expect(getByRole('table')).toBeInTheDocument();
});
test('renders if there is a limit in query.results but not queryLimit', async () => {
- const props = { ...mockedProps, query: queryWithNoQueryLimit };
- const { getByRole } = setup(props, mockStore(initialState));
+ const props = { ...mockedProps, queryId: queryWithNoQueryLimit.id };
+ const { getByRole } = setup(
+ props,
+ mockStore({
+ ...initialState,
+ user,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queries: {
+ [queryWithNoQueryLimit.id]: queryWithNoQueryLimit,
+ },
+ },
+ }),
+ );
expect(getByRole('table')).toBeInTheDocument();
});
test('Async queries - renders "Fetch data preview" button when data preview has no results', () => {
- setup(asyncRefetchDataPreviewProps, mockStore(initialState));
+ const asyncRefetchDataPreviewQuery = {
+ ...queries[0],
+ state: 'success',
+ results: undefined,
+ isDataPreview: true,
+ };
+ setup(
+ { ...asyncQueryProps, queryId: asyncRefetchDataPreviewQuery.id },
+ mockStore({
+ ...initialState,
+ user,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queries: {
+ [asyncRefetchDataPreviewQuery.id]: asyncRefetchDataPreviewQuery,
+ },
+ },
+ }),
+ );
expect(
screen.getByRole('button', {
name: /fetch data preview/i,
}),
).toBeVisible();
- expect(screen.queryByRole('grid')).toBe(null);
+ expect(screen.queryByRole('table')).toBe(null);
});
test('Async queries - renders "Refetch results" button when a query has no results', () => {
- setup(asyncRefetchResultsTableProps, mockStore(initialState));
+ const asyncRefetchResultsTableQuery = {
+ ...queries[0],
+ state: 'success',
+ results: undefined,
+ resultsKey: 'async results key',
+ };
+
+ setup(
+ { ...asyncQueryProps, queryId: asyncRefetchResultsTableQuery.id },
+ mockStore({
+ ...initialState,
+ user,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queries: {
+ [asyncRefetchResultsTableQuery.id]: asyncRefetchResultsTableQuery,
+ },
+ },
+ }),
+ );
expect(
screen.getByRole('button', {
name: /refetch results/i,
}),
).toBeVisible();
- expect(screen.queryByRole('grid')).toBe(null);
+ expect(screen.queryByRole('table')).toBe(null);
});
test('Async queries - renders on the first call', () => {
- setup(asyncQueryProps, mockStore(initialState));
+ const query = {
+ ...queries[0],
+ };
+ setup(
+ { ...asyncQueryProps, queryId: query.id },
+ mockStore({
+ ...initialState,
+ user,
+ sqlLab: {
+ ...initialState.sqlLab,
+ queries: {
+ [query.id]: query,
+ },
+ },
+ }),
+ );
expect(screen.getByRole('table')).toBeVisible();
expect(
screen.queryByRole('button', {
diff --git a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx
index 358802d544..87ba0370ed 100644
--- a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx
+++ b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx
@@ -17,14 +17,14 @@
* under the License.
*/
import React, { useCallback, useEffect, useState } from 'react';
-import { useDispatch } from 'react-redux';
+import { shallowEqual, useDispatch, useSelector } from 'react-redux';
import { useHistory } from 'react-router-dom';
+import { pick } from 'lodash';
import ButtonGroup from 'src/components/ButtonGroup';
import Alert from 'src/components/Alert';
import Button from 'src/components/Button';
import shortid from 'shortid';
import {
- QueryResponse,
QueryState,
styled,
t,
@@ -41,8 +41,7 @@ import {
ISimpleColumn,
SaveDatasetModal,
} from 'src/SqlLab/components/SaveDatasetModal';
-import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
-import { EXPLORE_CHART_DEFAULT } from 'src/SqlLab/types';
+import { EXPLORE_CHART_DEFAULT, SqlLabRootState } from 'src/SqlLab/types';
import { mountExploreUrl } from 'src/explore/exploreUtils';
import { postFormData } from 'src/explore/exploreUtils/formData';
import ProgressBar from 'src/components/ProgressBar';
@@ -82,12 +81,11 @@ export interface ResultSetProps {
database?: Record<string, any>;
displayLimit: number;
height: number;
- query: QueryResponse;
+ queryId: string;
search?: boolean;
showSql?: boolean;
showSqlInline?: boolean;
visualize?: boolean;
- user: UserWithPermissionsAndRoles;
defaultQueryLimit: number;
}
@@ -145,14 +143,44 @@ const ResultSet = ({
database = {},
displayLimit,
height,
- query,
+ queryId,
search = true,
showSql = false,
showSqlInline = false,
visualize = true,
- user,
defaultQueryLimit,
}: ResultSetProps) => {
+ const user = useSelector(({ user }: SqlLabRootState) => user, shallowEqual);
+ const query = useSelector(
+ ({ sqlLab: { queries } }: SqlLabRootState) =>
+ pick(queries[queryId], [
+ 'id',
+ 'errorMessage',
+ 'cached',
+ 'results',
+ 'resultsKey',
+ 'dbId',
+ 'tab',
+ 'sql',
+ 'templateParams',
+ 'schema',
+ 'rows',
+ 'queryLimit',
+ 'limitingFactor',
+ 'trackingUrl',
+ 'state',
+ 'errors',
+ 'link',
+ 'ctas',
+ 'ctas_method',
+ 'tempSchema',
+ 'tempTable',
+ 'isDataPreview',
+ 'progress',
+ 'extra',
+ ]),
+ shallowEqual,
+ );
const ResultTable =
extensionsRegistry.get('sqleditor.extension.resultTable') ??
FilterableTable;
@@ -179,8 +207,8 @@ const ResultSet = ({
reRunQueryIfSessionTimeoutErrorOnMount();
}, [reRunQueryIfSessionTimeoutErrorOnMount]);
- const fetchResults = (query: QueryResponse) => {
- dispatch(fetchQueryResults(query, displayLimit));
+ const fetchResults = (q: typeof query) => {
+ dispatch(fetchQueryResults(q, displayLimit));
};
const prevQuery = usePrevious(query);
@@ -478,7 +506,7 @@ const ResultSet = ({
<ResultlessStyles>
<ErrorMessageWithStackTrace
title={t('Database error')}
- error={query?.errors?.[0]}
+ error={query?.extra?.errors?.[0] || query?.errors?.[0]}
subtitle={<MonospaceDiv>{query.errorMessage}</MonospaceDiv>}
copyText={query.errorMessage || undefined}
link={query.link}
@@ -661,4 +689,4 @@ const ResultSet = ({
);
};
-export default ResultSet;
+export default React.memo(ResultSet);
diff --git a/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.tsx b/superset-frontend/src/SqlLab/components/SouthPane/Results.test.tsx
similarity index 60%
copy from superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.tsx
copy to superset-frontend/src/SqlLab/components/SouthPane/Results.test.tsx
index 80a102ff21..c70c039fe5 100644
--- a/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.tsx
+++ b/superset-frontend/src/SqlLab/components/SouthPane/Results.test.tsx
@@ -17,15 +17,11 @@
* under the License.
*/
import React from 'react';
-import configureStore from 'redux-mock-store';
-import thunk from 'redux-thunk';
-import { render, screen, waitFor } from 'spec/helpers/testing-library';
-import SouthPane, { SouthPaneProps } from 'src/SqlLab/components/SouthPane';
-import '@testing-library/jest-dom/extend-expect';
-import { STATUS_OPTIONS } from 'src/SqlLab/constants';
+import { render } from 'spec/helpers/testing-library';
import { initialState, table, defaultQueryEditor } from 'src/SqlLab/fixtures';
import { denormalizeTimestamp } from '@superset-ui/core';
-import { Store } from 'redux';
+import { LOCALSTORAGE_MAX_QUERY_AGE_MS } from 'src/SqlLab/constants';
+import Results from './Results';
const mockedProps = {
queryEditorId: defaultQueryEditor.id,
@@ -37,19 +33,21 @@ const mockedProps = {
const mockedEmptyProps = {
queryEditorId: 'random_id',
- latestQueryId: '',
+ latestQueryId: 'empty_query_id',
height: 100,
displayLimit: 100,
defaultQueryLimit: 100,
};
-jest.mock('src/SqlLab/components/SqlEditorLeftBar', () => jest.fn());
+const mockedExpiredProps = {
+ ...mockedEmptyProps,
+ latestQueryId: 'expired_query_id',
+};
const latestQueryProgressMsg = 'LATEST QUERY MESSAGE - LCly_kkIN';
+const expireDateTime = Date.now() - LOCALSTORAGE_MAX_QUERY_AGE_MS - 1;
-const middlewares = [thunk];
-const mockStore = configureStore(middlewares);
-const store = mockStore({
+const mockState = {
...initialState,
sqlLab: {
...initialState,
@@ -72,6 +70,7 @@ const store = mockStore({
startDttm: Date.now(),
sqlEditorId: defaultQueryEditor.id,
extra: { progress: latestQueryProgressMsg },
+ sql: 'select * from table1',
},
lXJa7F9_r: {
cached: false,
@@ -81,6 +80,7 @@ const store = mockStore({
id: 'lXJa7F9_r',
startDttm: 1559238500401,
sqlEditorId: defaultQueryEditor.id,
+ sql: 'select * from table2',
},
'2g2_iRFMl': {
cached: false,
@@ -90,53 +90,46 @@ const store = mockStore({
id: '2g2_iRFMl',
startDttm: 1559238506925,
sqlEditorId: defaultQueryEditor.id,
+ sql: 'select * from table3',
},
- erWdqEWPm: {
+ expired_query_id: {
cached: false,
- changed_on: denormalizeTimestamp(new Date(1559238516395).toISOString()),
+ changed_on: denormalizeTimestamp(
+ new Date(expireDateTime).toISOString(),
+ ),
db: 'main',
dbId: 1,
- id: 'erWdqEWPm',
- startDttm: 1559238516395,
+ id: 'expired_query_id',
+ startDttm: expireDateTime,
sqlEditorId: defaultQueryEditor.id,
+ sql: 'select * from table4',
},
},
},
-});
-const setup = (props: SouthPaneProps, store: Store) =>
- render(<SouthPane {...props} />, {
- useRedux: true,
- ...(store && { store }),
- });
-
-describe('SouthPane', () => {
- const renderAndWait = (props: SouthPaneProps, store: Store) =>
- waitFor(async () => setup(props, store));
+};
- it('Renders an empty state for results', async () => {
- await renderAndWait(mockedEmptyProps, store);
- const emptyStateText = screen.getByText(/run a query to display results/i);
- expect(emptyStateText).toBeVisible();
+test('Renders an empty state for results', async () => {
+ const { getByText } = render(<Results {...mockedEmptyProps} />, {
+ useRedux: true,
+ initialState: mockState,
});
+ const emptyStateText = getByText(/run a query to display results/i);
+ expect(emptyStateText).toBeVisible();
+});
- it('should render offline when the state is offline', async () => {
- await renderAndWait(
- mockedEmptyProps,
- mockStore({
- ...initialState,
- sqlLab: {
- ...initialState.sqlLab,
- offline: true,
- },
- }),
- );
-
- expect(screen.getByText(STATUS_OPTIONS.offline)).toBeVisible();
+test('Renders an empty state for expired results', async () => {
+ const { getByText } = render(<Results {...mockedExpiredProps} />, {
+ useRedux: true,
+ initialState: mockState,
});
+ const emptyStateText = getByText(/run a query to display results/i);
+ expect(emptyStateText).toBeVisible();
+});
- it('should pass latest query down to ResultSet component', async () => {
- await renderAndWait(mockedProps, store);
-
- expect(screen.getByText(latestQueryProgressMsg)).toBeVisible();
+test('should pass latest query down to ResultSet component', async () => {
+ const { getByText } = render(<Results {...mockedProps} />, {
+ useRedux: true,
+ initialState: mockState,
});
+ expect(getByText(latestQueryProgressMsg)).toBeVisible();
});
diff --git a/superset-frontend/src/SqlLab/components/SouthPane/Results.tsx b/superset-frontend/src/SqlLab/components/SouthPane/Results.tsx
new file mode 100644
index 0000000000..b5167be61f
--- /dev/null
+++ b/superset-frontend/src/SqlLab/components/SouthPane/Results.tsx
@@ -0,0 +1,106 @@
+/**
+ * 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 { shallowEqual, useSelector } from 'react-redux';
+import Alert from 'src/components/Alert';
+import { EmptyStateMedium } from 'src/components/EmptyState';
+import { FeatureFlag, styled, t, isFeatureEnabled } from '@superset-ui/core';
+
+import { SqlLabRootState } from 'src/SqlLab/types';
+import ResultSet from '../ResultSet';
+import { LOCALSTORAGE_MAX_QUERY_AGE_MS } from '../../constants';
+
+const EXTRA_HEIGHT_RESULTS = 8; // we need extra height in RESULTS tab. because the height from props was calculated based on PREVIEW tab.
+
+type Props = {
+ latestQueryId: string;
+ height: number;
+ displayLimit: number;
+ defaultQueryLimit: number;
+};
+
+const StyledEmptyStateWrapper = styled.div`
+ height: 100%;
+ .ant-empty-image img {
+ margin-right: 28px;
+ }
+
+ p {
+ margin-right: 28px;
+ }
+`;
+
+const Results: React.FC<Props> = ({
+ latestQueryId,
+ height,
+ displayLimit,
+ defaultQueryLimit,
+}) => {
+ const databases = useSelector(
+ ({ sqlLab: { databases } }: SqlLabRootState) => databases,
+ shallowEqual,
+ );
+ const latestQuery = useSelector(
+ ({ sqlLab: { queries } }: SqlLabRootState) => queries[latestQueryId || ''],
+ shallowEqual,
+ );
+
+ if (
+ !latestQuery ||
+ Date.now() - latestQuery.startDttm > LOCALSTORAGE_MAX_QUERY_AGE_MS
+ ) {
+ return (
+ <StyledEmptyStateWrapper>
+ <EmptyStateMedium
+ title={t('Run a query to display results')}
+ image="document.svg"
+ />
+ </StyledEmptyStateWrapper>
+ );
+ }
+
+ if (
+ isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) &&
+ latestQuery.state === 'success' &&
+ !latestQuery.resultsKey &&
+ !latestQuery.results
+ ) {
+ return (
+ <Alert
+ type="warning"
+ message={t('No stored results found, you need to re-run your query')}
+ />
+ );
+ }
+
+ return (
+ <ResultSet
+ search
+ queryId={latestQuery.id}
+ height={height + EXTRA_HEIGHT_RESULTS}
+ database={databases[latestQuery.dbId]}
+ displayLimit={displayLimit}
+ defaultQueryLimit={defaultQueryLimit}
+ showSql
+ showSqlInline
+ />
+ );
+};
+
+export default Results;
diff --git a/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.tsx b/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.tsx
index 80a102ff21..c978a4ca32 100644
--- a/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.tsx
+++ b/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.tsx
@@ -17,15 +17,12 @@
* under the License.
*/
import React from 'react';
-import configureStore from 'redux-mock-store';
-import thunk from 'redux-thunk';
-import { render, screen, waitFor } from 'spec/helpers/testing-library';
-import SouthPane, { SouthPaneProps } from 'src/SqlLab/components/SouthPane';
+import { render } from 'spec/helpers/testing-library';
+import SouthPane from 'src/SqlLab/components/SouthPane';
import '@testing-library/jest-dom/extend-expect';
import { STATUS_OPTIONS } from 'src/SqlLab/constants';
import { initialState, table, defaultQueryEditor } from 'src/SqlLab/fixtures';
import { denormalizeTimestamp } from '@superset-ui/core';
-import { Store } from 'redux';
const mockedProps = {
queryEditorId: defaultQueryEditor.id,
@@ -37,29 +34,32 @@ const mockedProps = {
const mockedEmptyProps = {
queryEditorId: 'random_id',
- latestQueryId: '',
+ latestQueryId: 'empty_query_id',
height: 100,
displayLimit: 100,
defaultQueryLimit: 100,
};
-jest.mock('src/SqlLab/components/SqlEditorLeftBar', () => jest.fn());
-
const latestQueryProgressMsg = 'LATEST QUERY MESSAGE - LCly_kkIN';
-const middlewares = [thunk];
-const mockStore = configureStore(middlewares);
-const store = mockStore({
+const mockState = {
...initialState,
sqlLab: {
- ...initialState,
+ ...initialState.sqlLab,
offline: false,
tables: [
{
...table,
+ name: 'table3',
dataPreviewQueryId: '2g2_iRFMl',
queryEditorId: defaultQueryEditor.id,
},
+ {
+ ...table,
+ name: 'table4',
+ dataPreviewQueryId: 'erWdqEWPm',
+ queryEditorId: defaultQueryEditor.id,
+ },
],
databases: {},
queries: {
@@ -72,6 +72,7 @@ const store = mockStore({
startDttm: Date.now(),
sqlEditorId: defaultQueryEditor.id,
extra: { progress: latestQueryProgressMsg },
+ sql: 'select * from table1',
},
lXJa7F9_r: {
cached: false,
@@ -81,6 +82,7 @@ const store = mockStore({
id: 'lXJa7F9_r',
startDttm: 1559238500401,
sqlEditorId: defaultQueryEditor.id,
+ sql: 'select * from table2',
},
'2g2_iRFMl': {
cached: false,
@@ -90,6 +92,7 @@ const store = mockStore({
id: '2g2_iRFMl',
startDttm: 1559238506925,
sqlEditorId: defaultQueryEditor.id,
+ sql: 'select * from table3',
},
erWdqEWPm: {
cached: false,
@@ -99,44 +102,38 @@ const store = mockStore({
id: 'erWdqEWPm',
startDttm: 1559238516395,
sqlEditorId: defaultQueryEditor.id,
+ sql: 'select * from table4',
},
},
},
-});
-const setup = (props: SouthPaneProps, store: Store) =>
- render(<SouthPane {...props} />, {
- useRedux: true,
- ...(store && { store }),
- });
-
-describe('SouthPane', () => {
- const renderAndWait = (props: SouthPaneProps, store: Store) =>
- waitFor(async () => setup(props, store));
+};
- it('Renders an empty state for results', async () => {
- await renderAndWait(mockedEmptyProps, store);
- const emptyStateText = screen.getByText(/run a query to display results/i);
- expect(emptyStateText).toBeVisible();
+test('should render offline when the state is offline', async () => {
+ const { getByText } = render(<SouthPane {...mockedEmptyProps} />, {
+ useRedux: true,
+ initialState: {
+ ...initialState,
+ sqlLab: {
+ ...initialState.sqlLab,
+ offline: true,
+ },
+ },
});
- it('should render offline when the state is offline', async () => {
- await renderAndWait(
- mockedEmptyProps,
- mockStore({
- ...initialState,
- sqlLab: {
- ...initialState.sqlLab,
- offline: true,
- },
- }),
- );
+ expect(getByText(STATUS_OPTIONS.offline)).toBeVisible();
+});
- expect(screen.getByText(STATUS_OPTIONS.offline)).toBeVisible();
+test('should render tabs for table preview queries', () => {
+ const { getAllByRole } = render(<SouthPane {...mockedProps} />, {
+ useRedux: true,
+ initialState: mockState,
});
- it('should pass latest query down to ResultSet component', async () => {
- await renderAndWait(mockedProps, store);
-
- expect(screen.getByText(latestQueryProgressMsg)).toBeVisible();
+ const tabs = getAllByRole('tab');
+ expect(tabs).toHaveLength(mockState.sqlLab.tables.length + 2);
+ expect(tabs[0]).toHaveTextContent('Results');
+ expect(tabs[1]).toHaveTextContent('Query history');
+ mockState.sqlLab.tables.forEach(({ name }, index) => {
+ expect(tabs[index + 2]).toHaveTextContent(`Preview: \`${name}\``);
});
});
diff --git a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx
index 9aa0b75755..0bbce99b1c 100644
--- a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx
+++ b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx
@@ -19,10 +19,8 @@
import React, { createRef, useMemo } from 'react';
import { shallowEqual, useDispatch, useSelector } from 'react-redux';
import shortid from 'shortid';
-import Alert from 'src/components/Alert';
import Tabs from 'src/components/Tabs';
-import { EmptyStateMedium } from 'src/components/EmptyState';
-import { FeatureFlag, styled, t, isFeatureEnabled } from '@superset-ui/core';
+import { styled, t } from '@superset-ui/core';
import { setActiveSouthPaneTab } from 'src/SqlLab/actions/sqlLab';
@@ -33,11 +31,11 @@ import ResultSet from '../ResultSet';
import {
STATUS_OPTIONS,
STATE_TYPE_MAP,
- LOCALSTORAGE_MAX_QUERY_AGE_MS,
STATUS_OPTIONS_LOCALIZED,
} from '../../constants';
+import Results from './Results';
-const TAB_HEIGHT = 140;
+const TAB_HEIGHT = 130;
/*
editorQueries are queries executed by users passed from SqlEditor component
@@ -85,18 +83,6 @@ const StyledPane = styled.div<StyledPaneProps>`
}
`;
-const EXTRA_HEIGHT_RESULTS = 24; // we need extra height in RESULTS tab. because the height from props was calculated based on PREVIEW tab.
-const StyledEmptyStateWrapper = styled.div`
- height: 100%;
- .ant-empty-image img {
- margin-right: 28px;
- }
-
- p {
- margin-right: 28px;
- }
-`;
-
const SouthPane = ({
queryEditorId,
latestQueryId,
@@ -105,128 +91,43 @@ const SouthPane = ({
defaultQueryLimit,
}: SouthPaneProps) => {
const dispatch = useDispatch();
- const user = useSelector(({ user }: SqlLabRootState) => user, shallowEqual);
- const { databases, offline, queries, tables } = useSelector(
- ({ sqlLab: { databases, offline, queries, tables } }: SqlLabRootState) => ({
- databases,
+ const { offline, tables } = useSelector(
+ ({ sqlLab: { offline, tables } }: SqlLabRootState) => ({
offline,
- queries,
tables,
}),
shallowEqual,
);
- const editorQueries = useMemo(
- () =>
- Object.values(queries).filter(
- ({ sqlEditorId }) => sqlEditorId === queryEditorId,
- ),
- [queries, queryEditorId],
- );
- const dataPreviewQueries = useMemo(
- () =>
- tables
- .filter(
- ({ dataPreviewQueryId, queryEditorId: qeId }) =>
- dataPreviewQueryId &&
- queryEditorId === qeId &&
- queries[dataPreviewQueryId],
- )
- .map(({ name, dataPreviewQueryId }) => ({
- ...queries[dataPreviewQueryId || ''],
- tableName: name,
- })),
- [queries, queryEditorId, tables],
- );
- const latestQuery = useMemo(
- () => editorQueries.find(({ id }) => id === latestQueryId),
- [editorQueries, latestQueryId],
+ const queries = useSelector(
+ ({ sqlLab: { queries } }: SqlLabRootState) => Object.keys(queries),
+ shallowEqual,
);
-
const activeSouthPaneTab =
useSelector<SqlLabRootState, string>(
state => state.sqlLab.activeSouthPaneTab as string,
) ?? 'Results';
+
+ const querySet = useMemo(() => new Set(queries), [queries]);
+ const dataPreviewQueries = useMemo(
+ () =>
+ tables.filter(
+ ({ dataPreviewQueryId, queryEditorId: qeId }) =>
+ dataPreviewQueryId &&
+ queryEditorId === qeId &&
+ querySet.has(dataPreviewQueryId),
+ ),
+ [queryEditorId, tables, querySet],
+ );
const innerTabContentHeight = height - TAB_HEIGHT;
const southPaneRef = createRef<HTMLDivElement>();
const switchTab = (id: string) => {
dispatch(setActiveSouthPaneTab(id));
};
- const renderOfflineStatus = () => (
+
+ return offline ? (
<Label className="m-r-3" type={STATE_TYPE_MAP[STATUS_OPTIONS.offline]}>
{STATUS_OPTIONS_LOCALIZED.offline}
</Label>
- );
-
- const renderResults = () => {
- let results;
- if (latestQuery) {
- if (latestQuery?.extra?.errors) {
- latestQuery.errors = latestQuery.extra.errors;
- }
- if (
- isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) &&
- latestQuery.state === 'success' &&
- !latestQuery.resultsKey &&
- !latestQuery.results
- ) {
- results = (
- <Alert
- type="warning"
- message={t(
- 'No stored results found, you need to re-run your query',
- )}
- />
- );
- return results;
- }
- if (Date.now() - latestQuery.startDttm <= LOCALSTORAGE_MAX_QUERY_AGE_MS) {
- results = (
- <ResultSet
- search
- query={latestQuery}
- user={user}
- height={innerTabContentHeight + EXTRA_HEIGHT_RESULTS}
- database={databases[latestQuery.dbId]}
- displayLimit={displayLimit}
- defaultQueryLimit={defaultQueryLimit}
- showSql
- showSqlInline
- />
- );
- }
- } else {
- results = (
- <StyledEmptyStateWrapper>
- <EmptyStateMedium
- title={t('Run a query to display results')}
- image="document.svg"
- />
- </StyledEmptyStateWrapper>
- );
- }
- return results;
- };
-
- const renderDataPreviewTabs = () =>
- dataPreviewQueries.map(query => (
- <Tabs.TabPane
- tab={t('Preview: `%s`', decodeURIComponent(query.tableName))}
- key={query.id}
- >
- <ResultSet
- query={query}
- visualize={false}
- csv={false}
- cache
- user={user}
- height={innerTabContentHeight}
- displayLimit={displayLimit}
- defaultQueryLimit={defaultQueryLimit}
- />
- </Tabs.TabPane>
- ));
- return offline ? (
- renderOfflineStatus()
) : (
<StyledPane
data-test="south-pane"
@@ -243,16 +144,41 @@ const SouthPane = ({
animated={false}
>
<Tabs.TabPane tab={t('Results')} key="Results">
- {renderResults()}
+ {latestQueryId && (
+ <Results
+ height={innerTabContentHeight}
+ latestQueryId={latestQueryId}
+ displayLimit={displayLimit}
+ defaultQueryLimit={defaultQueryLimit}
+ />
+ )}
</Tabs.TabPane>
<Tabs.TabPane tab={t('Query history')} key="History">
<QueryHistory
- queries={editorQueries}
+ queryEditorId={queryEditorId}
displayLimit={displayLimit}
latestQueryId={latestQueryId}
/>
</Tabs.TabPane>
- {renderDataPreviewTabs()}
+ {dataPreviewQueries.map(
+ ({ name, dataPreviewQueryId }) =>
+ dataPreviewQueryId && (
+ <Tabs.TabPane
+ tab={t('Preview: `%s`', decodeURIComponent(name))}
+ key={dataPreviewQueryId}
+ >
+ <ResultSet
+ queryId={dataPreviewQueryId}
+ visualize={false}
+ csv={false}
+ cache
+ height={innerTabContentHeight}
+ displayLimit={displayLimit}
+ defaultQueryLimit={defaultQueryLimit}
+ />
+ </Tabs.TabPane>
+ ),
+ )}
</Tabs>
</StyledPane>
);
diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx
index 63f67170d0..6a25492ce5 100644
--- a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx
+++ b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx
@@ -145,8 +145,8 @@ describe('SqlEditor', () => {
(SqlEditorLeftBar as jest.Mock).mockImplementation(() => (
<div data-test="mock-sql-editor-left-bar" />
));
- (ResultSet as jest.Mock).mockClear();
- (ResultSet as jest.Mock).mockImplementation(() => (
+ (ResultSet as unknown as jest.Mock).mockClear();
+ (ResultSet as unknown as jest.Mock).mockImplementation(() => (
<div data-test="mock-result-set" />
));
});
@@ -182,7 +182,8 @@ describe('SqlEditor', () => {
const editor = await findByTestId('react-ace');
const sql = 'select *';
const renderCount = (SqlEditorLeftBar as jest.Mock).mock.calls.length;
- const renderCountForSouthPane = (ResultSet as jest.Mock).mock.calls.length;
+ const renderCountForSouthPane = (ResultSet as unknown as jest.Mock).mock
+ .calls.length;
expect(SqlEditorLeftBar).toHaveBeenCalledTimes(renderCount);
expect(ResultSet).toHaveBeenCalledTimes(renderCountForSouthPane);
fireEvent.change(editor, { target: { value: sql } });
diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx
index 23de528066..8e9d84bd74 100644
--- a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx
+++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx
@@ -244,29 +244,33 @@ const SqlEditor: React.FC<Props> = ({
const theme = useTheme();
const dispatch = useDispatch();
- const { database, latestQuery, hideLeftBar } = useSelector<
- SqlLabRootState,
- {
- database?: DatabaseObject;
- latestQuery?: QueryResponse;
- hideLeftBar?: boolean;
- }
- >(({ sqlLab: { unsavedQueryEditor, databases, queries } }) => {
- let { dbId, latestQueryId, hideLeftBar } = queryEditor;
- if (unsavedQueryEditor?.id === queryEditor.id) {
- dbId = unsavedQueryEditor.dbId || dbId;
- latestQueryId = unsavedQueryEditor.latestQueryId || latestQueryId;
- hideLeftBar = isBoolean(unsavedQueryEditor.hideLeftBar)
- ? unsavedQueryEditor.hideLeftBar
- : hideLeftBar;
- }
- return {
- database: databases[dbId || ''],
- latestQuery: queries[latestQueryId || ''],
- hideLeftBar,
- };
- }, shallowEqual);
-
+ const { database, latestQuery, hideLeftBar, currentQueryEditorId } =
+ useSelector<
+ SqlLabRootState,
+ {
+ database?: DatabaseObject;
+ latestQuery?: QueryResponse;
+ hideLeftBar?: boolean;
+ currentQueryEditorId: QueryEditor['id'];
+ }
+ >(({ sqlLab: { unsavedQueryEditor, databases, queries, tabHistory } }) => {
+ let { dbId, latestQueryId, hideLeftBar } = queryEditor;
+ if (unsavedQueryEditor?.id === queryEditor.id) {
+ dbId = unsavedQueryEditor.dbId || dbId;
+ latestQueryId = unsavedQueryEditor.latestQueryId || latestQueryId;
+ hideLeftBar = isBoolean(unsavedQueryEditor.hideLeftBar)
+ ? unsavedQueryEditor.hideLeftBar
+ : hideLeftBar;
+ }
+ return {
+ database: databases[dbId || ''],
+ latestQuery: queries[latestQueryId || ''],
+ hideLeftBar,
+ currentQueryEditorId: tabHistory.slice(-1)[0],
+ };
+ }, shallowEqual);
+
+ const isActive = currentQueryEditorId === queryEditor.id;
const [height, setHeight] = useState(0);
const [autorun, setAutorun] = useState(queryEditor.autorun);
const [ctas, setCtas] = useState('');
@@ -498,16 +502,17 @@ const SqlEditor: React.FC<Props> = ({
() => setHeight(getSqlEditorHeight()),
WINDOW_RESIZE_THROTTLE_MS,
);
-
- window.addEventListener('resize', handleWindowResizeWithThrottle);
- window.addEventListener('beforeunload', onBeforeUnload);
+ if (isActive) {
+ window.addEventListener('resize', handleWindowResizeWithThrottle);
+ window.addEventListener('beforeunload', onBeforeUnload);
+ }
return () => {
window.removeEventListener('resize', handleWindowResizeWithThrottle);
window.removeEventListener('beforeunload', onBeforeUnload);
};
// TODO: Remove useEffectEvent deps once https://github.com/facebook/react/pull/25881 is released
- }, [onBeforeUnload]);
+ }, [onBeforeUnload, isActive]);
useEffect(() => {
if (!database || isEmpty(database)) {
@@ -518,15 +523,14 @@ const SqlEditor: React.FC<Props> = ({
useEffect(() => {
// setup hotkeys
const hotkeys = getHotkeyConfig();
- hotkeys.forEach(keyConfig => {
- Mousetrap.bind([keyConfig.key], keyConfig.func);
- });
- return () => {
+ if (isActive) {
+ // MouseTrap always override the same key
+ // Unbind (reset) will be called when App component unmount
hotkeys.forEach(keyConfig => {
- Mousetrap.unbind(keyConfig.key);
+ Mousetrap.bind([keyConfig.key], keyConfig.func);
});
- };
- }, [getHotkeyConfig, latestQuery]);
+ }
+ }, [getHotkeyConfig, latestQuery, isActive]);
const onResizeStart = () => {
// Set the heights on the ace editor and the ace content area after drag starts
diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx
index 62ecfb5dcc..078276ad26 100644
--- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx
+++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.tsx
@@ -281,7 +281,6 @@ class TabbedSqlEditors extends React.PureComponent<TabbedSqlEditorsProps> {
return (
<StyledEditableTabs
- destroyInactiveTabPane
activeKey={this.props.tabHistory[this.props.tabHistory.length - 1]}
id="a11y-query-editor-tabs"
className="SqlEditorTabs"