Skip to content

Commit

Permalink
Fixed error handling in Report modal (#17988)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lyndsiWilliams
lyndsiWilliams authored Jan 10, 2022
1 parent c139312 commit f7add54
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 49 deletions.
75 changes: 43 additions & 32 deletions superset-frontend/src/components/ReportModal/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import React, {
FunctionComponent,
} from 'react';
import { t, SupersetTheme } from '@superset-ui/core';
import { getClientErrorObject } from 'src/utils/getClientErrorObject';
import { bindActionCreators } from 'redux';
import { connect, useDispatch, useSelector } from 'react-redux';
import { addReport, editReport } from 'src/reports/actions/reports';
Expand Down Expand Up @@ -72,6 +73,7 @@ export interface ReportObject {
working_timeout: number;
creation_method: string;
force_screenshot: boolean;
error?: string;
}

interface ChartObject {
Expand All @@ -88,8 +90,6 @@ interface ChartObject {
}

interface ReportProps {
addDangerToast: (msg: string) => void;
addSuccessToast: (msg: string) => void;
addReport: (report?: ReportObject) => {};
onHide: () => {};
onReportAdd: (report?: ReportObject) => {};
Expand All @@ -111,6 +111,7 @@ enum ActionType {
inputChange,
fetched,
reset,
error,
}

type ReportActionType =
Expand All @@ -124,6 +125,12 @@ type ReportActionType =
}
| {
type: ActionType.reset;
}
| {
type: ActionType.error;
payload: {
name: string[];
};
};

const TEXT_BASED_VISUALIZATION_TYPES = [
Expand Down Expand Up @@ -161,6 +168,11 @@ const reportReducer = (
};
case ActionType.reset:
return { ...initialState };
case ActionType.error:
return {
...state,
error: action.payload.name[0],
};
default:
return state;
}
Expand All @@ -181,11 +193,10 @@ const ReportModal: FunctionComponent<ReportProps> = ({
const [currentReport, setCurrentReport] = useReducer<
Reducer<Partial<ReportObject> | null, ReportActionType>
>(reportReducer, null);
const onChange = useCallback((type: any, payload: any) => {
const onReducerChange = useCallback((type: any, payload: any) => {
setCurrentReport({ type, payload });
}, []);
const [error, setError] = useState<CronError>();
// const [isLoading, setLoading] = useState<boolean>(false);
const [cronError, setCronError] = useState<CronError>();
const dispatch = useDispatch();
// Report fetch logic
const reports = useSelector<any, AlertObject>(state => state.reports);
Expand All @@ -205,9 +216,7 @@ const ReportModal: FunctionComponent<ReportProps> = ({
});
}
}, [reports]);
const onClose = () => {
onHide();
};

const onSave = async () => {
// Create new Report
const newReportValues: Partial<ReportObject> = {
Expand Down Expand Up @@ -235,15 +244,19 @@ const ReportModal: FunctionComponent<ReportProps> = ({
await dispatch(
editReport(currentReport?.id, newReportValues as ReportObject),
);
onHide();
} else {
await dispatch(addReport(newReportValues as ReportObject));
try {
await dispatch(addReport(newReportValues as ReportObject));
onHide();
} catch (e) {
const parsedError = await getClientErrorObject(e);
const errorMessage = parsedError.message;
onReducerChange(ActionType.error, errorMessage);
}
}

if (onReportAdd) {
onReportAdd();
}

onClose();
if (onReportAdd) onReportAdd();
};

const wrappedTitle = (
Expand All @@ -257,7 +270,7 @@ const ReportModal: FunctionComponent<ReportProps> = ({

const renderModalFooter = (
<>
<StyledFooterButton key="back" onClick={onClose}>
<StyledFooterButton key="back" onClick={onHide}>
{t('Cancel')}
</StyledFooterButton>
<StyledFooterButton
Expand All @@ -279,7 +292,7 @@ const ReportModal: FunctionComponent<ReportProps> = ({
<div className="inline-container">
<StyledRadioGroup
onChange={(event: RadioChangeEvent) => {
onChange(ActionType.inputChange, {
onReducerChange(ActionType.inputChange, {
name: 'report_format',
value: event.target.value,
});
Expand All @@ -305,7 +318,7 @@ const ReportModal: FunctionComponent<ReportProps> = ({
return (
<StyledModal
show={show}
onHide={onClose}
onHide={onHide}
title={wrappedTitle}
footer={renderModalFooter}
width="432"
Expand All @@ -316,18 +329,16 @@ const ReportModal: FunctionComponent<ReportProps> = ({
id="name"
name="name"
value={currentReport?.name || ''}
placeholder="Weekly Report"
placeholder={t('Weekly Report')}
required
validationMethods={{
onChange: ({ target }: { target: HTMLInputElement }) =>
onChange(ActionType.inputChange, {
onReducerChange(ActionType.inputChange, {
name: target.name,
value: target.value,
}),
}}
errorMessage={
currentReport?.name === 'error' ? t('REPORT NAME ERROR') : ''
}
errorMessage={currentReport?.error || ''}
label="Report Name"
data-test="report-name-test"
/>
Expand All @@ -338,16 +349,16 @@ const ReportModal: FunctionComponent<ReportProps> = ({
value={currentReport?.description || ''}
validationMethods={{
onChange: ({ target }: { target: HTMLInputElement }) =>
onChange(ActionType.inputChange, {
onReducerChange(ActionType.inputChange, {
name: target.name,
value: target.value,
}),
}}
errorMessage={
currentReport?.description === 'error' ? t('DESCRIPTION ERROR') : ''
}
label="Description"
placeholder="Include a description that will be sent with your report"
errorMessage=""
label={t('Description')}
placeholder={t(
'Include a description that will be sent with your report',
)}
css={noBottomMargin}
data-test="report-description-test"
/>
Expand All @@ -363,16 +374,16 @@ const ReportModal: FunctionComponent<ReportProps> = ({

<StyledCronPicker
clearButton={false}
value={currentReport?.crontab || '0 12 * * 1'}
value={t(currentReport?.crontab || '0 12 * * 1')}
setValue={(newValue: string) => {
onChange(ActionType.inputChange, {
onReducerChange(ActionType.inputChange, {
name: 'crontab',
value: newValue,
});
}}
onError={setError}
onError={setCronError}
/>
<StyledCronError>{error}</StyledCronError>
<StyledCronError>{cronError}</StyledCronError>
<div
className="control-label"
css={(theme: SupersetTheme) => TimezoneHeaderStyle(theme)}
Expand Down
21 changes: 4 additions & 17 deletions superset-frontend/src/reports/actions/reports.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
/* eslint camelcase: 0 */
import { t, SupersetClient } from '@superset-ui/core';
import rison from 'rison';
import { getClientErrorObject } from 'src/utils/getClientErrorObject';
import {
addDangerToast,
addSuccessToast,
Expand Down Expand Up @@ -105,22 +104,10 @@ export const addReport = report => dispatch =>
SupersetClient.post({
endpoint: `/api/v1/report/`,
jsonPayload: report,
})
.then(({ json }) => {
dispatch({ type: ADD_REPORT, json });
dispatch(addSuccessToast(t('The report has been created')));
})
.catch(async e => {
const parsedError = await getClientErrorObject(e);
const errorMessage = parsedError.message;
const errorArr = Object.keys(errorMessage);
const error = errorMessage[errorArr[0]];
dispatch(
addDangerToast(
t('An error occurred while editing this report: %s', error),
),
);
});
}).then(({ json }) => {
dispatch({ type: ADD_REPORT, json });
dispatch(addSuccessToast(t('The report has been created')));
});

export const EDIT_REPORT = 'EDIT_REPORT';

Expand Down

0 comments on commit f7add54

Please sign in to comment.