diff --git a/superset-frontend/src/SqlLab/components/App/App.test.tsx b/superset-frontend/src/SqlLab/components/App/App.test.tsx index b609419cb14c4..027347daacad3 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', () => () => (
)); +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()).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(, { 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 4d2e1d222c0e8..973fc79dcb101 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 { // 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 6fd84a0d2a290..ad1881b5d93ed 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 cab1160144a3d..311a125d556c3 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 ? ( @@ -67,5 +81,6 @@ const QueryHistory = ({ /> ); +}; export default QueryHistory; diff --git a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx index 5dc8a43c19310..3282a939ef768 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={ { + 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(); - expect(store.getActions()).toHaveLength(2); - expect(store.getActions()[0].query.results).toEqual( - cachedQueryProps.query.results, - ); + rerender(); + 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 358802d544592..87ba0370edafb 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; 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 = ({ {query.errorMessage}} 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/Results.test.tsx b/superset-frontend/src/SqlLab/components/SouthPane/Results.test.tsx new file mode 100644 index 0000000000000..c70c039fe5840 --- /dev/null +++ b/superset-frontend/src/SqlLab/components/SouthPane/Results.test.tsx @@ -0,0 +1,135 @@ +/** + * 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 { render } from 'spec/helpers/testing-library'; +import { initialState, table, defaultQueryEditor } from 'src/SqlLab/fixtures'; +import { denormalizeTimestamp } from '@superset-ui/core'; +import { LOCALSTORAGE_MAX_QUERY_AGE_MS } from 'src/SqlLab/constants'; +import Results from './Results'; + +const mockedProps = { + queryEditorId: defaultQueryEditor.id, + latestQueryId: 'LCly_kkIN', + height: 1, + displayLimit: 1, + defaultQueryLimit: 100, +}; + +const mockedEmptyProps = { + queryEditorId: 'random_id', + latestQueryId: 'empty_query_id', + height: 100, + displayLimit: 100, + defaultQueryLimit: 100, +}; + +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 mockState = { + ...initialState, + sqlLab: { + ...initialState, + offline: false, + tables: [ + { + ...table, + dataPreviewQueryId: '2g2_iRFMl', + queryEditorId: defaultQueryEditor.id, + }, + ], + databases: {}, + queries: { + LCly_kkIN: { + cached: false, + changed_on: denormalizeTimestamp(new Date().toISOString()), + db: 'main', + dbId: 1, + id: 'LCly_kkIN', + startDttm: Date.now(), + sqlEditorId: defaultQueryEditor.id, + extra: { progress: latestQueryProgressMsg }, + sql: 'select * from table1', + }, + lXJa7F9_r: { + cached: false, + changed_on: denormalizeTimestamp(new Date(1559238500401).toISOString()), + db: 'main', + dbId: 1, + id: 'lXJa7F9_r', + startDttm: 1559238500401, + sqlEditorId: defaultQueryEditor.id, + sql: 'select * from table2', + }, + '2g2_iRFMl': { + cached: false, + changed_on: denormalizeTimestamp(new Date(1559238506925).toISOString()), + db: 'main', + dbId: 1, + id: '2g2_iRFMl', + startDttm: 1559238506925, + sqlEditorId: defaultQueryEditor.id, + sql: 'select * from table3', + }, + expired_query_id: { + cached: false, + changed_on: denormalizeTimestamp( + new Date(expireDateTime).toISOString(), + ), + db: 'main', + dbId: 1, + id: 'expired_query_id', + startDttm: expireDateTime, + sqlEditorId: defaultQueryEditor.id, + sql: 'select * from table4', + }, + }, + }, +}; + +test('Renders an empty state for results', async () => { + const { getByText } = render(, { + useRedux: true, + initialState: mockState, + }); + const emptyStateText = getByText(/run a query to display results/i); + expect(emptyStateText).toBeVisible(); +}); + +test('Renders an empty state for expired results', async () => { + const { getByText } = render(, { + useRedux: true, + initialState: mockState, + }); + const emptyStateText = getByText(/run a query to display results/i); + expect(emptyStateText).toBeVisible(); +}); + +test('should pass latest query down to ResultSet component', async () => { + const { getByText } = render(, { + 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 0000000000000..b5167be61f4bf --- /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 = ({ + 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 ( + + + + ); + } + + if ( + isFeatureEnabled(FeatureFlag.SqllabBackendPersistence) && + latestQuery.state === 'success' && + !latestQuery.resultsKey && + !latestQuery.results + ) { + return ( + + ); + } + + return ( + + ); +}; + +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 80a102ff214af..c978a4ca3233b 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(, { - 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(, { + 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(, { + 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 9aa0b75755a9f..0bbce99b1c43e 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` } `; -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( 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(); const switchTab = (id: string) => { dispatch(setActiveSouthPaneTab(id)); }; - const renderOfflineStatus = () => ( + + return offline ? ( - ); - - 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 = ( - - ); - return results; - } - if (Date.now() - latestQuery.startDttm <= LOCALSTORAGE_MAX_QUERY_AGE_MS) { - results = ( - - ); - } - } else { - results = ( - - - - ); - } - return results; - }; - - const renderDataPreviewTabs = () => - dataPreviewQueries.map(query => ( - - - - )); - return offline ? ( - renderOfflineStatus() ) : ( - {renderResults()} + {latestQueryId && ( + + )} - {renderDataPreviewTabs()} + {dataPreviewQueries.map( + ({ name, dataPreviewQueryId }) => + dataPreviewQueryId && ( + + + + ), + )} ); diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx index 63f67170d05ff..6a25492ce5a8d 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(() => (
)); - (ResultSet as jest.Mock).mockClear(); - (ResultSet as jest.Mock).mockImplementation(() => ( + (ResultSet as unknown as jest.Mock).mockClear(); + (ResultSet as unknown as jest.Mock).mockImplementation(() => (
)); }); @@ -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 23de528066310..8e9d84bd74118 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 = ({ 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 = ({ () => 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 = ({ 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 62ecfb5dcc5d9..078276ad26abf 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 { return (