Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: dashbaord unable to refresh #20220

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions superset-frontend/src/components/Chart/chartAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,7 @@ export function exploreJSON(
return Promise.all([
chartDataRequestCaught,
dispatch(triggerQuery(false, key)),
dispatch(updateQueryFormData(formData, key)),
...annotationLayers.map(annotation =>
dispatch(
runAnnotationQuery({
Expand Down
43 changes: 27 additions & 16 deletions superset-frontend/src/components/Chart/chartActions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand All @@ -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);
});
});

Expand All @@ -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();
});
});
Expand All @@ -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');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ describe('Dashboard', () => {
removeSliceFromDashboard() {},
triggerQuery() {},
logEvent() {},
updateQueryFormData() {},
},
initMessages: [],
dashboardState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => ({
Expand Down Expand Up @@ -57,7 +58,7 @@ const createProps = () => ({
extra_form_data: {},
},
queryForce: false,
chartStatus: 'rendered',
chartStatus: 'rendered' as ChartStatus,
onCollapseChange: jest.fn(),
queriesResponse: [
{
Expand Down Expand Up @@ -150,7 +151,7 @@ describe('DataTablesPane', () => {
<DataTablesPane
{...{
...props,
chartStatus: 'success',
chartStatus: 'rendered',
queriesResponse: [
{
colnames: ['__timestamp', 'genre'],
Expand Down Expand Up @@ -202,7 +203,7 @@ describe('DataTablesPane', () => {
<DataTablesPane
{...{
...props,
chartStatus: 'success',
chartStatus: 'rendered',
queriesResponse: [
{
colnames: ['__timestamp', 'genre'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,15 @@ export const DataTablesPane = ({
datasource,
queryForce,
onCollapseChange,
chartStatus,
ownState,
errorMessage,
actions,
}: DataTablesPaneProps) => {
const theme = useTheme();
const [activeTabKey, setActiveTabKey] = useState<string>(ResultTypes.Results);
const [isRequest, setIsRequest] = useState<Record<ResultTypes, boolean>>({
results: getItem(LocalStorageKeys.is_datapanel_open, false),
results: false,
samples: false,
});
const [panelOpen, setPanelOpen] = useState(
Expand All @@ -111,7 +112,11 @@ export const DataTablesPane = ({
});
}

if (panelOpen && activeTabKey === ResultTypes.Results) {
if (
panelOpen &&
activeTabKey === ResultTypes.Results &&
chartStatus === 'rendered'
) {
setIsRequest({
results: true,
samples: false,
Expand All @@ -124,7 +129,7 @@ export const DataTablesPane = ({
samples: true,
});
}
}, [panelOpen, activeTabKey]);
}, [panelOpen, activeTabKey, chartStatus]);

const handleCollapseChange = useCallback(
(isOpen: boolean) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const ResultsPane = ({
const [data, setData] = useState<Record<string, any>[][]>([]);
const [colnames, setColnames] = useState<string[]>([]);
const [coltypes, setColtypes] = useState<GenericDataType[]>([]);
const [isLoading, setIsLoading] = useState<boolean>(false);
const [isLoading, setIsLoading] = useState<boolean>(true);
const [responseError, setResponseError] = useState<string>('');

useEffect(() => {
Expand Down Expand Up @@ -105,6 +105,12 @@ export const ResultsPane = ({
}
}, [queryFormData, isRequest]);

useEffect(() => {
if (errorMessage) {
setIsLoading(false);
}
}, [errorMessage]);

const originalFormattedTimeColumns = useOriginalFormattedTimeColumns(
queryFormData.datasource,
);
Expand All @@ -119,15 +125,15 @@ export const ResultsPane = ({
);
const filteredData = useFilteredTableData(filterText, data);

if (isLoading) {
return <Loading />;
}

if (errorMessage) {
const title = t('Run a query to display results');
return <EmptyStateMedium image="document.svg" title={title} />;
}

if (isLoading) {
return <Loading />;
}

if (responseError) {
return (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ const createProps = () => ({
fetchFaveStar: jest.fn(),
saveFaveStar: jest.fn(),
redirectSQLLab: jest.fn(),
updateQueryFormData: jest.fn(),
},
user: {
userId: 1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ const ExploreChartPanel = ({
datasource={datasource}
queryForce={force}
onCollapseChange={onCollapseChange}
chartStatus={chart.chartStatus}
errorMessage={errorMessage}
actions={actions}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down