Skip to content

Commit

Permalink
fix(dashboard): Chart stuck in loading state when when datasets reque…
Browse files Browse the repository at this point in the history
…st and chart request fail (apache#19327)
  • Loading branch information
kgabryje authored and philipher29 committed Jun 9, 2022
1 parent baa8403 commit 6b8a6fd
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 15 deletions.
6 changes: 5 additions & 1 deletion superset-frontend/src/components/Chart/Chart.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import ErrorBoundary from 'src/components/ErrorBoundary';
import { LOG_ACTIONS_RENDER_CHART, Logger } from 'src/logger/LogUtils';
import { URL_PARAMS } from 'src/constants';
import { getUrlParam } from 'src/utils/urlUtils';
import { ResourceStatus } from 'src/hooks/apiResources/apiResources';
import ChartRenderer from './ChartRenderer';
import { ChartErrorMessage } from './ChartErrorMessage';

Expand Down Expand Up @@ -72,6 +73,7 @@ const propTypes = {
onFilterMenuClose: PropTypes.func,
ownState: PropTypes.object,
postTransformProps: PropTypes.func,
datasetsStatus: PropTypes.oneOf(['loading', 'error', 'complete']),
};

const BLANK = {};
Expand Down Expand Up @@ -207,6 +209,7 @@ class Chart extends React.PureComponent {
datasource,
dashboardId,
height,
datasetsStatus,
} = this.props;

const error = queryResponse?.errors?.[0];
Expand All @@ -216,7 +219,8 @@ class Chart extends React.PureComponent {
if (
chartAlert !== undefined &&
chartAlert !== NONEXISTENT_DATASET &&
datasource === PLACEHOLDER_DATASOURCE
datasource === PLACEHOLDER_DATASOURCE &&
datasetsStatus !== ResourceStatus.ERROR
) {
return (
<Styles
Expand Down
8 changes: 8 additions & 0 deletions superset-frontend/src/dashboard/actions/dashboardState.js
Original file line number Diff line number Diff line change
Expand Up @@ -638,3 +638,11 @@ export function maxUndoHistoryToast() {
);
};
}

export const SET_DATASETS_STATUS = 'SET_DATASETS_STATUS';
export function setDatasetsStatus(status) {
return {
type: SET_DATASETS_STATUS,
status,
};
}
2 changes: 2 additions & 0 deletions superset-frontend/src/dashboard/actions/hydrate.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import { TIME_RANGE } from 'src/visualizations/FilterBox/FilterBox';
import { URL_PARAMS } from 'src/constants';
import { getUrlParam } from 'src/utils/urlUtils';
import { FILTER_BOX_MIGRATION_STATES } from 'src/explore/constants';
import { ResourceStatus } from 'src/hooks/apiResources/apiResources';
import { FeatureFlag, isFeatureEnabled } from '../../featureFlags';
import extractUrlParams from '../util/extractUrlParams';
import getNativeFilterConfig from '../util/filterboxMigrationHelper';
Expand Down Expand Up @@ -400,6 +401,7 @@ export const hydrateDashboard =
isFiltersRefreshing: false,
activeTabs: dashboardState?.activeTabs || [],
filterboxMigrationState,
datasetsStatus: ResourceStatus.LOADING,
},
dashboardLayout,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ const propTypes = {
ownState: PropTypes.object,
filterState: PropTypes.object,
postTransformProps: PropTypes.func,
datasetsStatus: PropTypes.oneOf(['loading', 'error', 'complete']),
};

const defaultProps = {
Expand Down Expand Up @@ -338,6 +339,7 @@ export default class Chart extends React.Component {
isFullSize,
filterboxMigrationState,
postTransformProps,
datasetsStatus,
} = this.props;

const { width } = this.state;
Expand Down Expand Up @@ -463,6 +465,7 @@ export default class Chart extends React.Component {
isDeactivatedViz={isDeactivatedViz}
filterboxMigrationState={filterboxMigrationState}
postTransformProps={postTransformProps}
datasetsStatus={datasetsStatus}
/>
</div>
</div>
Expand Down
3 changes: 2 additions & 1 deletion superset-frontend/src/dashboard/containers/Chart.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ function mapStateToProps(
const datasource =
(chart && chart.form_data && datasources[chart.form_data.datasource]) ||
PLACEHOLDER_DATASOURCE;
const { colorScheme, colorNamespace } = dashboardState;
const { colorScheme, colorNamespace, datasetsStatus } = dashboardState;
const labelColors = dashboardInfo?.metadata?.label_colors || {};
const sharedLabelColors = dashboardInfo?.metadata?.shared_label_colors || {};
// note: this method caches filters if possible to prevent render cascades
Expand Down Expand Up @@ -101,6 +101,7 @@ function mapStateToProps(
filterState: dataMask[id]?.filterState,
maxRows: common.conf.SQL_MAX_ROW,
filterboxMigrationState: dashboardState.filterboxMigrationState,
datasetsStatus,
};
}

Expand Down
12 changes: 10 additions & 2 deletions superset-frontend/src/dashboard/containers/DashboardPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import { URL_PARAMS } from 'src/constants';
import { getUrlParam } from 'src/utils/urlUtils';
import { canUserEditDashboard } from 'src/dashboard/util/findPermission';
import { getFilterSets } from '../actions/nativeFilters';
import { setDatasetsStatus } from '../actions/dashboardState';
import {
getFilterValue,
getPermalinkValue,
Expand Down Expand Up @@ -90,8 +91,11 @@ const DashboardPage: FC = () => {
useDashboard(idOrSlug);
const { result: charts, error: chartsApiError } =
useDashboardCharts(idOrSlug);
const { result: datasets, error: datasetsApiError } =
useDashboardDatasets(idOrSlug);
const {
result: datasets,
error: datasetsApiError,
status,
} = useDashboardDatasets(idOrSlug);
const isDashboardHydrated = useRef(false);

const error = dashboardApiError || chartsApiError;
Expand All @@ -107,6 +111,10 @@ const DashboardPage: FC = () => {
migrationStateParam || FILTER_BOX_MIGRATION_STATES.NOOP,
);

useEffect(() => {
dispatch(setDatasetsStatus(status));
}, [dispatch, status]);

useEffect(() => {
// should convert filter_box to filter component?
const hasFilterBox =
Expand Down
29 changes: 18 additions & 11 deletions superset-frontend/src/dashboard/reducers/dashboardState.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,28 +20,29 @@
import {
ADD_SLICE,
ON_CHANGE,
ON_FILTERS_REFRESH,
ON_FILTERS_REFRESH_SUCCESS,
ON_REFRESH,
ON_REFRESH_SUCCESS,
ON_SAVE,
REMOVE_SLICE,
RESET_SLICE,
SET_ACTIVE_TABS,
SET_COLOR_SCHEME,
SET_DIRECT_PATH,
SET_EDIT_MODE,
SET_FOCUSED_FILTER_FIELD,
SET_FULL_SIZE_CHART_ID,
SET_MAX_UNDO_HISTORY_EXCEEDED,
SET_REFRESH_FREQUENCY,
SET_UNSAVED_CHANGES,
SHOW_BUILDER_PANE,
TOGGLE_EXPAND_SLICE,
TOGGLE_FAVE_STAR,
TOGGLE_PUBLISHED,
UNSET_FOCUSED_FILTER_FIELD,
UPDATE_CSS,
SET_REFRESH_FREQUENCY,
ON_REFRESH,
ON_REFRESH_SUCCESS,
SET_DIRECT_PATH,
SET_FOCUSED_FILTER_FIELD,
UNSET_FOCUSED_FILTER_FIELD,
SET_ACTIVE_TABS,
SET_FULL_SIZE_CHART_ID,
RESET_SLICE,
ON_FILTERS_REFRESH,
ON_FILTERS_REFRESH_SUCCESS,
SET_DATASETS_STATUS,
} from '../actions/dashboardState';
import { HYDRATE_DASHBOARD } from '../actions/hydrate';

Expand Down Expand Up @@ -212,6 +213,12 @@ export default function dashboardStateReducer(state = {}, action) {
fullSizeChartId: action.chartId,
};
},
[SET_DATASETS_STATUS]() {
return {
...state,
datasetsStatus: action.status,
};
},
};

if (action.type in actionHandlers) {
Expand Down

0 comments on commit 6b8a6fd

Please sign in to comment.