From 8716b861308392d0e3e75b0598aa75b350797ec6 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Mon, 23 Nov 2020 16:14:23 +0000 Subject: [PATCH 01/21] renamed Resolved to Recovered --- .../alerts/common/builtin_action_groups.ts | 10 +++--- .../alerts/server/alert_type_registry.test.ts | 14 ++++---- ...rt_instance_summary_from_event_log.test.ts | 2 +- .../alert_instance_summary_from_event_log.ts | 2 +- x-pack/plugins/alerts/server/plugin.ts | 2 +- .../server/task_runner/task_runner.test.ts | 12 +++---- .../alerts/server/task_runner/task_runner.ts | 32 ++++++++++--------- .../email/email_params.tsx | 6 ++-- .../server_log/server_log_params.tsx | 6 ++-- .../slack/slack_params.tsx | 6 ++-- .../public/application/constants/index.ts | 6 ++-- .../action_form.test.tsx | 22 ++++++------- .../action_type_form.tsx | 10 +++--- 13 files changed, 66 insertions(+), 64 deletions(-) diff --git a/x-pack/plugins/alerts/common/builtin_action_groups.ts b/x-pack/plugins/alerts/common/builtin_action_groups.ts index d31f75357d370..d9c5ae613f787 100644 --- a/x-pack/plugins/alerts/common/builtin_action_groups.ts +++ b/x-pack/plugins/alerts/common/builtin_action_groups.ts @@ -6,13 +6,13 @@ import { i18n } from '@kbn/i18n'; import { ActionGroup } from './alert_type'; -export const ResolvedActionGroup: ActionGroup = { - id: 'resolved', - name: i18n.translate('xpack.alerts.builtinActionGroups.resolved', { - defaultMessage: 'Resolved', +export const RecoveredActionGroup: ActionGroup = { + id: 'recovered', + name: i18n.translate('xpack.alerts.builtinActionGroups.recovered', { + defaultMessage: 'Recovered', }), }; export function getBuiltinActionGroups(): ActionGroup[] { - return [ResolvedActionGroup]; + return [RecoveredActionGroup]; } diff --git a/x-pack/plugins/alerts/server/alert_type_registry.test.ts b/x-pack/plugins/alerts/server/alert_type_registry.test.ts index 8dc387f96addb..b04871a047e4b 100644 --- a/x-pack/plugins/alerts/server/alert_type_registry.test.ts +++ b/x-pack/plugins/alerts/server/alert_type_registry.test.ts @@ -105,8 +105,8 @@ describe('register()', () => { name: 'Default', }, { - id: 'resolved', - name: 'Resolved', + id: 'recovered', + name: 'Recovered', }, ], defaultActionGroupId: 'default', @@ -117,7 +117,7 @@ describe('register()', () => { expect(() => registry.register(alertType)).toThrowError( new Error( - `Alert type [id="${alertType.id}"] cannot be registered. Action groups [resolved] are reserved by the framework.` + `Alert type [id="${alertType.id}"] cannot be registered. Action groups [recovered] are reserved by the framework.` ) ); }); @@ -229,8 +229,8 @@ describe('get()', () => { "name": "Default", }, Object { - "id": "resolved", - "name": "Resolved", + "id": "recovered", + "name": "Recovered", }, ], "actionVariables": Object { @@ -287,8 +287,8 @@ describe('list()', () => { "name": "Test Action Group", }, Object { - "id": "resolved", - "name": "Resolved", + "id": "recovered", + "name": "Recovered", }, ], "actionVariables": Object { diff --git a/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts b/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts index f9e4a2908d6ce..7b45bcf736846 100644 --- a/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts +++ b/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts @@ -566,7 +566,7 @@ export class EventsFactory { '@timestamp': this.date, event: { provider: EVENT_LOG_PROVIDER, - action: EVENT_LOG_ACTIONS.resolvedInstance, + action: EVENT_LOG_ACTIONS.recoveredInstance, }, kibana: { alerting: { instance_id: instanceId } }, }); diff --git a/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.ts b/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.ts index 8fed97a74435d..9d6d9bed5aa93 100644 --- a/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.ts +++ b/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.ts @@ -80,7 +80,7 @@ export function alertInstanceSummaryFromEventLog( status.status = 'Active'; status.actionGroupId = event?.kibana?.alerting?.action_group_id; break; - case EVENT_LOG_ACTIONS.resolvedInstance: + case EVENT_LOG_ACTIONS.recoveredInstance: status.status = 'OK'; status.activeStartDate = undefined; status.actionGroupId = undefined; diff --git a/x-pack/plugins/alerts/server/plugin.ts b/x-pack/plugins/alerts/server/plugin.ts index 4bfb44425544a..ab41a33f2d4a4 100644 --- a/x-pack/plugins/alerts/server/plugin.ts +++ b/x-pack/plugins/alerts/server/plugin.ts @@ -82,7 +82,7 @@ export const EVENT_LOG_ACTIONS = { execute: 'execute', executeAction: 'execute-action', newInstance: 'new-instance', - resolvedInstance: 'resolved-instance', + recoveredInstance: 'recovered-instance', activeInstance: 'active-instance', }; diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts b/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts index 07d08f5837d54..d5218e7eea341 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts @@ -26,12 +26,12 @@ import { alertsMock, alertsClientMock } from '../mocks'; import { eventLoggerMock } from '../../../event_log/server/event_logger.mock'; import { IEventLogger } from '../../../event_log/server'; import { SavedObjectsErrorHelpers } from '../../../../../src/core/server'; -import { Alert, ResolvedActionGroup } from '../../common'; +import { Alert, RecoveredActionGroup } from '../../common'; import { omit } from 'lodash'; const alertType = { id: 'test', name: 'My test alert', - actionGroups: [{ id: 'default', name: 'Default' }, ResolvedActionGroup], + actionGroups: [{ id: 'default', name: 'Default' }, RecoveredActionGroup], defaultActionGroupId: 'default', executor: jest.fn(), producer: 'alerts', @@ -114,7 +114,7 @@ describe('Task Runner', () => { }, }, { - group: ResolvedActionGroup.id, + group: RecoveredActionGroup.id, id: '2', actionTypeId: 'action', params: { @@ -517,7 +517,7 @@ describe('Task Runner', () => { `); }); - test('fire resolved actions for execution for the alertInstances which is in the resolved state', async () => { + test('fire recovered actions for execution for the alertInstances which is in the recovered state', async () => { taskRunnerFactoryInitializerParams.actionsPlugin.isActionTypeEnabled.mockReturnValue(true); taskRunnerFactoryInitializerParams.actionsPlugin.isActionExecutable.mockReturnValue(true); @@ -650,7 +650,7 @@ describe('Task Runner', () => { Array [ Object { "event": Object { - "action": "resolved-instance", + "action": "recovered-instance", }, "kibana": Object { "alerting": Object { @@ -666,7 +666,7 @@ describe('Task Runner', () => { }, ], }, - "message": "test:1: 'alert-name' resolved instance: '2'", + "message": "test:1: 'alert-name' recovered instance: '2'", }, ], Array [ diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner.ts b/x-pack/plugins/alerts/server/task_runner/task_runner.ts index 24d96788c3395..624552ef015b8 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner.ts @@ -39,7 +39,7 @@ import { IEvent, IEventLogger, SAVED_OBJECT_REL_PRIMARY } from '../../../event_l import { isAlertSavedObjectNotFoundError } from '../lib/is_alert_not_found_error'; import { AlertsClient } from '../alerts_client'; import { partiallyUpdateAlert } from '../saved_objects'; -import { ResolvedActionGroup } from '../../common'; +import { RecoveredActionGroup } from '../../common'; const FALLBACK_RETRY_INTERVAL = '5m'; @@ -219,7 +219,7 @@ export class TaskRunner { alertInstance.hasScheduledActions() ); - generateNewAndResolvedInstanceEvents({ + generateNewAndRecoveredInstanceEvents({ eventLogger, originalAlertInstances, currentAlertInstances: instancesWithScheduledActions, @@ -229,7 +229,7 @@ export class TaskRunner { }); if (!muteAll) { - scheduleActionsForResolvedInstances( + scheduleActionsForRecoveredInstances( alertInstances, executionHandler, originalAlertInstances, @@ -436,7 +436,7 @@ export class TaskRunner { } } -interface GenerateNewAndResolvedInstanceEventsParams { +interface GenerateNewAndRecoveredInstanceEventsParams { eventLogger: IEventLogger; originalAlertInstances: Dictionary; currentAlertInstances: Dictionary; @@ -445,18 +445,20 @@ interface GenerateNewAndResolvedInstanceEventsParams { namespace: string | undefined; } -function generateNewAndResolvedInstanceEvents(params: GenerateNewAndResolvedInstanceEventsParams) { +function generateNewAndRecoveredInstanceEvents( + params: GenerateNewAndRecoveredInstanceEventsParams +) { const { eventLogger, alertId, namespace, currentAlertInstances, originalAlertInstances } = params; const originalAlertInstanceIds = Object.keys(originalAlertInstances); const currentAlertInstanceIds = Object.keys(currentAlertInstances); const newIds = without(currentAlertInstanceIds, ...originalAlertInstanceIds); - const resolvedIds = without(originalAlertInstanceIds, ...currentAlertInstanceIds); + const recoveredIds = without(originalAlertInstanceIds, ...currentAlertInstanceIds); - for (const id of resolvedIds) { + for (const id of recoveredIds) { const actionGroup = originalAlertInstances[id].getLastScheduledActions()?.group; - const message = `${params.alertLabel} resolved instance: '${id}'`; - logInstanceEvent(id, EVENT_LOG_ACTIONS.resolvedInstance, message, actionGroup); + const message = `${params.alertLabel} recovered instance: '${id}'`; + logInstanceEvent(id, EVENT_LOG_ACTIONS.recoveredInstance, message, actionGroup); } for (const id of newIds) { @@ -496,7 +498,7 @@ function generateNewAndResolvedInstanceEvents(params: GenerateNewAndResolvedInst } } -function scheduleActionsForResolvedInstances( +function scheduleActionsForRecoveredInstances( alertInstancesMap: Record, executionHandler: ReturnType, originalAlertInstances: Record, @@ -505,22 +507,22 @@ function scheduleActionsForResolvedInstances( ) { const currentAlertInstanceIds = Object.keys(currentAlertInstances); const originalAlertInstanceIds = Object.keys(originalAlertInstances); - const resolvedIds = without( + const recoveredIds = without( originalAlertInstanceIds, ...currentAlertInstanceIds, ...mutedInstanceIds ); - for (const id of resolvedIds) { + for (const id of recoveredIds) { const instance = alertInstancesMap[id]; - instance.updateLastScheduledActions(ResolvedActionGroup.id); + instance.updateLastScheduledActions(RecoveredActionGroup.id); instance.unscheduleActions(); executionHandler({ - actionGroup: ResolvedActionGroup.id, + actionGroup: RecoveredActionGroup.id, context: {}, state: {}, alertInstanceId: id, }); - instance.scheduleActions(ResolvedActionGroup.id); + instance.scheduleActions(RecoveredActionGroup.id); } } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.tsx index eacdf20747fdc..26ab5b21e167a 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.tsx @@ -11,7 +11,7 @@ import { ActionParamsProps } from '../../../../types'; import { EmailActionParams } from '../types'; import { TextFieldWithMessageVariables } from '../../text_field_with_message_variables'; import { TextAreaWithMessageVariables } from '../../text_area_with_message_variables'; -import { resolvedActionGroupMessage } from '../../../constants'; +import { recoveredActionGroupMessage } from '../../../constants'; export const EmailParamsFields = ({ actionParams, @@ -29,10 +29,10 @@ export const EmailParamsFields = ({ const [addBCC, setAddBCC] = useState(false); useEffect(() => { - if (defaultMessage === resolvedActionGroupMessage) { + if (defaultMessage === recoveredActionGroupMessage) { editAction('message', defaultMessage, index); } else if ( - (!message || message === resolvedActionGroupMessage) && + (!message || message === recoveredActionGroupMessage) && defaultMessage && defaultMessage.length > 0 ) { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx index c4f434f138747..cef58f2769373 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx @@ -9,7 +9,7 @@ import { EuiSelect, EuiFormRow } from '@elastic/eui'; import { ActionParamsProps } from '../../../../types'; import { ServerLogActionParams } from '.././types'; import { TextAreaWithMessageVariables } from '../../text_area_with_message_variables'; -import { resolvedActionGroupMessage } from '../../../constants'; +import { recoveredActionGroupMessage } from '../../../constants'; export const ServerLogParamsFields: React.FunctionComponent { - if (defaultMessage === resolvedActionGroupMessage) { + if (defaultMessage === recoveredActionGroupMessage) { editAction('message', defaultMessage, index); } else if ( - (!message || message === resolvedActionGroupMessage) && + (!message || message === recoveredActionGroupMessage) && defaultMessage && defaultMessage.length > 0 ) { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/slack/slack_params.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/slack/slack_params.tsx index d1498567218d3..cbc6da3607c19 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/slack/slack_params.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/slack/slack_params.tsx @@ -8,7 +8,7 @@ import { i18n } from '@kbn/i18n'; import { ActionParamsProps } from '../../../../types'; import { SlackActionParams } from '../types'; import { TextAreaWithMessageVariables } from '../../text_area_with_message_variables'; -import { resolvedActionGroupMessage } from '../../../constants'; +import { recoveredActionGroupMessage } from '../../../constants'; const SlackParamsFields: React.FunctionComponent> = ({ actionParams, @@ -20,10 +20,10 @@ const SlackParamsFields: React.FunctionComponent { const { message } = actionParams; useEffect(() => { - if (defaultMessage === resolvedActionGroupMessage) { + if (defaultMessage === recoveredActionGroupMessage) { editAction('message', defaultMessage, index); } else if ( - (!message || message === resolvedActionGroupMessage) && + (!message || message === recoveredActionGroupMessage) && defaultMessage && defaultMessage.length > 0 ) { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/constants/index.ts b/x-pack/plugins/triggers_actions_ui/public/application/constants/index.ts index 7af8e5ba88300..156f65f094342 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/constants/index.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/constants/index.ts @@ -16,10 +16,10 @@ export const routeToConnectors = `/connectors`; export const routeToAlerts = `/alerts`; export const routeToAlertDetails = `/alert/:alertId`; -export const resolvedActionGroupMessage = i18n.translate( - 'xpack.triggersActionsUI.sections.actionForm.ResolvedMessage', +export const recoveredActionGroupMessage = i18n.translate( + 'xpack.triggersActionsUI.sections.actionForm.RecoveredMessage', { - defaultMessage: 'Resolved', + defaultMessage: 'Recovered', } ); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx index 38c9687ae581e..06082441bedb4 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx @@ -10,7 +10,7 @@ import { act } from 'react-dom/test-utils'; import { actionTypeRegistryMock } from '../../action_type_registry.mock'; import { ValidationResult, Alert, AlertAction } from '../../../types'; import ActionForm from './action_form'; -import { ResolvedActionGroup } from '../../../../../alerts/common'; +import { RecoveredActionGroup } from '../../../../../alerts/common'; jest.mock('../../lib/action_connector_api', () => ({ loadAllActions: jest.fn(), loadActionTypes: jest.fn(), @@ -232,7 +232,7 @@ describe('action_form', () => { }} actionGroups={[ { id: 'default', name: 'Default' }, - { id: 'resolved', name: 'Resolved' }, + { id: 'recovered', name: 'Recovered' }, ]} setActionGroupIdByIndex={(group: string, index: number) => { initialAlert.actions[index].group = group; @@ -355,18 +355,18 @@ describe('action_form', () => { "value": "default", }, Object { - "data-test-subj": "addNewActionConnectorActionGroup-0-option-resolved", - "inputDisplay": "Resolved", - "value": "resolved", + "data-test-subj": "addNewActionConnectorActionGroup-0-option-recovered", + "inputDisplay": "Recovered", + "value": "recovered", }, ] `); }); - it('renders selected Resolved action group', async () => { + it('renders selected Recovered action group', async () => { const wrapper = await setup([ { - group: ResolvedActionGroup.id, + group: RecoveredActionGroup.id, id: 'test', actionTypeId: actionType.id, params: { @@ -389,14 +389,14 @@ describe('action_form', () => { "value": "default", }, Object { - "data-test-subj": "addNewActionConnectorActionGroup-0-option-resolved", - "inputDisplay": "Resolved", - "value": "resolved", + "data-test-subj": "addNewActionConnectorActionGroup-0-option-recovered", + "inputDisplay": "Recovered", + "value": "recovered", }, ] `); expect(actionGroupsSelect.first().text()).toEqual( - 'Select an option: Resolved, is selectedResolved' + 'Select an option: Recovered, is selectedResolved' ); }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx index 5f1798d101d94..9c906e492a846 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx @@ -25,7 +25,7 @@ import { EuiLoadingSpinner, EuiBadge, } from '@elastic/eui'; -import { AlertActionParam, ResolvedActionGroup } from '../../../../../alerts/common'; +import { AlertActionParam, RecoveredActionGroup } from '../../../../../alerts/common'; import { IErrorObject, AlertAction, @@ -38,7 +38,7 @@ import { checkActionFormActionTypeEnabled } from '../../lib/check_action_type_en import { hasSaveActionsCapability } from '../../lib/capabilities'; import { ActionAccordionFormProps } from './action_form'; import { transformActionVariables } from '../../lib/action_variables'; -import { resolvedActionGroupMessage } from '../../constants'; +import { recoveredActionGroupMessage } from '../../constants'; export type ActionTypeFormProps = { actionItem: AlertAction; @@ -106,8 +106,8 @@ export const ActionTypeForm = ({ useEffect(() => { setAvailableActionVariables(getAvailableActionVariables(messageVariables, actionItem.group)); const res = - actionItem.group === ResolvedActionGroup.id - ? resolvedActionGroupMessage + actionItem.group === RecoveredActionGroup.id + ? recoveredActionGroupMessage : defaultActionMessage; setAvailableDefaultActionMessage(res); // eslint-disable-next-line react-hooks/exhaustive-deps @@ -370,7 +370,7 @@ function getAvailableActionVariables( return []; } const filteredActionVariables = - actionGroup === ResolvedActionGroup.id + actionGroup === RecoveredActionGroup.id ? { params: actionVariables.params, state: actionVariables.state } : actionVariables; From 6dd664d5f711745b63386b1034d17e7c469b5d3b Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Mon, 23 Nov 2020 18:29:09 +0000 Subject: [PATCH 02/21] fixed missing import --- .../inventory_metric_threshold_executor.ts | 4 ++-- .../metric_threshold/metric_threshold_executor.test.ts | 6 +++--- .../alerting/metric_threshold/metric_threshold_executor.ts | 4 ++-- .../spaces_only/tests/alerting/alerts_base.ts | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts index 14785f64cffac..1941ec6326ddb 100644 --- a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts +++ b/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/inventory_metric_threshold_executor.ts @@ -9,7 +9,7 @@ import moment from 'moment'; import { getCustomMetricLabel } from '../../../../common/formatters/get_custom_metric_label'; import { toMetricOpt } from '../../../../common/snapshot_metric_i18n'; import { AlertStates, InventoryMetricConditions } from './types'; -import { ResolvedActionGroup } from '../../../../../alerts/common'; +import { RecoveredActionGroup } from '../../../../../alerts/common'; import { AlertExecutorOptions } from '../../../../../alerts/server'; import { InventoryItemType, SnapshotMetricType } from '../../../../common/inventory_models/types'; import { InfraBackendLibs } from '../../infra_types'; @@ -103,7 +103,7 @@ export const createInventoryMetricThresholdExecutor = (libs: InfraBackendLibs) = } if (reason) { const actionGroupId = - nextState === AlertStates.OK ? ResolvedActionGroup.id : FIRED_ACTIONS.id; + nextState === AlertStates.OK ? RecoveredActionGroup.id : FIRED_ACTIONS.id; alertInstance.scheduleActions(actionGroupId, { group: item, alertState: stateToAlertMessage[nextState], diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts index b31afba8ac9cc..a1d6428f3b52b 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts @@ -6,7 +6,7 @@ import { createMetricThresholdExecutor, FIRED_ACTIONS } from './metric_threshold_executor'; import { Comparator, AlertStates } from './types'; import * as mocks from './test_mocks'; -import { ResolvedActionGroup } from '../../../../../alerts/common'; +import { RecoveredActionGroup } from '../../../../../alerts/common'; import { AlertExecutorOptions } from '../../../../../alerts/server'; import { alertsMock, @@ -367,7 +367,7 @@ describe('The metric threshold alert type', () => { expect(mostRecentAction(instanceID).id).toBe(FIRED_ACTIONS.id); expect(getState(instanceID).alertState).toBe(AlertStates.ALERT); await execute([2]); - expect(mostRecentAction(instanceID).id).toBe(ResolvedActionGroup.id); + expect(mostRecentAction(instanceID).id).toBe(RecoveredActionGroup.id); expect(getState(instanceID).alertState).toBe(AlertStates.OK); }); test('does not continue to send a recovery alert if the metric is still OK', async () => { @@ -383,7 +383,7 @@ describe('The metric threshold alert type', () => { expect(mostRecentAction(instanceID).id).toBe(FIRED_ACTIONS.id); expect(getState(instanceID).alertState).toBe(AlertStates.ALERT); await execute([2]); - expect(mostRecentAction(instanceID).id).toBe(ResolvedActionGroup.id); + expect(mostRecentAction(instanceID).id).toBe(RecoveredActionGroup.id); expect(getState(instanceID).alertState).toBe(AlertStates.OK); }); }); diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts index 7c3918c37ebbf..60790648d9a9b 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts @@ -6,7 +6,7 @@ import { first, last } from 'lodash'; import { i18n } from '@kbn/i18n'; import moment from 'moment'; -import { ResolvedActionGroup } from '../../../../../alerts/common'; +import { RecoveredActionGroup } from '../../../../../alerts/common'; import { AlertExecutorOptions } from '../../../../../alerts/server'; import { InfraBackendLibs } from '../../infra_types'; import { @@ -89,7 +89,7 @@ export const createMetricThresholdExecutor = (libs: InfraBackendLibs) => const firstResult = first(alertResults); const timestamp = (firstResult && firstResult[group].timestamp) ?? moment().toISOString(); const actionGroupId = - nextState === AlertStates.OK ? ResolvedActionGroup.id : FIRED_ACTIONS.id; + nextState === AlertStates.OK ? RecoveredActionGroup.id : FIRED_ACTIONS.id; alertInstance.scheduleActions(actionGroupId, { group, alertState: stateToAlertMessage[nextState], diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts index 64e99190e183a..a314b02b4be88 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts @@ -6,7 +6,7 @@ import expect from '@kbn/expect'; import { Response as SupertestResponse } from 'supertest'; -import { ResolvedActionGroup } from '../../../../../plugins/alerts/common'; +import { RecoveredActionGroup } from '../../../../../plugins/alerts/common'; import { Space } from '../../../common/types'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; import { @@ -174,7 +174,7 @@ instanceStateValue: true params: {}, }, { - group: ResolvedActionGroup.id, + group: RecoveredActionGroup.id, id: indexRecordActionId, params: { index: ES_TEST_INDEX_NAME, @@ -237,7 +237,7 @@ instanceStateValue: true params: {}, }, { - group: ResolvedActionGroup.id, + group: RecoveredActionGroup.id, id: indexRecordActionId, params: { index: ES_TEST_INDEX_NAME, From 00f7ef3510638eefced17f6854d1f7710c1c3562 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Tue, 24 Nov 2020 12:23:43 +0000 Subject: [PATCH 03/21] fixed buggy default message behaviour --- .../email/email_params.tsx | 17 ++++++------ .../server_log/server_log_params.tsx | 26 +++++++++---------- .../slack/slack_params.tsx | 19 +++++++------- 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.tsx index eacdf20747fdc..1030ce34d2569 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.tsx @@ -11,7 +11,6 @@ import { ActionParamsProps } from '../../../../types'; import { EmailActionParams } from '../types'; import { TextFieldWithMessageVariables } from '../../text_field_with_message_variables'; import { TextAreaWithMessageVariables } from '../../text_area_with_message_variables'; -import { resolvedActionGroupMessage } from '../../../constants'; export const EmailParamsFields = ({ actionParams, @@ -28,17 +27,19 @@ export const EmailParamsFields = ({ const [addCC, setAddCC] = useState(false); const [addBCC, setAddBCC] = useState(false); + const [[isUsingDefault, defaultMessageUsed], setDefaultMessageUsage] = useState< + [boolean, string | undefined] + >([false, defaultMessage]); useEffect(() => { - if (defaultMessage === resolvedActionGroupMessage) { - editAction('message', defaultMessage, index); - } else if ( - (!message || message === resolvedActionGroupMessage) && - defaultMessage && - defaultMessage.length > 0 + if ( + !actionParams?.message || + (isUsingDefault && + actionParams?.message === defaultMessageUsed && + defaultMessageUsed !== defaultMessage) ) { + setDefaultMessageUsage([true, defaultMessage]); editAction('message', defaultMessage, index); } - // eslint-disable-next-line react-hooks/exhaustive-deps }, [defaultMessage]); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx index e8c427371c4a5..ea734439ef41a 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx @@ -3,17 +3,16 @@ * 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, { Fragment, useEffect } from 'react'; +import React, { Fragment, useEffect, useState } from 'react'; import { i18n } from '@kbn/i18n'; import { EuiSelect, EuiFormRow } from '@elastic/eui'; import { ActionParamsProps } from '../../../../types'; import { ServerLogActionParams } from '.././types'; import { TextAreaWithMessageVariables } from '../../text_area_with_message_variables'; -import { resolvedActionGroupMessage } from '../../../constants'; -export const ServerLogParamsFields: React.FunctionComponent< - ActionParamsProps -> = ({ actionParams, editAction, index, errors, messageVariables, defaultMessage }) => { +export const ServerLogParamsFields: React.FunctionComponent> = ({ actionParams, editAction, index, errors, messageVariables, defaultMessage }) => { const { message, level } = actionParams; const levelOptions = [ { value: 'trace', text: 'Trace' }, @@ -23,7 +22,6 @@ export const ServerLogParamsFields: React.FunctionComponent< { value: 'error', text: 'Error' }, { value: 'fatal', text: 'Fatal' }, ]; - useEffect(() => { if (!actionParams?.level) { editAction('level', 'info', index); @@ -31,17 +29,19 @@ export const ServerLogParamsFields: React.FunctionComponent< // eslint-disable-next-line react-hooks/exhaustive-deps }, []); + const [[isUsingDefault, defaultMessageUsed], setDefaultMessageUsage] = useState< + [boolean, string | undefined] + >([false, defaultMessage]); useEffect(() => { - if (defaultMessage === resolvedActionGroupMessage) { - editAction('message', defaultMessage, index); - } else if ( - (!message || message === resolvedActionGroupMessage) && - defaultMessage && - defaultMessage.length > 0 + if ( + !actionParams?.message || + (isUsingDefault && + actionParams?.message === defaultMessageUsed && + defaultMessageUsed !== defaultMessage) ) { + setDefaultMessageUsage([true, defaultMessage]); editAction('message', defaultMessage, index); } - // eslint-disable-next-line react-hooks/exhaustive-deps }, [defaultMessage]); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/slack/slack_params.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/slack/slack_params.tsx index d1498567218d3..40818c327af60 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/slack/slack_params.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/slack/slack_params.tsx @@ -3,12 +3,11 @@ * 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, { useEffect } from 'react'; +import React, { useEffect, useState } from 'react'; import { i18n } from '@kbn/i18n'; import { ActionParamsProps } from '../../../../types'; import { SlackActionParams } from '../types'; import { TextAreaWithMessageVariables } from '../../text_area_with_message_variables'; -import { resolvedActionGroupMessage } from '../../../constants'; const SlackParamsFields: React.FunctionComponent> = ({ actionParams, @@ -19,17 +18,19 @@ const SlackParamsFields: React.FunctionComponent { const { message } = actionParams; + const [[isUsingDefault, defaultMessageUsed], setDefaultMessageUsage] = useState< + [boolean, string | undefined] + >([false, defaultMessage]); useEffect(() => { - if (defaultMessage === resolvedActionGroupMessage) { - editAction('message', defaultMessage, index); - } else if ( - (!message || message === resolvedActionGroupMessage) && - defaultMessage && - defaultMessage.length > 0 + if ( + !actionParams?.message || + (isUsingDefault && + actionParams?.message === defaultMessageUsed && + defaultMessageUsed !== defaultMessage) ) { + setDefaultMessageUsage([true, defaultMessage]); editAction('message', defaultMessage, index); } - // eslint-disable-next-line react-hooks/exhaustive-deps }, [defaultMessage]); From f9c7560cd96df8a2fcac209f423da2173114daf0 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Tue, 24 Nov 2020 15:26:31 +0000 Subject: [PATCH 04/21] added missing test --- .../email/email_params.test.tsx | 93 +++++++++++++++++ .../server_log/server_log_params.test.tsx | 99 +++++++++++++++++-- .../server_log/server_log_params.tsx | 8 +- .../triggers_actions_ui/public/types.ts | 2 +- 4 files changed, 189 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.test.tsx index 3cd54b58bf29a..1020b784f3c58 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.test.tsx @@ -33,4 +33,97 @@ describe('EmailParamsFields renders', () => { expect(wrapper.find('[data-test-subj="subjectInput"]').length > 0).toBeTruthy(); expect(wrapper.find('[data-test-subj="messageTextArea"]').length > 0).toBeTruthy(); }); + + test('message param field is rendered with default value if not set', () => { + const actionParams = { + cc: [], + bcc: [], + to: ['test@test.com'], + subject: 'test', + }; + + const editAction = jest.fn(); + const wrapper = mountWithIntl( + + ); + + expect(editAction).toHaveBeenCalledWith('message', 'Some default message', 0); + }); + + test('when the default message changes, so is the underlying message if it was set by the previous default', () => { + const actionParams = { + cc: [], + bcc: [], + to: ['test@test.com'], + subject: 'test', + }; + + const editAction = jest.fn(); + const wrapper = mountWithIntl( + + ); + + expect(editAction).toHaveBeenCalledWith('message', 'Some default message', 0); + + wrapper.setProps({ + defaultMessage: 'Some different default message', + }); + + expect(editAction).toHaveBeenCalledWith('message', 'Some different default message', 0); + }); + + test('when the default message changes, it doesnt change the underlying message if it wasnt set by a previous default', () => { + const actionParams = { + cc: [], + bcc: [], + to: ['test@test.com'], + subject: 'test', + }; + + const editAction = jest.fn(); + const wrapper = mountWithIntl( + + ); + + expect(editAction).toHaveBeenCalledWith('message', 'Some default message', 0); + + // simulate value being updated + const valueToSimulate = 'some new value'; + wrapper + .find('[data-test-subj="messageTextArea"]') + .first() + .simulate('change', { target: { value: valueToSimulate } }); + expect(editAction).toHaveBeenCalledWith('message', valueToSimulate, 0); + wrapper.setProps({ + actionParams: { + ...actionParams, + message: valueToSimulate, + }, + }); + + // simulate default changing + wrapper.setProps({ + defaultMessage: 'Some different default message', + }); + + expect(editAction).not.toHaveBeenCalledWith('message', 'Some different default message', 0); + }); }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.test.tsx index 0552a126ca66f..86dd304bf6b12 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.test.tsx @@ -9,8 +9,8 @@ import { ServerLogLevelOptions } from '.././types'; import ServerLogParamsFields from './server_log_params'; describe('ServerLogParamsFields renders', () => { - const editAction = jest.fn(); test('all params fields is rendered', () => { + const editAction = jest.fn(); const actionParams = { level: ServerLogLevelOptions.TRACE, message: 'test', @@ -35,20 +35,103 @@ describe('ServerLogParamsFields renders', () => { test('level param field is rendered with default value if not selected', () => { const actionParams = { message: 'test message', - level: ServerLogLevelOptions.INFO, }; + const editAction = jest.fn(); + + mountWithIntl( + + ); + + expect(editAction).toHaveBeenCalledWith('level', 'info', 0); + }); + + test('message param field is rendered with default value if not set', () => { + const actionParams = { + level: ServerLogLevelOptions.TRACE, + }; + + const editAction = jest.fn(); + + mountWithIntl( + + ); + + expect(editAction).toHaveBeenCalledWith('message', 'Some default message', 0); + }); + + test('when the default message changes, so is the underlying message if it was set by the previous default', () => { + const actionParams = { + level: ServerLogLevelOptions.TRACE, + }; + + const editAction = jest.fn(); const wrapper = mountWithIntl( {}} + editAction={editAction} index={0} /> ); - expect(wrapper.find('[data-test-subj="loggingLevelSelect"]').length > 0).toBeTruthy(); - expect( - wrapper.find('[data-test-subj="loggingLevelSelect"]').first().prop('value') - ).toStrictEqual('info'); - expect(wrapper.find('[data-test-subj="messageTextArea"]').length > 0).toBeTruthy(); + + expect(editAction).toHaveBeenCalledWith('message', 'Some default message', 0); + + wrapper.setProps({ + defaultMessage: 'Some different default message', + }); + + expect(editAction).toHaveBeenCalledWith('message', 'Some different default message', 0); + }); + + test('when the default message changes, it doesnt change the underlying message if it wasnt set by a previous default', () => { + const actionParams = { + level: ServerLogLevelOptions.TRACE, + }; + + const editAction = jest.fn(); + const wrapper = mountWithIntl( + + ); + + expect(editAction).toHaveBeenCalledWith('message', 'Some default message', 0); + + // simulate value being updated + const valueToSimulate = 'some new value'; + wrapper + .find('[data-test-subj="messageTextArea"]') + .first() + .simulate('change', { target: { value: valueToSimulate } }); + expect(editAction).toHaveBeenCalledWith('message', valueToSimulate, 0); + wrapper.setProps({ + actionParams: { + ...actionParams, + message: valueToSimulate, + }, + }); + + // simulate default changing + wrapper.setProps({ + defaultMessage: 'Some different default message', + }); + + expect(editAction).not.toHaveBeenCalledWith('message', 'Some different default message', 0); }); }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx index ea734439ef41a..ce426c72b64b4 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/server_log/server_log_params.tsx @@ -10,9 +10,9 @@ import { ActionParamsProps } from '../../../../types'; import { ServerLogActionParams } from '.././types'; import { TextAreaWithMessageVariables } from '../../text_area_with_message_variables'; -export const ServerLogParamsFields: React.FunctionComponent> = ({ actionParams, editAction, index, errors, messageVariables, defaultMessage }) => { +export const ServerLogParamsFields: React.FunctionComponent< + ActionParamsProps +> = ({ actionParams, editAction, index, errors, messageVariables, defaultMessage }) => { const { message, level } = actionParams; const levelOptions = [ { value: 'trace', text: 'Trace' }, @@ -23,7 +23,7 @@ export const ServerLogParamsFields: React.FunctionComponent { - if (!actionParams?.level) { + if (!actionParams.level) { editAction('level', 'info', index); } // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/x-pack/plugins/triggers_actions_ui/public/types.ts b/x-pack/plugins/triggers_actions_ui/public/types.ts index be8b7b9757e9e..a4eac1ab1da21 100644 --- a/x-pack/plugins/triggers_actions_ui/public/types.ts +++ b/x-pack/plugins/triggers_actions_ui/public/types.ts @@ -48,7 +48,7 @@ export interface ActionConnectorFieldsProps { } export interface ActionParamsProps { - actionParams: TParams; + actionParams: Partial; index: number; editAction: (key: string, value: AlertActionParam, index: number) => void; errors: IErrorObject; From b1eb47b114a525774438335cdc8d2aa3e2511893 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Thu, 26 Nov 2020 09:51:31 +0000 Subject: [PATCH 05/21] fixed typing --- .../components/builtin_action_types/email/email_params.test.tsx | 2 +- .../components/builtin_action_types/jira/jira_params.tsx | 2 +- .../builtin_action_types/jira/use_get_fields_by_issue_type.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.test.tsx index 1020b784f3c58..6ead65d958bd5 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email/email_params.test.tsx @@ -43,7 +43,7 @@ describe('EmailParamsFields renders', () => { }; const editAction = jest.fn(); - const wrapper = mountWithIntl( + mountWithIntl( - {hasParent && ( + {hasParent && parent && ( <> diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/use_get_fields_by_issue_type.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/use_get_fields_by_issue_type.tsx index 8685ee1e615b0..6129f42923e45 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/use_get_fields_by_issue_type.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/use_get_fields_by_issue_type.tsx @@ -23,7 +23,7 @@ interface Props { ToastsApi, 'get$' | 'add' | 'remove' | 'addSuccess' | 'addWarning' | 'addDanger' | 'addError' >; - issueType: string; + issueType: string | undefined; actionConnector?: ActionConnector; } From 0d5771658929d0bf3eba73217dd7ccbfe77c7389 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Thu, 26 Nov 2020 09:54:18 +0000 Subject: [PATCH 06/21] fixed resolved in tests --- .../tests/alerting/list_alert_types.ts | 4 ++-- .../spaces_only/tests/alerting/alerts_base.ts | 10 +++++----- .../spaces_only/tests/alerting/event_log.ts | 8 ++++---- .../tests/alerting/get_alert_instance_summary.ts | 2 +- .../spaces_only/tests/alerting/list_alert_types.ts | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts index b3635b9f40e27..bfaf8a2a4788e 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts @@ -17,7 +17,7 @@ export default function listAlertTypes({ getService }: FtrProviderContext) { const expectedNoOpType = { actionGroups: [ { id: 'default', name: 'Default' }, - { id: 'resolved', name: 'Resolved' }, + { id: 'recovered', name: 'Recovered' }, ], defaultActionGroupId: 'default', id: 'test.noop', @@ -33,7 +33,7 @@ export default function listAlertTypes({ getService }: FtrProviderContext) { const expectedRestrictedNoOpType = { actionGroups: [ { id: 'default', name: 'Default' }, - { id: 'resolved', name: 'Resolved' }, + { id: 'recovered', name: 'Recovered' }, ], defaultActionGroupId: 'default', id: 'test.restricted-noop', diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts index a314b02b4be88..8dab199271da8 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts @@ -137,7 +137,7 @@ instanceStateValue: true await taskManagerUtils.waitForActionTaskParamsToBeCleanedUp(testStart); }); - it('should fire actions when an alert instance is resolved', async () => { + it('should fire actions when an alert instance is recovered', async () => { const reference = alertUtils.generateReference(); const { body: createdAction } = await supertestWithoutAuth @@ -179,7 +179,7 @@ instanceStateValue: true params: { index: ES_TEST_INDEX_NAME, reference, - message: 'Resolved message', + message: 'Recovered message', }, }, ], @@ -194,10 +194,10 @@ instanceStateValue: true await esTestIndexTool.waitForDocs('action:test.index-record', reference) )[0]; - expect(actionTestRecord._source.params.message).to.eql('Resolved message'); + expect(actionTestRecord._source.params.message).to.eql('Recovered message'); }); - it('should not fire actions when an alert instance is resolved, but alert is muted', async () => { + it('should not fire actions when an alert instance is recovered, but alert is muted', async () => { const testStart = new Date(); const reference = alertUtils.generateReference(); @@ -242,7 +242,7 @@ instanceStateValue: true params: { index: ES_TEST_INDEX_NAME, reference, - message: 'Resolved message', + message: 'Recovered message', }, }, ], diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts index 937045b6218c6..f12cb59acbb7a 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts @@ -77,7 +77,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) { 'execute-action', 'new-instance', 'active-instance', - 'resolved-instance', + 'recovered-instance', ], }); }); @@ -86,7 +86,7 @@ export default function eventLogTests({ getService }: FtrProviderContext) { const executeEvents = getEventsByAction(events, 'execute'); const executeActionEvents = getEventsByAction(events, 'execute-action'); const newInstanceEvents = getEventsByAction(events, 'new-instance'); - const resolvedInstanceEvents = getEventsByAction(events, 'resolved-instance'); + const resolvedInstanceEvents = getEventsByAction(events, 'recovered-instance'); expect(executeEvents.length >= 4).to.be(true); expect(executeActionEvents.length).to.be(2); @@ -135,8 +135,8 @@ export default function eventLogTests({ getService }: FtrProviderContext) { case 'new-instance': validateInstanceEvent(event, `created new instance: 'instance'`); break; - case 'resolved-instance': - validateInstanceEvent(event, `resolved instance: 'instance'`); + case 'recovered-instance': + validateInstanceEvent(event, `recovered instance: 'instance'`); break; case 'active-instance': validateInstanceEvent(event, `active instance: 'instance' in actionGroup: 'default'`); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get_alert_instance_summary.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get_alert_instance_summary.ts index 22034328e5275..404c6020fa237 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get_alert_instance_summary.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get_alert_instance_summary.ts @@ -216,7 +216,7 @@ export default function createGetAlertInstanceSummaryTests({ getService }: FtrPr await alertUtils.muteInstance(createdAlert.id, 'instanceC'); await alertUtils.muteInstance(createdAlert.id, 'instanceD'); - await waitForEvents(createdAlert.id, ['new-instance', 'resolved-instance']); + await waitForEvents(createdAlert.id, ['new-instance', 'recovered-instance']); const response = await supertest.get( `${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert/${createdAlert.id}/_instance_summary` ); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/list_alert_types.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/list_alert_types.ts index 3fb2cc40437d8..9d38f4abb7f3f 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/list_alert_types.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/list_alert_types.ts @@ -25,7 +25,7 @@ export default function listAlertTypes({ getService }: FtrProviderContext) { expect(fixtureAlertType).to.eql({ actionGroups: [ { id: 'default', name: 'Default' }, - { id: 'resolved', name: 'Resolved' }, + { id: 'recovered', name: 'Recovered' }, ], defaultActionGroupId: 'default', id: 'test.noop', From 50e612e016096cadae478653be953cdeaa72b8d6 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Thu, 26 Nov 2020 12:53:03 +0000 Subject: [PATCH 07/21] allows alert types to specify their own custom recovery group name --- .../server/alert_types/always_firing.ts | 1 + .../server/alert_types/astros.ts | 1 + .../alerts/common/builtin_action_groups.ts | 6 +-- .../alerts/server/alert_type_registry.test.ts | 31 +++++++++++++++ .../alerts/server/alert_type_registry.ts | 32 ++++++++++----- .../task_runner/create_execution_handler.ts | 6 ++- .../transform_action_params.test.ts | 39 +++++++++++++++++++ .../task_runner/transform_action_params.ts | 3 ++ x-pack/plugins/alerts/server/types.ts | 1 + .../application/lib/action_variables.test.ts | 16 ++++++++ .../application/lib/action_variables.ts | 11 ++++++ .../action_type_form.tsx | 26 +++++-------- .../alerts_restricted/server/alert_types.ts | 1 + .../tests/alerting/list_alert_types.ts | 2 +- 14 files changed, 144 insertions(+), 32 deletions(-) diff --git a/x-pack/examples/alerting_example/server/alert_types/always_firing.ts b/x-pack/examples/alerting_example/server/alert_types/always_firing.ts index 1900f55a51a55..693489598e3ef 100644 --- a/x-pack/examples/alerting_example/server/alert_types/always_firing.ts +++ b/x-pack/examples/alerting_example/server/alert_types/always_firing.ts @@ -41,6 +41,7 @@ export const alertType: AlertType = { name: 'Always firing', actionGroups: ACTION_GROUPS, defaultActionGroupId: DEFAULT_ACTION_GROUP, + recoveryActionGroupName: 'Has landed back on Earth', async executor({ services, params: { instances = DEFAULT_INSTANCES_TO_GENERATE, thresholds }, diff --git a/x-pack/examples/alerting_example/server/alert_types/astros.ts b/x-pack/examples/alerting_example/server/alert_types/astros.ts index 852e6f57d1106..934b6e67d1f90 100644 --- a/x-pack/examples/alerting_example/server/alert_types/astros.ts +++ b/x-pack/examples/alerting_example/server/alert_types/astros.ts @@ -43,6 +43,7 @@ export const alertType: AlertType = { name: 'People In Space Right Now', actionGroups: [{ id: 'default', name: 'default' }], defaultActionGroupId: 'default', + recoveryActionGroupName: 'Has landed back on Earth', async executor({ services, params }) { const { outerSpaceCapacity, craft: craftToTriggerBy, op } = params; diff --git a/x-pack/plugins/alerts/common/builtin_action_groups.ts b/x-pack/plugins/alerts/common/builtin_action_groups.ts index d9c5ae613f787..e23bbcc54b24d 100644 --- a/x-pack/plugins/alerts/common/builtin_action_groups.ts +++ b/x-pack/plugins/alerts/common/builtin_action_groups.ts @@ -6,13 +6,13 @@ import { i18n } from '@kbn/i18n'; import { ActionGroup } from './alert_type'; -export const RecoveredActionGroup: ActionGroup = { +export const RecoveredActionGroup: Readonly = { id: 'recovered', name: i18n.translate('xpack.alerts.builtinActionGroups.recovered', { defaultMessage: 'Recovered', }), }; -export function getBuiltinActionGroups(): ActionGroup[] { - return [RecoveredActionGroup]; +export function getBuiltinActionGroups(customRecoveryGroup?: ActionGroup): ActionGroup[] { + return [customRecoveryGroup ?? Object.freeze(RecoveredActionGroup)]; } diff --git a/x-pack/plugins/alerts/server/alert_type_registry.test.ts b/x-pack/plugins/alerts/server/alert_type_registry.test.ts index b04871a047e4b..bcdf413c4e9a6 100644 --- a/x-pack/plugins/alerts/server/alert_type_registry.test.ts +++ b/x-pack/plugins/alerts/server/alert_type_registry.test.ts @@ -122,6 +122,37 @@ describe('register()', () => { ); }); + test('allows an AlertType to specify a custom recovery group name', () => { + const alertType = { + id: 'test', + name: 'Test', + actionGroups: [ + { + id: 'default', + name: 'Default', + }, + ], + defaultActionGroupId: 'default', + recoveryActionGroupName: 'Back To Awesome', + executor: jest.fn(), + producer: 'alerts', + }; + const registry = new AlertTypeRegistry(alertTypeRegistryParams); + registry.register(alertType); + expect(registry.get('test').actionGroups).toMatchInlineSnapshot(` + Array [ + Object { + "id": "default", + "name": "Default", + }, + Object { + "id": "recovered", + "name": "Back To Awesome", + }, + ] + `); + }); + test('registers the executor with the task manager', () => { const alertType = { id: 'test', diff --git a/x-pack/plugins/alerts/server/alert_type_registry.ts b/x-pack/plugins/alerts/server/alert_type_registry.ts index 8fe2ab06acd9a..629e670000f46 100644 --- a/x-pack/plugins/alerts/server/alert_type_registry.ts +++ b/x-pack/plugins/alerts/server/alert_type_registry.ts @@ -8,8 +8,7 @@ import Boom from '@hapi/boom'; import { i18n } from '@kbn/i18n'; import { schema } from '@kbn/config-schema'; import typeDetect from 'type-detect'; -import { intersection } from 'lodash'; -import _ from 'lodash'; +import { intersection, defaults } from 'lodash'; import { RunContext, TaskManagerSetupContract } from '../../task_manager/server'; import { TaskRunnerFactory } from './task_runner'; import { @@ -20,7 +19,7 @@ import { AlertInstanceContext, ActionGroup, } from './types'; -import { getBuiltinActionGroups } from '../common'; +import { RecoveredActionGroup, getBuiltinActionGroups } from '../common'; interface ConstructorOptions { taskManager: TaskManagerSetupContract; @@ -86,8 +85,15 @@ export class AlertTypeRegistry { ); } alertType.actionVariables = normalizedActionVariables(alertType.actionVariables); - validateActionGroups(alertType.id, alertType.actionGroups); - alertType.actionGroups = [...alertType.actionGroups, ..._.cloneDeep(getBuiltinActionGroups())]; + + alertType.actionGroups = validateActionGroups( + alertType.id, + alertType.actionGroups, + getBuiltinActionGroups( + defaults({ name: alertType.recoveryActionGroupName }, RecoveredActionGroup) + ) + ); + this.alertTypes.set(alertIdSchema.validate(alertType.id), { ...alertType } as AlertType); this.taskManager.registerTaskDefinitions({ [`alerting:${alertType.id}`]: { @@ -144,21 +150,27 @@ function normalizedActionVariables(actionVariables: AlertType['actionVariables'] }; } -function validateActionGroups(alertTypeId: string, actionGroups: ActionGroup[]) { - const reservedActionGroups = intersection( +function validateActionGroups( + alertTypeId: string, + actionGroups: ActionGroup[], + reservedActionGroups: ActionGroup[] +) { + const intersectingReservedActionGroups = intersection( actionGroups.map((item) => item.id), - getBuiltinActionGroups().map((item) => item.id) + reservedActionGroups.map((item) => item.id) ); - if (reservedActionGroups.length > 0) { + if (intersectingReservedActionGroups.length > 0) { throw new Error( i18n.translate('xpack.alerts.alertTypeRegistry.register.reservedActionGroupUsageError', { defaultMessage: 'Alert type [id="{alertTypeId}"] cannot be registered. Action groups [{actionGroups}] are reserved by the framework.', values: { - actionGroups: reservedActionGroups.join(', '), + actionGroups: intersectingReservedActionGroups.join(', '), alertTypeId, }, }) ); } + + return [...actionGroups, ...reservedActionGroups]; } diff --git a/x-pack/plugins/alerts/server/task_runner/create_execution_handler.ts b/x-pack/plugins/alerts/server/task_runner/create_execution_handler.ts index ccd1f6c20ba52..3d68ba3adbd6b 100644 --- a/x-pack/plugins/alerts/server/task_runner/create_execution_handler.ts +++ b/x-pack/plugins/alerts/server/task_runner/create_execution_handler.ts @@ -4,7 +4,6 @@ * you may not use this file except in compliance with the Elastic License. */ -import { map } from 'lodash'; import { Logger, KibanaRequest } from '../../../../../src/core/server'; import { transformActionParams } from './transform_action_params'; import { @@ -58,7 +57,9 @@ export function createExecutionHandler({ request, alertParams, }: CreateExecutionHandlerOptions) { - const alertTypeActionGroups = new Set(map(alertType.actionGroups, 'id')); + const alertTypeActionGroups = new Map( + alertType.actionGroups.map((actionGroup) => [actionGroup.id, actionGroup.name]) + ); return async ({ actionGroup, context, state, alertInstanceId }: ExecutionHandlerOptions) => { if (!alertTypeActionGroups.has(actionGroup)) { logger.error(`Invalid action group "${actionGroup}" for alert "${alertType.id}".`); @@ -76,6 +77,7 @@ export function createExecutionHandler({ tags, alertInstanceId, alertActionGroup: actionGroup, + alertActionGroupName: alertTypeActionGroups.get(actionGroup)!, context, actionParams: action.params, state, diff --git a/x-pack/plugins/alerts/server/task_runner/transform_action_params.test.ts b/x-pack/plugins/alerts/server/task_runner/transform_action_params.test.ts index 9a4cfbbca792d..782b9fc07207b 100644 --- a/x-pack/plugins/alerts/server/task_runner/transform_action_params.test.ts +++ b/x-pack/plugins/alerts/server/task_runner/transform_action_params.test.ts @@ -25,6 +25,7 @@ test('skips non string parameters', () => { spaceId: 'spaceId-A', alertInstanceId: '2', alertActionGroup: 'action-group', + alertActionGroupName: 'Action Group', alertParams: { foo: 'test', }, @@ -56,6 +57,7 @@ test('missing parameters get emptied out', () => { spaceId: 'spaceId-A', alertInstanceId: '2', alertActionGroup: 'action-group', + alertActionGroupName: 'Action Group', alertParams: {}, }); expect(result).toMatchInlineSnapshot(` @@ -80,6 +82,7 @@ test('context parameters are passed to templates', () => { spaceId: 'spaceId-A', alertInstanceId: '2', alertActionGroup: 'action-group', + alertActionGroupName: 'Action Group', alertParams: {}, }); expect(result).toMatchInlineSnapshot(` @@ -103,6 +106,7 @@ test('state parameters are passed to templates', () => { spaceId: 'spaceId-A', alertInstanceId: '2', alertActionGroup: 'action-group', + alertActionGroupName: 'Action Group', alertParams: {}, }); expect(result).toMatchInlineSnapshot(` @@ -126,6 +130,7 @@ test('alertId is passed to templates', () => { spaceId: 'spaceId-A', alertInstanceId: '2', alertActionGroup: 'action-group', + alertActionGroupName: 'Action Group', alertParams: {}, }); expect(result).toMatchInlineSnapshot(` @@ -149,6 +154,7 @@ test('alertName is passed to templates', () => { spaceId: 'spaceId-A', alertInstanceId: '2', alertActionGroup: 'action-group', + alertActionGroupName: 'Action Group', alertParams: {}, }); expect(result).toMatchInlineSnapshot(` @@ -172,6 +178,7 @@ test('tags is passed to templates', () => { spaceId: 'spaceId-A', alertInstanceId: '2', alertActionGroup: 'action-group', + alertActionGroupName: 'Action Group', alertParams: {}, }); expect(result).toMatchInlineSnapshot(` @@ -194,6 +201,7 @@ test('undefined tags is passed to templates', () => { spaceId: 'spaceId-A', alertInstanceId: '2', alertActionGroup: 'action-group', + alertActionGroupName: 'Action Group', alertParams: {}, }); expect(result).toMatchInlineSnapshot(` @@ -217,6 +225,7 @@ test('empty tags is passed to templates', () => { spaceId: 'spaceId-A', alertInstanceId: '2', alertActionGroup: 'action-group', + alertActionGroupName: 'Action Group', alertParams: {}, }); expect(result).toMatchInlineSnapshot(` @@ -240,6 +249,7 @@ test('spaceId is passed to templates', () => { spaceId: 'spaceId-A', alertInstanceId: '2', alertActionGroup: 'action-group', + alertActionGroupName: 'Action Group', alertParams: {}, }); expect(result).toMatchInlineSnapshot(` @@ -263,6 +273,7 @@ test('alertInstanceId is passed to templates', () => { spaceId: 'spaceId-A', alertInstanceId: '2', alertActionGroup: 'action-group', + alertActionGroupName: 'Action Group', alertParams: {}, }); expect(result).toMatchInlineSnapshot(` @@ -286,6 +297,7 @@ test('alertActionGroup is passed to templates', () => { spaceId: 'spaceId-A', alertInstanceId: '2', alertActionGroup: 'action-group', + alertActionGroupName: 'Action Group', alertParams: {}, }); expect(result).toMatchInlineSnapshot(` @@ -295,6 +307,30 @@ test('alertActionGroup is passed to templates', () => { `); }); +test('alertActionGroupName is passed to templates', () => { + const actionParams = { + message: 'Value "{{alertActionGroupName}}" exists', + }; + const result = transformActionParams({ + actionParams, + state: {}, + context: {}, + alertId: '1', + alertName: 'alert-name', + tags: ['tag-A', 'tag-B'], + spaceId: 'spaceId-A', + alertInstanceId: '2', + alertActionGroup: 'action-group', + alertActionGroupName: 'Action Group', + alertParams: {}, + }); + expect(result).toMatchInlineSnapshot(` + Object { + "message": "Value \\"Action Group\\" exists", + } + `); +}); + test('date is passed to templates', () => { const actionParams = { message: '{{date}}', @@ -310,6 +346,7 @@ test('date is passed to templates', () => { spaceId: 'spaceId-A', alertInstanceId: '2', alertActionGroup: 'action-group', + alertActionGroupName: 'Action Group', alertParams: {}, }); const dateAfter = Date.now(); @@ -335,6 +372,7 @@ test('works recursively', () => { spaceId: 'spaceId-A', alertInstanceId: '2', alertActionGroup: 'action-group', + alertActionGroupName: 'Action Group', alertParams: {}, }); expect(result).toMatchInlineSnapshot(` @@ -362,6 +400,7 @@ test('works recursively with arrays', () => { spaceId: 'spaceId-A', alertInstanceId: '2', alertActionGroup: 'action-group', + alertActionGroupName: 'Action Group', alertParams: {}, }); expect(result).toMatchInlineSnapshot(` diff --git a/x-pack/plugins/alerts/server/task_runner/transform_action_params.ts b/x-pack/plugins/alerts/server/task_runner/transform_action_params.ts index b02285d56aa9a..fa4a5b7f1b4ab 100644 --- a/x-pack/plugins/alerts/server/task_runner/transform_action_params.ts +++ b/x-pack/plugins/alerts/server/task_runner/transform_action_params.ts @@ -20,6 +20,7 @@ interface TransformActionParamsOptions { tags?: string[]; alertInstanceId: string; alertActionGroup: string; + alertActionGroupName: string; actionParams: AlertActionParams; alertParams: AlertTypeParams; state: AlertInstanceState; @@ -33,6 +34,7 @@ export function transformActionParams({ tags, alertInstanceId, alertActionGroup, + alertActionGroupName, context, actionParams, state, @@ -51,6 +53,7 @@ export function transformActionParams({ tags, alertInstanceId, alertActionGroup, + alertActionGroupName, context, date: new Date().toISOString(), state, diff --git a/x-pack/plugins/alerts/server/types.ts b/x-pack/plugins/alerts/server/types.ts index 500c681a1d2b9..f1166d47799e3 100644 --- a/x-pack/plugins/alerts/server/types.ts +++ b/x-pack/plugins/alerts/server/types.ts @@ -96,6 +96,7 @@ export interface AlertType< }; actionGroups: ActionGroup[]; defaultActionGroupId: ActionGroup['id']; + recoveryActionGroupName?: ActionGroup['name']; executor: ({ services, params, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.test.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.test.ts index 6317896a5ecd2..165a231cd1d67 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.test.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.test.ts @@ -43,6 +43,10 @@ describe('transformActionVariables', () => { "description": "The alert action group that was used to scheduled actions for the alert.", "name": "alertActionGroup", }, + Object { + "description": "The human readable name of the alert action group that was used to scheduled actions for the alert.", + "name": "alertActionGroupName", + }, ] `); }); @@ -86,6 +90,10 @@ describe('transformActionVariables', () => { "description": "The alert action group that was used to scheduled actions for the alert.", "name": "alertActionGroup", }, + Object { + "description": "The human readable name of the alert action group that was used to scheduled actions for the alert.", + "name": "alertActionGroupName", + }, Object { "description": "foo-description", "name": "context.foo", @@ -137,6 +145,10 @@ describe('transformActionVariables', () => { "description": "The alert action group that was used to scheduled actions for the alert.", "name": "alertActionGroup", }, + Object { + "description": "The human readable name of the alert action group that was used to scheduled actions for the alert.", + "name": "alertActionGroupName", + }, Object { "description": "foo-description", "name": "state.foo", @@ -191,6 +203,10 @@ describe('transformActionVariables', () => { "description": "The alert action group that was used to scheduled actions for the alert.", "name": "alertActionGroup", }, + Object { + "description": "The human readable name of the alert action group that was used to scheduled actions for the alert.", + "name": "alertActionGroupName", + }, Object { "description": "fooC-description", "name": "context.fooC", diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.ts index 296185211d043..4bcf003b5f587 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.ts @@ -79,5 +79,16 @@ function getAlwaysProvidedActionVariables(): ActionVariable[] { }), }); + result.push({ + name: 'alertActionGroupName', + description: i18n.translate( + 'xpack.triggersActionsUI.actionVariables.alertActionGroupNameLabel', + { + defaultMessage: + 'The human readable name of the alert action group that was used to scheduled actions for the alert.', + } + ), + }); + return result; } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx index 345631c7348c3..72ee7ceda6372 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx @@ -102,12 +102,14 @@ export const ActionTypeForm = ({ >(undefined); useEffect(() => { - setAvailableActionVariables(getAvailableActionVariables(messageVariables, actionItem.group)); - const res = + setAvailableActionVariables( + messageVariables ? getAvailableActionVariables(messageVariables, actionItem.group) : [] + ); + setAvailableDefaultActionMessage( actionItem.group === RecoveredActionGroup.id ? recoveredActionGroupMessage - : defaultActionMessage; - setAvailableDefaultActionMessage(res); + : defaultActionMessage + ); // eslint-disable-next-line react-hooks/exhaustive-deps }, [actionItem.group]); @@ -360,18 +362,10 @@ export const ActionTypeForm = ({ }; function getAvailableActionVariables( - actionVariables: ActionVariables | undefined, + { params, state, context }: ActionVariables, actionGroup: string ) { - if (!actionVariables) { - return []; - } - const filteredActionVariables = - actionGroup === RecoveredActionGroup.id - ? { params: actionVariables.params, state: actionVariables.state } - : actionVariables; - - return transformActionVariables(filteredActionVariables).sort((a, b) => - a.name.toUpperCase().localeCompare(b.name.toUpperCase()) - ); + return transformActionVariables( + actionGroup === RecoveredActionGroup.id ? { params, state } : { params, state, context } + ).sort((a, b) => a.name.toUpperCase().localeCompare(b.name.toUpperCase())); } diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts_restricted/server/alert_types.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts_restricted/server/alert_types.ts index 00d06c30aa910..968b0e9dd8d14 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts_restricted/server/alert_types.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts_restricted/server/alert_types.ts @@ -18,6 +18,7 @@ export function defineAlertTypes( actionGroups: [{ id: 'default', name: 'Default' }], producer: 'alertsRestrictedFixture', defaultActionGroupId: 'default', + recoveryActionGroupName: 'Restricted Recovery', async executor({ services, params, state }: AlertExecutorOptions) {}, }; const noopUnrestrictedAlertType: AlertType = { diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts index bfaf8a2a4788e..df469c4cbca9e 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts @@ -33,7 +33,7 @@ export default function listAlertTypes({ getService }: FtrProviderContext) { const expectedRestrictedNoOpType = { actionGroups: [ { id: 'default', name: 'Default' }, - { id: 'recovered', name: 'Recovered' }, + { id: 'recovered', name: 'Restricted Recovery' }, ], defaultActionGroupId: 'default', id: 'test.restricted-noop', From 095159afcdda7f57d2fbde18ff9f9bb4025953df Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Thu, 26 Nov 2020 15:25:50 +0000 Subject: [PATCH 08/21] removed unnecesery field on always fires --- .../alerting_example/server/alert_types/always_firing.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/examples/alerting_example/server/alert_types/always_firing.ts b/x-pack/examples/alerting_example/server/alert_types/always_firing.ts index 693489598e3ef..1900f55a51a55 100644 --- a/x-pack/examples/alerting_example/server/alert_types/always_firing.ts +++ b/x-pack/examples/alerting_example/server/alert_types/always_firing.ts @@ -41,7 +41,6 @@ export const alertType: AlertType = { name: 'Always firing', actionGroups: ACTION_GROUPS, defaultActionGroupId: DEFAULT_ACTION_GROUP, - recoveryActionGroupName: 'Has landed back on Earth', async executor({ services, params: { instances = DEFAULT_INSTANCES_TO_GENERATE, thresholds }, From 0ea276f205b4651188f67c63ba00d01d2eba52c7 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Thu, 26 Nov 2020 18:17:14 +0000 Subject: [PATCH 09/21] allows alert types to specify their own custom recovery group --- .../server/alert_types/astros.ts | 5 +- x-pack/plugins/alerts/common/alert_type.ts | 1 + .../alerts/server/alert_type_registry.test.ts | 40 ++++++- .../alerts/server/alert_type_registry.ts | 105 +++++++++++++----- .../server/task_runner/task_runner.test.ts | 104 +++++++++++++++++ .../alerts/server/task_runner/task_runner.ts | 16 +-- .../server/task_runner/task_runner_factory.ts | 5 +- x-pack/plugins/alerts/server/types.ts | 2 +- .../action_form.test.tsx | 44 +------- .../action_connector_form/action_form.tsx | 18 +-- .../action_type_form.tsx | 36 +++--- .../sections/alert_form/alert_form.tsx | 23 ++-- .../triggers_actions_ui/public/types.ts | 14 ++- .../alerts_restricted/server/alert_types.ts | 2 +- 14 files changed, 287 insertions(+), 128 deletions(-) diff --git a/x-pack/examples/alerting_example/server/alert_types/astros.ts b/x-pack/examples/alerting_example/server/alert_types/astros.ts index 934b6e67d1f90..f0f47adffa109 100644 --- a/x-pack/examples/alerting_example/server/alert_types/astros.ts +++ b/x-pack/examples/alerting_example/server/alert_types/astros.ts @@ -43,7 +43,10 @@ export const alertType: AlertType = { name: 'People In Space Right Now', actionGroups: [{ id: 'default', name: 'default' }], defaultActionGroupId: 'default', - recoveryActionGroupName: 'Has landed back on Earth', + recoveryActionGroup: { + id: 'hasLandedBackOnEarth', + name: 'Has landed back on Earth', + }, async executor({ services, params }) { const { outerSpaceCapacity, craft: craftToTriggerBy, op } = params; diff --git a/x-pack/plugins/alerts/common/alert_type.ts b/x-pack/plugins/alerts/common/alert_type.ts index f07555c334074..a06c6d2fd5af2 100644 --- a/x-pack/plugins/alerts/common/alert_type.ts +++ b/x-pack/plugins/alerts/common/alert_type.ts @@ -8,6 +8,7 @@ export interface AlertType { id: string; name: string; actionGroups: ActionGroup[]; + recoveryActionGroup: ActionGroup; actionVariables: string[]; defaultActionGroupId: ActionGroup['id']; producer: string; diff --git a/x-pack/plugins/alerts/server/alert_type_registry.test.ts b/x-pack/plugins/alerts/server/alert_type_registry.test.ts index bcdf413c4e9a6..99bf76c7f81ad 100644 --- a/x-pack/plugins/alerts/server/alert_type_registry.test.ts +++ b/x-pack/plugins/alerts/server/alert_type_registry.test.ts @@ -122,7 +122,7 @@ describe('register()', () => { ); }); - test('allows an AlertType to specify a custom recovery group name', () => { + test('allows an AlertType to specify a custom recovery group', () => { const alertType = { id: 'test', name: 'Test', @@ -133,7 +133,10 @@ describe('register()', () => { }, ], defaultActionGroupId: 'default', - recoveryActionGroupName: 'Back To Awesome', + recoveryActionGroup: { + id: 'backToAwesome', + name: 'Back To Awesome', + }, executor: jest.fn(), producer: 'alerts', }; @@ -146,13 +149,44 @@ describe('register()', () => { "name": "Default", }, Object { - "id": "recovered", + "id": "backToAwesome", "name": "Back To Awesome", }, ] `); }); + test('throws if the custom recovery group is contained in the AlertType action groups', () => { + const alertType = { + id: 'test', + name: 'Test', + actionGroups: [ + { + id: 'default', + name: 'Default', + }, + { + id: 'backToAwesome', + name: 'Back To Awesome', + }, + ], + recoveryActionGroup: { + id: 'backToAwesome', + name: 'Back To Awesome', + }, + defaultActionGroupId: 'default', + executor: jest.fn(), + producer: 'alerts', + }; + const registry = new AlertTypeRegistry(alertTypeRegistryParams); + + expect(() => registry.register(alertType)).toThrowError( + new Error( + `Alert type [id="${alertType.id}"] cannot be registered. Action group [backToAwesome] cannot be used as both a recovery and an active action group.` + ) + ); + }); + test('registers the executor with the task manager', () => { const alertType = { id: 'test', diff --git a/x-pack/plugins/alerts/server/alert_type_registry.ts b/x-pack/plugins/alerts/server/alert_type_registry.ts index 629e670000f46..fe3df5e9dbe74 100644 --- a/x-pack/plugins/alerts/server/alert_type_registry.ts +++ b/x-pack/plugins/alerts/server/alert_type_registry.ts @@ -6,9 +6,10 @@ import Boom from '@hapi/boom'; import { i18n } from '@kbn/i18n'; +// import { Required } from 'utility-types'; import { schema } from '@kbn/config-schema'; import typeDetect from 'type-detect'; -import { intersection, defaults } from 'lodash'; +import { intersection } from 'lodash'; import { RunContext, TaskManagerSetupContract } from '../../task_manager/server'; import { TaskRunnerFactory } from './task_runner'; import { @@ -17,7 +18,6 @@ import { AlertTypeState, AlertInstanceState, AlertInstanceContext, - ActionGroup, } from './types'; import { RecoveredActionGroup, getBuiltinActionGroups } from '../common'; @@ -28,8 +28,13 @@ interface ConstructorOptions { export interface RegistryAlertType extends Pick< - AlertType, - 'name' | 'actionGroups' | 'defaultActionGroupId' | 'actionVariables' | 'producer' + NormalizedAlertType, + | 'name' + | 'actionGroups' + | 'recoveryActionGroup' + | 'defaultActionGroupId' + | 'actionVariables' + | 'producer' > { id: string; } @@ -54,9 +59,17 @@ const alertIdSchema = schema.string({ }, }); +export type NormalizedAlertType< + Params extends AlertTypeParams = AlertTypeParams, + State extends AlertTypeState = AlertTypeState, + InstanceState extends AlertInstanceState = AlertInstanceState, + InstanceContext extends AlertInstanceContext = AlertInstanceContext +> = Omit, 'recoveryActionGroup'> & + Pick>, 'recoveryActionGroup'>; + export class AlertTypeRegistry { private readonly taskManager: TaskManagerSetupContract; - private readonly alertTypes: Map = new Map(); + private readonly alertTypes: Map = new Map(); private readonly taskRunnerFactory: TaskRunnerFactory; constructor({ taskManager, taskRunnerFactory }: ConstructorOptions) { @@ -86,20 +99,14 @@ export class AlertTypeRegistry { } alertType.actionVariables = normalizedActionVariables(alertType.actionVariables); - alertType.actionGroups = validateActionGroups( - alertType.id, - alertType.actionGroups, - getBuiltinActionGroups( - defaults({ name: alertType.recoveryActionGroupName }, RecoveredActionGroup) - ) - ); + const normalizedAlertType = augmentActionGroupsWithReserved(alertType as AlertType); - this.alertTypes.set(alertIdSchema.validate(alertType.id), { ...alertType } as AlertType); + this.alertTypes.set(alertIdSchema.validate(alertType.id), normalizedAlertType); this.taskManager.registerTaskDefinitions({ [`alerting:${alertType.id}`]: { title: alertType.name, createTaskRunner: (context: RunContext) => - this.taskRunnerFactory.create({ ...alertType } as AlertType, context), + this.taskRunnerFactory.create(normalizedAlertType, context), }, }); } @@ -109,7 +116,7 @@ export class AlertTypeRegistry { State extends AlertTypeState = AlertTypeState, InstanceState extends AlertInstanceState = AlertInstanceState, InstanceContext extends AlertInstanceContext = AlertInstanceContext - >(id: string): AlertType { + >(id: string): NormalizedAlertType { if (!this.has(id)) { throw Boom.badRequest( i18n.translate('xpack.alerts.alertTypeRegistry.get.missingAlertTypeError', { @@ -120,19 +127,32 @@ export class AlertTypeRegistry { }) ); } - return this.alertTypes.get(id)! as AlertType; + return this.alertTypes.get(id)! as NormalizedAlertType< + Params, + State, + InstanceState, + InstanceContext + >; } public list(): Set { return new Set( Array.from(this.alertTypes).map( - ([id, { name, actionGroups, defaultActionGroupId, actionVariables, producer }]: [ - string, - AlertType - ]) => ({ + ([ + id, + { + name, + actionGroups, + recoveryActionGroup, + defaultActionGroupId, + actionVariables, + producer, + }, + ]: [string, NormalizedAlertType]) => ({ id, name, actionGroups, + recoveryActionGroup, defaultActionGroupId, actionVariables, producer, @@ -150,27 +170,52 @@ function normalizedActionVariables(actionVariables: AlertType['actionVariables'] }; } -function validateActionGroups( - alertTypeId: string, - actionGroups: ActionGroup[], - reservedActionGroups: ActionGroup[] -) { +function augmentActionGroupsWithReserved< + Params extends AlertTypeParams, + State extends AlertTypeState, + InstanceState extends AlertInstanceState, + InstanceContext extends AlertInstanceContext +>( + alertType: AlertType +): NormalizedAlertType { + const reservedActionGroups = getBuiltinActionGroups(alertType.recoveryActionGroup); + const { id, actionGroups, recoveryActionGroup } = alertType; + + const activeActionGroups = new Set(actionGroups.map((item) => item.id)); const intersectingReservedActionGroups = intersection( - actionGroups.map((item) => item.id), + [...activeActionGroups.values()], reservedActionGroups.map((item) => item.id) ); - if (intersectingReservedActionGroups.length > 0) { + if (recoveryActionGroup && activeActionGroups.has(recoveryActionGroup.id)) { + throw new Error( + i18n.translate( + 'xpack.alerts.alertTypeRegistry.register.customRecoveryActionGroupUsageError', + { + defaultMessage: + 'Alert type [id="{id}"] cannot be registered. Action group [{actionGroup}] cannot be used as both a recovery and an active action group.', + values: { + actionGroup: recoveryActionGroup.id, + id, + }, + } + ) + ); + } else if (intersectingReservedActionGroups.length > 0) { throw new Error( i18n.translate('xpack.alerts.alertTypeRegistry.register.reservedActionGroupUsageError', { defaultMessage: - 'Alert type [id="{alertTypeId}"] cannot be registered. Action groups [{actionGroups}] are reserved by the framework.', + 'Alert type [id="{id}"] cannot be registered. Action groups [{actionGroups}] are reserved by the framework.', values: { actionGroups: intersectingReservedActionGroups.join(', '), - alertTypeId, + id, }, }) ); } - return [...actionGroups, ...reservedActionGroups]; + return { + ...alertType, + actionGroups: [...actionGroups, ...reservedActionGroups], + recoveryActionGroup: recoveryActionGroup ?? RecoveredActionGroup, + }; } diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts b/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts index d5218e7eea341..4bb3e94c23476 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts @@ -33,6 +33,7 @@ const alertType = { name: 'My test alert', actionGroups: [{ id: 'default', name: 'Default' }, RecoveredActionGroup], defaultActionGroupId: 'default', + recoveryActionGroup: RecoveredActionGroup, executor: jest.fn(), producer: 'alerts', }; @@ -590,6 +591,109 @@ describe('Task Runner', () => { `); }); + test('fire actions under a custom recovery group when specified on an alert type for alertInstances which are in the recovered state', async () => { + taskRunnerFactoryInitializerParams.actionsPlugin.isActionTypeEnabled.mockReturnValue(true); + taskRunnerFactoryInitializerParams.actionsPlugin.isActionExecutable.mockReturnValue(true); + + const recoveryActionGroup = { + id: 'customRecovered', + name: 'Custom Recovered', + }; + const alertTypeWithCustomRecovery = { + ...alertType, + recoveryActionGroup, + actionGroups: [{ id: 'default', name: 'Default' }, recoveryActionGroup], + }; + + alertTypeWithCustomRecovery.executor.mockImplementation( + ({ services: executorServices }: AlertExecutorOptions) => { + executorServices.alertInstanceFactory('1').scheduleActions('default'); + } + ); + const taskRunner = new TaskRunner( + alertTypeWithCustomRecovery, + { + ...mockedTaskInstance, + state: { + ...mockedTaskInstance.state, + alertInstances: { + '1': { meta: {}, state: { bar: false } }, + '2': { meta: {}, state: { bar: false } }, + }, + }, + }, + taskRunnerFactoryInitializerParams + ); + alertsClient.get.mockResolvedValue({ + ...mockedAlertTypeSavedObject, + actions: [ + { + group: 'default', + id: '1', + actionTypeId: 'action', + params: { + foo: true, + }, + }, + { + group: recoveryActionGroup.id, + id: '2', + actionTypeId: 'action', + params: { + isResolved: true, + }, + }, + ], + }); + encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValue({ + id: '1', + type: 'alert', + attributes: { + apiKey: Buffer.from('123:abc').toString('base64'), + }, + references: [], + }); + const runnerResult = await taskRunner.run(); + expect(runnerResult.state.alertInstances).toMatchInlineSnapshot(` + Object { + "1": Object { + "meta": Object { + "lastScheduledActions": Object { + "date": 1970-01-01T00:00:00.000Z, + "group": "default", + }, + }, + "state": Object { + "bar": false, + }, + }, + } + `); + + const eventLogger = taskRunnerFactoryInitializerParams.eventLogger; + expect(eventLogger.logEvent).toHaveBeenCalledTimes(5); + expect(actionsClient.enqueueExecution).toHaveBeenCalledTimes(2); + expect(actionsClient.enqueueExecution.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "apiKey": "MTIzOmFiYw==", + "id": "2", + "params": Object { + "isResolved": true, + }, + "source": Object { + "source": Object { + "id": "1", + "type": "alert", + }, + "type": "SAVED_OBJECT", + }, + "spaceId": undefined, + }, + ] + `); + }); + test('persists alertInstances passed in from state, only if they are scheduled for execution', async () => { alertType.executor.mockImplementation( ({ services: executorServices }: AlertExecutorOptions) => { diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner.ts b/x-pack/plugins/alerts/server/task_runner/task_runner.ts index 624552ef015b8..8c6e2ca5a1c6b 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner.ts @@ -20,7 +20,6 @@ import { ErrorWithReason, } from '../lib'; import { - AlertType, RawAlert, IntervalSchedule, Services, @@ -39,7 +38,8 @@ import { IEvent, IEventLogger, SAVED_OBJECT_REL_PRIMARY } from '../../../event_l import { isAlertSavedObjectNotFoundError } from '../lib/is_alert_not_found_error'; import { AlertsClient } from '../alerts_client'; import { partiallyUpdateAlert } from '../saved_objects'; -import { RecoveredActionGroup } from '../../common'; +import { ActionGroup } from '../../common'; +import { NormalizedAlertType } from '../alert_type_registry'; const FALLBACK_RETRY_INTERVAL = '5m'; @@ -58,10 +58,10 @@ export class TaskRunner { private context: TaskRunnerContext; private logger: Logger; private taskInstance: AlertTaskInstance; - private alertType: AlertType; + private alertType: NormalizedAlertType; constructor( - alertType: AlertType, + alertType: NormalizedAlertType, taskInstance: ConcreteTaskInstance, context: TaskRunnerContext ) { @@ -230,6 +230,7 @@ export class TaskRunner { if (!muteAll) { scheduleActionsForRecoveredInstances( + this.alertType.recoveryActionGroup, alertInstances, executionHandler, originalAlertInstances, @@ -499,6 +500,7 @@ function generateNewAndRecoveredInstanceEvents( } function scheduleActionsForRecoveredInstances( + recoveryActionGroup: ActionGroup, alertInstancesMap: Record, executionHandler: ReturnType, originalAlertInstances: Record, @@ -514,15 +516,15 @@ function scheduleActionsForRecoveredInstances( ); for (const id of recoveredIds) { const instance = alertInstancesMap[id]; - instance.updateLastScheduledActions(RecoveredActionGroup.id); + instance.updateLastScheduledActions(recoveryActionGroup.id); instance.unscheduleActions(); executionHandler({ - actionGroup: RecoveredActionGroup.id, + actionGroup: recoveryActionGroup.id, context: {}, state: {}, alertInstanceId: id, }); - instance.scheduleActions(RecoveredActionGroup.id); + instance.scheduleActions(recoveryActionGroup.id); } } diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner_factory.ts b/x-pack/plugins/alerts/server/task_runner/task_runner_factory.ts index 2a2d74c1fc259..405afbf53c075 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner_factory.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner_factory.ts @@ -13,10 +13,11 @@ import { import { RunContext } from '../../../task_manager/server'; import { EncryptedSavedObjectsClient } from '../../../encrypted_saved_objects/server'; import { PluginStartContract as ActionsPluginStartContract } from '../../../actions/server'; -import { AlertType, GetServicesFunction, SpaceIdToNamespaceFunction } from '../types'; +import { GetServicesFunction, SpaceIdToNamespaceFunction } from '../types'; import { TaskRunner } from './task_runner'; import { IEventLogger } from '../../../event_log/server'; import { AlertsClient } from '../alerts_client'; +import { NormalizedAlertType } from '../alert_type_registry'; export interface TaskRunnerContext { logger: Logger; @@ -42,7 +43,7 @@ export class TaskRunnerFactory { this.taskRunnerContext = taskRunnerContext; } - public create(alertType: AlertType, { taskInstance }: RunContext) { + public create(alertType: NormalizedAlertType, { taskInstance }: RunContext) { if (!this.isInitialized) { throw new Error('TaskRunnerFactory not initialized'); } diff --git a/x-pack/plugins/alerts/server/types.ts b/x-pack/plugins/alerts/server/types.ts index f1166d47799e3..7847b6f6249a8 100644 --- a/x-pack/plugins/alerts/server/types.ts +++ b/x-pack/plugins/alerts/server/types.ts @@ -96,7 +96,7 @@ export interface AlertType< }; actionGroups: ActionGroup[]; defaultActionGroupId: ActionGroup['id']; - recoveryActionGroupName?: ActionGroup['name']; + recoveryActionGroup?: ActionGroup; executor: ({ services, params, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx index 9f4d8f5676d9f..5b2c8bd63a2f5 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx @@ -10,7 +10,6 @@ import { act } from 'react-dom/test-utils'; import { actionTypeRegistryMock } from '../../action_type_registry.mock'; import { ValidationResult, Alert, AlertAction } from '../../../types'; import ActionForm from './action_form'; -import { RecoveredActionGroup } from '../../../../../alerts/common'; import { useKibana } from '../../../common/lib/kibana'; jest.mock('../../../common/lib/kibana'); jest.mock('../../lib/action_connector_api', () => ({ @@ -211,6 +210,7 @@ describe('action_form', () => { mutedInstanceIds: [], } as unknown) as Alert; + const defaultActionMessage = 'Alert [{{context.metadata.name}}] has exceeded the threshold'; const wrapper = mountWithIntl( { initialAlert.actions[index].id = id; }} actionGroups={[ - { id: 'default', name: 'Default' }, + { id: 'default', name: 'Default', defaultActionMessage }, { id: 'recovered', name: 'Recovered' }, ]} setActionGroupIdByIndex={(group: string, index: number) => { initialAlert.actions[index].group = group; }} - setAlertProperty={(_updatedActions: AlertAction[]) => {}} + setActions={(_updatedActions: AlertAction[]) => {}} setActionParamsProperty={(key: string, value: any, index: number) => (initialAlert.actions[index] = { ...initialAlert.actions[index], [key]: value }) } actionTypeRegistry={actionTypeRegistry} setHasActionsWithBrokenConnector={setHasActionsWithBrokenConnector} - defaultActionMessage={'Alert [{{ctx.metadata.name}}] has exceeded the threshold'} actionTypes={[ { id: actionType.id, @@ -355,43 +354,6 @@ describe('action_form', () => { `); }); - it('renders selected Recovered action group', async () => { - const wrapper = await setup([ - { - group: RecoveredActionGroup.id, - id: 'test', - actionTypeId: actionType.id, - params: { - message: '', - }, - }, - ]); - const actionOption = wrapper.find( - `[data-test-subj="${actionType.id}-ActionTypeSelectOption"]` - ); - actionOption.first().simulate('click'); - const actionGroupsSelect = wrapper.find( - `[data-test-subj="addNewActionConnectorActionGroup-0"]` - ); - expect((actionGroupsSelect.first().props() as any).options).toMatchInlineSnapshot(` - Array [ - Object { - "data-test-subj": "addNewActionConnectorActionGroup-0-option-default", - "inputDisplay": "Default", - "value": "default", - }, - Object { - "data-test-subj": "addNewActionConnectorActionGroup-0-option-recovered", - "inputDisplay": "Recovered", - "value": "recovered", - }, - ] - `); - expect(actionGroupsSelect.first().text()).toEqual( - 'Select an option: Recovered, is selectedResolved' - ); - }); - it('renders available connectors for the selected action type', async () => { const wrapper = await setup(); const actionOption = wrapper.find( diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx index d62b8e7694089..b5758539d8b25 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx @@ -38,17 +38,21 @@ import { VIEW_LICENSE_OPTIONS_LINK, DEFAULT_HIDDEN_ACTION_TYPES } from '../../.. import { ActionGroup, AlertActionParam } from '../../../../../alerts/common'; import { useKibana } from '../../../common/lib/kibana'; +export interface ActionGroupWithMessageVariables extends ActionGroup { + omitOptionalMessageVariables?: boolean; + defaultActionMessage?: string; +} + export interface ActionAccordionFormProps { actions: AlertAction[]; defaultActionGroupId: string; - actionGroups?: ActionGroup[]; + actionGroups?: ActionGroupWithMessageVariables[]; setActionIdByIndex: (id: string, index: number) => void; setActionGroupIdByIndex?: (group: string, index: number) => void; - setAlertProperty: (actions: AlertAction[]) => void; + setActions: (actions: AlertAction[]) => void; setActionParamsProperty: (key: string, value: AlertActionParam, index: number) => void; actionTypes?: ActionType[]; messageVariables?: ActionVariables; - defaultActionMessage?: string; setHasActionsDisabled?: (value: boolean) => void; setHasActionsWithBrokenConnector?: (value: boolean) => void; actionTypeRegistry: ActionTypeRegistryContract; @@ -65,11 +69,10 @@ export const ActionForm = ({ actionGroups, setActionIdByIndex, setActionGroupIdByIndex, - setAlertProperty, + setActions, setActionParamsProperty, actionTypes, messageVariables, - defaultActionMessage, setHasActionsDisabled, setHasActionsWithBrokenConnector, actionTypeRegistry, @@ -303,7 +306,7 @@ export const ActionForm = ({ const updatedActions = actions.filter( (_item: AlertAction, i: number) => i !== index ); - setAlertProperty(updatedActions); + setActions(updatedActions); setIsAddActionPanelOpen( updatedActions.filter((item: AlertAction) => item.id !== actionItem.id) .length === 0 @@ -333,7 +336,6 @@ export const ActionForm = ({ actionTypesIndex={actionTypesIndex} connectors={connectors} defaultActionGroupId={defaultActionGroupId} - defaultActionMessage={defaultActionMessage} messageVariables={messageVariables} actionGroups={actionGroups} setActionGroupIdByIndex={setActionGroupIdByIndex} @@ -349,7 +351,7 @@ export const ActionForm = ({ const updatedActions = actions.filter( (_item: AlertAction, i: number) => i !== index ); - setAlertProperty(updatedActions); + setActions(updatedActions); setIsAddActionPanelOpen( updatedActions.filter((item: AlertAction) => item.id !== actionItem.id).length === 0 diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx index 72ee7ceda6372..c3471a2d48024 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx @@ -26,7 +26,8 @@ import { EuiBadge, EuiErrorBoundary, } from '@elastic/eui'; -import { AlertActionParam, RecoveredActionGroup } from '../../../../../alerts/common'; +import { pick } from 'lodash'; +import { AlertActionParam } from '../../../../../alerts/common'; import { IErrorObject, AlertAction, @@ -35,12 +36,12 @@ import { ActionVariables, ActionVariable, ActionTypeRegistryContract, + REQUIRED_ACTION_VARIABLES, } from '../../../types'; import { checkActionFormActionTypeEnabled } from '../../lib/check_action_type_enabled'; import { hasSaveActionsCapability } from '../../lib/capabilities'; -import { ActionAccordionFormProps } from './action_form'; +import { ActionAccordionFormProps, ActionGroupWithMessageVariables } from './action_form'; import { transformActionVariables } from '../../lib/action_variables'; -import { recoveredActionGroupMessage } from '../../constants'; import { useKibana } from '../../../common/lib/kibana'; export type ActionTypeFormProps = { @@ -64,7 +65,6 @@ export type ActionTypeFormProps = { | 'setActionGroupIdByIndex' | 'setActionParamsProperty' | 'messageVariables' - | 'defaultActionMessage' >; const preconfiguredMessage = i18n.translate( @@ -86,7 +86,6 @@ export const ActionTypeForm = ({ actionTypesIndex, connectors, defaultActionGroupId, - defaultActionMessage, messageVariables, actionGroups, setActionGroupIdByIndex, @@ -97,18 +96,13 @@ export const ActionTypeForm = ({ } = useKibana().services; const [isOpen, setIsOpen] = useState(true); const [availableActionVariables, setAvailableActionVariables] = useState([]); - const [availableDefaultActionMessage, setAvailableDefaultActionMessage] = useState< - string | undefined - >(undefined); + const defaultActionGroup = actionGroups?.find(({ id }) => id === defaultActionGroupId); + const selectedActionGroup = + actionGroups?.find(({ id }) => id === actionItem.group) ?? defaultActionGroup; useEffect(() => { setAvailableActionVariables( - messageVariables ? getAvailableActionVariables(messageVariables, actionItem.group) : [] - ); - setAvailableDefaultActionMessage( - actionItem.group === RecoveredActionGroup.id - ? recoveredActionGroupMessage - : defaultActionMessage + messageVariables ? getAvailableActionVariables(messageVariables, selectedActionGroup) : [] ); // eslint-disable-next-line react-hooks/exhaustive-deps }, [actionItem.group]); @@ -162,10 +156,6 @@ export const ActionTypeForm = ({ connectors.filter((connector) => connector.isPreconfigured) ); - const defaultActionGroup = actionGroups?.find(({ id }) => id === defaultActionGroupId); - const selectedActionGroup = - actionGroups?.find(({ id }) => id === actionItem.group) ?? defaultActionGroup; - const accordionContent = checkEnabledResult.isEnabled ? ( {actionGroups && selectedActionGroup && setActionGroupIdByIndex && ( @@ -270,7 +260,7 @@ export const ActionTypeForm = ({ errors={actionParamsErrors.errors} editAction={setActionParamsProperty} messageVariables={availableActionVariables} - defaultMessage={availableDefaultActionMessage} + defaultMessage={selectedActionGroup?.defaultActionMessage} actionConnector={actionConnector} /> @@ -362,10 +352,12 @@ export const ActionTypeForm = ({ }; function getAvailableActionVariables( - { params, state, context }: ActionVariables, - actionGroup: string + actionVariables: ActionVariables, + actionGroup?: ActionGroupWithMessageVariables ) { return transformActionVariables( - actionGroup === RecoveredActionGroup.id ? { params, state } : { params, state, context } + actionGroup?.omitOptionalMessageVariables + ? pick(actionVariables, ...REQUIRED_ACTION_VARIABLES) + : actionVariables ).sort((a, b) => a.name.toUpperCase().localeCompare(b.name.toUpperCase())); } 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 a950af9c99a51..406d69d54f1ae 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 @@ -58,6 +58,7 @@ import { AlertActionParam, ALERTS_FEATURE_ID } from '../../../../../alerts/commo import { hasAllPrivilege, hasShowActionsCapability } from '../../lib/capabilities'; import { SolutionFilter } from './solution_filter'; import './alert_form.scss'; +import { recoveredActionGroupMessage } from '../../constants'; const ENTER_KEY = 13; @@ -306,6 +307,7 @@ export const AlertForm = ({ ? !item.alertTypeModel.requiresAppContext : item.alertType!.producer === alert.consumer ); + const selectedAlertType = alert?.alertTypeId && alertTypesIndex?.get(alert?.alertTypeId); const tagsOptions = alert.tags ? alert.tags.map((label: string) => ({ label })) : []; @@ -461,7 +463,7 @@ export const AlertForm = ({ {AlertParamsExpressionComponent && defaultActionGroupId && alert.alertTypeId && - alertTypesIndex?.has(alert.alertTypeId) ? ( + selectedAlertType ? ( }> @@ -482,22 +484,29 @@ export const AlertForm = ({ defaultActionGroupId && alertTypeModel && alert.alertTypeId && - alertTypesIndex?.has(alert.alertTypeId) ? ( + selectedAlertType ? ( + actionGroup.id === selectedAlertType.recoveryActionGroup.id + ? { + ...actionGroup, + omitOptionalMessageVariables: true, + defaultActionMessage: recoveredActionGroupMessage, + } + : { ...actionGroup, defaultActionMessage: alertTypeModel?.defaultActionMessage } + )} setActionIdByIndex={(id: string, index: number) => setActionProperty('id', id, index)} setActionGroupIdByIndex={(group: string, index: number) => setActionProperty('group', group, index) } - setAlertProperty={setActions} + setActions={setActions} setActionParamsProperty={setActionParamsProperty} actionTypeRegistry={actionTypeRegistry} - defaultActionMessage={alertTypeModel?.defaultActionMessage} /> ) : null} diff --git a/x-pack/plugins/triggers_actions_ui/public/types.ts b/x-pack/plugins/triggers_actions_ui/public/types.ts index be8b7b9757e9e..996a4cda9d457 100644 --- a/x-pack/plugins/triggers_actions_ui/public/types.ts +++ b/x-pack/plugins/triggers_actions_ui/public/types.ts @@ -6,6 +6,7 @@ import type { PublicMethodsOf } from '@kbn/utility-types'; import type { DocLinksStart } from 'kibana/public'; import { ComponentType } from 'react'; +import { RequiredKeys } from 'utility-types'; import { ActionGroup, AlertActionParam } from '../../alerts/common'; import { ActionType } from '../../actions/common'; import { TypeRegistry } from './application/type_registry'; @@ -127,16 +128,19 @@ export interface ActionVariable { description: string; } -export interface ActionVariables { - context?: ActionVariable[]; - state: ActionVariable[]; - params: ActionVariable[]; -} +type AsActionVariables = { + [Req in Keys]: ActionVariable[]; +}; +export const REQUIRED_ACTION_VARIABLES = ['state', 'params'] as const; +export const OPTIONAL_ACTION_VARIABLES = ['context'] as const; +export type ActionVariables = AsActionVariables & + Partial>; export interface AlertType { id: string; name: string; actionGroups: ActionGroup[]; + recoveryActionGroup: ActionGroup; actionVariables: ActionVariables; defaultActionGroupId: ActionGroup['id']; authorizedConsumers: Record; diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts_restricted/server/alert_types.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts_restricted/server/alert_types.ts index 968b0e9dd8d14..3e3c44f2c2784 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts_restricted/server/alert_types.ts +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts_restricted/server/alert_types.ts @@ -18,7 +18,7 @@ export function defineAlertTypes( actionGroups: [{ id: 'default', name: 'Default' }], producer: 'alertsRestrictedFixture', defaultActionGroupId: 'default', - recoveryActionGroupName: 'Restricted Recovery', + recoveryActionGroup: { id: 'restrictedRecovered', name: 'Restricted Recovery' }, async executor({ services, params, state }: AlertExecutorOptions) {}, }; const noopUnrestrictedAlertType: AlertType = { From c040c6fa583faa9ffd831f638311af8379e4f0cb Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Thu, 26 Nov 2020 19:49:47 +0000 Subject: [PATCH 10/21] fixed mock alert types throughout unit tests --- .../alert_navigation_registry.test.ts | 4 ++ .../alerts/server/alert_type_registry.test.ts | 8 +++ .../alerts_client/tests/aggregate.test.ts | 8 +++ .../server/alerts_client/tests/create.test.ts | 4 ++ .../server/alerts_client/tests/find.test.ts | 2 + .../alerts/server/alerts_client/tests/lib.ts | 4 ++ .../tests/list_alert_types.test.ts | 20 +++++++ .../server/alerts_client/tests/update.test.ts | 12 ++++ .../alerts_client_conflict_retries.test.ts | 8 +++ .../alerts_authorization.test.ts | 56 +++++++++++++++++++ .../alerts_authorization_kuery.test.ts | 20 +++++++ .../server/routes/list_alert_types.test.ts | 16 ++++++ .../create_execution_handler.test.ts | 4 ++ .../task_runner/task_runner_factory.test.ts | 4 ++ .../components/add_message_variables.tsx | 2 +- .../tests/alerting/list_alert_types.ts | 4 ++ .../tests/alerting/list_alert_types.ts | 4 ++ .../alert_create_flyout.ts | 4 +- 18 files changed, 181 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/alerts/public/alert_navigation_registry/alert_navigation_registry.test.ts b/x-pack/plugins/alerts/public/alert_navigation_registry/alert_navigation_registry.test.ts index 72c955923a0cc..11abcdc4da87d 100644 --- a/x-pack/plugins/alerts/public/alert_navigation_registry/alert_navigation_registry.test.ts +++ b/x-pack/plugins/alerts/public/alert_navigation_registry/alert_navigation_registry.test.ts @@ -14,6 +14,10 @@ const mockAlertType = (id: string): AlertType => ({ id, name: id, actionGroups: [], + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, actionVariables: [], defaultActionGroupId: 'default', producer: 'alerts', diff --git a/x-pack/plugins/alerts/server/alert_type_registry.test.ts b/x-pack/plugins/alerts/server/alert_type_registry.test.ts index 99bf76c7f81ad..e4811daa3611b 100644 --- a/x-pack/plugins/alerts/server/alert_type_registry.test.ts +++ b/x-pack/plugins/alerts/server/alert_type_registry.test.ts @@ -308,6 +308,10 @@ describe('get()', () => { "id": "test", "name": "Test", "producer": "alerts", + "recoveryActionGroup": Object { + "id": "recovered", + "name": "Recovered", + }, } `); }); @@ -365,6 +369,10 @@ describe('list()', () => { "id": "test", "name": "Test", "producer": "alerts", + "recoveryActionGroup": Object { + "id": "recovered", + "name": "Recovered", + }, }, } `); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/aggregate.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/aggregate.test.ts index cc5d10c3346e8..38ddf44f95a7f 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/aggregate.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/aggregate.test.ts @@ -53,6 +53,10 @@ describe('aggregate()', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, id: 'myType', name: 'myType', producer: 'myApp', @@ -102,6 +106,10 @@ describe('aggregate()', () => { name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, producer: 'alerts', authorizedConsumers: { myApp: { read: true, all: true }, diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts index 171ed13763c46..cd0786ec08e76 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts @@ -683,6 +683,10 @@ describe('create()', () => { }, ], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, validate: { params: schema.object({ param1: schema.string(), diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/find.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/find.test.ts index 3d7473a746986..6e35fb04e992a 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/find.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/find.test.ts @@ -52,6 +52,7 @@ describe('find()', () => { const listedTypes = new Set([ { actionGroups: [], + recoveryActionGroup: { id: 'recovered', name: 'recovered' }, actionVariables: undefined, defaultActionGroupId: 'default', id: 'myType', @@ -108,6 +109,7 @@ describe('find()', () => { id: 'myType', name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'recovered' }, defaultActionGroupId: 'default', producer: 'alerts', authorizedConsumers: { diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/lib.ts b/x-pack/plugins/alerts/server/alerts_client/tests/lib.ts index 028a7c6737474..04ece9cbd96eb 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/lib.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/lib.ts @@ -82,6 +82,10 @@ export function getBeforeSetup( id: '123', name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, defaultActionGroupId: 'default', async executor() {}, producer: 'alerts', diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/list_alert_types.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/list_alert_types.test.ts index 8cbe47655ef68..a85c6a2419117 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/list_alert_types.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/list_alert_types.test.ts @@ -50,6 +50,10 @@ describe('listAlertTypes', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, id: 'alertingAlertType', name: 'alertingAlertType', producer: 'alerts', @@ -58,6 +62,10 @@ describe('listAlertTypes', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, id: 'myAppAlertType', name: 'myAppAlertType', producer: 'myApp', @@ -96,6 +104,10 @@ describe('listAlertTypes', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, id: 'myType', name: 'myType', producer: 'myApp', @@ -105,6 +117,10 @@ describe('listAlertTypes', () => { name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, producer: 'alerts', }, ]); @@ -119,6 +135,10 @@ describe('listAlertTypes', () => { name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, producer: 'alerts', authorizedConsumers: { myApp: { read: true, all: true }, diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts index 046d7ec63c048..60868bf6f9069 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts @@ -97,6 +97,10 @@ describe('update()', () => { name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, async executor() {}, producer: 'alerts', }); @@ -676,6 +680,10 @@ describe('update()', () => { name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, validate: { params: schema.object({ param1: schema.string(), @@ -1021,6 +1029,10 @@ describe('update()', () => { name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, async executor() {}, producer: 'alerts', }); diff --git a/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts b/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts index ca9389ece310c..c788f50a62ae6 100644 --- a/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts @@ -331,6 +331,10 @@ beforeEach(() => { name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, async executor() {}, producer: 'alerts', })); @@ -340,6 +344,10 @@ beforeEach(() => { name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, async executor() {}, producer: 'alerts', }); diff --git a/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts b/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts index eb116b9e208dc..8ad56cbc9bd60 100644 --- a/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts +++ b/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts @@ -172,6 +172,10 @@ beforeEach(() => { name: 'My Alert Type', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, async executor() {}, producer: 'myApp', })); @@ -534,6 +538,10 @@ describe('AlertsAuthorization', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, id: 'myOtherAppAlertType', name: 'myOtherAppAlertType', producer: 'alerts', @@ -542,6 +550,10 @@ describe('AlertsAuthorization', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, id: 'myAppAlertType', name: 'myAppAlertType', producer: 'myApp', @@ -550,6 +562,10 @@ describe('AlertsAuthorization', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, id: 'mySecondAppAlertType', name: 'mySecondAppAlertType', producer: 'myApp', @@ -824,6 +840,10 @@ describe('AlertsAuthorization', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, id: 'myOtherAppAlertType', name: 'myOtherAppAlertType', producer: 'myOtherApp', @@ -832,6 +852,10 @@ describe('AlertsAuthorization', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, id: 'myAppAlertType', name: 'myAppAlertType', producer: 'myApp', @@ -880,6 +904,10 @@ describe('AlertsAuthorization', () => { "id": "myAppAlertType", "name": "myAppAlertType", "producer": "myApp", + "recoveryActionGroup": Object { + "id": "recovered", + "name": "Recovered", + }, }, Object { "actionGroups": Array [], @@ -906,6 +934,10 @@ describe('AlertsAuthorization', () => { "id": "myOtherAppAlertType", "name": "myOtherAppAlertType", "producer": "myOtherApp", + "recoveryActionGroup": Object { + "id": "recovered", + "name": "Recovered", + }, }, } `); @@ -972,6 +1004,10 @@ describe('AlertsAuthorization', () => { "id": "myOtherAppAlertType", "name": "myOtherAppAlertType", "producer": "myOtherApp", + "recoveryActionGroup": Object { + "id": "recovered", + "name": "Recovered", + }, }, Object { "actionGroups": Array [], @@ -994,6 +1030,10 @@ describe('AlertsAuthorization', () => { "id": "myAppAlertType", "name": "myAppAlertType", "producer": "myApp", + "recoveryActionGroup": Object { + "id": "recovered", + "name": "Recovered", + }, }, } `); @@ -1055,6 +1095,10 @@ describe('AlertsAuthorization', () => { "id": "myAppAlertType", "name": "myAppAlertType", "producer": "myApp", + "recoveryActionGroup": Object { + "id": "recovered", + "name": "Recovered", + }, }, } `); @@ -1145,6 +1189,10 @@ describe('AlertsAuthorization', () => { "id": "myOtherAppAlertType", "name": "myOtherAppAlertType", "producer": "myOtherApp", + "recoveryActionGroup": Object { + "id": "recovered", + "name": "Recovered", + }, }, Object { "actionGroups": Array [], @@ -1167,6 +1215,10 @@ describe('AlertsAuthorization', () => { "id": "myAppAlertType", "name": "myAppAlertType", "producer": "myApp", + "recoveryActionGroup": Object { + "id": "recovered", + "name": "Recovered", + }, }, } `); @@ -1241,6 +1293,10 @@ describe('AlertsAuthorization', () => { "id": "myOtherAppAlertType", "name": "myOtherAppAlertType", "producer": "myOtherApp", + "recoveryActionGroup": Object { + "id": "recovered", + "name": "Recovered", + }, }, } `); diff --git a/x-pack/plugins/alerts/server/authorization/alerts_authorization_kuery.test.ts b/x-pack/plugins/alerts/server/authorization/alerts_authorization_kuery.test.ts index e4b9f8c54c38d..c36c26a1036a3 100644 --- a/x-pack/plugins/alerts/server/authorization/alerts_authorization_kuery.test.ts +++ b/x-pack/plugins/alerts/server/authorization/alerts_authorization_kuery.test.ts @@ -17,6 +17,10 @@ describe('asFiltersByAlertTypeAndConsumer', () => { { actionGroups: [], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, id: 'myAppAlertType', name: 'myAppAlertType', producer: 'myApp', @@ -40,6 +44,10 @@ describe('asFiltersByAlertTypeAndConsumer', () => { { actionGroups: [], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, id: 'myAppAlertType', name: 'myAppAlertType', producer: 'myApp', @@ -65,6 +73,10 @@ describe('asFiltersByAlertTypeAndConsumer', () => { { actionGroups: [], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, id: 'myAppAlertType', name: 'myAppAlertType', producer: 'myApp', @@ -78,6 +90,10 @@ describe('asFiltersByAlertTypeAndConsumer', () => { { actionGroups: [], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, id: 'myOtherAppAlertType', name: 'myOtherAppAlertType', producer: 'alerts', @@ -91,6 +107,10 @@ describe('asFiltersByAlertTypeAndConsumer', () => { { actionGroups: [], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, id: 'mySecondAppAlertType', name: 'mySecondAppAlertType', producer: 'myApp', diff --git a/x-pack/plugins/alerts/server/routes/list_alert_types.test.ts b/x-pack/plugins/alerts/server/routes/list_alert_types.test.ts index af20dd6e202ba..f09151cfbfaab 100644 --- a/x-pack/plugins/alerts/server/routes/list_alert_types.test.ts +++ b/x-pack/plugins/alerts/server/routes/list_alert_types.test.ts @@ -43,6 +43,10 @@ describe('listAlertTypesRoute', () => { }, ], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, authorizedConsumers: {}, actionVariables: { context: [], @@ -74,6 +78,10 @@ describe('listAlertTypesRoute', () => { "id": "1", "name": "name", "producer": "test", + "recoveryActionGroup": Object { + "id": "recovered", + "name": "Recovered", + }, }, ], } @@ -107,6 +115,10 @@ describe('listAlertTypesRoute', () => { }, ], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, authorizedConsumers: {}, actionVariables: { context: [], @@ -156,6 +168,10 @@ describe('listAlertTypesRoute', () => { }, ], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, authorizedConsumers: {}, actionVariables: { context: [], diff --git a/x-pack/plugins/alerts/server/task_runner/create_execution_handler.test.ts b/x-pack/plugins/alerts/server/task_runner/create_execution_handler.test.ts index ed73fec24db26..59eca88a9ada3 100644 --- a/x-pack/plugins/alerts/server/task_runner/create_execution_handler.test.ts +++ b/x-pack/plugins/alerts/server/task_runner/create_execution_handler.test.ts @@ -20,6 +20,10 @@ const alertType: AlertType = { { id: 'other-group', name: 'Other Group' }, ], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, executor: jest.fn(), producer: 'alerts', }; diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner_factory.test.ts b/x-pack/plugins/alerts/server/task_runner/task_runner_factory.test.ts index 1c10a997d8cdd..4c685d2fdec82 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner_factory.test.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner_factory.test.ts @@ -22,6 +22,10 @@ const alertType = { name: 'My test alert', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, executor: jest.fn(), producer: 'alerts', }; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/add_message_variables.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/add_message_variables.tsx index 2bcd87830901b..79a69a6af0828 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/add_message_variables.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/add_message_variables.tsx @@ -32,7 +32,7 @@ export const AddMessageVariables: React.FunctionComponent = ({ messageVariables?.map((variable: ActionVariable, i: number) => ( { onSelectEventHandler(variable.name); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts index df469c4cbca9e..deaa6d9d80960 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts @@ -28,6 +28,10 @@ export default function listAlertTypes({ getService }: FtrProviderContext) { params: [], }, producer: 'alertsFixture', + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, }; const expectedRestrictedNoOpType = { diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/list_alert_types.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/list_alert_types.ts index 9d38f4abb7f3f..c76a43b05b172 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/list_alert_types.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/list_alert_types.ts @@ -35,6 +35,10 @@ export default function listAlertTypes({ getService }: FtrProviderContext) { params: [], context: [], }, + recoveryActionGroup: { + id: 'recovered', + name: 'Recovered', + }, producer: 'alertsFixture', }); expect(Object.keys(authorizedConsumers)).to.contain('alertsFixture'); 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 ea9441a2e788b..ae48950f4f47a 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 @@ -89,14 +89,14 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { ); await testSubjects.setValue('messageTextArea', 'test message '); await testSubjects.click('messageAddVariableButton'); - await testSubjects.click('variableMenuButton-0'); + await testSubjects.click('variableMenuButton-alertActionGroup'); expect(await messageTextArea.getAttribute('value')).to.eql( 'test message {{alertActionGroup}}' ); await messageTextArea.type(' some additional text '); await testSubjects.click('messageAddVariableButton'); - await testSubjects.click('variableMenuButton-1'); + await testSubjects.click('variableMenuButton-alertId'); expect(await messageTextArea.getAttribute('value')).to.eql( 'test message {{alertActionGroup}} some additional text {{alertId}}' From 269966098d9294ba4578bec0d844bfc9f69a1b90 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Mon, 30 Nov 2020 09:49:53 +0000 Subject: [PATCH 11/21] fixed typing issues --- .../plugins/alerts/public/alert_api.test.ts | 4 ++++ .../rules/rule_actions_field/index.tsx | 2 +- .../application/lib/action_variables.test.ts | 1 + .../public/application/lib/alert_api.test.ts | 1 + .../action_connector_form/action_form.tsx | 5 ++++- .../action_type_form.tsx | 4 +++- .../components/alert_details.test.tsx | 19 +++++++++++++++++++ .../components/alert_instances.test.tsx | 1 + .../components/alert_instances_route.test.tsx | 1 + .../sections/alert_form/alert_add.test.tsx | 1 + .../sections/alert_form/alert_form.test.tsx | 3 +++ .../components/alerts_list.test.tsx | 1 + .../triggers_actions_ui/public/types.ts | 1 - 13 files changed, 40 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/alerts/public/alert_api.test.ts b/x-pack/plugins/alerts/public/alert_api.test.ts index 3ee67b79b7bda..40d93a6dfdeca 100644 --- a/x-pack/plugins/alerts/public/alert_api.test.ts +++ b/x-pack/plugins/alerts/public/alert_api.test.ts @@ -22,6 +22,7 @@ describe('loadAlertTypes', () => { actionVariables: ['var1'], actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, producer: 'alerts', }, ]; @@ -45,6 +46,7 @@ describe('loadAlertType', () => { actionVariables: ['var1'], actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, producer: 'alerts', }; http.get.mockResolvedValueOnce([alertType]); @@ -65,6 +67,7 @@ describe('loadAlertType', () => { actionVariables: [], actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, producer: 'alerts', }; http.get.mockResolvedValueOnce([alertType]); @@ -80,6 +83,7 @@ describe('loadAlertType', () => { actionVariables: [], actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, producer: 'alerts', }, ]); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/rule_actions_field/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/rule_actions_field/index.tsx index b653fc05850af..a856acb520d4e 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/rule_actions_field/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/rule_actions_field/index.tsx @@ -119,7 +119,7 @@ export const RuleActionsField: React.FC = ({ field, messageVariables }) = messageVariables={messageVariables} defaultActionGroupId={DEFAULT_ACTION_GROUP_ID} setActionIdByIndex={setActionIdByIndex} - setAlertProperty={setAlertProperty} + setActions={setAlertProperty} setActionParamsProperty={setActionParamsProperty} actionTypeRegistry={actionTypeRegistry} actionTypes={supportedActionTypes} diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.test.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.test.ts index 165a231cd1d67..80e94f8a80f0e 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.test.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.test.ts @@ -239,6 +239,7 @@ function getAlertType(actionVariables: ActionVariables): AlertType { actionVariables, actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, authorizedConsumers: {}, producer: ALERTS_FEATURE_ID, }; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.test.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.test.ts index 0817be3796fdf..e1011e2fe69b9 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.test.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/alert_api.test.ts @@ -48,6 +48,7 @@ describe('loadAlertTypes', () => { }, producer: ALERTS_FEATURE_ID, actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, defaultActionGroupId: 'default', authorizedConsumers: {}, }, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx index b5758539d8b25..f1297c4a1b561 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx @@ -47,6 +47,7 @@ export interface ActionAccordionFormProps { actions: AlertAction[]; defaultActionGroupId: string; actionGroups?: ActionGroupWithMessageVariables[]; + defaultActionMessage?: string; setActionIdByIndex: (id: string, index: number) => void; setActionGroupIdByIndex?: (group: string, index: number) => void; setActions: (actions: AlertAction[]) => void; @@ -66,13 +67,14 @@ interface ActiveActionConnectorState { export const ActionForm = ({ actions, defaultActionGroupId, - actionGroups, setActionIdByIndex, setActionGroupIdByIndex, setActions, setActionParamsProperty, actionTypes, messageVariables, + actionGroups, + defaultActionMessage, setHasActionsDisabled, setHasActionsWithBrokenConnector, actionTypeRegistry, @@ -338,6 +340,7 @@ export const ActionForm = ({ defaultActionGroupId={defaultActionGroupId} messageVariables={messageVariables} actionGroups={actionGroups} + defaultActionMessage={defaultActionMessage} setActionGroupIdByIndex={setActionGroupIdByIndex} onAddConnector={() => { setActiveActionItem({ actionTypeId: actionItem.actionTypeId, index }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx index c3471a2d48024..eb8798f709358 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx @@ -65,6 +65,7 @@ export type ActionTypeFormProps = { | 'setActionGroupIdByIndex' | 'setActionParamsProperty' | 'messageVariables' + | 'defaultActionMessage' >; const preconfiguredMessage = i18n.translate( @@ -86,6 +87,7 @@ export const ActionTypeForm = ({ actionTypesIndex, connectors, defaultActionGroupId, + defaultActionMessage, messageVariables, actionGroups, setActionGroupIdByIndex, @@ -260,7 +262,7 @@ export const ActionTypeForm = ({ errors={actionParamsErrors.errors} editAction={setActionParamsProperty} messageVariables={availableActionVariables} - defaultMessage={selectedActionGroup?.defaultActionMessage} + defaultMessage={selectedActionGroup?.defaultActionMessage ?? defaultActionMessage} actionConnector={actionConnector} /> diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx index 2f7a31721fa07..1a33cee7b2bb1 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx @@ -58,6 +58,7 @@ describe('alert_details', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -83,6 +84,7 @@ describe('alert_details', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -111,6 +113,7 @@ describe('alert_details', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -145,6 +148,7 @@ describe('alert_details', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -199,6 +203,7 @@ describe('alert_details', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -258,6 +263,7 @@ describe('alert_details', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -278,6 +284,7 @@ describe('alert_details', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -307,6 +314,7 @@ describe('disable button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -335,6 +343,7 @@ describe('disable button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -363,6 +372,7 @@ describe('disable button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -400,6 +410,7 @@ describe('disable button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -440,6 +451,7 @@ describe('mute button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -469,6 +481,7 @@ describe('mute button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -498,6 +511,7 @@ describe('mute button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -536,6 +550,7 @@ describe('mute button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -574,6 +589,7 @@ describe('mute button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -639,6 +655,7 @@ describe('edit button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: 'alerting', @@ -681,6 +698,7 @@ describe('edit button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: 'alerting', @@ -716,6 +734,7 @@ describe('edit button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: 'alerting', diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances.test.tsx index 52a85e8bc57bd..f7b00a2ccf0b9 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances.test.tsx @@ -307,6 +307,7 @@ function mockAlertType(overloads: Partial = {}): AlertType { params: [], }, defaultActionGroupId: 'default', + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, authorizedConsumers: {}, producer: 'alerts', ...overloads, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances_route.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances_route.test.tsx index 2256efe30831b..24e20c5d477f7 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances_route.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_instances_route.test.tsx @@ -147,6 +147,7 @@ function mockAlertType(overloads: Partial = {}): AlertType { params: [], }, defaultActionGroupId: 'default', + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, authorizedConsumers: {}, producer: 'alerts', ...overloads, 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 index 084da8905663e..608a4482543e2 100644 --- 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 @@ -61,6 +61,7 @@ describe('alert_add', () => { }, ], defaultActionGroupId: 'testActionGroup', + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, producer: ALERTS_FEATURE_ID, authorizedConsumers: { [ALERTS_FEATURE_ID]: { read: true, all: true }, 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 6b5f0a31d345c..0c712f326a7ee 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 @@ -85,6 +85,7 @@ describe('alert_form', () => { }, ], defaultActionGroupId: 'testActionGroup', + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, producer: ALERTS_FEATURE_ID, authorizedConsumers: { [ALERTS_FEATURE_ID]: { read: true, all: true }, @@ -218,6 +219,7 @@ describe('alert_form', () => { }, ], defaultActionGroupId: 'testActionGroup', + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, producer: ALERTS_FEATURE_ID, authorizedConsumers: { [ALERTS_FEATURE_ID]: { read: true, all: true }, @@ -234,6 +236,7 @@ describe('alert_form', () => { }, ], defaultActionGroupId: 'testActionGroup', + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, producer: 'test', authorizedConsumers: { [ALERTS_FEATURE_ID]: { read: true, all: true }, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alerts_list/components/alerts_list.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alerts_list/components/alerts_list.test.tsx index 351eccf2934be..cb4d6d8097463 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alerts_list/components/alerts_list.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alerts_list/components/alerts_list.test.tsx @@ -56,6 +56,7 @@ const alertTypeFromApi = { id: 'test_alert_type', name: 'some alert type', actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, actionVariables: { context: [], state: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, diff --git a/x-pack/plugins/triggers_actions_ui/public/types.ts b/x-pack/plugins/triggers_actions_ui/public/types.ts index b9cb6f5ef09b3..8c69643f19b8d 100644 --- a/x-pack/plugins/triggers_actions_ui/public/types.ts +++ b/x-pack/plugins/triggers_actions_ui/public/types.ts @@ -6,7 +6,6 @@ import type { PublicMethodsOf } from '@kbn/utility-types'; import type { DocLinksStart } from 'kibana/public'; import { ComponentType } from 'react'; -import { RequiredKeys } from 'utility-types'; import { ActionGroup, AlertActionParam } from '../../alerts/common'; import { ActionType } from '../../actions/common'; import { TypeRegistry } from './application/type_registry'; From 23f1433ef3279efbba8e6e39c8454af8a8e9e95a Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Mon, 30 Nov 2020 10:42:21 +0000 Subject: [PATCH 12/21] reduce repetition of mock data --- .../plugins/alerts/public/alert_api.test.ts | 10 ++--- .../alert_navigation_registry.test.ts | 7 +--- .../alerts_client/tests/aggregate.test.ts | 11 ++--- .../server/alerts_client/tests/create.test.ts | 6 +-- .../server/alerts_client/tests/find.test.ts | 5 ++- .../alerts/server/alerts_client/tests/lib.ts | 6 +-- .../tests/list_alert_types.test.ts | 26 +++--------- .../server/alerts_client/tests/update.test.ts | 16 ++------ .../alerts_client_conflict_retries.test.ts | 11 ++--- .../alerts_authorization.test.ts | 31 ++++---------- .../alerts_authorization_kuery.test.ts | 26 +++--------- .../server/routes/list_alert_types.test.ts | 16 ++------ .../components/alert_details.test.tsx | 40 +++++++++---------- .../sections/alert_form/alert_form.test.tsx | 8 ++-- 14 files changed, 71 insertions(+), 148 deletions(-) diff --git a/x-pack/plugins/alerts/public/alert_api.test.ts b/x-pack/plugins/alerts/public/alert_api.test.ts index 40d93a6dfdeca..49d39e57f3832 100644 --- a/x-pack/plugins/alerts/public/alert_api.test.ts +++ b/x-pack/plugins/alerts/public/alert_api.test.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { AlertType } from '../common'; +import { AlertType, RecoveredActionGroup } from '../common'; import { httpServiceMock } from '../../../../src/core/public/mocks'; import { loadAlert, loadAlertState, loadAlertType, loadAlertTypes } from './alert_api'; import uuid from 'uuid'; @@ -22,7 +22,7 @@ describe('loadAlertTypes', () => { actionVariables: ['var1'], actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup: RecoveredActionGroup, producer: 'alerts', }, ]; @@ -46,7 +46,7 @@ describe('loadAlertType', () => { actionVariables: ['var1'], actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup: RecoveredActionGroup, producer: 'alerts', }; http.get.mockResolvedValueOnce([alertType]); @@ -67,7 +67,7 @@ describe('loadAlertType', () => { actionVariables: [], actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup: RecoveredActionGroup, producer: 'alerts', }; http.get.mockResolvedValueOnce([alertType]); @@ -83,7 +83,7 @@ describe('loadAlertType', () => { actionVariables: [], actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup: RecoveredActionGroup, producer: 'alerts', }, ]); diff --git a/x-pack/plugins/alerts/public/alert_navigation_registry/alert_navigation_registry.test.ts b/x-pack/plugins/alerts/public/alert_navigation_registry/alert_navigation_registry.test.ts index 11abcdc4da87d..bf005e07f959e 100644 --- a/x-pack/plugins/alerts/public/alert_navigation_registry/alert_navigation_registry.test.ts +++ b/x-pack/plugins/alerts/public/alert_navigation_registry/alert_navigation_registry.test.ts @@ -5,7 +5,7 @@ */ import { AlertNavigationRegistry } from './alert_navigation_registry'; -import { AlertType, SanitizedAlert } from '../../common'; +import { AlertType, RecoveredActionGroup, SanitizedAlert } from '../../common'; import uuid from 'uuid'; beforeEach(() => jest.resetAllMocks()); @@ -14,10 +14,7 @@ const mockAlertType = (id: string): AlertType => ({ id, name: id, actionGroups: [], - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, actionVariables: [], defaultActionGroupId: 'default', producer: 'alerts', diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/aggregate.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/aggregate.test.ts index 38ddf44f95a7f..b21e3dcdf563d 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/aggregate.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/aggregate.test.ts @@ -14,6 +14,7 @@ import { AlertsAuthorization } from '../../authorization/alerts_authorization'; import { ActionsAuthorization } from '../../../../actions/server'; import { getBeforeSetup, setGlobalDate } from './lib'; import { AlertExecutionStatusValues } from '../../types'; +import { RecoveredActionGroup } from '../../../common'; const taskManager = taskManagerMock.createStart(); const alertTypeRegistry = alertTypeRegistryMock.create(); @@ -53,10 +54,7 @@ describe('aggregate()', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, id: 'myType', name: 'myType', producer: 'myApp', @@ -106,10 +104,7 @@ describe('aggregate()', () => { name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, producer: 'alerts', authorizedConsumers: { myApp: { read: true, all: true }, diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts index cd0786ec08e76..dcbb33d849405 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts @@ -15,6 +15,7 @@ import { AlertsAuthorization } from '../../authorization/alerts_authorization'; import { ActionsAuthorization, ActionsClient } from '../../../../actions/server'; import { TaskStatus } from '../../../../task_manager/server'; import { getBeforeSetup, setGlobalDate } from './lib'; +import { RecoveredActionGroup } from '../../../common'; const taskManager = taskManagerMock.createStart(); const alertTypeRegistry = alertTypeRegistryMock.create(); @@ -683,10 +684,7 @@ describe('create()', () => { }, ], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, validate: { params: schema.object({ param1: schema.string(), diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/find.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/find.test.ts index 6e35fb04e992a..336cb536d702b 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/find.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/find.test.ts @@ -15,6 +15,7 @@ import { actionsAuthorizationMock } from '../../../../actions/server/mocks'; import { AlertsAuthorization } from '../../authorization/alerts_authorization'; import { ActionsAuthorization } from '../../../../actions/server'; import { getBeforeSetup, setGlobalDate } from './lib'; +import { RecoveredActionGroup } from '../../../common'; const taskManager = taskManagerMock.createStart(); const alertTypeRegistry = alertTypeRegistryMock.create(); @@ -52,7 +53,7 @@ describe('find()', () => { const listedTypes = new Set([ { actionGroups: [], - recoveryActionGroup: { id: 'recovered', name: 'recovered' }, + recoveryActionGroup: RecoveredActionGroup, actionVariables: undefined, defaultActionGroupId: 'default', id: 'myType', @@ -109,7 +110,7 @@ describe('find()', () => { id: 'myType', name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'recovered' }, + recoveryActionGroup: RecoveredActionGroup, defaultActionGroupId: 'default', producer: 'alerts', authorizedConsumers: { diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/lib.ts b/x-pack/plugins/alerts/server/alerts_client/tests/lib.ts index 04ece9cbd96eb..8f692cf548a9a 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/lib.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/lib.ts @@ -9,6 +9,7 @@ import { actionsClientMock } from '../../../../actions/server/mocks'; import { ConstructorOptions } from '../alerts_client'; import { eventLogClientMock } from '../../../../event_log/server/mocks'; import { AlertTypeRegistry } from '../../alert_type_registry'; +import { RecoveredActionGroup } from '../../../common'; export const mockedDateString = '2019-02-12T21:01:22.479Z'; @@ -82,10 +83,7 @@ export function getBeforeSetup( id: '123', name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, defaultActionGroupId: 'default', async executor() {}, producer: 'alerts', diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/list_alert_types.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/list_alert_types.test.ts index a85c6a2419117..f3521965d615d 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/list_alert_types.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/list_alert_types.test.ts @@ -13,6 +13,7 @@ import { actionsAuthorizationMock } from '../../../../actions/server/mocks'; import { AlertsAuthorization } from '../../authorization/alerts_authorization'; import { ActionsAuthorization } from '../../../../actions/server'; import { getBeforeSetup } from './lib'; +import { RecoveredActionGroup } from '../../../common'; const taskManager = taskManagerMock.createStart(); const alertTypeRegistry = alertTypeRegistryMock.create(); @@ -50,10 +51,7 @@ describe('listAlertTypes', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, id: 'alertingAlertType', name: 'alertingAlertType', producer: 'alerts', @@ -62,10 +60,7 @@ describe('listAlertTypes', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, id: 'myAppAlertType', name: 'myAppAlertType', producer: 'myApp', @@ -104,10 +99,7 @@ describe('listAlertTypes', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, id: 'myType', name: 'myType', producer: 'myApp', @@ -117,10 +109,7 @@ describe('listAlertTypes', () => { name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, producer: 'alerts', }, ]); @@ -135,10 +124,7 @@ describe('listAlertTypes', () => { name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, producer: 'alerts', authorizedConsumers: { myApp: { read: true, all: true }, diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts index 60868bf6f9069..b42ee096777fe 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts @@ -11,6 +11,7 @@ import { taskManagerMock } from '../../../../task_manager/server/mocks'; import { alertTypeRegistryMock } from '../../alert_type_registry.mock'; import { alertsAuthorizationMock } from '../../authorization/alerts_authorization.mock'; import { IntervalSchedule, InvalidatePendingApiKey } from '../../types'; +import { RecoveredActionGroup } from '../../../common'; import { encryptedSavedObjectsMock } from '../../../../encrypted_saved_objects/server/mocks'; import { actionsAuthorizationMock } from '../../../../actions/server/mocks'; import { AlertsAuthorization } from '../../authorization/alerts_authorization'; @@ -97,10 +98,7 @@ describe('update()', () => { name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, async executor() {}, producer: 'alerts', }); @@ -680,10 +678,7 @@ describe('update()', () => { name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, validate: { params: schema.object({ param1: schema.string(), @@ -1029,10 +1024,7 @@ describe('update()', () => { name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, async executor() {}, producer: 'alerts', }); diff --git a/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts b/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts index c788f50a62ae6..60e733b49b041 100644 --- a/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client_conflict_retries.test.ts @@ -18,6 +18,7 @@ import { ActionsAuthorization } from '../../actions/server'; import { SavedObjectsErrorHelpers } from '../../../../src/core/server'; import { RetryForConflictsAttempts } from './lib/retry_if_conflicts'; import { TaskStatus } from '../../../plugins/task_manager/server/task'; +import { RecoveredActionGroup } from '../common'; let alertsClient: AlertsClient; @@ -331,10 +332,7 @@ beforeEach(() => { name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, async executor() {}, producer: 'alerts', })); @@ -344,10 +342,7 @@ beforeEach(() => { name: 'Test', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, async executor() {}, producer: 'alerts', }); diff --git a/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts b/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts index 8ad56cbc9bd60..ccc325d468c54 100644 --- a/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts +++ b/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts @@ -16,6 +16,7 @@ import { AlertsAuthorization, WriteOperations, ReadOperations } from './alerts_a import { alertsAuthorizationAuditLoggerMock } from './audit_logger.mock'; import { AlertsAuthorizationAuditLogger, AuthorizationResult } from './audit_logger'; import uuid from 'uuid'; +import { RecoveredActionGroup } from '../../common'; const alertTypeRegistry = alertTypeRegistryMock.create(); const features: jest.Mocked = featuresPluginMock.createStart(); @@ -172,10 +173,7 @@ beforeEach(() => { name: 'My Alert Type', actionGroups: [{ id: 'default', name: 'Default' }], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, async executor() {}, producer: 'myApp', })); @@ -538,10 +536,7 @@ describe('AlertsAuthorization', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, id: 'myOtherAppAlertType', name: 'myOtherAppAlertType', producer: 'alerts', @@ -550,10 +545,7 @@ describe('AlertsAuthorization', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, id: 'myAppAlertType', name: 'myAppAlertType', producer: 'myApp', @@ -562,10 +554,7 @@ describe('AlertsAuthorization', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, id: 'mySecondAppAlertType', name: 'mySecondAppAlertType', producer: 'myApp', @@ -840,10 +829,7 @@ describe('AlertsAuthorization', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, id: 'myOtherAppAlertType', name: 'myOtherAppAlertType', producer: 'myOtherApp', @@ -852,10 +838,7 @@ describe('AlertsAuthorization', () => { actionGroups: [], actionVariables: undefined, defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, id: 'myAppAlertType', name: 'myAppAlertType', producer: 'myApp', diff --git a/x-pack/plugins/alerts/server/authorization/alerts_authorization_kuery.test.ts b/x-pack/plugins/alerts/server/authorization/alerts_authorization_kuery.test.ts index c36c26a1036a3..4be52f12da9c7 100644 --- a/x-pack/plugins/alerts/server/authorization/alerts_authorization_kuery.test.ts +++ b/x-pack/plugins/alerts/server/authorization/alerts_authorization_kuery.test.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ import { esKuery } from '../../../../../src/plugins/data/server'; +import { RecoveredActionGroup } from '../../common'; import { asFiltersByAlertTypeAndConsumer, ensureFieldIsSafeForQuery, @@ -17,10 +18,7 @@ describe('asFiltersByAlertTypeAndConsumer', () => { { actionGroups: [], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, id: 'myAppAlertType', name: 'myAppAlertType', producer: 'myApp', @@ -44,10 +42,7 @@ describe('asFiltersByAlertTypeAndConsumer', () => { { actionGroups: [], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, id: 'myAppAlertType', name: 'myAppAlertType', producer: 'myApp', @@ -73,10 +68,7 @@ describe('asFiltersByAlertTypeAndConsumer', () => { { actionGroups: [], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, id: 'myAppAlertType', name: 'myAppAlertType', producer: 'myApp', @@ -90,10 +82,7 @@ describe('asFiltersByAlertTypeAndConsumer', () => { { actionGroups: [], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, id: 'myOtherAppAlertType', name: 'myOtherAppAlertType', producer: 'alerts', @@ -107,10 +96,7 @@ describe('asFiltersByAlertTypeAndConsumer', () => { { actionGroups: [], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, id: 'mySecondAppAlertType', name: 'mySecondAppAlertType', producer: 'myApp', diff --git a/x-pack/plugins/alerts/server/routes/list_alert_types.test.ts b/x-pack/plugins/alerts/server/routes/list_alert_types.test.ts index f09151cfbfaab..b18c79fd67484 100644 --- a/x-pack/plugins/alerts/server/routes/list_alert_types.test.ts +++ b/x-pack/plugins/alerts/server/routes/list_alert_types.test.ts @@ -10,6 +10,7 @@ import { mockLicenseState } from '../lib/license_state.mock'; import { verifyApiAccess } from '../lib/license_api_access'; import { mockHandlerArguments } from './_mock_handler_arguments'; import { alertsClientMock } from '../alerts_client.mock'; +import { RecoveredActionGroup } from '../../common'; const alertsClient = alertsClientMock.create(); @@ -43,10 +44,7 @@ describe('listAlertTypesRoute', () => { }, ], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, authorizedConsumers: {}, actionVariables: { context: [], @@ -115,10 +113,7 @@ describe('listAlertTypesRoute', () => { }, ], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, authorizedConsumers: {}, actionVariables: { context: [], @@ -168,10 +163,7 @@ describe('listAlertTypesRoute', () => { }, ], defaultActionGroupId: 'default', - recoveryActionGroup: { - id: 'recovered', - name: 'Recovered', - }, + recoveryActionGroup: RecoveredActionGroup, authorizedConsumers: {}, actionVariables: { context: [], diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx index 1a33cee7b2bb1..b19b6eb5f7a3e 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.test.tsx @@ -47,8 +47,8 @@ const mockAlertApis = { const authorizedConsumers = { [ALERTS_FEATURE_ID]: { read: true, all: true }, }; +const recoveryActionGroup = { id: 'recovered', name: 'Recovered' }; -// const AlertDetails = withBulkAlertOperations(RawAlertDetails); describe('alert_details', () => { // mock Api handlers @@ -58,7 +58,7 @@ describe('alert_details', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -84,7 +84,7 @@ describe('alert_details', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -113,7 +113,7 @@ describe('alert_details', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -148,7 +148,7 @@ describe('alert_details', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -203,7 +203,7 @@ describe('alert_details', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -263,7 +263,7 @@ describe('alert_details', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -284,7 +284,7 @@ describe('alert_details', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -314,7 +314,7 @@ describe('disable button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -343,7 +343,7 @@ describe('disable button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -372,7 +372,7 @@ describe('disable button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -410,7 +410,7 @@ describe('disable button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -451,7 +451,7 @@ describe('mute button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -481,7 +481,7 @@ describe('mute button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -511,7 +511,7 @@ describe('mute button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -550,7 +550,7 @@ describe('mute button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -589,7 +589,7 @@ describe('mute button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: ALERTS_FEATURE_ID, @@ -655,7 +655,7 @@ describe('edit button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: 'alerting', @@ -698,7 +698,7 @@ describe('edit button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: 'alerting', @@ -734,7 +734,7 @@ describe('edit button', () => { id: '.noop', name: 'No Op', actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup, actionVariables: { context: [], state: [], params: [] }, defaultActionGroupId: 'default', producer: 'alerting', 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 0c712f326a7ee..0d5972d075f42 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 @@ -13,7 +13,7 @@ import { ValidationResult, Alert } from '../../../types'; import { AlertForm } from './alert_form'; import { AlertsContextProvider } from '../../context/alerts_context'; import { coreMock } from 'src/core/public/mocks'; -import { ALERTS_FEATURE_ID } from '../../../../../alerts/common'; +import { ALERTS_FEATURE_ID, RecoveredActionGroup } from '../../../../../alerts/common'; const actionTypeRegistry = actionTypeRegistryMock.create(); const alertTypeRegistry = alertTypeRegistryMock.create(); @@ -85,7 +85,7 @@ describe('alert_form', () => { }, ], defaultActionGroupId: 'testActionGroup', - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup: RecoveredActionGroup, producer: ALERTS_FEATURE_ID, authorizedConsumers: { [ALERTS_FEATURE_ID]: { read: true, all: true }, @@ -219,7 +219,7 @@ describe('alert_form', () => { }, ], defaultActionGroupId: 'testActionGroup', - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup: RecoveredActionGroup, producer: ALERTS_FEATURE_ID, authorizedConsumers: { [ALERTS_FEATURE_ID]: { read: true, all: true }, @@ -236,7 +236,7 @@ describe('alert_form', () => { }, ], defaultActionGroupId: 'testActionGroup', - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + recoveryActionGroup: RecoveredActionGroup, producer: 'test', authorizedConsumers: { [ALERTS_FEATURE_ID]: { read: true, all: true }, From 4c20c3bcd023efa0de4bb675ee2829db6717e2a2 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Mon, 30 Nov 2020 14:47:02 +0000 Subject: [PATCH 13/21] fixed alert type list test --- .../security_and_spaces/tests/alerting/list_alert_types.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts index deaa6d9d80960..1ce04683f79bf 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/list_alert_types.ts @@ -37,8 +37,12 @@ export default function listAlertTypes({ getService }: FtrProviderContext) { const expectedRestrictedNoOpType = { actionGroups: [ { id: 'default', name: 'Default' }, - { id: 'recovered', name: 'Restricted Recovery' }, + { id: 'restrictedRecovered', name: 'Restricted Recovery' }, ], + recoveryActionGroup: { + id: 'restrictedRecovered', + name: 'Restricted Recovery', + }, defaultActionGroupId: 'default', id: 'test.restricted-noop', name: 'Test: Restricted Noop', From a4f8a9c37ba3a5685db55b0f96dc7c8968889c01 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Mon, 30 Nov 2020 17:15:14 +0000 Subject: [PATCH 14/21] support legacy event log alert recovery syntax --- .../tests/get_alert_instance_summary.test.ts | 2 +- ...rt_instance_summary_from_event_log.test.ts | 60 +++++++++++++++++-- .../alert_instance_summary_from_event_log.ts | 3 +- x-pack/plugins/alerts/server/plugin.ts | 3 + .../server/task_runner/task_runner.test.ts | 2 +- .../alerts/server/task_runner/task_runner.ts | 2 +- .../spaces_only/tests/alerting/event_log.ts | 8 +-- 7 files changed, 66 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_instance_summary.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_instance_summary.test.ts index 9bd61c0fe66d2..0a764ea768591 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_instance_summary.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_instance_summary.test.ts @@ -122,7 +122,7 @@ describe('getAlertInstanceSummary()', () => { .addActiveInstance('instance-previously-active', 'action group B') .advanceTime(10000) .addExecute() - .addResolvedInstance('instance-previously-active') + .addRecoveredInstance('instance-previously-active') .addActiveInstance('instance-currently-active', 'action group A') .getEvents(); const eventsResult = { diff --git a/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts b/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts index 7b45bcf736846..1d5ebe2b5911e 100644 --- a/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts +++ b/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.test.ts @@ -6,7 +6,7 @@ import { SanitizedAlert, AlertInstanceSummary } from '../types'; import { IValidatedEvent } from '../../../event_log/server'; -import { EVENT_LOG_ACTIONS, EVENT_LOG_PROVIDER } from '../plugin'; +import { EVENT_LOG_ACTIONS, EVENT_LOG_PROVIDER, LEGACY_EVENT_LOG_ACTIONS } from '../plugin'; import { alertInstanceSummaryFromEventLog } from './alert_instance_summary_from_event_log'; const ONE_HOUR_IN_MILLIS = 60 * 60 * 1000; @@ -189,7 +189,43 @@ describe('alertInstanceSummaryFromEventLog', () => { .addActiveInstance('instance-1', 'action group A') .advanceTime(10000) .addExecute() - .addResolvedInstance('instance-1') + .addRecoveredInstance('instance-1') + .getEvents(); + + const summary: AlertInstanceSummary = alertInstanceSummaryFromEventLog({ + alert, + events, + dateStart, + dateEnd, + }); + + const { lastRun, status, instances } = summary; + expect({ lastRun, status, instances }).toMatchInlineSnapshot(` + Object { + "instances": Object { + "instance-1": Object { + "actionGroupId": undefined, + "activeStartDate": undefined, + "muted": false, + "status": "OK", + }, + }, + "lastRun": "2020-06-18T00:00:10.000Z", + "status": "OK", + } + `); + }); + + test('legacy alert with currently inactive instance', async () => { + const alert = createAlert({}); + const eventsFactory = new EventsFactory(); + const events = eventsFactory + .addExecute() + .addNewInstance('instance-1') + .addActiveInstance('instance-1', 'action group A') + .advanceTime(10000) + .addExecute() + .addLegacyResolvedInstance('instance-1') .getEvents(); const summary: AlertInstanceSummary = alertInstanceSummaryFromEventLog({ @@ -224,7 +260,7 @@ describe('alertInstanceSummaryFromEventLog', () => { .addActiveInstance('instance-1', 'action group A') .advanceTime(10000) .addExecute() - .addResolvedInstance('instance-1') + .addRecoveredInstance('instance-1') .getEvents(); const summary: AlertInstanceSummary = alertInstanceSummaryFromEventLog({ @@ -406,7 +442,7 @@ describe('alertInstanceSummaryFromEventLog', () => { .advanceTime(10000) .addExecute() .addActiveInstance('instance-1', 'action group A') - .addResolvedInstance('instance-2') + .addRecoveredInstance('instance-2') .getEvents(); const summary: AlertInstanceSummary = alertInstanceSummaryFromEventLog({ @@ -451,7 +487,7 @@ describe('alertInstanceSummaryFromEventLog', () => { .advanceTime(10000) .addExecute() .addActiveInstance('instance-1', 'action group A') - .addResolvedInstance('instance-2') + .addRecoveredInstance('instance-2') .advanceTime(10000) .addExecute() .addActiveInstance('instance-1', 'action group B') @@ -561,7 +597,7 @@ export class EventsFactory { return this; } - addResolvedInstance(instanceId: string): EventsFactory { + addRecoveredInstance(instanceId: string): EventsFactory { this.events.push({ '@timestamp': this.date, event: { @@ -572,6 +608,18 @@ export class EventsFactory { }); return this; } + + addLegacyResolvedInstance(instanceId: string): EventsFactory { + this.events.push({ + '@timestamp': this.date, + event: { + provider: EVENT_LOG_PROVIDER, + action: LEGACY_EVENT_LOG_ACTIONS.resolvedInstance, + }, + kibana: { alerting: { instance_id: instanceId } }, + }); + return this; + } } function createAlert(overrides: Partial): SanitizedAlert { diff --git a/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.ts b/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.ts index 9d6d9bed5aa93..6fed8b4aa4ee6 100644 --- a/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.ts +++ b/x-pack/plugins/alerts/server/lib/alert_instance_summary_from_event_log.ts @@ -6,7 +6,7 @@ import { SanitizedAlert, AlertInstanceSummary, AlertInstanceStatus } from '../types'; import { IEvent } from '../../../event_log/server'; -import { EVENT_LOG_ACTIONS, EVENT_LOG_PROVIDER } from '../plugin'; +import { EVENT_LOG_ACTIONS, EVENT_LOG_PROVIDER, LEGACY_EVENT_LOG_ACTIONS } from '../plugin'; export interface AlertInstanceSummaryFromEventLogParams { alert: SanitizedAlert; @@ -80,6 +80,7 @@ export function alertInstanceSummaryFromEventLog( status.status = 'Active'; status.actionGroupId = event?.kibana?.alerting?.action_group_id; break; + case LEGACY_EVENT_LOG_ACTIONS.resolvedInstance: case EVENT_LOG_ACTIONS.recoveredInstance: status.status = 'OK'; status.activeStartDate = undefined; diff --git a/x-pack/plugins/alerts/server/plugin.ts b/x-pack/plugins/alerts/server/plugin.ts index ab41a33f2d4a4..bafb89c64076b 100644 --- a/x-pack/plugins/alerts/server/plugin.ts +++ b/x-pack/plugins/alerts/server/plugin.ts @@ -85,6 +85,9 @@ export const EVENT_LOG_ACTIONS = { recoveredInstance: 'recovered-instance', activeInstance: 'active-instance', }; +export const LEGACY_EVENT_LOG_ACTIONS = { + resolvedInstance: 'resolved-instance', +}; export interface PluginSetupContract { registerType: AlertTypeRegistry['register']; diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts b/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts index 4bb3e94c23476..a2b281036d4cc 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner.test.ts @@ -770,7 +770,7 @@ describe('Task Runner', () => { }, ], }, - "message": "test:1: 'alert-name' recovered instance: '2'", + "message": "test:1: 'alert-name' instance '2' has recovered", }, ], Array [ diff --git a/x-pack/plugins/alerts/server/task_runner/task_runner.ts b/x-pack/plugins/alerts/server/task_runner/task_runner.ts index 8c6e2ca5a1c6b..5cb86c32420e1 100644 --- a/x-pack/plugins/alerts/server/task_runner/task_runner.ts +++ b/x-pack/plugins/alerts/server/task_runner/task_runner.ts @@ -458,7 +458,7 @@ function generateNewAndRecoveredInstanceEvents( for (const id of recoveredIds) { const actionGroup = originalAlertInstances[id].getLastScheduledActions()?.group; - const message = `${params.alertLabel} recovered instance: '${id}'`; + const message = `${params.alertLabel} instance '${id}' has recovered`; logInstanceEvent(id, EVENT_LOG_ACTIONS.recoveredInstance, message, actionGroup); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts index e83c7b38504ee..3766785680925 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts @@ -87,25 +87,25 @@ export default function eventLogTests({ getService }: FtrProviderContext) { const executeEvents = getEventsByAction(events, 'execute'); const executeActionEvents = getEventsByAction(events, 'execute-action'); const newInstanceEvents = getEventsByAction(events, 'new-instance'); - const resolvedInstanceEvents = getEventsByAction(events, 'recovered-instance'); + const recoveredInstanceEvents = getEventsByAction(events, 'recovered-instance'); expect(executeEvents.length >= 4).to.be(true); expect(executeActionEvents.length).to.be(2); expect(newInstanceEvents.length).to.be(1); - expect(resolvedInstanceEvents.length).to.be(1); + expect(recoveredInstanceEvents.length).to.be(1); // make sure the events are in the right temporal order const executeTimes = getTimestamps(executeEvents); const executeActionTimes = getTimestamps(executeActionEvents); const newInstanceTimes = getTimestamps(newInstanceEvents); - const resolvedInstanceTimes = getTimestamps(resolvedInstanceEvents); + const recoveredInstanceTimes = getTimestamps(recoveredInstanceEvents); expect(executeTimes[0] < newInstanceTimes[0]).to.be(true); expect(executeTimes[1] <= newInstanceTimes[0]).to.be(true); expect(executeTimes[2] > newInstanceTimes[0]).to.be(true); expect(executeTimes[1] <= executeActionTimes[0]).to.be(true); expect(executeTimes[2] > executeActionTimes[0]).to.be(true); - expect(resolvedInstanceTimes[0] > newInstanceTimes[0]).to.be(true); + expect(recoveredInstanceTimes[0] > newInstanceTimes[0]).to.be(true); // validate each event let executeCount = 0; From 94a30d5671b39b2932593cd610f6b625d3c260c4 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Wed, 2 Dec 2020 10:05:36 +0000 Subject: [PATCH 15/21] added doc --- x-pack/plugins/alerts/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugins/alerts/README.md b/x-pack/plugins/alerts/README.md index 62058d47cbd44..0a112c6ae761a 100644 --- a/x-pack/plugins/alerts/README.md +++ b/x-pack/plugins/alerts/README.md @@ -91,6 +91,7 @@ The following table describes the properties of the `options` object. |name|A user-friendly name for the alert type. These will be displayed in dropdowns when choosing alert types.|string| |actionGroups|An explicit list of groups the alert type may schedule actions for, each specifying the ActionGroup's unique ID and human readable name. Alert `actions` validation will use this configuartion to ensure groups are valid. We highly encourage using `kbn-i18n` to translate the names of actionGroup when registering the AlertType. |Array<{id:string, name:string}>| |defaultActionGroupId|Default ID value for the group of the alert type.|string| +|recoveryActionGroup|An action group to use when an alert instance goes from an active state, to an inactive one. This action group should not be specified under the `actionGroups` property. If no recoveryActionGroup is specified, the default `recovered` action group will be used. |{id:string, name:string}| |actionVariables|An explicit list of action variables the alert type makes available via context and state in action parameter templates, and a short human readable description. Alert UI will use this to display prompts for the users for these variables, in action parameter editors. We highly encourage using `kbn-i18n` to translate the descriptions. |{ context: Array<{name:string, description:string}, state: Array<{name:string, description:string}>| |validate.params|When developing an alert type, you can choose to accept a series of parameters. You may also have the parameters validated before they are passed to the `executor` function or created as an alert saved object. In order to do this, provide a `@kbn/config-schema` schema that we will use to validate the `params` attribute.|@kbn/config-schema| |executor|This is where the code of the alert type lives. This is a function to be called when executing an alert on an interval basis. For full details, see executor section below.|Function| From 0010947f9d7e0cc2bb4697e6f6faedfb072e6689 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Wed, 2 Dec 2020 10:41:01 +0000 Subject: [PATCH 16/21] removed unneeded change in jira --- .../components/builtin_action_types/jira/jira_params.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira_params.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira_params.tsx index e8e303bc5b0f2..aaa9b697f32ec 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira_params.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/jira/jira_params.tsx @@ -198,7 +198,7 @@ const JiraParamsFields: React.FunctionComponent - {hasParent && parent && ( + {hasParent && ( <> From 124d002d2806c893e721726c31b817e59ace65ac Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Wed, 2 Dec 2020 10:44:45 +0000 Subject: [PATCH 17/21] correct callback name in siem --- .../detections/components/rules/rule_actions_field/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/rule_actions_field/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/rule_actions_field/index.tsx index a856acb520d4e..22815852da1a5 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/rule_actions_field/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/rule_actions_field/index.tsx @@ -63,7 +63,7 @@ export const RuleActionsField: React.FC = ({ field, messageVariables }) = [field.setValue, actions] ); - const setAlertProperty = useCallback( + const setAlertActionsProperty = useCallback( (updatedActions: AlertAction[]) => field.setValue(updatedActions), [field] ); @@ -119,7 +119,7 @@ export const RuleActionsField: React.FC = ({ field, messageVariables }) = messageVariables={messageVariables} defaultActionGroupId={DEFAULT_ACTION_GROUP_ID} setActionIdByIndex={setActionIdByIndex} - setActions={setAlertProperty} + setActions={setAlertActionsProperty} setActionParamsProperty={setActionParamsProperty} actionTypeRegistry={actionTypeRegistry} actionTypes={supportedActionTypes} From c690e67f8df0ec5eeb470dc5cad90c3340d0db4f Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Wed, 2 Dec 2020 10:51:11 +0000 Subject: [PATCH 18/21] renamed resolved to recovered --- x-pack/plugins/alerts/server/alert_type_registry.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/plugins/alerts/server/alert_type_registry.ts b/x-pack/plugins/alerts/server/alert_type_registry.ts index fe3df5e9dbe74..95e2fce5b7a75 100644 --- a/x-pack/plugins/alerts/server/alert_type_registry.ts +++ b/x-pack/plugins/alerts/server/alert_type_registry.ts @@ -6,7 +6,6 @@ import Boom from '@hapi/boom'; import { i18n } from '@kbn/i18n'; -// import { Required } from 'utility-types'; import { schema } from '@kbn/config-schema'; import typeDetect from 'type-detect'; import { intersection } from 'lodash'; @@ -202,7 +201,7 @@ function augmentActionGroupsWithReserved< ); } else if (intersectingReservedActionGroups.length > 0) { throw new Error( - i18n.translate('xpack.alerts.alertTypeRegistry.register.reservedActionGroupUsageError', { + i18n.translate('xpack.alerts.alertTypeRegistry.register.recoveryActionGroupUsageError', { defaultMessage: 'Alert type [id="{id}"] cannot be registered. Action groups [{actionGroups}] are reserved by the framework.', values: { From d7b3b3405a900137e184455a41524ddc3d124be2 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Wed, 2 Dec 2020 15:21:46 +0000 Subject: [PATCH 19/21] fixed mistaken rename --- x-pack/plugins/alerts/server/alert_type_registry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/alerts/server/alert_type_registry.ts b/x-pack/plugins/alerts/server/alert_type_registry.ts index 95e2fce5b7a75..a3e80fbd6c11a 100644 --- a/x-pack/plugins/alerts/server/alert_type_registry.ts +++ b/x-pack/plugins/alerts/server/alert_type_registry.ts @@ -201,7 +201,7 @@ function augmentActionGroupsWithReserved< ); } else if (intersectingReservedActionGroups.length > 0) { throw new Error( - i18n.translate('xpack.alerts.alertTypeRegistry.register.recoveryActionGroupUsageError', { + i18n.translate('xpack.alerts.alertTypeRegistry.register.reservedActionGroupUsageError', { defaultMessage: 'Alert type [id="{id}"] cannot be registered. Action groups [{actionGroups}] are reserved by the framework.', values: { From ca540130b2efd763a940cbda9aac81d3fd6c89d5 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Thu, 3 Dec 2020 12:59:15 +0000 Subject: [PATCH 20/21] elvated default params to alert concern instead of actions concern --- .../lib/get_defaults_for_action_params.test.ts | 8 ++++++-- .../application/lib/get_defaults_for_action_params.ts | 8 ++++++-- .../sections/action_connector_form/action_form.tsx | 4 ++++ .../action_connector_form/action_type_form.tsx | 11 ++++++----- .../application/sections/alert_form/alert_form.tsx | 4 ++++ 5 files changed, 26 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/get_defaults_for_action_params.test.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/get_defaults_for_action_params.test.ts index 57cc45786b2da..35470db23fb35 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/get_defaults_for_action_params.test.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/get_defaults_for_action_params.test.ts @@ -10,16 +10,20 @@ import { getDefaultsForActionParams } from './get_defaults_for_action_params'; describe('getDefaultsForActionParams', () => { test('pagerduty defaults', async () => { - expect(getDefaultsForActionParams('.pagerduty', 'test')).toEqual({ + expect(getDefaultsForActionParams(() => false)('.pagerduty', 'test')).toEqual({ dedupKey: `{{${AlertProvidedActionVariables.alertId}}}:{{${AlertProvidedActionVariables.alertInstanceId}}}`, eventAction: 'trigger', }); }); test('pagerduty defaults for recovered action group', async () => { - expect(getDefaultsForActionParams('.pagerduty', RecoveredActionGroup.id)).toEqual({ + const isRecoveryActionGroup = jest.fn().mockReturnValue(true); + expect( + getDefaultsForActionParams(isRecoveryActionGroup)('.pagerduty', RecoveredActionGroup.id) + ).toEqual({ dedupKey: `{{${AlertProvidedActionVariables.alertId}}}:{{${AlertProvidedActionVariables.alertInstanceId}}}`, eventAction: 'resolve', }); + expect(isRecoveryActionGroup).toHaveBeenCalledWith(RecoveredActionGroup.id); }); }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/get_defaults_for_action_params.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/get_defaults_for_action_params.ts index 36c054977ac30..22bda143425de 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/get_defaults_for_action_params.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/get_defaults_for_action_params.ts @@ -4,10 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ -import { AlertActionParam, RecoveredActionGroup } from '../../../../alerts/common'; +import { AlertActionParam } from '../../../../alerts/common'; import { AlertProvidedActionVariables } from './action_variables'; +export type DefaultActionParamsGetter = ReturnType; +export type DefaultActionParams = ReturnType; export const getDefaultsForActionParams = ( + isRecoveryActionGroup: (actionGroupId: string) => boolean +) => ( actionTypeId: string, actionGroupId: string ): Record | undefined => { @@ -17,7 +21,7 @@ export const getDefaultsForActionParams = ( dedupKey: `{{${AlertProvidedActionVariables.alertId}}}:{{${AlertProvidedActionVariables.alertInstanceId}}}`, eventAction: 'trigger', }; - if (actionGroupId === RecoveredActionGroup.id) { + if (isRecoveryActionGroup(actionGroupId)) { pagerDutyDefaults.eventAction = 'resolve'; } return pagerDutyDefaults; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx index f1297c4a1b561..4a3905a93ac55 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx @@ -37,6 +37,7 @@ import { checkActionFormActionTypeEnabled } from '../../lib/check_action_type_en import { VIEW_LICENSE_OPTIONS_LINK, DEFAULT_HIDDEN_ACTION_TYPES } from '../../../common/constants'; import { ActionGroup, AlertActionParam } from '../../../../../alerts/common'; import { useKibana } from '../../../common/lib/kibana'; +import { DefaultActionParamsGetter } from '../../lib/get_defaults_for_action_params'; export interface ActionGroupWithMessageVariables extends ActionGroup { omitOptionalMessageVariables?: boolean; @@ -57,6 +58,7 @@ export interface ActionAccordionFormProps { setHasActionsDisabled?: (value: boolean) => void; setHasActionsWithBrokenConnector?: (value: boolean) => void; actionTypeRegistry: ActionTypeRegistryContract; + getDefaultActionParams: DefaultActionParamsGetter; } interface ActiveActionConnectorState { @@ -78,6 +80,7 @@ export const ActionForm = ({ setHasActionsDisabled, setHasActionsWithBrokenConnector, actionTypeRegistry, + getDefaultActionParams, }: ActionAccordionFormProps) => { const { http, @@ -341,6 +344,7 @@ export const ActionForm = ({ messageVariables={messageVariables} actionGroups={actionGroups} defaultActionMessage={defaultActionMessage} + defaultParams={getDefaultActionParams(actionItem.actionTypeId, actionItem.group)} setActionGroupIdByIndex={setActionGroupIdByIndex} onAddConnector={() => { setActiveActionItem({ actionTypeId: actionItem.actionTypeId, index }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx index cd26390ec48fb..a5b133d2a50b7 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx @@ -43,7 +43,7 @@ import { hasSaveActionsCapability } from '../../lib/capabilities'; import { ActionAccordionFormProps, ActionGroupWithMessageVariables } from './action_form'; import { transformActionVariables } from '../../lib/action_variables'; import { useKibana } from '../../../common/lib/kibana'; -import { getDefaultsForActionParams } from '../../lib/get_defaults_for_action_params'; +import { DefaultActionParams } from '../../lib/get_defaults_for_action_params'; export type ActionTypeFormProps = { actionItem: AlertAction; @@ -59,6 +59,7 @@ export type ActionTypeFormProps = { actionTypesIndex: ActionTypeIndex; connectors: ActionConnector[]; actionTypeRegistry: ActionTypeRegistryContract; + defaultParams: DefaultActionParams; } & Pick< ActionAccordionFormProps, | 'defaultActionGroupId' @@ -93,6 +94,7 @@ export const ActionTypeForm = ({ actionGroups, setActionGroupIdByIndex, actionTypeRegistry, + defaultParams, }: ActionTypeFormProps) => { const { application: { capabilities }, @@ -107,14 +109,13 @@ export const ActionTypeForm = ({ setAvailableActionVariables( messageVariables ? getAvailableActionVariables(messageVariables, selectedActionGroup) : [] ); - const paramsDefaults = getDefaultsForActionParams(actionItem.actionTypeId, actionItem.group); - if (paramsDefaults) { - for (const [key, paramValue] of Object.entries(paramsDefaults)) { + if (defaultParams) { + for (const [key, paramValue] of Object.entries(defaultParams)) { setActionParamsProperty(key, paramValue, index); } } // eslint-disable-next-line react-hooks/exhaustive-deps - }, [actionItem.group]); + }, [actionItem.group, defaultParams]); const canSave = hasSaveActionsCapability(capabilities); const getSelectedOptions = (actionItemId: string) => { 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 406d69d54f1ae..014398b200124 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 @@ -59,6 +59,7 @@ import { hasAllPrivilege, hasShowActionsCapability } from '../../lib/capabilitie import { SolutionFilter } from './solution_filter'; import './alert_form.scss'; import { recoveredActionGroupMessage } from '../../constants'; +import { getDefaultsForActionParams } from '../../lib/get_defaults_for_action_params'; const ENTER_KEY = 13; @@ -500,6 +501,9 @@ export const AlertForm = ({ } : { ...actionGroup, defaultActionMessage: alertTypeModel?.defaultActionMessage } )} + getDefaultActionParams={getDefaultsForActionParams( + (actionGroupId) => actionGroupId === selectedAlertType.recoveryActionGroup.id + )} setActionIdByIndex={(id: string, index: number) => setActionProperty('id', id, index)} setActionGroupIdByIndex={(group: string, index: number) => setActionProperty('group', group, index) From 69fa7af9a9a5c57cc3c90c8b76fe34df66f027c1 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Thu, 3 Dec 2020 17:17:03 +0000 Subject: [PATCH 21/21] made default params optional --- .../sections/action_connector_form/action_form.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx index 4a3905a93ac55..0337f6879e24a 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx @@ -58,7 +58,7 @@ export interface ActionAccordionFormProps { setHasActionsDisabled?: (value: boolean) => void; setHasActionsWithBrokenConnector?: (value: boolean) => void; actionTypeRegistry: ActionTypeRegistryContract; - getDefaultActionParams: DefaultActionParamsGetter; + getDefaultActionParams?: DefaultActionParamsGetter; } interface ActiveActionConnectorState { @@ -344,7 +344,7 @@ export const ActionForm = ({ messageVariables={messageVariables} actionGroups={actionGroups} defaultActionMessage={defaultActionMessage} - defaultParams={getDefaultActionParams(actionItem.actionTypeId, actionItem.group)} + defaultParams={getDefaultActionParams?.(actionItem.actionTypeId, actionItem.group)} setActionGroupIdByIndex={setActionGroupIdByIndex} onAddConnector={() => { setActiveActionItem({ actionTypeId: actionItem.actionTypeId, index });