From 3d5ae6226bc5035fc86d3d3aa2cbc0202bede363 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Wed, 1 Jun 2022 20:37:19 +0800 Subject: [PATCH] fix: dashbaord unable to refresh (#20220) --- .../src/components/Chart/chartAction.js | 1 + .../src/components/Chart/chartActions.test.js | 43 ++++++++++++------- .../dashboard/components/Dashboard.test.jsx | 1 - .../DataTablesPane/DataTablesPane.test.tsx | 7 +-- .../DataTablesPane/DataTablesPane.tsx | 11 +++-- .../DataTablesPane/components/ResultsPane.tsx | 16 ++++--- .../components/DataTablesPane/types.ts | 2 + .../ExploreChartHeader.test.tsx | 1 - .../explore/components/ExploreChartPanel.jsx | 1 + .../components/ExploreViewContainer/index.jsx | 1 - 10 files changed, 54 insertions(+), 30 deletions(-) diff --git a/superset-frontend/src/components/Chart/chartAction.js b/superset-frontend/src/components/Chart/chartAction.js index 4ccf6a23cda1f..d52ac79177da0 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 08840fd6db6ae..7c7af00a4b2e4 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 logEvent async action', () => { + it('should dispatch UPDATE_QUERY_FORM_DATA action with the query', () => { 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[2][0].type).toBe(actions.UPDATE_QUERY_FORM_DATA); + }); + }); - dispatch.args[2][0](dispatch); + it('should dispatch logEvent async action', () => { + const actionThunk = actions.postChartFormData({}); + return actionThunk(dispatch).then(() => { + // 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 c76e1e3518219..a881d0cdadf93 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 16445a2e3ed02..76d4b6951d8ca 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 b334a980b52b1..a71a1232efbf9 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 aa71326aa4b71..c17ae06e407f8 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 b76796bfde97c..540a444ab0473 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 ff8a46b3c5149..9fc7caef62803 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 1bef9da0d2762..bf8a17a3650cc 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]);