From e320d0ffad39c5440355697b143098a42b49b3c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20C=C3=B4t=C3=A9?= Date: Wed, 11 Mar 2020 10:21:42 -0400 Subject: [PATCH] Fix UX in alerting UI forms when errors occur (#59444) (#59853) * 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 Co-authored-by: Elastic Machine --- .../action_connector_form.test.tsx | 1 - .../action_connector_form.tsx | 7 ++-- .../connector_add_flyout.tsx | 16 ++++++--- .../connector_add_modal.tsx | 11 +++--- .../connector_edit_flyout.tsx | 16 ++++++--- .../sections/alert_form/alert_add.tsx | 34 +++++++++---------- .../sections/alert_form/alert_edit.test.tsx | 30 +++++++++++++--- .../sections/alert_form/alert_edit.tsx | 24 ++++++------- .../sections/alert_form/alert_form.test.tsx | 14 ++------ .../sections/alert_form/alert_form.tsx | 14 ++------ 10 files changed, 91 insertions(+), 76 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_connector_form.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_connector_form.test.tsx index 800863e46034e..f68cc5759fb54 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_connector_form.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_connector_form.test.tsx @@ -66,7 +66,6 @@ describe('action_connector_form', () => { actionTypeName={'my-action-type-name'} connector={initialConnector} dispatch={() => {}} - serverError={null} errors={{ name: [] }} actionTypeRegistry={deps.actionTypeRegistry} /> diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_connector_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_connector_form.tsx index 9f2b203246280..e221fff64048e 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_connector_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_connector_form.tsx @@ -42,9 +42,9 @@ interface ActionConnectorProps { connector: ActionConnector; dispatch: React.Dispatch; actionTypeName: string; - serverError: { + serverError?: { body: { message: string; error: string }; - } | null; + }; errors: IErrorObject; actionTypeRegistry: TypeRegistry; } @@ -110,7 +110,7 @@ export const ActionConnectorForm = ({ const FieldsComponent = actionTypeRegistered.actionConnectorFields; return ( - + 0 && connector.name !== undefined} name="name" placeholder="Untitled" diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_flyout.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_flyout.tsx index 1b86116781084..f265a1de6f56a 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_flyout.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_flyout.tsx @@ -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; }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_modal.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_modal.tsx index 1cc26f39990ff..3a53f9f06e0dd 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_modal.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_modal.tsx @@ -58,14 +58,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) { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_edit_flyout.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_edit_flyout.tsx index c52bb8cc08f6f..d0dcff9ef6a94 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_edit_flyout.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_edit_flyout.tsx @@ -51,9 +51,6 @@ export const ConnectorEditFlyout = ({ connector: { ...initialConnector, secrets: {} }, }); const [isSaving, setIsSaving] = useState(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 = ({ { 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 { 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} /> diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.test.tsx index 4ebeba3924faf..9a335b2f2c242 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.test.tsx @@ -19,13 +19,17 @@ const alertTypeRegistry = alertTypeRegistryMock.create(); describe('alert_edit', () => { let deps: any; let wrapper: ReactWrapper; + let mockedCoreSetup: ReturnType; + + 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' + ); + }); }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.tsx index 06d21c05582e0..ac3951cfa98de 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.tsx @@ -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 = ({ - + diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.test.tsx index 6119b407a6590..b31be7ecb9a79 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.test.tsx @@ -86,12 +86,7 @@ describe('alert_form', () => { uiSettings: deps!.uiSettings, }} > - {}} - errors={{ name: [] }} - serverError={null} - /> + {}} errors={{ name: [] }} /> ); @@ -170,12 +165,7 @@ describe('alert_form', () => { uiSettings: deps!.uiSettings, }} > - {}} - errors={{ name: [] }} - serverError={null} - /> + {}} errors={{ name: [] }} /> ); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx index 10bd978e31e30..a6d3644a35d9c 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx @@ -68,19 +68,10 @@ interface AlertFormProps { alert: Alert; dispatch: React.Dispatch; 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; @@ -276,7 +267,7 @@ export const AlertForm = ({ ); return ( - + 0 && alert.name !== undefined} compressed name="name"