diff --git a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx index a2ffcf85cb413..f4808f52fdad2 100644 --- a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx @@ -45,7 +45,7 @@ export interface QueryAutoRefreshProps { // returns true if the Query.state matches one of the specifc values indicating the query is still processing on server export const isQueryRunning = (q: Query): boolean => - runningQueryStateList.includes(q?.state) && !q?.resultsKey; + runningQueryStateList.includes(q?.state); // returns true if at least one query is running and within the max age to poll timeframe export const shouldCheckForQueries = (queryList: QueryDictionary): boolean => { diff --git a/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx b/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx index ad1881b5d93ed..6fd84a0d2a290 100644 --- a/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx +++ b/superset-frontend/src/SqlLab/components/QueryHistory/QueryHistory.test.tsx @@ -19,10 +19,9 @@ 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 = { - queryEditorId: 123, + queries: [], displayLimit: 1000, latestQueryId: 'yhMUZCGb', }; @@ -33,7 +32,7 @@ const setup = (overrides = {}) => ( describe('QueryHistory', () => { it('Renders an empty state for query history', () => { - render(setup(), { useRedux: true, initialState }); + render(setup()); 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 311a125d556c3..cab1160144a3d 100644 --- a/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryHistory/index.tsx @@ -16,15 +16,13 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useMemo } from 'react'; -import { shallowEqual, useSelector } from 'react-redux'; +import React from 'react'; import { EmptyStateMedium } from 'src/components/EmptyState'; -import { t, styled } from '@superset-ui/core'; +import { t, styled, QueryResponse } from '@superset-ui/core'; import QueryTable from 'src/SqlLab/components/QueryTable'; -import { SqlLabRootState } from 'src/SqlLab/types'; interface QueryHistoryProps { - queryEditorId: string | number; + queries: QueryResponse[]; displayLimit: number; latestQueryId: string | undefined; } @@ -41,23 +39,11 @@ const StyledEmptyStateWrapper = styled.div` `; const QueryHistory = ({ - queryEditorId, + queries, displayLimit, latestQueryId, -}: 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 ? ( +}: QueryHistoryProps) => + queries.length > 0 ? ( @@ -81,6 +67,5 @@ 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 3282a939ef768..5dc8a43c19310 100644 --- a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx @@ -251,7 +251,8 @@ const QueryTable = ({ modalBody={ { - fetchMock.resetHistory(); -}); const middlewares = [thunk]; const mockStore = configureStore(middlewares); @@ -133,47 +107,25 @@ const setup = (props?: any, store?: Store) => describe('ResultSet', () => { test('renders a Table', async () => { - const { getByTestId } = setup( - mockedProps, - mockStore({ - ...initialState, - user, - sqlLab: { - ...initialState.sqlLab, - queries: { - [queries[0].id]: queries[0], - }, - }, - }), - ); + const { getByTestId } = setup(mockedProps, mockStore(initialState)); 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, - user, - sqlLab: { - ...initialState.sqlLab, - queries: { - [query.id]: query, - }, - }, - }), + mockStore(initialState), ); const table = getByTestId('table-container'); expect(table).toBeInTheDocument(); const firstColumn = queryAllByText( - query.results?.columns[0].column_name ?? '', + mockedProps.query.results?.columns[0].column_name ?? '', )[0]; const secondColumn = queryAllByText( - query.results?.columns[1].column_name ?? '', + mockedProps.query.results?.columns[1].column_name ?? '', )[0]; expect(firstColumn).toBeInTheDocument(); expect(secondColumn).toBeInTheDocument(); @@ -183,24 +135,12 @@ describe('ResultSet', () => { }); test('should render empty results', async () => { - const query = { - ...queries[0], - results: { data: [] }, + const props = { + ...mockedProps, + query: { ...mockedProps.query, results: { data: [] } }, }; await waitFor(() => { - setup( - mockedProps, - mockStore({ - ...initialState, - user, - sqlLab: { - ...initialState.sqlLab, - queries: { - [query.id]: query, - }, - }, - }), - ); + setup(props, mockStore(initialState)); }); const alert = screen.getByRole('alert'); @@ -209,70 +149,42 @@ describe('ResultSet', () => { }); test('should call reRunQuery if timed out', async () => { - const query = { - ...queries[0], - errorMessage: 'Your session timed out', + const store = mockStore(initialState); + const propsWithError = { + ...mockedProps, + query: { ...queries[0], errorMessage: 'Your session timed out' }, }; - const store = mockStore({ - ...initialState, - user, - sqlLab: { - ...initialState.sqlLab, - queries: { - [query.id]: query, - }, - }, - }); - expect(fetchMock.calls(reRunQueryEndpoint)).toHaveLength(0); - setup(mockedProps, store); + setup(propsWithError, 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 query = queries[0]; - const store = mockStore({ - ...initialState, - user, - sqlLab: { - ...initialState.sqlLab, - queries: { - [query.id]: query, - }, - }, - }); + const store = mockStore(initialState); setup(mockedProps, store); expect(store.getActions()).toEqual([]); - expect(fetchMock.calls(reRunQueryEndpoint)).toHaveLength(0); }); test('should render cached query', async () => { - const store = mockStore(cachedQueryState); - const { rerender } = setup( - { ...mockedProps, queryId: cachedQuery.id }, - store, - ); + const store = mockStore(initialState); + const { rerender } = setup(cachedQueryProps, store); // @ts-ignore - rerender(); - expect(store.getActions()).toHaveLength(1); - expect(store.getActions()[0].query.results).toEqual(cachedQuery.results); + rerender(); + expect(store.getActions()).toHaveLength(2); + expect(store.getActions()[0].query.results).toEqual( + cachedQueryProps.query.results, + ); expect(store.getActions()[0].type).toEqual('CLEAR_QUERY_RESULTS'); }); test('should render stopped query', async () => { await waitFor(() => { - setup( - { ...mockedProps, queryId: stoppedQuery.id }, - mockStore(stoppedQueryState), - ); + setup(stoppedQueryProps, mockStore(initialState)); }); const alert = screen.getByRole('alert'); @@ -280,18 +192,15 @@ describe('ResultSet', () => { }); test('should render running/pending/fetching query', async () => { - const { getByTestId } = setup( - { ...mockedProps, queryId: runningQuery.id }, - mockStore(runningQueryState), - ); + const { getByTestId } = setup(runningQueryProps, mockStore(initialState)); const progressBar = getByTestId('progress-bar'); expect(progressBar).toBeInTheDocument(); }); test('should render fetching w/ 100 progress query', async () => { const { getByRole, getByText } = setup( - mockedProps, - mockStore(fetchingQueryState), + fetchingQueryProps, + mockStore(initialState), ); const loading = getByRole('status'); expect(loading).toBeInTheDocument(); @@ -300,10 +209,7 @@ describe('ResultSet', () => { test('should render a failed query with an error message', async () => { await waitFor(() => { - setup( - { ...mockedProps, queryId: failedQueryWithErrorMessage.id }, - mockStore(failedQueryWithErrorMessageState), - ); + setup(failedQueryWithErrorMessageProps, mockStore(initialState)); }); expect(screen.getByText('Database error')).toBeInTheDocument(); @@ -312,129 +218,44 @@ describe('ResultSet', () => { test('should render a failed query with an errors object', async () => { await waitFor(() => { - setup( - { ...mockedProps, queryId: failedQueryWithErrors.id }, - mockStore(failedQueryWithErrorsState), - ); + setup(failedQueryWithErrorsProps, mockStore(initialState)); }); 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, queryId: queryWithNoQueryLimit.id }; - const { getByRole } = setup( - props, - mockStore({ - ...initialState, - user, - sqlLab: { - ...initialState.sqlLab, - queries: { - [queryWithNoQueryLimit.id]: queryWithNoQueryLimit, - }, - }, - }), - ); + const props = { ...mockedProps, query: queryWithNoQueryLimit }; + const { getByRole } = setup(props, mockStore(initialState)); expect(getByRole('table')).toBeInTheDocument(); }); test('Async queries - renders "Fetch data preview" button when data preview has no results', () => { - 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, - }, - }, - }), - ); + setup(asyncRefetchDataPreviewProps, mockStore(initialState)); expect( screen.getByRole('button', { name: /fetch data preview/i, }), ).toBeVisible(); - expect(screen.queryByRole('table')).toBe(null); + expect(screen.queryByRole('grid')).toBe(null); }); test('Async queries - renders "Refetch results" button when a query has no results', () => { - 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, - }, - }, - }), - ); + setup(asyncRefetchResultsTableProps, mockStore(initialState)); expect( screen.getByRole('button', { name: /refetch results/i, }), ).toBeVisible(); - expect(screen.queryByRole('table')).toBe(null); + expect(screen.queryByRole('grid')).toBe(null); }); test('Async queries - renders on the first call', () => { - const query = { - ...queries[0], - }; - setup( - { ...asyncQueryProps, queryId: query.id }, - mockStore({ - ...initialState, - user, - sqlLab: { - ...initialState.sqlLab, - queries: { - [query.id]: query, - }, - }, - }), - ); + setup(asyncQueryProps, mockStore(initialState)); 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 69e450843414d..58e55a1df7911 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 { shallowEqual, useDispatch, useSelector } from 'react-redux'; +import { useDispatch } from 'react-redux'; import { useHistory } from 'react-router-dom'; -import pick from 'lodash/pick'; 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,7 +41,8 @@ import { ISimpleColumn, SaveDatasetModal, } from 'src/SqlLab/components/SaveDatasetModal'; -import { EXPLORE_CHART_DEFAULT, SqlLabRootState } from 'src/SqlLab/types'; +import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes'; +import { EXPLORE_CHART_DEFAULT } from 'src/SqlLab/types'; import { mountExploreUrl } from 'src/explore/exploreUtils'; import { postFormData } from 'src/explore/exploreUtils/formData'; import ProgressBar from 'src/components/ProgressBar'; @@ -81,11 +82,12 @@ export interface ResultSetProps { database?: Record; displayLimit: number; height: number; - queryId: string; + query: QueryResponse; search?: boolean; showSql?: boolean; showSqlInline?: boolean; visualize?: boolean; + user: UserWithPermissionsAndRoles; defaultQueryLimit: number; } @@ -143,44 +145,14 @@ const ResultSet = ({ database = {}, displayLimit, height, - queryId, + query, 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; @@ -207,8 +179,8 @@ const ResultSet = ({ reRunQueryIfSessionTimeoutErrorOnMount(); }, [reRunQueryIfSessionTimeoutErrorOnMount]); - const fetchResults = (q: typeof query) => { - dispatch(fetchQueryResults(q, displayLimit)); + const fetchResults = (query: QueryResponse) => { + dispatch(fetchQueryResults(query, displayLimit)); }; const prevQuery = usePrevious(query); @@ -507,7 +479,7 @@ const ResultSet = ({ {query.errorMessage}} copyText={query.errorMessage || undefined} link={query.link} @@ -690,4 +662,4 @@ const ResultSet = ({ ); }; -export default React.memo(ResultSet); +export default ResultSet; diff --git a/superset-frontend/src/SqlLab/components/SouthPane/Results.test.tsx b/superset-frontend/src/SqlLab/components/SouthPane/Results.test.tsx deleted file mode 100644 index c70c039fe5840..0000000000000 --- a/superset-frontend/src/SqlLab/components/SouthPane/Results.test.tsx +++ /dev/null @@ -1,135 +0,0 @@ -/** - * 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 deleted file mode 100644 index 4e1b6219aeca6..0000000000000 --- a/superset-frontend/src/SqlLab/components/SouthPane/Results.tsx +++ /dev/null @@ -1,106 +0,0 @@ -/** - * 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.SQLLAB_BACKEND_PERSISTENCE) && - 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 c978a4ca3233b..80a102ff214af 100644 --- a/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.tsx +++ b/superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.tsx @@ -17,12 +17,15 @@ * under the License. */ import React from 'react'; -import { render } from 'spec/helpers/testing-library'; -import SouthPane from 'src/SqlLab/components/SouthPane'; +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 { initialState, table, defaultQueryEditor } from 'src/SqlLab/fixtures'; import { denormalizeTimestamp } from '@superset-ui/core'; +import { Store } from 'redux'; const mockedProps = { queryEditorId: defaultQueryEditor.id, @@ -34,32 +37,29 @@ const mockedProps = { const mockedEmptyProps = { queryEditorId: 'random_id', - latestQueryId: 'empty_query_id', + latestQueryId: '', height: 100, displayLimit: 100, defaultQueryLimit: 100, }; +jest.mock('src/SqlLab/components/SqlEditorLeftBar', () => jest.fn()); + const latestQueryProgressMsg = 'LATEST QUERY MESSAGE - LCly_kkIN'; -const mockState = { +const middlewares = [thunk]; +const mockStore = configureStore(middlewares); +const store = mockStore({ ...initialState, sqlLab: { - ...initialState.sqlLab, + ...initialState, offline: false, tables: [ { ...table, - name: 'table3', dataPreviewQueryId: '2g2_iRFMl', queryEditorId: defaultQueryEditor.id, }, - { - ...table, - name: 'table4', - dataPreviewQueryId: 'erWdqEWPm', - queryEditorId: defaultQueryEditor.id, - }, ], databases: {}, queries: { @@ -72,7 +72,6 @@ const mockState = { startDttm: Date.now(), sqlEditorId: defaultQueryEditor.id, extra: { progress: latestQueryProgressMsg }, - sql: 'select * from table1', }, lXJa7F9_r: { cached: false, @@ -82,7 +81,6 @@ const mockState = { id: 'lXJa7F9_r', startDttm: 1559238500401, sqlEditorId: defaultQueryEditor.id, - sql: 'select * from table2', }, '2g2_iRFMl': { cached: false, @@ -92,7 +90,6 @@ const mockState = { id: '2g2_iRFMl', startDttm: 1559238506925, sqlEditorId: defaultQueryEditor.id, - sql: 'select * from table3', }, erWdqEWPm: { cached: false, @@ -102,38 +99,44 @@ const mockState = { id: 'erWdqEWPm', startDttm: 1559238516395, sqlEditorId: defaultQueryEditor.id, - sql: 'select * from table4', }, }, }, -}; - -test('should render offline when the state is offline', async () => { - const { getByText } = render(, { +}); +const setup = (props: SouthPaneProps, store: Store) => + render(, { useRedux: true, - initialState: { - ...initialState, - sqlLab: { - ...initialState.sqlLab, - offline: true, - }, - }, + ...(store && { store }), }); - expect(getByText(STATUS_OPTIONS.offline)).toBeVisible(); -}); +describe('SouthPane', () => { + const renderAndWait = (props: SouthPaneProps, store: Store) => + waitFor(async () => setup(props, store)); -test('should render tabs for table preview queries', () => { - const { getAllByRole } = render(, { - useRedux: true, - initialState: mockState, + 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(); }); - 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}\``); + 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(); + }); + + it('should pass latest query down to ResultSet component', async () => { + await renderAndWait(mockedProps, store); + + expect(screen.getByText(latestQueryProgressMsg)).toBeVisible(); }); }); diff --git a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx index 0bbce99b1c43e..38a20f9f6df07 100644 --- a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx +++ b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx @@ -19,8 +19,10 @@ 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 { styled, t } from '@superset-ui/core'; +import { EmptyStateMedium } from 'src/components/EmptyState'; +import { FeatureFlag, styled, t, isFeatureEnabled } from '@superset-ui/core'; import { setActiveSouthPaneTab } from 'src/SqlLab/actions/sqlLab'; @@ -31,11 +33,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 = 130; +const TAB_HEIGHT = 140; /* editorQueries are queries executed by users passed from SqlEditor component @@ -83,6 +85,18 @@ 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, @@ -91,43 +105,128 @@ const SouthPane = ({ defaultQueryLimit, }: SouthPaneProps) => { const dispatch = useDispatch(); - const { offline, tables } = useSelector( - ({ sqlLab: { offline, tables } }: SqlLabRootState) => ({ + const user = useSelector(({ user }: SqlLabRootState) => user, shallowEqual); + const { databases, offline, queries, tables } = useSelector( + ({ sqlLab: { databases, offline, queries, tables } }: SqlLabRootState) => ({ + databases, offline, + queries, tables, }), shallowEqual, ); - const queries = useSelector( - ({ sqlLab: { queries } }: SqlLabRootState) => Object.keys(queries), - 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 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)); }; - - return offline ? ( + const renderOfflineStatus = () => ( + ); + + const renderResults = () => { + let results; + if (latestQuery) { + if (latestQuery?.extra?.errors) { + latestQuery.errors = latestQuery.extra.errors; + } + if ( + isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE) && + 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() ) : ( - {latestQueryId && ( - - )} + {renderResults()} - {dataPreviewQueries.map( - ({ name, dataPreviewQueryId }) => - dataPreviewQueryId && ( - - - - ), - )} + {renderDataPreviewTabs()} ); diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.tsx index 6a25492ce5a8d..63f67170d05ff 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 unknown as jest.Mock).mockClear(); - (ResultSet as unknown as jest.Mock).mockImplementation(() => ( + (ResultSet as jest.Mock).mockClear(); + (ResultSet as jest.Mock).mockImplementation(() => (
)); }); @@ -182,8 +182,7 @@ describe('SqlEditor', () => { const editor = await findByTestId('react-ace'); const sql = 'select *'; const renderCount = (SqlEditorLeftBar as jest.Mock).mock.calls.length; - const renderCountForSouthPane = (ResultSet as unknown as jest.Mock).mock - .calls.length; + const renderCountForSouthPane = (ResultSet 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/reducers/sqlLab.js b/superset-frontend/src/SqlLab/reducers/sqlLab.js index ce9eed9b9d670..59bd0558a1f1c 100644 --- a/superset-frontend/src/SqlLab/reducers/sqlLab.js +++ b/superset-frontend/src/SqlLab/reducers/sqlLab.js @@ -345,7 +345,7 @@ export default function sqlLabReducer(state = {}, action) { return state; } const alts = { - endDttm: action?.results?.query?.endDttm || now(), + endDttm: now(), progress: 100, results: action.results, rows: action?.results?.query?.rows || 0, @@ -674,14 +674,7 @@ export default function sqlLabReducer(state = {}, action) { if (!change) { newQueries = state.queries; } - return { - ...state, - queries: newQueries, - queriesLastUpdate: - queriesLastUpdate > state.queriesLastUpdate - ? queriesLastUpdate - : Date.now(), - }; + return { ...state, queries: newQueries, queriesLastUpdate }; }, [actions.CLEAR_INACTIVE_QUERIES]() { const { queries } = state; @@ -708,11 +701,7 @@ export default function sqlLabReducer(state = {}, action) { }, ]), ); - return { - ...state, - queries: cleanedQueries, - queriesLastUpdate: Date.now(), - }; + return { ...state, queries: cleanedQueries }; }, [actions.SET_USER_OFFLINE]() { return { ...state, offline: action.offline }; diff --git a/superset-frontend/src/SqlLab/reducers/sqlLab.test.js b/superset-frontend/src/SqlLab/reducers/sqlLab.test.js index 5a70f10bb3a14..e1a234734bca6 100644 --- a/superset-frontend/src/SqlLab/reducers/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/reducers/sqlLab.test.js @@ -20,7 +20,6 @@ import { QueryState } from '@superset-ui/core'; import sqlLabReducer from 'src/SqlLab/reducers/sqlLab'; import * as actions from 'src/SqlLab/actions/sqlLab'; import { table, initialState as mockState } from '../fixtures'; -import { QUERY_UPDATE_FREQ } from '../components/QueryAutoRefresh'; const initialState = mockState.sqlLab; @@ -405,7 +404,6 @@ describe('sqlLabReducer', () => { }; }); it('updates queries that have already been completed', () => { - const current = Date.now(); newState = sqlLabReducer( { ...newState, @@ -420,10 +418,9 @@ describe('sqlLabReducer', () => { }, }, }, - actions.clearInactiveQueries(QUERY_UPDATE_FREQ), + actions.clearInactiveQueries(Date.now()), ); expect(newState.queries.abcd.state).toBe(QueryState.SUCCESS); - expect(newState.queriesLastUpdate).toBeGreaterThanOrEqual(current); }); }); });