Fix UX in alerting UI forms when errors occur (#59444)

* Fix UX in UI forms when errors occur

* Create connector modal won't change

* ESLint fixes

* Fix type check

* Add some tests

* Remove if checks

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
Mike Côté 2020-03-10 21:11:18 -04:00 committed by GitHub
parent 9985597ad9
commit c51d287862
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 91 additions and 76 deletions

View file

@ -66,7 +66,6 @@ describe('action_connector_form', () => {
actionTypeName={'my-action-type-name'}
connector={initialConnector}
dispatch={() => {}}
serverError={null}
errors={{ name: [] }}
actionTypeRegistry={deps.actionTypeRegistry}
/>

View file

@ -42,9 +42,9 @@ interface ActionConnectorProps {
connector: ActionConnector;
dispatch: React.Dispatch<ReducerAction>;
actionTypeName: string;
serverError: {
serverError?: {
body: { message: string; error: string };
} | null;
};
errors: IErrorObject;
actionTypeRegistry: TypeRegistry<ActionTypeModel>;
}
@ -110,7 +110,7 @@ export const ActionConnectorForm = ({
const FieldsComponent = actionTypeRegistered.actionConnectorFields;
return (
<EuiForm isInvalid={serverError !== null} error={serverError?.body.message}>
<EuiForm isInvalid={!!serverError} error={serverError?.body.message}>
<EuiFormRow
id="actionName"
fullWidth
@ -125,6 +125,7 @@ export const ActionConnectorForm = ({
>
<EuiFieldText
fullWidth
autoFocus={true}
isInvalid={errors.name.length > 0 && connector.name !== undefined}
name="name"
placeholder="Untitled"

View file

@ -71,9 +71,6 @@ export const ConnectorAddFlyout = ({
setConnector(initialConnector);
}, [setAddFlyoutVisibility, initialConnector]);
const [serverError, setServerError] = useState<{
body: { message: string; error: string };
} | null>(null);
const canSave = hasSaveActionsCapability(capabilities);
if (!addFlyoutVisible) {
@ -105,7 +102,6 @@ export const ConnectorAddFlyout = ({
actionTypeName={actionType.name}
connector={connector}
dispatch={dispatch}
serverError={serverError}
errors={errors}
actionTypeRegistry={actionTypeRegistry}
/>
@ -131,7 +127,17 @@ export const ConnectorAddFlyout = ({
return savedConnector;
})
.catch(errorRes => {
setServerError(errorRes);
toastNotifications.addDanger(
i18n.translate(
'xpack.triggersActionsUI.sections.addConnectorForm.updateErrorNotificationText',
{
defaultMessage: 'Failed to create connector: {message}',
values: {
message: errorRes.body?.message ?? '',
},
}
)
);
return undefined;
});

View file

@ -59,14 +59,17 @@ export const ConnectorAddModal = ({
const setConnector = (value: any) => {
dispatch({ command: { type: 'setConnector' }, payload: { key: 'connector', value } });
};
const [serverError, setServerError] = useState<{
body: { message: string; error: string };
} | null>(null);
const [serverError, setServerError] = useState<
| {
body: { message: string; error: string };
}
| undefined
>(undefined);
const closeModal = useCallback(() => {
setAddModalVisibility(false);
setConnector(initialConnector);
setServerError(null);
setServerError(undefined);
}, [initialConnector, setAddModalVisibility]);
if (!addModalVisible) {

View file

@ -51,9 +51,6 @@ export const ConnectorEditFlyout = ({
connector: { ...initialConnector, secrets: {} },
});
const [isSaving, setIsSaving] = useState<boolean>(false);
const [serverError, setServerError] = useState<{
body: { message: string; error: string };
} | null>(null);
if (!editFlyoutVisible) {
return null;
@ -85,7 +82,17 @@ export const ConnectorEditFlyout = ({
return savedConnector;
})
.catch(errorRes => {
setServerError(errorRes);
toastNotifications.addDanger(
i18n.translate(
'xpack.triggersActionsUI.sections.editConnectorForm.updateErrorNotificationText',
{
defaultMessage: 'Failed to update connector: {message}',
values: {
message: errorRes.body?.message ?? '',
},
}
)
);
return undefined;
});
@ -124,7 +131,6 @@ export const ConnectorEditFlyout = ({
<EuiFlyoutBody>
<ActionConnectorForm
connector={connector}
serverError={serverError}
errors={errors}
actionTypeName={connector.actionType}
dispatch={dispatch}

View file

@ -69,13 +69,8 @@ export const AlertAdd = ({
const closeFlyout = useCallback(() => {
setAddFlyoutVisibility(false);
setAlert(initialAlert);
setServerError(null);
}, [initialAlert, setAddFlyoutVisibility]);
const [serverError, setServerError] = useState<{
body: { message: string; error: string };
} | null>(null);
if (!addFlyoutVisible) {
return null;
}
@ -110,20 +105,24 @@ export const AlertAdd = ({
async function onSaveAlert(): Promise<Alert | undefined> {
try {
const newAlert = await createAlert({ http, alert });
if (toastNotifications) {
toastNotifications.addSuccess(
i18n.translate('xpack.triggersActionsUI.sections.alertAdd.saveSuccessNotificationText', {
defaultMessage: "Saved '{alertName}'",
values: {
alertName: newAlert.name,
},
})
);
}
toastNotifications.addSuccess(
i18n.translate('xpack.triggersActionsUI.sections.alertAdd.saveSuccessNotificationText', {
defaultMessage: "Saved '{alertName}'",
values: {
alertName: newAlert.name,
},
})
);
return newAlert;
} catch (errorRes) {
setServerError(errorRes);
return undefined;
toastNotifications.addDanger(
i18n.translate('xpack.triggersActionsUI.sections.alertAdd.saveErrorNotificationText', {
defaultMessage: 'Failed to save alert: {message}',
values: {
message: errorRes.body?.message ?? '',
},
})
);
}
}
@ -161,7 +160,6 @@ export const AlertAdd = ({
alert={alert}
dispatch={dispatch}
errors={errors}
serverError={serverError}
canChangeTrigger={canChangeTrigger}
/>
</EuiFlyoutBody>

View file

@ -19,13 +19,17 @@ const alertTypeRegistry = alertTypeRegistryMock.create();
describe('alert_edit', () => {
let deps: any;
let wrapper: ReactWrapper<any>;
let mockedCoreSetup: ReturnType<typeof coreMock.createSetup>;
beforeEach(() => {
mockedCoreSetup = coreMock.createSetup();
});
async function setup() {
const mockes = coreMock.createSetup();
deps = {
toastNotifications: mockes.notifications.toasts,
http: mockes.http,
uiSettings: mockes.uiSettings,
toastNotifications: mockedCoreSetup.notifications.toasts,
http: mockedCoreSetup.http,
uiSettings: mockedCoreSetup.uiSettings,
actionTypeRegistry: actionTypeRegistry as any,
alertTypeRegistry: alertTypeRegistry as any,
};
@ -129,4 +133,22 @@ describe('alert_edit', () => {
expect(wrapper.find('[data-test-subj="editAlertFlyoutTitle"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="saveEditedAlertButton"]').exists()).toBeTruthy();
});
it('displays a toast message on save for server errors', async () => {
mockedCoreSetup.http.get.mockResolvedValue([]);
await setup();
const err = new Error() as any;
err.body = {};
err.body.message = 'Fail message';
mockedCoreSetup.http.put.mockRejectedValue(err);
await act(async () => {
wrapper
.find('[data-test-subj="saveEditedAlertButton"]')
.first()
.simulate('click');
});
expect(mockedCoreSetup.notifications.toasts.addDanger).toHaveBeenCalledWith(
'Failed to save alert: Fail message'
);
});
});

View file

@ -49,13 +49,8 @@ export const AlertEdit = ({
const closeFlyout = useCallback(() => {
setEditFlyoutVisibility(false);
setServerError(null);
}, [setEditFlyoutVisibility]);
const [serverError, setServerError] = useState<{
body: { message: string; error: string };
} | null>(null);
if (!editFlyoutVisible) {
return null;
}
@ -103,7 +98,16 @@ export const AlertEdit = ({
}
return newAlert;
} catch (errorRes) {
setServerError(errorRes);
if (toastNotifications) {
toastNotifications.addDanger(
i18n.translate('xpack.triggersActionsUI.sections.alertEdit.saveErrorNotificationText', {
defaultMessage: 'Failed to save alert: {message}',
values: {
message: errorRes.body?.message ?? '',
},
})
);
}
}
}
@ -137,13 +141,7 @@ export const AlertEdit = ({
</EuiTitle>
</EuiFlyoutHeader>
<EuiFlyoutBody>
<AlertForm
alert={alert}
dispatch={dispatch}
errors={errors}
serverError={serverError}
canChangeTrigger={false}
/>
<AlertForm alert={alert} dispatch={dispatch} errors={errors} canChangeTrigger={false} />
</EuiFlyoutBody>
<EuiFlyoutFooter>
<EuiFlexGroup justifyContent="spaceBetween">

View file

@ -86,12 +86,7 @@ describe('alert_form', () => {
uiSettings: deps!.uiSettings,
}}
>
<AlertForm
alert={initialAlert}
dispatch={() => {}}
errors={{ name: [] }}
serverError={null}
/>
<AlertForm alert={initialAlert} dispatch={() => {}} errors={{ name: [] }} />
</AlertsContextProvider>
);
@ -170,12 +165,7 @@ describe('alert_form', () => {
uiSettings: deps!.uiSettings,
}}
>
<AlertForm
alert={initialAlert}
dispatch={() => {}}
errors={{ name: [] }}
serverError={null}
/>
<AlertForm alert={initialAlert} dispatch={() => {}} errors={{ name: [] }} />
</AlertsContextProvider>
);

View file

@ -68,19 +68,10 @@ interface AlertFormProps {
alert: Alert;
dispatch: React.Dispatch<AlertReducerAction>;
errors: IErrorObject;
serverError: {
body: { message: string; error: string };
} | null;
canChangeTrigger?: boolean; // to hide Change trigger button
}
export const AlertForm = ({
alert,
canChangeTrigger = true,
dispatch,
errors,
serverError,
}: AlertFormProps) => {
export const AlertForm = ({ alert, canChangeTrigger = true, dispatch, errors }: AlertFormProps) => {
const alertsContext = useAlertsContext();
const { http, toastNotifications, alertTypeRegistry, actionTypeRegistry } = alertsContext;
@ -275,7 +266,7 @@ export const AlertForm = ({
);
return (
<EuiForm isInvalid={serverError !== null} error={serverError?.body.message}>
<EuiForm>
<EuiFlexGrid columns={2}>
<EuiFlexItem>
<EuiFormRow
@ -292,6 +283,7 @@ export const AlertForm = ({
>
<EuiFieldText
fullWidth
autoFocus={true}
isInvalid={errors.name.length > 0 && alert.name !== undefined}
compressed
name="name"