diff --git a/superset-frontend/src/components/Chart/chartAction.js b/superset-frontend/src/components/Chart/chartAction.js index 4ccf6a23cd..d52ac79177 100644 --- a/superset-frontend/src/components/Chart/chartAction.js +++ b/superset-frontend/src/components/Chart/chartAction.js @@ -481,6 +481,7 @@ export function exploreJSON( return Promise.all([ chartDataRequestCaught, dispatch(triggerQuery(false, key)), + dispatch(updateQueryFormData(formData, key)), ...annotationLayers.map(annotation => dispatch( runAnnotationQuery({ diff --git a/superset-frontend/src/components/Chart/chartActions.test.js b/superset-frontend/src/components/Chart/chartActions.test.js index 08840fd6db..7c7af00a4b 100644 --- a/superset-frontend/src/components/Chart/chartActions.test.js +++ b/superset-frontend/src/components/Chart/chartActions.test.js @@ -105,8 +105,8 @@ describe('chart actions', () => { const actionThunk = actions.postChartFormData({}); return actionThunk(dispatch).then(() => { - // chart update, trigger query, success - expect(dispatch.callCount).toBe(4); + // chart update, trigger query, update form data, success + expect(dispatch.callCount).toBe(5); expect(fetchMock.calls(MOCK_URL)).toHaveLength(1); expect(dispatch.args[0][0].type).toBe(actions.CHART_UPDATE_STARTED); }); @@ -116,32 +116,43 @@ describe('chart actions', () => { const actionThunk = actions.postChartFormData({}); return actionThunk(dispatch).then(() => { // chart update, trigger query, update form data, success - expect(dispatch.callCount).toBe(4); + expect(dispatch.callCount).toBe(5); expect(fetchMock.calls(MOCK_URL)).toHaveLength(1); expect(dispatch.args[1][0].type).toBe(actions.TRIGGER_QUERY); }); }); + it('should dispatch UPDATE_QUERY_FORM_DATA action with the query', () => { + const actionThunk = actions.postChartFormData({}); + return actionThunk(dispatch).then(() => { + // chart update, trigger query, update form data, success + expect(dispatch.callCount).toBe(5); + expect(fetchMock.calls(MOCK_URL)).toHaveLength(1); + expect(dispatch.args[2][0].type).toBe(actions.UPDATE_QUERY_FORM_DATA); + }); + }); + it('should dispatch logEvent async action', () => { const actionThunk = actions.postChartFormData({}); return actionThunk(dispatch).then(() => { - // chart update, trigger query, success - expect(dispatch.callCount).toBe(4); - expect(fetchMock.calls(MOCK_URL)).toHaveLength(1); - - dispatch.args[2][0](dispatch); + // chart update, trigger query, update form data, success expect(dispatch.callCount).toBe(5); - expect(dispatch.args[4][0].type).toBe(LOG_EVENT); + expect(fetchMock.calls(MOCK_URL)).toHaveLength(1); + expect(typeof dispatch.args[3][0]).toBe('function'); + + dispatch.args[3][0](dispatch); + expect(dispatch.callCount).toBe(6); + expect(dispatch.args[5][0].type).toBe(LOG_EVENT); }); }); it('should dispatch CHART_UPDATE_SUCCEEDED action upon success', () => { const actionThunk = actions.postChartFormData({}); return actionThunk(dispatch).then(() => { - // chart update, trigger query, success - expect(dispatch.callCount).toBe(4); + // chart update, trigger query, update form data, success + expect(dispatch.callCount).toBe(5); expect(fetchMock.calls(MOCK_URL)).toHaveLength(1); - expect(dispatch.args[3][0].type).toBe(actions.CHART_UPDATE_SUCCEEDED); + expect(dispatch.args[4][0].type).toBe(actions.CHART_UPDATE_SUCCEEDED); }); }); @@ -157,8 +168,8 @@ describe('chart actions', () => { return actionThunk(dispatch).then(() => { // chart update, trigger query, update form data, fail expect(fetchMock.calls(MOCK_URL)).toHaveLength(1); - expect(dispatch.callCount).toBe(4); - expect(dispatch.args[3][0].type).toBe(actions.CHART_UPDATE_FAILED); + expect(dispatch.callCount).toBe(5); + expect(dispatch.args[4][0].type).toBe(actions.CHART_UPDATE_FAILED); setupDefaultFetchMock(); }); }); @@ -174,9 +185,9 @@ describe('chart actions', () => { const actionThunk = actions.postChartFormData({}, false, timeoutInSec); return actionThunk(dispatch).then(() => { - // chart update, trigger query, fail - expect(dispatch.callCount).toBe(4); - const updateFailedAction = dispatch.args[3][0]; + // chart update, trigger query, update form data, fail + expect(dispatch.callCount).toBe(5); + const updateFailedAction = dispatch.args[4][0]; expect(updateFailedAction.type).toBe(actions.CHART_UPDATE_FAILED); expect(updateFailedAction.queriesResponse[0].error).toBe('misc error'); diff --git a/superset-frontend/src/dashboard/components/Dashboard.test.jsx b/superset-frontend/src/dashboard/components/Dashboard.test.jsx index c76e1e3518..a881d0cdad 100644 --- a/superset-frontend/src/dashboard/components/Dashboard.test.jsx +++ b/superset-frontend/src/dashboard/components/Dashboard.test.jsx @@ -48,7 +48,6 @@ describe('Dashboard', () => { removeSliceFromDashboard() {}, triggerQuery() {}, logEvent() {}, - updateQueryFormData() {}, }, initMessages: [], dashboardState, diff --git a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx index 16445a2e3e..76d4b6951d 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx @@ -28,6 +28,7 @@ import { } from 'spec/helpers/testing-library'; import { DatasourceType } from '@superset-ui/core'; import { exploreActions } from 'src/explore/actions/exploreActions'; +import { ChartStatus } from 'src/explore/types'; import { DataTablesPane } from '.'; const createProps = () => ({ @@ -57,7 +58,7 @@ const createProps = () => ({ extra_form_data: {}, }, queryForce: false, - chartStatus: 'rendered', + chartStatus: 'rendered' as ChartStatus, onCollapseChange: jest.fn(), queriesResponse: [ { @@ -150,7 +151,7 @@ describe('DataTablesPane', () => { { (ResultTypes.Results); const [isRequest, setIsRequest] = useState>({ - results: getItem(LocalStorageKeys.is_datapanel_open, false), + results: false, samples: false, }); const [panelOpen, setPanelOpen] = useState( @@ -111,7 +112,11 @@ export const DataTablesPane = ({ }); } - if (panelOpen && activeTabKey === ResultTypes.Results) { + if ( + panelOpen && + activeTabKey === ResultTypes.Results && + chartStatus === 'rendered' + ) { setIsRequest({ results: true, samples: false, @@ -124,7 +129,7 @@ export const DataTablesPane = ({ samples: true, }); } - }, [panelOpen, activeTabKey]); + }, [panelOpen, activeTabKey, chartStatus]); const handleCollapseChange = useCallback( (isOpen: boolean) => { diff --git a/superset-frontend/src/explore/components/DataTablesPane/components/ResultsPane.tsx b/superset-frontend/src/explore/components/DataTablesPane/components/ResultsPane.tsx index b334a980b5..a71a1232ef 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/components/ResultsPane.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/components/ResultsPane.tsx @@ -50,7 +50,7 @@ export const ResultsPane = ({ const [data, setData] = useState[][]>([]); const [colnames, setColnames] = useState([]); const [coltypes, setColtypes] = useState([]); - const [isLoading, setIsLoading] = useState(false); + const [isLoading, setIsLoading] = useState(true); const [responseError, setResponseError] = useState(''); useEffect(() => { @@ -105,6 +105,12 @@ export const ResultsPane = ({ } }, [queryFormData, isRequest]); + useEffect(() => { + if (errorMessage) { + setIsLoading(false); + } + }, [errorMessage]); + const originalFormattedTimeColumns = useOriginalFormattedTimeColumns( queryFormData.datasource, ); @@ -119,15 +125,15 @@ export const ResultsPane = ({ ); const filteredData = useFilteredTableData(filterText, data); + if (isLoading) { + return ; + } + if (errorMessage) { const title = t('Run a query to display results'); return ; } - if (isLoading) { - return ; - } - if (responseError) { return ( <> diff --git a/superset-frontend/src/explore/components/DataTablesPane/types.ts b/superset-frontend/src/explore/components/DataTablesPane/types.ts index aa71326aa4..c17ae06e40 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/types.ts +++ b/superset-frontend/src/explore/components/DataTablesPane/types.ts @@ -18,12 +18,14 @@ */ import { Datasource, JsonObject, QueryFormData } from '@superset-ui/core'; import { ExploreActions } from 'src/explore/actions/exploreActions'; +import { ChartStatus } from 'src/explore/types'; export interface DataTablesPaneProps { queryFormData: QueryFormData; datasource: Datasource; queryForce: boolean; ownState?: JsonObject; + chartStatus: ChartStatus; onCollapseChange: (isOpen: boolean) => void; errorMessage?: JSX.Element; actions: ExploreActions; diff --git a/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx b/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx index b76796bfde..540a444ab0 100644 --- a/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx +++ b/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx @@ -97,7 +97,6 @@ const createProps = () => ({ fetchFaveStar: jest.fn(), saveFaveStar: jest.fn(), redirectSQLLab: jest.fn(), - updateQueryFormData: jest.fn(), }, user: { userId: 1, diff --git a/superset-frontend/src/explore/components/ExploreChartPanel.jsx b/superset-frontend/src/explore/components/ExploreChartPanel.jsx index ff8a46b3c5..9fc7caef62 100644 --- a/superset-frontend/src/explore/components/ExploreChartPanel.jsx +++ b/superset-frontend/src/explore/components/ExploreChartPanel.jsx @@ -392,6 +392,7 @@ const ExploreChartPanel = ({ datasource={datasource} queryForce={force} onCollapseChange={onCollapseChange} + chartStatus={chart.chartStatus} errorMessage={errorMessage} actions={actions} /> diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx index 1bef9da0d2..bf8a17a365 100644 --- a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx +++ b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx @@ -267,7 +267,6 @@ function ExploreViewContainer(props) { const onQuery = useCallback(() => { props.actions.setForceQuery(false); props.actions.triggerQuery(true, props.chart.id); - props.actions.updateQueryFormData(props.form_data, props.chart.id); addHistory(); setLastQueriedControls(props.controls); }, [props.controls, addHistory, props.actions, props.chart.id]);