[Alerting] Showing confirmation modal on Alert Add/Edit when flyout c…

…losed without saving and changes made. (#86370) * Adding hasChanged check and showing confirmation modal if something has changed * Showing confirmation always on close * Adding functional test * Setting name and tags for APM alerts using initial values instead of setAlertProperty * Checking for alert param changes separately * Checking for alert param changes separately * Fixing functional test * Resetting initial alert params on alert type change * Fixing duplicate import * Cloning edited alert * PR fixes * PR fixes * Updating modal wording Co-authored-by: Kibana Machine <[email protected]>
elastic · Jan 11, 2021 · 666af32 · 666af32
1 parent 3ef7bd3
commit 666af32
Show file tree

Hide file tree

Showing 17 changed files with 408 additions and 44 deletions.
diff --git a/x-pack/plugins/apm/public/components/alerting/alerting_flyout/index.tsx b/x-pack/plugins/apm/public/components/alerting/alerting_flyout/index.tsx
@@ -4,10 +4,11 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 import React, { useCallback, useMemo } from 'react';
+import { useParams } from 'react-router-dom';
 import { useKibana } from '../../../../../../../src/plugins/kibana_react/public';
 import { AlertType } from '../../../../common/alert_types';
+import { getInitialAlertValues } from '../get_initial_alert_values';
 import { TriggersAndActionsUIPublicPluginStart } from '../../../../../triggers_actions_ui/public';
-
 interface Props {
   addFlyoutVisible: boolean;
   setAddFlyoutVisibility: React.Dispatch<React.SetStateAction<boolean>>;
@@ -20,10 +21,13 @@ interface KibanaDeps {
 
 export function AlertingFlyout(props: Props) {
   const { addFlyoutVisible, setAddFlyoutVisibility, alertType } = props;
+  const { serviceName } = useParams<{ serviceName?: string }>();
   const {
     services: { triggersActionsUi },
   } = useKibana<KibanaDeps>();
 
+  const initialValues = getInitialAlertValues(alertType, serviceName);
+
   const onCloseAddFlyout = useCallback(() => setAddFlyoutVisibility(false), [
     setAddFlyoutVisibility,
   ]);
@@ -36,7 +40,9 @@ export function AlertingFlyout(props: Props) {
         onClose: onCloseAddFlyout,
         alertTypeId: alertType,
         canChangeTrigger: false,
+        initialValues,
       }),
+    /* eslint-disable-next-line react-hooks/exhaustive-deps */
     [alertType, onCloseAddFlyout, triggersActionsUi]
   );
   return <>{addFlyoutVisible && addAlertFlyout}</>;

diff --git a/x-pack/plugins/apm/public/components/alerting/error_count_alert_trigger/index.tsx b/x-pack/plugins/apm/public/components/alerting/error_count_alert_trigger/index.tsx
@@ -8,7 +8,6 @@ import { i18n } from '@kbn/i18n';
 import React from 'react';
 import { useParams } from 'react-router-dom';
 import { ForLastExpression } from '../../../../../triggers_actions_ui/public';
-import { AlertType, ALERT_TYPES_CONFIG } from '../../../../common/alert_types';
 import { ENVIRONMENT_ALL } from '../../../../common/environment_filter_values';
 import { asInteger } from '../../../../common/utils/formatters';
 import { useUrlParams } from '../../../context/url_params_context/use_url_params';
@@ -110,7 +109,6 @@ export function ErrorCountAlertTrigger(props: Props) {
 
   return (
     <ServiceAlertTrigger
-      alertTypeName={ALERT_TYPES_CONFIG[AlertType.ErrorCount].name}
       defaults={defaults}
       fields={fields}
       setAlertParams={setAlertParams}

diff --git a/x-pack/plugins/apm/public/components/alerting/get_initial_alert_values.test.ts b/x-pack/plugins/apm/public/components/alerting/get_initial_alert_values.test.ts
@@ -0,0 +1,25 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { getInitialAlertValues } from './get_initial_alert_values';
+import { AlertType, ALERT_TYPES_CONFIG } from '../../../common/alert_types';
+
+test('handles null alert type and undefined service name', () => {
+  expect(getInitialAlertValues(null, undefined)).toEqual({ tags: ['apm'] });
+});
+
+test('handles valid alert type', () => {
+  const alertType = AlertType.ErrorCount;
+  expect(getInitialAlertValues(alertType, undefined)).toEqual({
+    name: ALERT_TYPES_CONFIG[alertType].name,
+    tags: ['apm'],
+  });
+
+  expect(getInitialAlertValues(alertType, 'Service Name')).toEqual({
+    name: `${ALERT_TYPES_CONFIG[alertType].name} | Service Name`,
+    tags: ['apm', `service.name:service name`],
+  });
+});
diff --git a/x-pack/plugins/apm/public/components/alerting/get_initial_alert_values.ts b/x-pack/plugins/apm/public/components/alerting/get_initial_alert_values.ts
@@ -0,0 +1,30 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { AlertType, ALERT_TYPES_CONFIG } from '../../../common/alert_types';
+
+export function getInitialAlertValues(
+  alertType: AlertType | null,
+  serviceName: string | undefined
+) {
+  const alertTypeName = alertType
+    ? ALERT_TYPES_CONFIG[alertType].name
+    : undefined;
+  const alertName = alertTypeName
+    ? serviceName
+      ? `${alertTypeName} | ${serviceName}`
+      : alertTypeName
+    : undefined;
+  const tags = ['apm'];
+  if (serviceName) {
+    tags.push(`service.name:${serviceName}`.toLowerCase());
+  }
+
+  return {
+    tags,
+    ...(alertName ? { name: alertName } : {}),
+  };
+}
diff --git a/x-pack/plugins/apm/public/components/alerting/service_alert_trigger/index.tsx b/x-pack/plugins/apm/public/components/alerting/service_alert_trigger/index.tsx
@@ -9,7 +9,6 @@ import React, { useEffect } from 'react';
 import { useParams } from 'react-router-dom';
 
 interface Props {
-  alertTypeName: string;
   setAlertParams: (key: string, value: any) => void;
   setAlertProperty: (key: string, value: any) => void;
   defaults: Record<string, any>;
@@ -20,14 +19,7 @@ interface Props {
 export function ServiceAlertTrigger(props: Props) {
   const { serviceName } = useParams<{ serviceName?: string }>();
 
-  const {
-    fields,
-    setAlertParams,
-    setAlertProperty,
-    alertTypeName,
-    defaults,
-    chartPreview,
-  } = props;
+  const { fields, setAlertParams, defaults, chartPreview } = props;
 
   const params: Record<string, any> = {
     ...defaults,
@@ -36,17 +28,6 @@ export function ServiceAlertTrigger(props: Props) {
 
   useEffect(() => {
     // we only want to run this on mount to set default values
-
-    const alertName = params.serviceName
-      ? `${alertTypeName} | ${params.serviceName}`
-      : alertTypeName;
-    setAlertProperty('name', alertName);
-
-    const tags = ['apm'];
-    if (params.serviceName) {
-      tags.push(`service.name:${params.serviceName}`.toLowerCase());
-    }
-    setAlertProperty('tags', tags);
     Object.keys(params).forEach((key) => {
       setAlertParams(key, params[key]);
     });

diff --git a/...ugins/apm/public/components/alerting/service_alert_trigger/service_alert_trigger.test.tsx b/...ugins/apm/public/components/alerting/service_alert_trigger/service_alert_trigger.test.tsx
@@ -18,7 +18,6 @@ describe('ServiceAlertTrigger', () => {
     expect(() =>
       render(
         <ServiceAlertTrigger
-          alertTypeName="test alert type name"
           defaults={{}}
           fields={[null]}
           setAlertParams={() => {}}

diff --git a/x-pack/plugins/apm/public/components/alerting/transaction_duration_alert_trigger/index.tsx b/x-pack/plugins/apm/public/components/alerting/transaction_duration_alert_trigger/index.tsx
@@ -10,7 +10,6 @@ import React from 'react';
 import { useParams } from 'react-router-dom';
 import { useFetcher } from '../../../../../observability/public';
 import { ForLastExpression } from '../../../../../triggers_actions_ui/public';
-import { ALERT_TYPES_CONFIG } from '../../../../common/alert_types';
 import { ENVIRONMENT_ALL } from '../../../../common/environment_filter_values';
 import { getDurationFormatter } from '../../../../common/utils/formatters';
 import { TimeSeries } from '../../../../typings/timeseries';
@@ -203,7 +202,6 @@ export function TransactionDurationAlertTrigger(props: Props) {
 
   return (
     <ServiceAlertTrigger
-      alertTypeName={ALERT_TYPES_CONFIG['apm.transaction_duration'].name}
       chartPreview={chartPreview}
       defaults={defaults}
       fields={fields}

diff --git a/...ugins/apm/public/components/alerting/transaction_duration_anomaly_alert_trigger/index.tsx b/...ugins/apm/public/components/alerting/transaction_duration_anomaly_alert_trigger/index.tsx
@@ -8,7 +8,6 @@ import { useParams } from 'react-router-dom';
 import { i18n } from '@kbn/i18n';
 import React from 'react';
 import { ANOMALY_SEVERITY } from '../../../../../ml/common';
-import { ALERT_TYPES_CONFIG } from '../../../../common/alert_types';
 import { useEnvironmentsFetcher } from '../../../hooks/use_environments_fetcher';
 import { useUrlParams } from '../../../context/url_params_context/use_url_params';
 import { ServiceAlertTrigger } from '../service_alert_trigger';
@@ -106,9 +105,6 @@ export function TransactionDurationAnomalyAlertTrigger(props: Props) {
 
   return (
     <ServiceAlertTrigger
-      alertTypeName={
-        ALERT_TYPES_CONFIG['apm.transaction_duration_anomaly'].name
-      }
       fields={fields}
       defaults={defaults}
       setAlertParams={setAlertParams}

diff --git a/x-pack/plugins/apm/public/components/alerting/transaction_error_rate_alert_trigger/index.tsx b/x-pack/plugins/apm/public/components/alerting/transaction_error_rate_alert_trigger/index.tsx
@@ -6,7 +6,6 @@
 import React from 'react';
 import { useParams } from 'react-router-dom';
 import { ForLastExpression } from '../../../../../triggers_actions_ui/public';
-import { AlertType, ALERT_TYPES_CONFIG } from '../../../../common/alert_types';
 import { ENVIRONMENT_ALL } from '../../../../common/environment_filter_values';
 import { asPercent } from '../../../../common/utils/formatters';
 import { useApmServiceContext } from '../../../context/apm_service/use_apm_service_context';
@@ -137,7 +136,6 @@ export function TransactionErrorRateAlertTrigger(props: Props) {
 
   return (
     <ServiceAlertTrigger
-      alertTypeName={ALERT_TYPES_CONFIG[AlertType.TransactionErrorRate].name}
       fields={fields}
       defaults={defaultParams}
       setAlertParams={setAlertParams}

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
@@ -3,25 +3,29 @@
  * 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, { useCallback, useReducer, useMemo, useState, useEffect } from 'react';
+import React, { useReducer, useMemo, useState, useEffect } from 'react';
 import { FormattedMessage } from '@kbn/i18n/react';
 import { EuiTitle, EuiFlyoutHeader, EuiFlyout, EuiFlyoutBody, EuiPortal } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
+import { isEmpty } from 'lodash';
 import {
   ActionTypeRegistryContract,
   Alert,
   AlertTypeRegistryContract,
+  AlertTypeParams,
   AlertUpdates,
 } from '../../../types';
 import { AlertForm, getAlertErrors, isValidAlert } from './alert_form';
 import { alertReducer, InitialAlert, InitialAlertReducer } from './alert_reducer';
 import { createAlert } from '../../lib/alert_api';
 import { HealthCheck } from '../../components/health_check';
 import { ConfirmAlertSave } from './confirm_alert_save';
+import { ConfirmAlertClose } from './confirm_alert_close';
 import { hasShowActionsCapability } from '../../lib/capabilities';
 import AlertAddFooter from './alert_add_footer';
 import { HealthContextProvider } from '../../context/health_context';
 import { useKibana } from '../../../common/lib/kibana';
+import { hasAlertChanged, haveAlertParamsChanged } from './has_alert_changed';
 import { getAlertWithInvalidatedFields } from '../../lib/value_validators';
 
 export interface AlertAddProps<MetaData = Record<string, any>> {
@@ -66,8 +70,10 @@ const AlertAdd = ({
   const [{ alert }, dispatch] = useReducer(alertReducer as InitialAlertReducer, {
     alert: initialAlert,
   });
+  const [initialAlertParams, setInitialAlertParams] = useState<AlertTypeParams>({});
   const [isSaving, setIsSaving] = useState<boolean>(false);
   const [isConfirmAlertSaveModalOpen, setIsConfirmAlertSaveModalOpen] = useState<boolean>(false);
+  const [isConfirmAlertCloseModalOpen, setIsConfirmAlertCloseModalOpen] = useState<boolean>(false);
 
   const setAlert = (value: InitialAlert) => {
     dispatch({ command: { type: 'setAlert' }, payload: { key: 'alert', value } });
@@ -91,16 +97,35 @@ const AlertAdd = ({
     }
   }, [alertTypeId]);
 
-  const closeFlyout = useCallback(() => {
-    setAlert(initialAlert);
-    onClose();
-  }, [initialAlert, onClose]);
+  useEffect(() => {
+    if (isEmpty(alert.params) && !isEmpty(initialAlertParams)) {
+      // alert params are explicitly cleared when the alert type is cleared.
+      // clear the "initial" params in order to capture the
+      // default when a new alert type is selected
+      setInitialAlertParams({});
+    } else if (isEmpty(initialAlertParams)) {
+      // captures the first change to the alert params,
+      // when consumers set a default value for the alert params
+      setInitialAlertParams(alert.params);
+    }
+  }, [alert.params, initialAlertParams, setInitialAlertParams]);
+
+  const checkForChangesAndCloseFlyout = () => {
+    if (
+      hasAlertChanged(alert, initialAlert, false) ||
+      haveAlertParamsChanged(alert.params, initialAlertParams)
+    ) {
+      setIsConfirmAlertCloseModalOpen(true);
+    } else {
+      onClose();
+    }
+  };
 
   const saveAlertAndCloseFlyout = async () => {
     const savedAlert = await onSaveAlert();
     setIsSaving(false);
     if (savedAlert) {
-      closeFlyout();
+      onClose();
       if (reloadAlerts) {
         reloadAlerts();
       }
@@ -142,7 +167,7 @@ const AlertAdd = ({
   return (
     <EuiPortal>
       <EuiFlyout
-        onClose={closeFlyout}
+        onClose={checkForChangesAndCloseFlyout}
         aria-labelledby="flyoutAlertAddTitle"
         size="m"
         maxWidth={620}
@@ -198,7 +223,7 @@ const AlertAdd = ({
                   await saveAlertAndCloseFlyout();
                 }
               }}
-              onCancel={closeFlyout}
+              onCancel={checkForChangesAndCloseFlyout}
             />
           </HealthCheck>
         </HealthContextProvider>
@@ -214,6 +239,17 @@ const AlertAdd = ({
             }}
           />
         )}
+        {isConfirmAlertCloseModalOpen && (
+          <ConfirmAlertClose
+            onConfirm={() => {
+              setIsConfirmAlertCloseModalOpen(false);
+              onClose();
+            }}
+            onCancel={() => {
+              setIsConfirmAlertCloseModalOpen(false);
+            }}
+          />
+        )}
       </EuiFlyout>
     </EuiPortal>
   );