This is an automated email from the ASF dual-hosted git repository. hugh pushed a commit to branch 2022.39.1 in repository https://gitbox.apache.org/repos/asf/superset.git
commit a169ed12258694200452482da59273c7adcf8070 Author: Joe Li <[email protected]> AuthorDate: Tue Oct 4 10:41:02 2022 -0700 Revert "chore: refactor ResultSet to functional component (#21186)" This reverts commit f6032956780380bdac1e3ae950687ae53eabf2e1. --- .../components/ExploreCtasResultsButton/index.tsx | 25 +- .../components/ExploreResultsButton/index.tsx | 1 - .../src/SqlLab/components/QueryTable/index.tsx | 1 + .../SqlLab/components/ResultSet/ResultSet.test.jsx | 219 ++++++++ .../SqlLab/components/ResultSet/ResultSet.test.tsx | 216 -------- .../src/SqlLab/components/ResultSet/index.tsx | 573 +++++++++++---------- .../src/SqlLab/components/SouthPane/index.tsx | 3 + .../src/components/FilterableTable/index.tsx | 3 +- .../src/components/ProgressBar/index.tsx | 2 +- 9 files changed, 542 insertions(+), 501 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/ExploreCtasResultsButton/index.tsx b/superset-frontend/src/SqlLab/components/ExploreCtasResultsButton/index.tsx index ac9e8b2fb4..fbcdc15bc5 100644 --- a/superset-frontend/src/SqlLab/components/ExploreCtasResultsButton/index.tsx +++ b/superset-frontend/src/SqlLab/components/ExploreCtasResultsButton/index.tsx @@ -17,19 +17,19 @@ * under the License. */ import React from 'react'; -import { useSelector, useDispatch } from 'react-redux'; -import { t, JsonObject } from '@superset-ui/core'; -import { - createCtasDatasource, - addInfoToast, - addDangerToast, -} from 'src/SqlLab/actions/sqlLab'; +import { useSelector } from 'react-redux'; +import { t } from '@superset-ui/core'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; import Button from 'src/components/Button'; import { exploreChart } from 'src/explore/exploreUtils'; import { SqlLabRootState } from 'src/SqlLab/types'; interface ExploreCtasResultsButtonProps { + actions: { + createCtasDatasource: Function; + addInfoToast: Function; + addDangerToast: Function; + }; table: string; schema?: string | null; dbId: number; @@ -37,15 +37,16 @@ interface ExploreCtasResultsButtonProps { } const ExploreCtasResultsButton = ({ + actions, table, schema, dbId, templateParams, }: ExploreCtasResultsButtonProps) => { + const { createCtasDatasource, addInfoToast, addDangerToast } = actions; const errorMessage = useSelector( (state: SqlLabRootState) => state.sqlLab.errorMessage, ); - const dispatch = useDispatch<(dispatch: any) => Promise<JsonObject>>(); const buildVizOptions = { datasourceName: table, @@ -55,7 +56,7 @@ const ExploreCtasResultsButton = ({ }; const visualize = () => { - dispatch(createCtasDatasource(buildVizOptions)) + createCtasDatasource(buildVizOptions) .then((data: { table_id: number }) => { const formData = { datasource: `${data.table_id}__table`, @@ -66,14 +67,12 @@ const ExploreCtasResultsButton = ({ all_columns: [], row_limit: 1000, }; - dispatch( - addInfoToast(t('Creating a data source and creating a new tab')), - ); + addInfoToast(t('Creating a data source and creating a new tab')); // open new window for data visualization exploreChart(formData); }) .catch(() => { - dispatch(addDangerToast(errorMessage || t('An error occurred'))); + addDangerToast(errorMessage || t('An error occurred')); }); }; diff --git a/superset-frontend/src/SqlLab/components/ExploreResultsButton/index.tsx b/superset-frontend/src/SqlLab/components/ExploreResultsButton/index.tsx index 4ab7777736..24d5e8686f 100644 --- a/superset-frontend/src/SqlLab/components/ExploreResultsButton/index.tsx +++ b/superset-frontend/src/SqlLab/components/ExploreResultsButton/index.tsx @@ -39,7 +39,6 @@ const ExploreResultsButton = ({ onClick={onClick} disabled={!allowsSubquery} tooltip={t('Explore the result set in the data exploration view')} - data-test="explore-results-button" > <InfoTooltipWithTrigger icon="line-chart" diff --git a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx index d7ef5ed51c..54edb7f97e 100644 --- a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx @@ -245,6 +245,7 @@ const QueryTable = ({ showSql user={user} query={query} + actions={actions} height={400} displayLimit={displayLimit} defaultQueryLimit={1000} diff --git a/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.jsx b/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.jsx new file mode 100644 index 0000000000..c04e236133 --- /dev/null +++ b/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.jsx @@ -0,0 +1,219 @@ +/** + * 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 { shallow } from 'enzyme'; +import { styledMount } from 'spec/helpers/theming'; +import { render, screen } from 'spec/helpers/testing-library'; +import { Provider } from 'react-redux'; +import sinon from 'sinon'; +import Alert from 'src/components/Alert'; +import ProgressBar from 'src/components/ProgressBar'; +import Loading from 'src/components/Loading'; +import configureStore from 'redux-mock-store'; +import thunk from 'redux-thunk'; +import fetchMock from 'fetch-mock'; +import FilterableTable from 'src/components/FilterableTable'; +import ExploreResultsButton from 'src/SqlLab/components/ExploreResultsButton'; +import ResultSet from 'src/SqlLab/components/ResultSet'; +import ErrorMessageWithStackTrace from 'src/components/ErrorMessage/ErrorMessageWithStackTrace'; +import { + cachedQuery, + failedQueryWithErrorMessage, + failedQueryWithErrors, + queries, + runningQuery, + stoppedQuery, + initialState, + user, + queryWithNoQueryLimit, +} from 'src/SqlLab/fixtures'; + +const mockStore = configureStore([thunk]); +const store = mockStore(initialState); +const clearQuerySpy = sinon.spy(); +const fetchQuerySpy = sinon.spy(); +const reRunQuerySpy = sinon.spy(); +const mockedProps = { + actions: { + clearQueryResults: clearQuerySpy, + fetchQueryResults: fetchQuerySpy, + reRunQuery: reRunQuerySpy, + }, + cache: true, + query: queries[0], + height: 140, + database: { allows_virtual_table_explore: true }, + user, + 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 cachedQueryProps = { ...mockedProps, query: cachedQuery }; +const failedQueryWithErrorMessageProps = { + ...mockedProps, + query: failedQueryWithErrorMessage, +}; +const failedQueryWithErrorsProps = { + ...mockedProps, + query: failedQueryWithErrors, +}; +const newProps = { + query: { + cached: false, + resultsKey: 'new key', + results: { + data: [{ a: 1 }], + }, + }, +}; +fetchMock.get('glob:*/api/v1/dataset?*', { result: [] }); + +test('is valid', () => { + expect(React.isValidElement(<ResultSet {...mockedProps} />)).toBe(true); +}); + +test('renders a Table', () => { + const wrapper = shallow(<ResultSet {...mockedProps} />); + expect(wrapper.find(FilterableTable)).toExist(); +}); + +describe('componentDidMount', () => { + const propsWithError = { + ...mockedProps, + query: { ...queries[0], errorMessage: 'Your session timed out' }, + }; + let spy; + beforeEach(() => { + reRunQuerySpy.resetHistory(); + spy = sinon.spy(ResultSet.prototype, 'componentDidMount'); + }); + afterEach(() => { + spy.restore(); + }); + it('should call reRunQuery if timed out', () => { + shallow(<ResultSet {...propsWithError} />); + expect(reRunQuerySpy.callCount).toBe(1); + }); + + it('should not call reRunQuery if no error', () => { + shallow(<ResultSet {...mockedProps} />); + expect(reRunQuerySpy.callCount).toBe(0); + }); +}); + +describe('UNSAFE_componentWillReceiveProps', () => { + const wrapper = shallow(<ResultSet {...mockedProps} />); + let spy; + beforeEach(() => { + clearQuerySpy.resetHistory(); + fetchQuerySpy.resetHistory(); + spy = sinon.spy(ResultSet.prototype, 'UNSAFE_componentWillReceiveProps'); + }); + afterEach(() => { + spy.restore(); + }); + it('should update cached data', () => { + wrapper.setProps(newProps); + + expect(wrapper.state().data).toEqual(newProps.query.results.data); + expect(clearQuerySpy.callCount).toBe(1); + expect(clearQuerySpy.getCall(0).args[0]).toEqual(newProps.query); + expect(fetchQuerySpy.callCount).toBe(1); + expect(fetchQuerySpy.getCall(0).args[0]).toEqual(newProps.query); + }); +}); + +test('should render success query', () => { + const wrapper = shallow(<ResultSet {...mockedProps} />); + const filterableTable = wrapper.find(FilterableTable); + expect(filterableTable.props().data).toBe(mockedProps.query.results.data); + expect(wrapper.find(ExploreResultsButton)).toExist(); +}); +test('should render empty results', () => { + const props = { + ...mockedProps, + query: { ...mockedProps.query, results: { data: [] } }, + }; + const wrapper = styledMount( + <Provider store={store}> + <ResultSet {...props} /> + </Provider>, + ); + expect(wrapper.find(FilterableTable)).not.toExist(); + expect(wrapper.find(Alert)).toExist(); + expect(wrapper.find(Alert).render().text()).toBe( + 'The query returned no data', + ); +}); + +test('should render cached query', () => { + const wrapper = shallow(<ResultSet {...cachedQueryProps} />); + const cachedData = [{ col1: 'a', col2: 'b' }]; + wrapper.setState({ data: cachedData }); + const filterableTable = wrapper.find(FilterableTable); + expect(filterableTable.props().data).toBe(cachedData); +}); + +test('should render stopped query', () => { + const wrapper = shallow(<ResultSet {...stoppedQueryProps} />); + expect(wrapper.find(Alert)).toExist(); +}); + +test('should render running/pending/fetching query', () => { + const wrapper = shallow(<ResultSet {...runningQueryProps} />); + expect(wrapper.find(ProgressBar)).toExist(); +}); + +test('should render fetching w/ 100 progress query', () => { + const wrapper = shallow(<ResultSet {...fetchingQueryProps} />); + expect(wrapper.find(Loading)).toExist(); +}); + +test('should render a failed query with an error message', () => { + const wrapper = shallow(<ResultSet {...failedQueryWithErrorMessageProps} />); + expect(wrapper.find(ErrorMessageWithStackTrace)).toExist(); +}); + +test('should render a failed query with an errors object', () => { + const wrapper = shallow(<ResultSet {...failedQueryWithErrorsProps} />); + expect(wrapper.find(ErrorMessageWithStackTrace)).toExist(); +}); + +test('renders if there is no limit in query.results but has queryLimit', () => { + render(<ResultSet {...mockedProps} />, { useRedux: true }); + expect(screen.getByRole('grid')).toBeInTheDocument(); +}); + +test('renders if there is a limit in query.results but not queryLimit', () => { + const props = { ...mockedProps, query: queryWithNoQueryLimit }; + render(<ResultSet {...props} />, { useRedux: true }); + expect(screen.getByRole('grid')).toBeInTheDocument(); +}); diff --git a/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.tsx b/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.tsx deleted file mode 100644 index 2818ed279c..0000000000 --- a/superset-frontend/src/SqlLab/components/ResultSet/ResultSet.test.tsx +++ /dev/null @@ -1,216 +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, screen, waitFor } from 'spec/helpers/testing-library'; -import configureStore from 'redux-mock-store'; -import { Store } from 'redux'; -import thunk from 'redux-thunk'; -import fetchMock from 'fetch-mock'; -import ResultSet from 'src/SqlLab/components/ResultSet'; -import { - cachedQuery, - failedQueryWithErrorMessage, - failedQueryWithErrors, - queries, - runningQuery, - stoppedQuery, - initialState, - user, - queryWithNoQueryLimit, -} from 'src/SqlLab/fixtures'; - -const mockedProps = { - cache: true, - query: queries[0], - height: 140, - database: { allows_virtual_table_explore: true }, - user, - 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 cachedQueryProps = { ...mockedProps, query: cachedQuery }; -const failedQueryWithErrorMessageProps = { - ...mockedProps, - query: failedQueryWithErrorMessage, -}; -const failedQueryWithErrorsProps = { - ...mockedProps, - query: failedQueryWithErrors, -}; -const newProps = { - query: { - cached: false, - resultsKey: 'new key', - results: { - data: [{ a: 1 }], - }, - }, -}; -fetchMock.get('glob:*/api/v1/dataset?*', { result: [] }); - -const middlewares = [thunk]; -const mockStore = configureStore(middlewares); -const setup = (props?: any, store?: Store) => - render(<ResultSet {...props} />, { - useRedux: true, - ...(store && { store }), - }); - -describe('ResultSet', () => { - it('renders a Table', async () => { - const { getByTestId } = setup(mockedProps, mockStore(initialState)); - const table = getByTestId('table-container'); - expect(table).toBeInTheDocument(); - }); - - it('should render success query', async () => { - const { queryAllByText, getByTestId } = setup( - mockedProps, - mockStore(initialState), - ); - - const table = getByTestId('table-container'); - expect(table).toBeInTheDocument(); - - const firstColumn = queryAllByText( - mockedProps.query.results?.columns[0].name ?? '', - )[0]; - const secondColumn = queryAllByText( - mockedProps.query.results?.columns[1].name ?? '', - )[0]; - expect(firstColumn).toBeInTheDocument(); - expect(secondColumn).toBeInTheDocument(); - - const exploreButton = getByTestId('explore-results-button'); - expect(exploreButton).toBeInTheDocument(); - }); - - it('should render empty results', async () => { - const props = { - ...mockedProps, - query: { ...mockedProps.query, results: { data: [] } }, - }; - await waitFor(() => { - setup(props, mockStore(initialState)); - }); - - const alert = screen.getByRole('alert'); - expect(alert).toBeInTheDocument(); - expect(alert).toHaveTextContent('The query returned no data'); - }); - - it('should call reRunQuery if timed out', async () => { - const store = mockStore(initialState); - const propsWithError = { - ...mockedProps, - query: { ...queries[0], errorMessage: 'Your session timed out' }, - }; - - 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'); - }); - - it('should not call reRunQuery if no error', async () => { - const store = mockStore(initialState); - setup(mockedProps, store); - expect(store.getActions()).toEqual([]); - }); - - it('should render cached query', async () => { - const store = mockStore(initialState); - const { rerender } = setup(cachedQueryProps, store); - - // @ts-ignore - rerender(<ResultSet {...newProps} />); - expect(store.getActions()).toHaveLength(1); - expect(store.getActions()[0].query.results).toEqual( - cachedQueryProps.query.results, - ); - expect(store.getActions()[0].type).toEqual('CLEAR_QUERY_RESULTS'); - }); - - it('should render stopped query', async () => { - await waitFor(() => { - setup(stoppedQueryProps, mockStore(initialState)); - }); - - const alert = screen.getByRole('alert'); - expect(alert).toBeInTheDocument(); - }); - - it('should render running/pending/fetching query', async () => { - const { getByTestId } = setup(runningQueryProps, mockStore(initialState)); - const progressBar = getByTestId('progress-bar'); - expect(progressBar).toBeInTheDocument(); - }); - - it('should render fetching w/ 100 progress query', async () => { - const { getByRole, getByText } = setup( - fetchingQueryProps, - mockStore(initialState), - ); - const loading = getByRole('status'); - expect(loading).toBeInTheDocument(); - expect(getByText('fetching')).toBeInTheDocument(); - }); - - it('should render a failed query with an error message', async () => { - await waitFor(() => { - setup(failedQueryWithErrorMessageProps, mockStore(initialState)); - }); - - expect(screen.getByText('Database error')).toBeInTheDocument(); - expect(screen.getByText('Something went wrong')).toBeInTheDocument(); - }); - - it('should render a failed query with an errors object', async () => { - await waitFor(() => { - setup(failedQueryWithErrorsProps, mockStore(initialState)); - }); - expect(screen.getByText('Database error')).toBeInTheDocument(); - }); - - it('renders if there is no limit in query.results but has queryLimit', async () => { - const { getByRole } = setup(mockedProps, mockStore(initialState)); - expect(getByRole('grid')).toBeInTheDocument(); - }); - - it('renders if there is a limit in query.results but not queryLimit', async () => { - const props = { ...mockedProps, query: queryWithNoQueryLimit }; - const { getByRole } = setup(props, mockStore(initialState)); - expect(getByRole('grid')).toBeInTheDocument(); - }); -}); diff --git a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx index 78387f6dc4..27913d7fc4 100644 --- a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx +++ b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx @@ -16,14 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useState, useEffect, useCallback } from 'react'; -import { useDispatch } from 'react-redux'; +import React from 'react'; import ButtonGroup from 'src/components/ButtonGroup'; import Alert from 'src/components/Alert'; import Button from 'src/components/Button'; import shortid from 'shortid'; import { styled, t, QueryResponse } from '@superset-ui/core'; -import { usePrevious } from 'src/hooks/usePrevious'; import ErrorMessageWithStackTrace from 'src/components/ErrorMessage/ErrorMessageWithStackTrace'; import { ISaveableDatasource, @@ -42,14 +40,7 @@ import FilterableTable, { import CopyToClipboard from 'src/components/CopyToClipboard'; import { addDangerToast } from 'src/components/MessageToasts/actions'; import { prepareCopyToClipboardTabularData } from 'src/utils/common'; -import { - CtasEnum, - clearQueryResults, - addQueryEditor, - fetchQueryResults, - reFetchQueryResults, - reRunQuery, -} from 'src/SqlLab/actions/sqlLab'; +import { CtasEnum } from 'src/SqlLab/actions/sqlLab'; import { URL_PARAMS } from 'src/constants'; import ExploreCtasResultsButton from '../ExploreCtasResultsButton'; import ExploreResultsButton from '../ExploreResultsButton'; @@ -63,7 +54,9 @@ enum LIMITING_FACTOR { NOT_LIMITED = 'NOT_LIMITED', } -export interface ResultSetProps { +interface ResultSetProps { + showControls?: boolean; + actions: Record<string, any>; cache?: boolean; csv?: boolean; database?: Record<string, any>; @@ -77,9 +70,17 @@ export interface ResultSetProps { defaultQueryLimit: number; } +interface ResultSetState { + searchText: string; + showExploreResultsButton: boolean; + data: Record<string, any>[]; + showSaveDatasetModal: boolean; + alertIsOpen: boolean; +} + const ResultlessStyles = styled.div` position: relative; - min-height: ${({ theme }) => theme.gridUnit * 25}px; + min-height: 100px; [role='alert'] { margin-top: ${({ theme }) => theme.gridUnit * 2}px; } @@ -99,8 +100,8 @@ const MonospaceDiv = styled.div` `; const ReturnedRows = styled.div` - font-size: ${({ theme }) => theme.typography.sizes.s}px; - line-height: ${({ theme }) => theme.gridUnit * 6}px; + font-size: 13px; + line-height: 24px; `; const ResultSetControls = styled.div` @@ -120,84 +121,115 @@ const LimitMessage = styled.span` margin-left: ${({ theme }) => theme.gridUnit * 2}px; `; -const ResultSet = ({ - cache = false, - csv = true, - database = {}, - displayLimit, - height, - query, - search = true, - showSql = false, - visualize = true, - user, - defaultQueryLimit, -}: ResultSetProps) => { - const [searchText, setSearchText] = useState(''); - const [cachedData, setCachedData] = useState<Record<string, unknown>[]>([]); - const [showSaveDatasetModal, setShowSaveDatasetModal] = useState(false); - const [alertIsOpen, setAlertIsOpen] = useState(false); - - const dispatch = useDispatch(); - - const reRunQueryIfSessionTimeoutErrorOnMount = useCallback(() => { - if ( - query.errorMessage && - query.errorMessage.indexOf('session timed out') > 0 - ) { - dispatch(reRunQuery(query)); - } - }, []); +export default class ResultSet extends React.PureComponent< + ResultSetProps, + ResultSetState +> { + static defaultProps = { + cache: false, + csv: true, + database: {}, + search: true, + showSql: false, + visualize: true, + }; - useEffect(() => { - // only do this the first time the component is rendered/mounted - reRunQueryIfSessionTimeoutErrorOnMount(); - }, [reRunQueryIfSessionTimeoutErrorOnMount]); + constructor(props: ResultSetProps) { + super(props); + this.state = { + searchText: '', + showExploreResultsButton: false, + data: [], + showSaveDatasetModal: false, + alertIsOpen: false, + }; + this.changeSearch = this.changeSearch.bind(this); + this.fetchResults = this.fetchResults.bind(this); + this.popSelectStar = this.popSelectStar.bind(this); + this.reFetchQueryResults = this.reFetchQueryResults.bind(this); + this.toggleExploreResultsButton = + this.toggleExploreResultsButton.bind(this); + } - const fetchResults = (query: QueryResponse) => { - dispatch(fetchQueryResults(query, displayLimit)); - }; + async componentDidMount() { + // only do this the first time the component is rendered/mounted + this.reRunQueryIfSessionTimeoutErrorOnMount(); + } - const prevQuery = usePrevious(query); - useEffect(() => { - if (cache && query.cached && query?.results?.data?.length > 0) { - setCachedData(query.results.data); - dispatch(clearQueryResults(query)); + UNSAFE_componentWillReceiveProps(nextProps: ResultSetProps) { + // when new results comes in, save them locally and clear in store + if ( + this.props.cache && + !nextProps.query.cached && + nextProps.query.results && + nextProps.query.results.data && + nextProps.query.results.data.length > 0 + ) { + this.setState({ data: nextProps.query.results.data }, () => + this.clearQueryResults(nextProps.query), + ); } if ( - query.resultsKey && - prevQuery?.resultsKey && - query.resultsKey !== prevQuery.resultsKey + nextProps.query.resultsKey && + nextProps.query.resultsKey !== this.props.query.resultsKey ) { - fetchResults(query); + this.fetchResults(nextProps.query); } - }, [query, cache]); + } - const calculateAlertRefHeight = (alertElement: HTMLElement | null) => { + calculateAlertRefHeight = (alertElement: HTMLElement | null) => { if (alertElement) { - setAlertIsOpen(true); + this.setState({ alertIsOpen: true }); } else { - setAlertIsOpen(false); + this.setState({ alertIsOpen: false }); } }; - const popSelectStar = (tempSchema: string | null, tempTable: string) => { + clearQueryResults(query: QueryResponse) { + this.props.actions.clearQueryResults(query); + } + + popSelectStar(tempSchema: string | null, tempTable: string) { const qe = { id: shortid.generate(), name: tempTable, autorun: false, - dbId: query.dbId, + dbId: this.props.query.dbId, sql: `SELECT * FROM ${tempSchema ? `${tempSchema}.` : ''}${tempTable}`, }; - dispatch(addQueryEditor(qe)); - }; + this.props.actions.addQueryEditor(qe); + } - const changeSearch = (event: React.ChangeEvent<HTMLInputElement>) => { - setSearchText(event.target.value); - }; + toggleExploreResultsButton() { + this.setState(prevState => ({ + showExploreResultsButton: !prevState.showExploreResultsButton, + })); + } + + changeSearch(event: React.ChangeEvent<HTMLInputElement>) { + this.setState({ searchText: event.target.value }); + } + + fetchResults(query: QueryResponse) { + this.props.actions.fetchQueryResults(query, this.props.displayLimit); + } - const createExploreResultsOnClick = async () => { - const { results } = query; + reFetchQueryResults(query: QueryResponse) { + this.props.actions.reFetchQueryResults(query); + } + + reRunQueryIfSessionTimeoutErrorOnMount() { + const { query } = this.props; + if ( + query.errorMessage && + query.errorMessage.indexOf('session timed out') > 0 + ) { + this.props.actions.reRunQuery(query); + } + } + + createExploreResultsOnClick = async () => { + const { results } = this.props.query; if (results?.query_id) { const key = await postFormData(results.query_id, 'query', { @@ -216,14 +248,16 @@ const ResultSet = ({ } }; - const renderControls = () => { - if (search || visualize || csv) { - let { data } = query.results; - if (cache && query.cached) { - data = cachedData; + renderControls() { + if (this.props.search || this.props.visualize || this.props.csv) { + let { data } = this.props.query.results; + if (this.props.cache && this.props.query.cached) { + ({ data } = this.state); } - const { columns } = query.results; + const { columns } = this.props.query.results; // Added compute logic to stop user from being able to Save & Explore + const { showSaveDatasetModal } = this.state; + const { query } = this.props; const datasource: ISaveableDatasource = { columns: query.results.columns as ISimpleColumn[], @@ -238,7 +272,7 @@ const ResultSet = ({ <ResultSetControls> <SaveDatasetModal visible={showSaveDatasetModal} - onHide={() => setShowSaveDatasetModal(false)} + onHide={() => this.setState({ showSaveDatasetModal: false })} buttonTextOnSave={t('Save & Explore')} buttonTextOnOverwrite={t('Overwrite & Explore')} modalDescription={t( @@ -247,13 +281,14 @@ const ResultSet = ({ datasource={datasource} /> <ResultSetButtons> - {visualize && database?.allows_virtual_table_explore && ( - <ExploreResultsButton - database={database} - onClick={createExploreResultsOnClick} - /> - )} - {csv && ( + {this.props.visualize && + this.props.database?.allows_virtual_table_explore && ( + <ExploreResultsButton + database={this.props.database} + onClick={this.createExploreResultsOnClick} + /> + )} + {this.props.csv && ( <Button buttonSize="small" href={`/superset/csv/${query.id}`}> <i className="fa fa-file-text-o" /> {t('Download to CSV')} </Button> @@ -270,11 +305,11 @@ const ResultSet = ({ hideTooltip /> </ResultSetButtons> - {search && ( + {this.props.search && ( <input type="text" - onChange={changeSearch} - value={searchText} + onChange={this.changeSearch} + value={this.state.searchText} className="form-control input-sm" disabled={columns.length > MAX_COLUMNS_FOR_TABLE} placeholder={ @@ -288,14 +323,14 @@ const ResultSet = ({ ); } return <div />; - }; + } - const renderRowsReturned = () => { - const { results, rows, queryLimit, limitingFactor } = query; + renderRowsReturned() { + const { results, rows, queryLimit, limitingFactor } = this.props.query; let limitMessage; const limitReached = results?.displayLimitReached; const limit = queryLimit || results.query.limit; - const isAdmin = !!user?.roles?.Admin; + const isAdmin = !!this.props.user?.roles?.Admin; const rowsCount = Math.min(rows || 0, results?.data?.length || 0); const displayMaxRowsReachedMessage = { @@ -313,10 +348,10 @@ const ResultSet = ({ ), }; const shouldUseDefaultDropdownAlert = - limit === defaultQueryLimit && + limit === this.props.defaultQueryLimit && limitingFactor === LIMITING_FACTOR.DROPDOWN; - if (limitingFactor === LIMITING_FACTOR.QUERY && csv) { + if (limitingFactor === LIMITING_FACTOR.QUERY && this.props.csv) { limitMessage = t( 'The number of rows displayed is limited to %(rows)d by the query', { rows }, @@ -351,11 +386,11 @@ const ResultSet = ({ </span> )} {!limitReached && shouldUseDefaultDropdownAlert && ( - <div ref={calculateAlertRefHeight}> + <div ref={this.calculateAlertRefHeight}> <Alert type="warning" message={t('%(rows)d rows returned', { rows })} - onClose={() => setAlertIsOpen(false)} + onClose={() => this.setState({ alertIsOpen: false })} description={t( 'The number of rows displayed is limited to %s by the dropdown.', rows, @@ -364,10 +399,10 @@ const ResultSet = ({ </div> )} {limitReached && ( - <div ref={calculateAlertRefHeight}> + <div ref={this.calculateAlertRefHeight}> <Alert type="warning" - onClose={() => setAlertIsOpen(false)} + onClose={() => this.setState({ alertIsOpen: false })} message={t('%(rows)d rows returned', { rows: rowsCount })} description={ isAdmin @@ -379,191 +414,193 @@ const ResultSet = ({ )} </ReturnedRows> ); - }; - - const limitReached = query?.results?.displayLimitReached; - let sql; - let exploreDBId = query.dbId; - if (database?.explore_database_id) { - exploreDBId = database.explore_database_id; } - let trackingUrl; - if ( - query.trackingUrl && - query.state !== 'success' && - query.state !== 'fetching' - ) { - trackingUrl = ( - <Button - className="sql-result-track-job" - buttonSize="small" - href={query.trackingUrl} - target="_blank" - > - {query.state === 'running' ? t('Track job') : t('See query details')} - </Button> - ); - } - - if (showSql) { - sql = <HighlightedSql sql={query.sql} />; - } - - if (query.state === 'stopped') { - return <Alert type="warning" message={t('Query was stopped')} />; - } + render() { + const { query } = this.props; + const limitReached = query?.results?.displayLimitReached; + let sql; + let exploreDBId = query.dbId; + if (this.props.database && this.props.database.explore_database_id) { + exploreDBId = this.props.database.explore_database_id; + } + let trackingUrl; + if ( + query.trackingUrl && + query.state !== 'success' && + query.state !== 'fetching' + ) { + trackingUrl = ( + <Button + className="sql-result-track-job" + buttonSize="small" + href={query.trackingUrl} + target="_blank" + > + {query.state === 'running' ? t('Track job') : t('See query details')} + </Button> + ); + } - if (query.state === 'failed') { - return ( - <ResultlessStyles> - <ErrorMessageWithStackTrace - title={t('Database error')} - error={query?.errors?.[0]} - subtitle={<MonospaceDiv>{query.errorMessage}</MonospaceDiv>} - copyText={query.errorMessage || undefined} - link={query.link} - source="sqllab" - /> - {trackingUrl} - </ResultlessStyles> - ); - } + if (this.props.showSql) sql = <HighlightedSql sql={query.sql} />; - if (query.state === 'success' && query.ctas) { - const { tempSchema, tempTable } = query; - let object = 'Table'; - if (query.ctas_method === CtasEnum.VIEW) { - object = 'View'; + if (query.state === 'stopped') { + return <Alert type="warning" message={t('Query was stopped')} />; } - return ( - <div> - <Alert - type="info" - message={ - <> - {t(object)} [ - <strong> - {tempSchema ? `${tempSchema}.` : ''} - {tempTable} - </strong> - ] {t('was created')} - <ButtonGroup> - <Button - buttonSize="small" - className="m-r-5" - onClick={() => popSelectStar(tempSchema, tempTable)} - > - {t('Query in a new tab')} - </Button> - <ExploreCtasResultsButton - table={tempTable} - schema={tempSchema} - dbId={exploreDBId} - /> - </ButtonGroup> - </> - } - /> - </div> - ); - } - - if (query.state === 'success' && query.results) { - const { results } = query; - // Accounts for offset needed for height of ResultSetRowsReturned component if !limitReached - const rowMessageHeight = !limitReached ? 32 : 0; - // Accounts for offset needed for height of Alert if this.state.alertIsOpen - const alertContainerHeight = 70; - // We need to calculate the height of this.renderRowsReturned() - // if we want results panel to be proper height because the - // FilterTable component needs an explicit height to render - // react-virtualized Table component - const rowsHeight = alertIsOpen - ? height - alertContainerHeight - : height - rowMessageHeight; - let data; - if (cache && query.cached) { - data = cachedData; - } else if (results?.data) { - ({ data } = results); + if (query.state === 'failed') { + return ( + <ResultlessStyles> + <ErrorMessageWithStackTrace + title={t('Database error')} + error={query?.errors?.[0]} + subtitle={<MonospaceDiv>{query.errorMessage}</MonospaceDiv>} + copyText={query.errorMessage || undefined} + link={query.link} + source="sqllab" + /> + {trackingUrl} + </ResultlessStyles> + ); } - if (data && data.length > 0) { - const expandedColumns = results.expanded_columns - ? results.expanded_columns.map(col => col.name) - : []; + if (query.state === 'success' && query.ctas) { + const { tempSchema, tempTable } = query; + let object = 'Table'; + if (query.ctas_method === CtasEnum.VIEW) { + object = 'View'; + } return ( - <> - {renderControls()} - {renderRowsReturned()} - {sql} - <FilterableTable - data={data} - orderedColumnKeys={results.columns.map(col => col.name)} - height={rowsHeight} - filterText={searchText} - expandedColumns={expandedColumns} + <div> + <Alert + type="info" + message={ + <> + {t(object)} [ + <strong> + {tempSchema ? `${tempSchema}.` : ''} + {tempTable} + </strong> + ] {t('was created')} + <ButtonGroup> + <Button + buttonSize="small" + className="m-r-5" + onClick={() => this.popSelectStar(tempSchema, tempTable)} + > + {t('Query in a new tab')} + </Button> + <ExploreCtasResultsButton + // @ts-ignore Redux types are difficult to work with, ignoring for now + actions={this.props.actions} + table={tempTable} + schema={tempSchema} + dbId={exploreDBId} + /> + </ButtonGroup> + </> + } /> - </> + </div> ); } - if (data && data.length === 0) { - return <Alert type="warning" message={t('The query returned no data')} />; + if (query.state === 'success' && query.results) { + const { results } = query; + // Accounts for offset needed for height of ResultSetRowsReturned component if !limitReached + const rowMessageHeight = !limitReached ? 32 : 0; + // Accounts for offset needed for height of Alert if this.state.alertIsOpen + const alertContainerHeight = 70; + // We need to calculate the height of this.renderRowsReturned() + // if we want results panel to be propper height because the + // FilterTable component nedds an explcit height to render + // react-virtualized Table component + const height = this.state.alertIsOpen + ? this.props.height - alertContainerHeight + : this.props.height - rowMessageHeight; + let data; + if (this.props.cache && query.cached) { + ({ data } = this.state); + } else if (results && results.data) { + ({ data } = results); + } + if (data && data.length > 0) { + const expandedColumns = results.expanded_columns + ? results.expanded_columns.map(col => col.name) + : []; + return ( + <> + {this.renderControls()} + {this.renderRowsReturned()} + {sql} + <FilterableTable + data={data} + orderedColumnKeys={results.columns.map(col => col.name)} + height={height} + filterText={this.state.searchText} + expandedColumns={expandedColumns} + /> + </> + ); + } + if (data && data.length === 0) { + return ( + <Alert type="warning" message={t('The query returned no data')} /> + ); + } } - } - - if (query.cached || (query.state === 'success' && !query.results)) { - if (query.isDataPreview) { - return ( - <Button - buttonSize="small" - buttonStyle="primary" - onClick={() => - dispatch( - reFetchQueryResults({ + if (query.cached || (query.state === 'success' && !query.results)) { + if (query.isDataPreview) { + return ( + <Button + buttonSize="small" + buttonStyle="primary" + onClick={() => + this.reFetchQueryResults({ ...query, isDataPreview: true, - }), - ) - } - > - {t('Fetch data preview')} - </Button> - ); + }) + } + > + {t('Fetch data preview')} + </Button> + ); + } + if (query.resultsKey) { + return ( + <Button + buttonSize="small" + buttonStyle="primary" + onClick={() => this.fetchResults(query)} + > + {t('Refetch results')} + </Button> + ); + } } - if (query.resultsKey) { - return ( - <Button - buttonSize="small" - buttonStyle="primary" - onClick={() => fetchResults(query)} - > - {t('Refetch results')} - </Button> + let progressBar; + if (query.progress > 0) { + progressBar = ( + <ProgressBar + percent={parseInt(query.progress.toFixed(0), 10)} + striped + /> ); } - } + const progressMsg = + query && query.extra && query.extra.progress + ? query.extra.progress + : null; - let progressBar; - if (query.progress > 0) { - progressBar = ( - <ProgressBar percent={parseInt(query.progress.toFixed(0), 10)} striped /> + return ( + <ResultlessStyles> + <div>{!progressBar && <Loading position="normal" />}</div> + {/* show loading bar whenever progress bar is completed but needs time to render */} + <div>{query.progress === 100 && <Loading position="normal" />}</div> + <QueryStateLabel query={query} /> + <div> + {progressMsg && <Alert type="success" message={progressMsg} />} + </div> + <div>{query.progress !== 100 && progressBar}</div> + {trackingUrl && <div>{trackingUrl}</div>} + </ResultlessStyles> ); } - - const progressMsg = query?.extra?.progress ?? null; - - return ( - <ResultlessStyles> - <div>{!progressBar && <Loading position="normal" />}</div> - {/* show loading bar whenever progress bar is completed but needs time to render */} - <div>{query.progress === 100 && <Loading position="normal" />}</div> - <QueryStateLabel query={query} /> - <div>{progressMsg && <Alert type="success" message={progressMsg} />}</div> - <div>{query.progress !== 100 && progressBar}</div> - {trackingUrl && <div>{trackingUrl}</div>} - </ResultlessStyles> - ); -}; - -export default ResultSet; +} diff --git a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx index 2be88f6fe2..ddcd972f98 100644 --- a/superset-frontend/src/SqlLab/components/SouthPane/index.tsx +++ b/superset-frontend/src/SqlLab/components/SouthPane/index.tsx @@ -164,8 +164,10 @@ export default function SouthPane({ if (Date.now() - latestQuery.startDttm <= LOCALSTORAGE_MAX_QUERY_AGE_MS) { results = ( <ResultSet + showControls search query={latestQuery} + actions={actions} user={user} height={innerTabContentHeight + EXTRA_HEIGHT_RESULTS} database={databases[latestQuery.dbId]} @@ -197,6 +199,7 @@ export default function SouthPane({ query={query} visualize={false} csv={false} + actions={actions} cache user={user} height={innerTabContentHeight} diff --git a/superset-frontend/src/components/FilterableTable/index.tsx b/superset-frontend/src/components/FilterableTable/index.tsx index 16ae37e671..8324e93b90 100644 --- a/superset-frontend/src/components/FilterableTable/index.tsx +++ b/superset-frontend/src/components/FilterableTable/index.tsx @@ -578,7 +578,7 @@ const FilterableTable = ({ // fix height of filterable table return ( - <StyledFilterableTable data-test="grid-container"> + <StyledFilterableTable> <ScrollSync> {({ onScroll, scrollLeft }) => ( <> @@ -659,7 +659,6 @@ const FilterableTable = ({ return ( <StyledFilterableTable className="filterable-table-container" - data-test="table-container" ref={container} > {fitted && ( diff --git a/superset-frontend/src/components/ProgressBar/index.tsx b/superset-frontend/src/components/ProgressBar/index.tsx index ba69fc90c6..93b4315e52 100644 --- a/superset-frontend/src/components/ProgressBar/index.tsx +++ b/superset-frontend/src/components/ProgressBar/index.tsx @@ -27,7 +27,7 @@ export interface ProgressBarProps extends ProgressProps { // eslint-disable-next-line @typescript-eslint/no-unused-vars const ProgressBar = styled(({ striped, ...props }: ProgressBarProps) => ( - <AntdProgress data-test="progress-bar" {...props} /> + <AntdProgress {...props} /> ))` line-height: 0; position: static;
