From 666af32be4077f0af78eb386dc9e2302abba1541 Mon Sep 17 00:00:00 2001 From: ymao1 Date: Mon, 11 Jan 2021 16:16:10 -0500 Subject: [PATCH] [Alerting] Showing confirmation modal on Alert Add/Edit when flyout closed 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 <42973632+kibanamachine@users.noreply.github.com> --- .../alerting/alerting_flyout/index.tsx | 8 +- .../error_count_alert_trigger/index.tsx | 2 - .../alerting/get_initial_alert_values.test.ts | 25 +++ .../alerting/get_initial_alert_values.ts | 30 ++++ .../alerting/service_alert_trigger/index.tsx | 21 +-- .../service_alert_trigger.test.tsx | 1 - .../index.tsx | 2 - .../index.tsx | 4 - .../index.tsx | 2 - .../sections/alert_form/alert_add.tsx | 52 +++++- .../sections/alert_form/alert_edit.tsx | 29 +++- .../alert_form/confirm_alert_close.tsx | 52 ++++++ .../alert_form/has_alert_changed.test.ts | 164 ++++++++++++++++++ .../sections/alert_form/has_alert_changed.ts | 40 +++++ .../triggers_actions_ui/public/types.ts | 5 +- .../alert_create_flyout.ts | 13 ++ .../apps/triggers_actions_ui/details.ts | 2 + 17 files changed, 408 insertions(+), 44 deletions(-) create mode 100644 x-pack/plugins/apm/public/components/alerting/get_initial_alert_values.test.ts create mode 100644 x-pack/plugins/apm/public/components/alerting/get_initial_alert_values.ts create mode 100644 x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/confirm_alert_close.tsx create mode 100644 x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/has_alert_changed.test.ts create mode 100644 x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/has_alert_changed.ts 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 index 88a897d7baf5..83bbf49691be 100644 --- 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>; @@ -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(); + 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 index cce973f8587d..d7375d14e17c 100644 --- 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 ( { + 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 new file mode 100644 index 000000000000..3655c9f90c4b --- /dev/null +++ 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 index 0a12f79bf61a..a04679bcb689 100644 --- 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; @@ -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 = { ...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/x-pack/plugins/apm/public/components/alerting/service_alert_trigger/service_alert_trigger.test.tsx b/x-pack/plugins/apm/public/components/alerting/service_alert_trigger/service_alert_trigger.test.tsx index 72611043bbed..7f9a27e884e8 100644 --- a/x-pack/plugins/apm/public/components/alerting/service_alert_trigger/service_alert_trigger.test.tsx +++ b/x-pack/plugins/apm/public/components/alerting/service_alert_trigger/service_alert_trigger.test.tsx @@ -18,7 +18,6 @@ describe('ServiceAlertTrigger', () => { expect(() => render( {}} 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 index 22840bc2e6ed..7c0a74f2e1b6 100644 --- 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 ( > { @@ -66,8 +70,10 @@ const AlertAdd = ({ const [{ alert }, dispatch] = useReducer(alertReducer as InitialAlertReducer, { alert: initialAlert, }); + const [initialAlertParams, setInitialAlertParams] = useState({}); const [isSaving, setIsSaving] = useState(false); const [isConfirmAlertSaveModalOpen, setIsConfirmAlertSaveModalOpen] = useState(false); + const [isConfirmAlertCloseModalOpen, setIsConfirmAlertCloseModalOpen] = useState(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 ( @@ -214,6 +239,17 @@ const AlertAdd = ({ }} /> )} + {isConfirmAlertCloseModalOpen && ( + { + setIsConfirmAlertCloseModalOpen(false); + onClose(); + }} + onCancel={() => { + setIsConfirmAlertCloseModalOpen(false); + }} + /> + )} ); 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 024907c7bbbc..6190853096f2 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 @@ -19,6 +19,7 @@ import { EuiCallOut, EuiSpacer, } from '@elastic/eui'; +import { cloneDeep } from 'lodash'; import { i18n } from '@kbn/i18n'; import { ActionTypeRegistryContract, Alert, AlertTypeRegistryContract } from '../../../types'; import { AlertForm, getAlertErrors, isValidAlert } from './alert_form'; @@ -27,6 +28,8 @@ import { updateAlert } from '../../lib/alert_api'; import { HealthCheck } from '../../components/health_check'; import { HealthContextProvider } from '../../context/health_context'; import { useKibana } from '../../../common/lib/kibana'; +import { ConfirmAlertClose } from './confirm_alert_close'; +import { hasAlertChanged } from './has_alert_changed'; import { getAlertWithInvalidatedFields } from '../../lib/value_validators'; export interface AlertEditProps> { @@ -47,13 +50,14 @@ export const AlertEdit = ({ metadata, }: AlertEditProps) => { const [{ alert }, dispatch] = useReducer(alertReducer as ConcreteAlertReducer, { - alert: initialAlert, + alert: cloneDeep(initialAlert), }); const [isSaving, setIsSaving] = useState(false); const [hasActionsDisabled, setHasActionsDisabled] = useState(false); const [hasActionsWithBrokenConnector, setHasActionsWithBrokenConnector] = useState( false ); + const [isConfirmAlertCloseModalOpen, setIsConfirmAlertCloseModalOpen] = useState(false); const { http, @@ -71,6 +75,14 @@ export const AlertEdit = ({ alertType ); + const checkForChangesAndCloseFlyout = () => { + if (hasAlertChanged(alert, initialAlert, true)) { + setIsConfirmAlertCloseModalOpen(true); + } else { + onClose(); + } + }; + async function onSaveAlert(): Promise { try { if (isValidAlert(alert, alertErrors, alertActionsErrors) && !hasActionsWithBrokenConnector) { @@ -107,7 +119,7 @@ export const AlertEdit = ({ return ( onClose()} + onClose={checkForChangesAndCloseFlyout} aria-labelledby="flyoutAlertEditTitle" size="m" maxWidth={620} @@ -160,7 +172,7 @@ export const AlertEdit = ({ onClose()} + onClick={() => checkForChangesAndCloseFlyout()} > {i18n.translate( 'xpack.triggersActionsUI.sections.alertEdit.cancelButtonLabel', @@ -200,6 +212,17 @@ export const AlertEdit = ({ + {isConfirmAlertCloseModalOpen && ( + { + setIsConfirmAlertCloseModalOpen(false); + onClose(); + }} + onCancel={() => { + setIsConfirmAlertCloseModalOpen(false); + }} + /> + )} ); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/confirm_alert_close.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/confirm_alert_close.tsx new file mode 100644 index 000000000000..3c0ab3faacf6 --- /dev/null +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/confirm_alert_close.tsx @@ -0,0 +1,52 @@ +/* + * 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 React from 'react'; +import { EuiOverlayMask, EuiConfirmModal } from '@elastic/eui'; +import { FormattedMessage } from '@kbn/i18n/react'; +import { i18n } from '@kbn/i18n'; + +interface Props { + onConfirm: () => void; + onCancel: () => void; +} + +export const ConfirmAlertClose: React.FC = ({ onConfirm, onCancel }) => { + return ( + + +

+ +

+ + + ); +}; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/has_alert_changed.test.ts b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/has_alert_changed.test.ts new file mode 100644 index 000000000000..73e33aa6f5cd --- /dev/null +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/has_alert_changed.test.ts @@ -0,0 +1,164 @@ +/* + * 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 { InitialAlert } from './alert_reducer'; +import { hasAlertChanged } from './has_alert_changed'; + +function createAlert(overrides = {}): InitialAlert { + return { + params: {}, + consumer: 'test', + alertTypeId: 'test', + schedule: { + interval: '1m', + }, + actions: [], + tags: [], + notifyWhen: 'onActionGroupChange', + ...overrides, + }; +} + +test('should return false for same alert', () => { + const a = createAlert(); + expect(hasAlertChanged(a, a, true)).toEqual(false); +}); + +test('should return true for different alert', () => { + const a = createAlert(); + const b = createAlert({ alertTypeId: 'differentTest' }); + expect(hasAlertChanged(a, b, true)).toEqual(true); +}); + +test('should correctly compare name field', () => { + // name field doesn't exist initially + const a = createAlert(); + // set name to actual value + const b = createAlert({ name: 'myAlert' }); + // set name to different value + const c = createAlert({ name: 'anotherAlert' }); + // set name to various empty/null/undefined states + const d = createAlert({ name: '' }); + const e = createAlert({ name: undefined }); + const f = createAlert({ name: null }); + + expect(hasAlertChanged(a, b, true)).toEqual(true); + expect(hasAlertChanged(a, c, true)).toEqual(true); + expect(hasAlertChanged(a, d, true)).toEqual(false); + expect(hasAlertChanged(a, e, true)).toEqual(false); + expect(hasAlertChanged(a, f, true)).toEqual(false); + + expect(hasAlertChanged(b, c, true)).toEqual(true); + expect(hasAlertChanged(b, d, true)).toEqual(true); + expect(hasAlertChanged(b, e, true)).toEqual(true); + expect(hasAlertChanged(b, f, true)).toEqual(true); + + expect(hasAlertChanged(c, d, true)).toEqual(true); + expect(hasAlertChanged(c, e, true)).toEqual(true); + expect(hasAlertChanged(c, f, true)).toEqual(true); + + expect(hasAlertChanged(d, e, true)).toEqual(false); + expect(hasAlertChanged(d, f, true)).toEqual(false); +}); + +test('should correctly compare alertTypeId field', () => { + const a = createAlert(); + + // set alertTypeId to different value + const b = createAlert({ alertTypeId: 'myAlertId' }); + // set alertTypeId to various empty/null/undefined states + const c = createAlert({ alertTypeId: '' }); + const d = createAlert({ alertTypeId: undefined }); + const e = createAlert({ alertTypeId: null }); + + expect(hasAlertChanged(a, b, true)).toEqual(true); + expect(hasAlertChanged(a, c, true)).toEqual(true); + expect(hasAlertChanged(a, d, true)).toEqual(true); + expect(hasAlertChanged(a, e, true)).toEqual(true); + + expect(hasAlertChanged(b, c, true)).toEqual(true); + expect(hasAlertChanged(b, d, true)).toEqual(true); + expect(hasAlertChanged(b, e, true)).toEqual(true); + + expect(hasAlertChanged(c, d, true)).toEqual(false); + expect(hasAlertChanged(c, e, true)).toEqual(false); + expect(hasAlertChanged(d, e, true)).toEqual(false); +}); + +test('should correctly compare throttle field', () => { + // throttle field doesn't exist initially + const a = createAlert(); + // set throttle to actual value + const b = createAlert({ throttle: '1m' }); + // set throttle to different value + const c = createAlert({ throttle: '1h' }); + // set throttle to various empty/null/undefined states + const d = createAlert({ throttle: '' }); + const e = createAlert({ throttle: undefined }); + const f = createAlert({ throttle: null }); + + expect(hasAlertChanged(a, b, true)).toEqual(true); + expect(hasAlertChanged(a, c, true)).toEqual(true); + expect(hasAlertChanged(a, d, true)).toEqual(false); + expect(hasAlertChanged(a, e, true)).toEqual(false); + expect(hasAlertChanged(a, f, true)).toEqual(false); + + expect(hasAlertChanged(b, c, true)).toEqual(true); + expect(hasAlertChanged(b, d, true)).toEqual(true); + expect(hasAlertChanged(b, e, true)).toEqual(true); + expect(hasAlertChanged(b, f, true)).toEqual(true); + + expect(hasAlertChanged(c, d, true)).toEqual(true); + expect(hasAlertChanged(c, e, true)).toEqual(true); + expect(hasAlertChanged(c, f, true)).toEqual(true); + + expect(hasAlertChanged(d, e, true)).toEqual(false); + expect(hasAlertChanged(d, f, true)).toEqual(false); +}); + +test('should correctly compare tags field', () => { + const a = createAlert(); + const b = createAlert({ tags: ['first'] }); + + expect(hasAlertChanged(a, b, true)).toEqual(true); +}); + +test('should correctly compare schedule field', () => { + const a = createAlert(); + const b = createAlert({ schedule: { interval: '3h' } }); + + expect(hasAlertChanged(a, b, true)).toEqual(true); +}); + +test('should correctly compare actions field', () => { + const a = createAlert(); + const b = createAlert({ + actions: [{ actionTypeId: 'action', group: 'group', id: 'actionId', params: {} }], + }); + + expect(hasAlertChanged(a, b, true)).toEqual(true); +}); + +test('should skip comparing params field if compareParams=false', () => { + const a = createAlert(); + const b = createAlert({ params: { newParam: 'value' } }); + + expect(hasAlertChanged(a, b, false)).toEqual(false); +}); + +test('should correctly compare params field if compareParams=true', () => { + const a = createAlert(); + const b = createAlert({ params: { newParam: 'value' } }); + + expect(hasAlertChanged(a, b, true)).toEqual(true); +}); + +test('should correctly compare notifyWhen field', () => { + const a = createAlert(); + const b = createAlert({ notifyWhen: 'onActiveAlert' }); + + expect(hasAlertChanged(a, b, true)).toEqual(true); +}); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/has_alert_changed.ts b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/has_alert_changed.ts new file mode 100644 index 000000000000..806d4314dfe8 --- /dev/null +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/has_alert_changed.ts @@ -0,0 +1,40 @@ +/* + * 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 deepEqual from 'fast-deep-equal'; +import { pick } from 'lodash'; +import { AlertTypeParams } from '../../../types'; +import { InitialAlert } from './alert_reducer'; + +const DEEP_COMPARE_FIELDS = ['tags', 'schedule', 'actions', 'notifyWhen']; + +function getNonNullCompareFields(alert: InitialAlert) { + const { name, alertTypeId, throttle } = alert; + return { + ...(!!(name && name.length > 0) ? { name } : {}), + ...(!!(alertTypeId && alertTypeId.length > 0) ? { alertTypeId } : {}), + ...(!!(throttle && throttle.length > 0) ? { throttle } : {}), + }; +} + +export function hasAlertChanged(a: InitialAlert, b: InitialAlert, compareParams: boolean) { + // Deep compare these fields + let objectsAreEqual = deepEqual(pick(a, DEEP_COMPARE_FIELDS), pick(b, DEEP_COMPARE_FIELDS)); + if (compareParams) { + objectsAreEqual = objectsAreEqual && deepEqual(a.params, b.params); + } + + const nonNullCompareFieldsAreEqual = deepEqual( + getNonNullCompareFields(a), + getNonNullCompareFields(b) + ); + + return !objectsAreEqual || !nonNullCompareFieldsAreEqual; +} + +export function haveAlertParamsChanged(a: AlertTypeParams, b: AlertTypeParams) { + return !deepEqual(a, b); +} diff --git a/x-pack/plugins/triggers_actions_ui/public/types.ts b/x-pack/plugins/triggers_actions_ui/public/types.ts index f4dc4c3d4ef2..22969f6e4191 100644 --- a/x-pack/plugins/triggers_actions_ui/public/types.ts +++ b/x-pack/plugins/triggers_actions_ui/public/types.ts @@ -8,11 +8,12 @@ import type { DocLinksStart } from 'kibana/public'; import { ComponentType } from 'react'; import { ChartsPluginSetup } from 'src/plugins/charts/public'; import { DataPublicPluginStart } from 'src/plugins/data/public'; -import { ActionGroup, AlertActionParam, AlertTypeParams } from '../../alerts/common'; import { ActionType } from '../../actions/common'; import { TypeRegistry } from './application/type_registry'; import { AlertType as CommonAlertType } from '../../alerts/common'; import { + ActionGroup, + AlertActionParam, SanitizedAlert, AlertAction, AlertAggregations, @@ -22,6 +23,7 @@ import { RawAlertInstance, AlertingFrameworkHealth, AlertNotifyWhenType, + AlertTypeParams, } from '../../alerts/common'; // In Triggers and Actions we treat all `Alert`s as `SanitizedAlert` @@ -38,6 +40,7 @@ export { RawAlertInstance, AlertingFrameworkHealth, AlertNotifyWhenType, + AlertTypeParams, }; export { ActionType }; diff --git a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alert_create_flyout.ts b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alert_create_flyout.ts index bb56b1a7b526..352652d9601d 100644 --- a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alert_create_flyout.ts +++ b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alert_create_flyout.ts @@ -204,5 +204,18 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { const alertsToDelete = await getAlertsByName(alertName); await deleteAlerts(alertsToDelete.map((alertItem: { id: string }) => alertItem.id)); }); + + it('should show discard confirmation before closing flyout without saving', async () => { + await pageObjects.triggersActionsUI.clickCreateAlertButton(); + await testSubjects.click('cancelSaveAlertButton'); + await testSubjects.missingOrFail('confirmAlertCloseModal'); + + await pageObjects.triggersActionsUI.clickCreateAlertButton(); + await testSubjects.setValue('intervalInput', '10'); + await testSubjects.click('cancelSaveAlertButton'); + await testSubjects.existOrFail('confirmAlertCloseModal'); + await testSubjects.click('confirmAlertCloseModal > confirmModalCancelButton'); + await testSubjects.missingOrFail('confirmAlertCloseModal'); + }); }); }; diff --git a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details.ts b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details.ts index 4b81d317b84f..1ed0b38a238b 100644 --- a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details.ts +++ b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details.ts @@ -298,6 +298,8 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { }); await testSubjects.click('cancelSaveEditedAlertButton'); + await testSubjects.existOrFail('confirmAlertCloseModal'); + await testSubjects.click('confirmAlertCloseModal > confirmModalConfirmButton'); await find.waitForDeletedByCssSelector('[data-test-subj="cancelSaveEditedAlertButton"]'); await editButton.click();