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

…c#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 <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
ThomThomson · Mar 11, 2020 · e320d0f · e320d0f
1 parent f388e5f
commit e320d0f
Show file tree

Hide file tree

Showing 10 changed files with 91 additions and 76 deletions.
diff --git a/...tions_ui/public/application/sections/action_connector_form/action_connector_form.test.tsx b/...tions_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/...rs_actions_ui/public/application/sections/action_connector_form/action_connector_form.tsx b/...rs_actions_ui/public/application/sections/action_connector_form/action_connector_form.tsx
@@ -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"

diff --git a/...ers_actions_ui/public/application/sections/action_connector_form/connector_add_flyout.tsx b/...ers_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/...gers_actions_ui/public/application/sections/action_connector_form/connector_add_modal.tsx b/...gers_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/...rs_actions_ui/public/application/sections/action_connector_form/connector_edit_flyout.tsx b/...rs_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<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}

diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_add.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_add.tsx
@@ -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>

diff --git a/...ck/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.test.tsx b/...ck/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<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'
+    );
+  });
 });
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
@@ -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">

diff --git a/...ck/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.test.tsx b/...ck/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.test.tsx
@@ -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>
       );
 

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
@@ -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;
 
@@ -276,7 +267,7 @@ export const AlertForm = ({
   );
 
   return (
-    <EuiForm isInvalid={serverError !== null} error={serverError?.body.message}>
+    <EuiForm>
       <EuiFlexGrid columns={2}>
         <EuiFlexItem>
           <EuiFormRow
@@ -293,6 +284,7 @@ export const AlertForm = ({
           >
             <EuiFieldText
               fullWidth
+              autoFocus={true}
               isInvalid={errors.name.length > 0 && alert.name !== undefined}
               compressed
               name="name"