[Alerts] Remove Add Alerts flyout onClose (elastic#85462)

* Remove add alerts flyout after onClose * Updating tests Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Gidi Meir Morris <[email protected]>
ymao1 · Dec 15, 2020 · dd358b8 · dd358b8
1 parent 4b57ac5
commit dd358b8
Show file tree

Hide file tree

Showing 10 changed files with 67 additions and 55 deletions.
diff --git a/x-pack/examples/alerting_example/public/components/create_alert.tsx b/x-pack/examples/alerting_example/public/components/create_alert.tsx
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import React, { useMemo, useState } from 'react';
+import React, { useMemo, useState, useCallback } from 'react';
 
 import { EuiIcon, EuiFlexItem, EuiCard, EuiFlexGroup } from '@elastic/eui';
 
@@ -14,15 +14,18 @@ import { ALERTING_EXAMPLE_APP_ID } from '../../common/constants';
 export const CreateAlert = ({ triggersActionsUi }: AlertingExampleComponentParams) => {
   const [alertFlyoutVisible, setAlertFlyoutVisibility] = useState<boolean>(false);
 
+  const onCloseAlertFlyout = useCallback(() => setAlertFlyoutVisibility(false), [
+    setAlertFlyoutVisibility,
+  ]);
+
   const AddAlertFlyout = useMemo(
     () =>
       triggersActionsUi.getAddAlertFlyout({
         consumer: ALERTING_EXAMPLE_APP_ID,
-        addFlyoutVisible: alertFlyoutVisible,
-        setAddFlyoutVisibility: setAlertFlyoutVisibility,
+        onClose: onCloseAlertFlyout,
       }),
     // eslint-disable-next-line react-hooks/exhaustive-deps
-    [alertFlyoutVisible]
+    [onCloseAlertFlyout]
   );
 
   return (
@@ -35,7 +38,7 @@ export const CreateAlert = ({ triggersActionsUi }: AlertingExampleComponentParam
           onClick={() => setAlertFlyoutVisibility(true)}
         />
       </EuiFlexItem>
-      <EuiFlexItem>{AddAlertFlyout}</EuiFlexItem>
+      <EuiFlexItem>{alertFlyoutVisible && AddAlertFlyout}</EuiFlexItem>
     </EuiFlexGroup>
   );
 };
diff --git a/x-pack/plugins/apm/public/components/alerting/AlertingFlyout/index.tsx b/x-pack/plugins/apm/public/components/alerting/AlertingFlyout/index.tsx
@@ -3,7 +3,7 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-import React, { useMemo } from 'react';
+import React, { useCallback, useMemo } from 'react';
 import { useKibana } from '../../../../../../../src/plugins/kibana_react/public';
 import { AlertType } from '../../../../common/alert_types';
 import { TriggersAndActionsUIPublicPluginStart } from '../../../../../triggers_actions_ui/public';
@@ -23,17 +23,21 @@ export function AlertingFlyout(props: Props) {
   const {
     services: { triggersActionsUi },
   } = useKibana<KibanaDeps>();
+
+  const onCloseAddFlyout = useCallback(() => setAddFlyoutVisibility(false), [
+    setAddFlyoutVisibility,
+  ]);
+
   const addAlertFlyout = useMemo(
     () =>
       alertType &&
       triggersActionsUi.getAddAlertFlyout({
         consumer: 'apm',
-        addFlyoutVisible,
-        setAddFlyoutVisibility,
+        onClose: onCloseAddFlyout,
         alertTypeId: alertType,
         canChangeTrigger: false,
       }),
-    [addFlyoutVisible, alertType, setAddFlyoutVisibility, triggersActionsUi]
+    [alertType, onCloseAddFlyout, triggersActionsUi]
   );
-  return <>{addAlertFlyout}</>;
+  return <>{addFlyoutVisible && addAlertFlyout}</>;
 }
diff --git a/x-pack/plugins/infra/public/alerting/inventory/components/alert_flyout.tsx b/x-pack/plugins/infra/public/alerting/inventory/components/alert_flyout.tsx
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import React, { useContext, useMemo } from 'react';
+import React, { useCallback, useContext, useMemo } from 'react';
 
 import { TriggerActionsContext } from '../../../utils/triggers_actions_context';
 // eslint-disable-next-line @kbn/eslint/no-restricted-paths
@@ -26,14 +26,13 @@ export const AlertFlyout = ({ options, nodeType, filter, visible, setVisible }:
 
   const { inventoryPrefill } = useAlertPrefillContext();
   const { customMetrics } = inventoryPrefill;
-
+  const onCloseFlyout = useCallback(() => setVisible(false), [setVisible]);
   const AddAlertFlyout = useMemo(
     () =>
       triggersActionsUI &&
       triggersActionsUI.getAddAlertFlyout({
         consumer: 'infrastructure',
-        addFlyoutVisible: visible!,
-        setAddFlyoutVisibility: setVisible,
+        onClose: onCloseFlyout,
         canChangeTrigger: false,
         alertTypeId: METRIC_INVENTORY_THRESHOLD_ALERT_TYPE_ID,
         metadata: {
@@ -47,5 +46,5 @@ export const AlertFlyout = ({ options, nodeType, filter, visible, setVisible }:
     [triggersActionsUI, visible]
   );
 
-  return <>{AddAlertFlyout}</>;
+  return <>{visible && AddAlertFlyout}</>;
 };
diff --git a/x-pack/plugins/infra/public/alerting/log_threshold/components/alert_flyout.tsx b/x-pack/plugins/infra/public/alerting/log_threshold/components/alert_flyout.tsx
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import React, { useContext, useMemo } from 'react';
+import React, { useCallback, useContext, useMemo } from 'react';
 import { TriggerActionsContext } from '../../../utils/triggers_actions_context';
 import { LOG_DOCUMENT_COUNT_ALERT_TYPE_ID } from '../../../../common/alerting/logs/log_threshold/types';
 
@@ -14,24 +14,23 @@ interface Props {
 }
 
 export const AlertFlyout = (props: Props) => {
+  const { visible, setVisible } = props;
   const { triggersActionsUI } = useContext(TriggerActionsContext);
-
+  const onCloseFlyout = useCallback(() => setVisible(false), [setVisible]);
   const AddAlertFlyout = useMemo(
     () =>
       triggersActionsUI &&
       triggersActionsUI.getAddAlertFlyout({
         consumer: 'logs',
-        addFlyoutVisible: props.visible!,
-        setAddFlyoutVisibility: props.setVisible,
+        onClose: onCloseFlyout,
         canChangeTrigger: false,
         alertTypeId: LOG_DOCUMENT_COUNT_ALERT_TYPE_ID,
         metadata: {
           isInternal: true,
         },
       }),
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-    [triggersActionsUI, props.visible]
+    [triggersActionsUI, onCloseFlyout]
   );
 
-  return <>{AddAlertFlyout}</>;
+  return <>{visible && AddAlertFlyout}</>;
 };
diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_flyout.tsx b/x-pack/plugins/infra/public/alerting/metric_threshold/components/alert_flyout.tsx
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import React, { useContext, useMemo } from 'react';
+import React, { useCallback, useContext, useMemo } from 'react';
 import { TriggerActionsContext } from '../../../utils/triggers_actions_context';
 // eslint-disable-next-line @kbn/eslint/no-restricted-paths
 import { METRIC_THRESHOLD_ALERT_TYPE_ID } from '../../../../server/lib/alerting/metric_threshold/types';
@@ -19,15 +19,15 @@ interface Props {
 }
 
 export const AlertFlyout = (props: Props) => {
+  const { visible, setVisible } = props;
   const { triggersActionsUI } = useContext(TriggerActionsContext);
-
+  const onCloseFlyout = useCallback(() => setVisible(false), [setVisible]);
   const AddAlertFlyout = useMemo(
     () =>
       triggersActionsUI &&
       triggersActionsUI.getAddAlertFlyout({
         consumer: 'infrastructure',
-        addFlyoutVisible: props.visible!,
-        setAddFlyoutVisibility: props.setVisible,
+        onClose: onCloseFlyout,
         canChangeTrigger: false,
         alertTypeId: METRIC_THRESHOLD_ALERT_TYPE_ID,
         metadata: {
@@ -36,8 +36,8 @@ export const AlertFlyout = (props: Props) => {
         },
       }),
     // eslint-disable-next-line react-hooks/exhaustive-deps
-    [triggersActionsUI, props.visible]
+    [triggersActionsUI, onCloseFlyout]
   );
 
-  return <>{AddAlertFlyout}</>;
+  return <>{visible && AddAlertFlyout}</>;
 };
diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_add.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_add.test.tsx
@@ -128,8 +128,7 @@ describe('alert_add', () => {
     wrapper = mountWithIntl(
       <AlertAdd
         consumer={ALERTS_FEATURE_ID}
-        addFlyoutVisible={true}
-        setAddFlyoutVisibility={() => {}}
+        onClose={() => {}}
         initialValues={initialValues}
         reloadAlerts={() => {
           return new Promise<void>(() => {});

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
@@ -26,10 +26,9 @@ import { useKibana } from '../../../common/lib/kibana';
 
 export interface AlertAddProps<MetaData = Record<string, any>> {
   consumer: string;
-  addFlyoutVisible: boolean;
   alertTypeRegistry: AlertTypeRegistryContract;
   actionTypeRegistry: ActionTypeRegistryContract;
-  setAddFlyoutVisibility: React.Dispatch<React.SetStateAction<boolean>>;
+  onClose: () => void;
   alertTypeId?: string;
   canChangeTrigger?: boolean;
   initialValues?: Partial<Alert>;
@@ -39,10 +38,9 @@ export interface AlertAddProps<MetaData = Record<string, any>> {
 
 const AlertAdd = ({
   consumer,
-  addFlyoutVisible,
   alertTypeRegistry,
   actionTypeRegistry,
-  setAddFlyoutVisibility,
+  onClose,
   canChangeTrigger,
   alertTypeId,
   initialValues,
@@ -92,9 +90,9 @@ const AlertAdd = ({
   }, [alertTypeId]);
 
   const closeFlyout = useCallback(() => {
-    setAddFlyoutVisibility(false);
     setAlert(initialAlert);
-  }, [initialAlert, setAddFlyoutVisibility]);
+    onClose();
+  }, [initialAlert, onClose]);
 
   const saveAlertAndCloseFlyout = async () => {
     const savedAlert = await onSaveAlert();
@@ -107,10 +105,6 @@ const AlertAdd = ({
     }
   };
 
-  if (!addFlyoutVisible) {
-    return null;
-  }
-
   const alertType = alert.alertTypeId ? alertTypeRegistry.get(alert.alertTypeId) : null;
   const errors = {
     ...(alertType ? alertType.validate(alert.params).errors : []),

diff --git a/...iggers_actions_ui/public/application/sections/alerts_list/components/alerts_list.test.tsx b/...iggers_actions_ui/public/application/sections/alerts_list/components/alerts_list.test.tsx
@@ -26,6 +26,7 @@ jest.mock('../../../lib/action_connector_api', () => ({
 jest.mock('../../../lib/alert_api', () => ({
   loadAlerts: jest.fn(),
   loadAlertTypes: jest.fn(),
+  health: jest.fn(() => ({ isSufficientlySecure: true, hasPermanentEncryptionKey: true })),
 }));
 jest.mock('react-router-dom', () => ({
   useHistory: () => ({
@@ -114,7 +115,16 @@ describe('alerts_list component empty', () => {
     expect(
       wrapper.find('[data-test-subj="createFirstAlertButton"]').find('EuiButton')
     ).toHaveLength(1);
-    expect(wrapper.find('AlertAdd')).toHaveLength(1);
+    expect(wrapper.find('AlertAdd').exists()).toBeFalsy();
+
+    wrapper.find('button[data-test-subj="createFirstAlertButton"]').simulate('click');
+
+    // When the AlertAdd component is rendered, it waits for the healthcheck to resolve
+    await new Promise((resolve) => {
+      setTimeout(resolve, 1000);
+    });
+    wrapper.update();
+    expect(wrapper.find('AlertAdd').exists()).toEqual(true);
   });
 });
 

diff --git a/...ns/triggers_actions_ui/public/application/sections/alerts_list/components/alerts_list.tsx b/...ns/triggers_actions_ui/public/application/sections/alerts_list/components/alerts_list.tsx
@@ -653,14 +653,17 @@ export const AlertsList: React.FunctionComponent = () => {
       ) : (
         noPermissionPrompt
       )}
-      <AlertAdd
-        consumer={ALERTS_FEATURE_ID}
-        addFlyoutVisible={alertFlyoutVisible}
-        setAddFlyoutVisibility={setAlertFlyoutVisibility}
-        actionTypeRegistry={actionTypeRegistry}
-        alertTypeRegistry={alertTypeRegistry}
-        reloadAlerts={loadAlertsData}
-      />
+      {alertFlyoutVisible && (
+        <AlertAdd
+          consumer={ALERTS_FEATURE_ID}
+          onClose={() => {
+            setAlertFlyoutVisibility(false);
+          }}
+          actionTypeRegistry={actionTypeRegistry}
+          alertTypeRegistry={alertTypeRegistry}
+          reloadAlerts={loadAlertsData}
+        />
+      )}
     </section>
   );
 };

diff --git a/x-pack/plugins/uptime/public/components/overview/alerts/uptime_alerts_flyout_wrapper.tsx b/x-pack/plugins/uptime/public/components/overview/alerts/uptime_alerts_flyout_wrapper.tsx
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import React, { useMemo } from 'react';
+import React, { useCallback, useMemo } from 'react';
 import { useKibana } from '../../../../../../../src/plugins/kibana_react/public';
 import { TriggersAndActionsUIPublicPluginStart } from '../../../../../../plugins/triggers_actions_ui/public';
 
@@ -24,19 +24,20 @@ export const UptimeAlertsFlyoutWrapperComponent = ({
   setAlertFlyoutVisibility,
 }: Props) => {
   const { triggersActionsUi } = useKibana<KibanaDeps>().services;
-
+  const onCloseAlertFlyout = useCallback(() => setAlertFlyoutVisibility(false), [
+    setAlertFlyoutVisibility,
+  ]);
   const AddAlertFlyout = useMemo(
     () =>
       triggersActionsUi.getAddAlertFlyout({
         consumer: 'uptime',
-        addFlyoutVisible: alertFlyoutVisible,
-        setAddFlyoutVisibility: setAlertFlyoutVisibility,
+        onClose: onCloseAlertFlyout,
         alertTypeId,
         canChangeTrigger: !alertTypeId,
       }),
     // eslint-disable-next-line react-hooks/exhaustive-deps
-    [alertFlyoutVisible, alertTypeId]
+    [onCloseAlertFlyout, alertTypeId]
   );
 
-  return <>{AddAlertFlyout}</>;
+  return <>{alertFlyoutVisible && AddAlertFlyout}</>;
 };