From 68d6ab21354bcf0504dc3664b818ab07f94340bc Mon Sep 17 00:00:00 2001 From: Alexi Doak <109488926+doakalexi@users.noreply.github.com> Date: Thu, 15 Feb 2024 09:13:06 -0800 Subject: [PATCH] [ResponseOps][FE] Alert creation delay based on user definition (#176346) Resolves https://github.com/elastic/kibana/issues/173009 ## Summary Adds a new input for the user to define the `alertDelay`. This input is available for life-cycled alerts (stack and o11y) rule types. ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### To verify - Using the UI create a rule with the `alertDelay` field set. - Verify that the field is saved properly and that you can edit the `alertDelay` - Verify that you can add the alert delay to existing rules. Create a rule in a different branch and switch to this one. Edit the rule and set the `alertDelay`. Verify that the rule saves and works as expected. --------- Co-authored-by: Lisa Cawley --- .../server/routes/lib/rewrite_rule.test.ts | 3 + .../server/routes/lib/rewrite_rule.ts | 2 + .../server/routes/update_rule.test.ts | 10 ++ .../alerting/server/routes/update_rule.ts | 10 +- .../server/rules_client/methods/update.ts | 3 +- .../server/rules_client/tests/update.test.ts | 12 ++ .../lib/rule_api/common_transformations.ts | 2 + .../application/lib/rule_api/create.test.ts | 9 ++ .../public/application/lib/rule_api/create.ts | 2 + .../application/lib/rule_api/update.test.ts | 5 +- .../public/application/lib/rule_api/update.ts | 15 +- .../sections/rule_form/rule_form.test.tsx | 21 +++ .../sections/rule_form/rule_form.tsx | 143 ++++++++++++------ .../sections/rule_form/rule_reducer.test.ts | 17 +++ .../sections/rule_form/rule_reducer.ts | 29 ++++ 15 files changed, 232 insertions(+), 51 deletions(-) diff --git a/x-pack/plugins/alerting/server/routes/lib/rewrite_rule.test.ts b/x-pack/plugins/alerting/server/routes/lib/rewrite_rule.test.ts index 7a348e583ac6c4..826ee952a6bb65 100644 --- a/x-pack/plugins/alerting/server/routes/lib/rewrite_rule.test.ts +++ b/x-pack/plugins/alerting/server/routes/lib/rewrite_rule.test.ts @@ -62,6 +62,9 @@ const sampleRule: SanitizedRule & { activeSnoozes?: string[] } = }, nextRun: DATE_2020, revision: 0, + alertDelay: { + active: 10, + }, }; describe('rewriteRule', () => { diff --git a/x-pack/plugins/alerting/server/routes/lib/rewrite_rule.ts b/x-pack/plugins/alerting/server/routes/lib/rewrite_rule.ts index 953211a5ef4f77..d0e59278b13c5b 100644 --- a/x-pack/plugins/alerting/server/routes/lib/rewrite_rule.ts +++ b/x-pack/plugins/alerting/server/routes/lib/rewrite_rule.ts @@ -37,6 +37,7 @@ export const rewriteRule = ({ activeSnoozes, lastRun, nextRun, + alertDelay, ...rest }: SanitizedRule & { activeSnoozes?: string[] }) => ({ ...rest, @@ -78,4 +79,5 @@ export const rewriteRule = ({ ...(lastRun ? { last_run: rewriteRuleLastRun(lastRun) } : {}), ...(nextRun ? { next_run: nextRun } : {}), ...(apiKeyCreatedByUser !== undefined ? { api_key_created_by_user: apiKeyCreatedByUser } : {}), + ...(alertDelay !== undefined ? { alert_delay: alertDelay } : {}), }); diff --git a/x-pack/plugins/alerting/server/routes/update_rule.test.ts b/x-pack/plugins/alerting/server/routes/update_rule.test.ts index 5a4b3a19c0d7ce..b48e6d72bef3f6 100644 --- a/x-pack/plugins/alerting/server/routes/update_rule.test.ts +++ b/x-pack/plugins/alerting/server/routes/update_rule.test.ts @@ -59,6 +59,9 @@ describe('updateRuleRoute', () => { }, ], notifyWhen: RuleNotifyWhen.CHANGE, + alertDelay: { + active: 10, + }, }; const updateRequest: AsApiContract['data']> = { @@ -73,6 +76,9 @@ describe('updateRuleRoute', () => { alerts_filter: mockedAlert.actions[0].alertsFilter, }, ], + alert_delay: { + active: 10, + }, }; const updateResult: AsApiContract> = { @@ -86,6 +92,7 @@ describe('updateRuleRoute', () => { connector_type_id: actionTypeId, alerts_filter: alertsFilter, })), + alert_delay: mockedAlert.alertDelay, }; it('updates a rule with proper parameters', async () => { @@ -135,6 +142,9 @@ describe('updateRuleRoute', () => { "uuid": "1234-5678", }, ], + "alertDelay": Object { + "active": 10, + }, "name": "abc", "notifyWhen": "onActionGroupChange", "params": Object { diff --git a/x-pack/plugins/alerting/server/routes/update_rule.ts b/x-pack/plugins/alerting/server/routes/update_rule.ts index d24af256de6139..9419d84d063413 100644 --- a/x-pack/plugins/alerting/server/routes/update_rule.ts +++ b/x-pack/plugins/alerting/server/routes/update_rule.ts @@ -52,16 +52,22 @@ const bodySchema = schema.object({ ) ) ), + alert_delay: schema.maybe( + schema.object({ + active: schema.number(), + }) + ), }); const rewriteBodyReq: RewriteRequestCase> = (result) => { - const { notify_when: notifyWhen, actions, ...rest } = result.data; + const { notify_when: notifyWhen, alert_delay: alertDelay, actions, ...rest } = result.data; return { ...result, data: { ...rest, notifyWhen, actions: rewriteActionsReq(actions), + alertDelay, }, }; }; @@ -83,6 +89,7 @@ const rewriteBodyRes: RewriteResponseCase> = ({ isSnoozedUntil, lastRun, nextRun, + alertDelay, ...rest }) => ({ ...rest, @@ -115,6 +122,7 @@ const rewriteBodyRes: RewriteResponseCase> = ({ ...(lastRun ? { last_run: rewriteRuleLastRun(lastRun) } : {}), ...(nextRun ? { next_run: nextRun } : {}), ...(apiKeyCreatedByUser !== undefined ? { api_key_created_by_user: apiKeyCreatedByUser } : {}), + ...(alertDelay ? { alert_delay: alertDelay } : {}), }); export const updateRuleRoute = ( diff --git a/x-pack/plugins/alerting/server/rules_client/methods/update.ts b/x-pack/plugins/alerting/server/rules_client/methods/update.ts index 33afee4c20d26f..1255173beefe49 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/update.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/update.ts @@ -17,7 +17,7 @@ import { } from '../../types'; import { validateRuleTypeParams, getRuleNotifyWhenType } from '../../lib'; import { WriteOperations, AlertingAuthorizationEntity } from '../../authorization'; -import { parseDuration, getRuleCircuitBreakerErrorMessage } from '../../../common'; +import { parseDuration, getRuleCircuitBreakerErrorMessage, AlertDelay } from '../../../common'; import { retryIfConflicts } from '../../lib/retry_if_conflicts'; import { bulkMarkApiKeysForInvalidation } from '../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation'; import { ruleAuditEvent, RuleAuditAction } from '../common/audit_events'; @@ -51,6 +51,7 @@ export interface UpdateOptions { params: Params; throttle?: string | null; notifyWhen?: RuleNotifyWhenType | null; + alertDelay?: AlertDelay; }; allowMissingConnectorSecrets?: boolean; shouldIncrementRevision?: ShouldIncrementRevision; diff --git a/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts index be3221c8ed2f17..7384ab467a8e97 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts @@ -279,6 +279,9 @@ describe('update()', () => { scheduledTaskId: 'task-123', createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(), + alertDelay: { + active: 5, + }, }, references: [ { @@ -334,6 +337,9 @@ describe('update()', () => { }, }, ], + alertDelay: { + active: 10, + }, }, }); expect(result).toMatchInlineSnapshot(` @@ -364,6 +370,9 @@ describe('update()', () => { }, }, ], + "alertDelay": Object { + "active": 5, + }, "createdAt": 2019-02-12T21:01:22.479Z, "enabled": true, "id": "1", @@ -422,6 +431,9 @@ describe('update()', () => { "uuid": "102", }, ], + "alertDelay": Object { + "active": 10, + }, "alertTypeId": "myType", "apiKey": null, "apiKeyCreatedByUser": null, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/common_transformations.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/common_transformations.ts index 64949d9014c507..b00b874b079ef5 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/common_transformations.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/common_transformations.ts @@ -77,6 +77,7 @@ export const transformRule: RewriteRequestCase = ({ active_snoozes: activeSnoozes, last_run: lastRun, next_run: nextRun, + alert_delay: alertDelay, ...rest }: any) => ({ ruleTypeId, @@ -99,6 +100,7 @@ export const transformRule: RewriteRequestCase = ({ ...(lastRun ? { lastRun: transformLastRun(lastRun) } : {}), ...(nextRun ? { nextRun } : {}), ...(apiKeyCreatedByUser !== undefined ? { apiKeyCreatedByUser } : {}), + ...(alertDelay ? { alertDelay } : {}), ...rest, }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/create.test.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/create.test.ts index 76e5fd7f09207f..b27d9cad0c0560 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/create.test.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/create.test.ts @@ -52,6 +52,9 @@ describe('createRule', () => { execution_status: { status: 'pending', last_execution_date: '2021-04-01T21:33:13.250Z' }, create_at: '2021-04-01T21:33:13.247Z', updated_at: '2021-04-01T21:33:13.247Z', + alert_delay: { + active: 10, + }, }; const ruleToCreate: Omit< RuleUpdates, @@ -96,6 +99,9 @@ describe('createRule', () => { updatedAt: new Date('2021-04-01T21:33:13.247Z'), apiKeyOwner: '', revision: 0, + alertDelay: { + active: 10, + }, }; http.post.mockResolvedValueOnce(resolvedValue); @@ -148,6 +154,9 @@ describe('createRule', () => { tags: [], updatedAt: '2021-04-01T21:33:13.247Z', updatedBy: undefined, + alertDelay: { + active: 10, + }, }); }); }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/create.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/create.ts index 2304ee8c489301..48fa1783f3c1ff 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/create.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/create.ts @@ -23,6 +23,7 @@ type RuleCreateBody = Omit< const rewriteBodyRequest: RewriteResponseCase = ({ ruleTypeId, actions, + alertDelay, ...res }): any => ({ ...res, @@ -43,6 +44,7 @@ const rewriteBodyRequest: RewriteResponseCase = ({ : {}), }) ), + ...(alertDelay ? { alert_delay: alertDelay } : {}), }); export async function createRule({ diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/update.test.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/update.test.ts index 8b3ebc3f96e523..591cdc83e86cf3 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/update.test.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/update.test.ts @@ -27,6 +27,9 @@ describe('updateRule', () => { apiKey: null, apiKeyOwner: null, revision: 0, + alertDelay: { + active: 10, + }, }; const resolvedValue: Rule = { ...ruleToUpdate, @@ -51,7 +54,7 @@ describe('updateRule', () => { Array [ "/api/alerting/rule/12%2F3", Object { - "body": "{\\"name\\":\\"test\\",\\"tags\\":[\\"foo\\"],\\"schedule\\":{\\"interval\\":\\"1m\\"},\\"params\\":{},\\"actions\\":[]}", + "body": "{\\"name\\":\\"test\\",\\"tags\\":[\\"foo\\"],\\"schedule\\":{\\"interval\\":\\"1m\\"},\\"params\\":{},\\"actions\\":[],\\"alert_delay\\":{\\"active\\":10}}", }, ] `); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/update.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/update.ts index 52158bfa2f0343..80346ff2f65daf 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/update.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/rule_api/update.ts @@ -13,9 +13,13 @@ import { transformRule } from './common_transformations'; type RuleUpdatesBody = Pick< RuleUpdates, - 'name' | 'tags' | 'schedule' | 'actions' | 'params' | 'throttle' | 'notifyWhen' + 'name' | 'tags' | 'schedule' | 'actions' | 'params' | 'throttle' | 'notifyWhen' | 'alertDelay' >; -const rewriteBodyRequest: RewriteResponseCase = ({ actions, ...res }): any => ({ +const rewriteBodyRequest: RewriteResponseCase = ({ + actions, + alertDelay, + ...res +}): any => ({ ...res, actions: actions.map( ({ group, id, params, frequency, uuid, alertsFilter, useAlertDataForTemplate }) => ({ @@ -34,6 +38,7 @@ const rewriteBodyRequest: RewriteResponseCase = ({ actions, ... ...(uuid && { uuid }), }) ), + ...(alertDelay ? { alert_delay: alertDelay } : {}), }); export async function updateRule({ @@ -42,14 +47,16 @@ export async function updateRule({ id, }: { http: HttpSetup; - rule: Pick; + rule: Pick; id: string; }): Promise { const res = await http.put>( `${BASE_ALERTING_API_PATH}/rule/${encodeURIComponent(id)}`, { body: JSON.stringify( - rewriteBodyRequest(pick(rule, ['name', 'tags', 'schedule', 'params', 'actions'])) + rewriteBodyRequest( + pick(rule, ['name', 'tags', 'schedule', 'params', 'actions', 'alertDelay']) + ) ), } ); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.test.tsx index 24592566d54655..8902b4d472ad2d 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.test.tsx @@ -375,6 +375,9 @@ describe('rule_form', () => { enabled: false, mutedInstanceIds: [], ...(!showRulesList ? { ruleTypeId: ruleType.id } : {}), + alertDelay: { + active: 1, + }, } as unknown as Rule; wrapper = mountWithIntl( @@ -1034,6 +1037,24 @@ describe('rule_form', () => { expect(wrapper.find(ActionForm).props().hasFieldsForAAD).toEqual(true); }); + + it('renders rule alert delay', async () => { + const getAlertDelayInput = () => { + return wrapper.find('[data-test-subj="alertDelayInput"] input').first(); + }; + + await setup(); + expect(getAlertDelayInput().props().value).toEqual(1); + + getAlertDelayInput().simulate('change', { target: { value: '2' } }); + expect(getAlertDelayInput().props().value).toEqual(2); + + getAlertDelayInput().simulate('change', { target: { value: '20' } }); + expect(getAlertDelayInput().props().value).toEqual(20); + + getAlertDelayInput().simulate('change', { target: { value: '999' } }); + expect(getAlertDelayInput().props().value).toEqual(999); + }); }); describe('rule_form create rule non ruleing consumer and producer', () => { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.tsx index 98e2547dabdd3a..b0b052544b625d 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.tsx @@ -215,6 +215,7 @@ export const RuleForm = ({ ? getDurationUnitValue(rule.schedule.interval) : defaultScheduleIntervalUnit ); + const [alertDelay, setAlertDelay] = useState(rule.alertDelay?.active ?? 1); const [defaultActionGroupId, setDefaultActionGroupId] = useState(undefined); const [availableRuleTypes, setAvailableRuleTypes] = useState([]); @@ -328,6 +329,12 @@ export const RuleForm = ({ } }, [rule.schedule.interval, defaultScheduleInterval, defaultScheduleIntervalUnit]); + useEffect(() => { + if (rule.alertDelay) { + setAlertDelay(rule.alertDelay.active); + } + }, [rule.alertDelay]); + useEffect(() => { if (!flyoutBodyOverflowRef.current) { // We're using this as a reliable way to reset the scroll position @@ -393,6 +400,10 @@ export const RuleForm = ({ [dispatch] ); + const setAlertDelayProperty = (key: string, value: any) => { + dispatch({ command: { type: 'setAlertDelayProperty' }, payload: { key, value } }); + }; + useEffect(() => { const searchValue = searchText ? searchText.trim().toLocaleLowerCase() : null; setFilteredRuleTypes( @@ -766,51 +777,95 @@ export const RuleForm = ({ ) : null} {hideInterval !== true && ( - - 0} - error={errors['schedule.interval']} - > - - - 0} - value={ruleInterval || ''} - name="interval" - data-test-subj="intervalInput" - onChange={(e) => { - const value = e.target.value; - if (value === '' || INTEGER_REGEX.test(value)) { - const parsedValue = value === '' ? '' : parseInt(value, 10); - setRuleInterval(parsedValue || undefined); - setScheduleProperty('interval', `${parsedValue}${ruleIntervalUnit}`); - } - }} - /> - - - { - setRuleIntervalUnit(e.target.value); - setScheduleProperty('interval', `${ruleInterval}${e.target.value}`); - }} - data-test-subj="intervalInputUnit" - /> - - - - + <> + + 0} + error={errors['schedule.interval']} + > + + + 0} + value={ruleInterval || ''} + name="interval" + data-test-subj="intervalInput" + onChange={(e) => { + const value = e.target.value; + if (value === '' || INTEGER_REGEX.test(value)) { + const parsedValue = value === '' ? '' : parseInt(value, 10); + setRuleInterval(parsedValue || undefined); + setScheduleProperty('interval', `${parsedValue}${ruleIntervalUnit}`); + } + }} + /> + + + { + setRuleIntervalUnit(e.target.value); + setScheduleProperty('interval', `${ruleInterval}${e.target.value}`); + }} + data-test-subj="intervalInputUnit" + /> + + + + + + )} + + + + + } + />, + ]} + append={i18n.translate( + 'xpack.triggersActionsUI.sections.ruleForm.alertDelayFieldAppendLabel', + { + defaultMessage: 'consecutive matches', + } + )} + onChange={(e) => { + const value = e.target.value; + if (value === '' || INTEGER_REGEX.test(value)) { + const parsedValue = value === '' ? '' : parseInt(value, 10); + setAlertDelayProperty('active', parsedValue || 1); + setAlertDelay(parsedValue || undefined); + } + }} + /> + + {shouldShowConsumerSelect && ( <> diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_reducer.test.ts b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_reducer.test.ts index 996676b73d59e8..6eadf1fce5ff43 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_reducer.test.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_reducer.test.ts @@ -21,6 +21,9 @@ describe('rule reducer', () => { actions: [], tags: [], notifyWhen: 'onActionGroupChange', + alertDelay: { + active: 5, + }, } as unknown as Rule; }); @@ -211,4 +214,18 @@ describe('rule reducer', () => { ); expect(updatedRule.rule.actions[0].frequency?.notifyWhen).toBe('onThrottleInterval'); }); + + test('if initial alert delay property was updated', () => { + const updatedRule = ruleReducer( + { rule: initialRule }, + { + command: { type: 'setAlertDelayProperty' }, + payload: { + key: 'active', + value: 10, + }, + } + ); + expect(updatedRule.rule.alertDelay?.active).toBe(10); + }); }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_reducer.ts b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_reducer.ts index 54f3871928fb35..257df764ebc1ef 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_reducer.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_reducer.ts @@ -12,6 +12,7 @@ import { RuleActionParam, IntervalSchedule, RuleActionAlertsFilterProperty, + AlertDelay, } from '@kbn/alerting-plugin/common'; import { isEmpty } from 'lodash/fp'; import { Rule, RuleAction } from '../../../types'; @@ -30,6 +31,7 @@ interface CommandType< | 'setRuleActionProperty' | 'setRuleActionFrequency' | 'setRuleActionAlertsFilter' + | 'setAlertDelayProperty' > { type: T; } @@ -62,6 +64,12 @@ interface RuleSchedulePayload { index?: number; } +interface AlertDelayPayload { + key: Key; + value: AlertDelay[Key] | null; + index?: number; +} + export type RuleReducerAction = | { command: CommandType<'setRule'>; @@ -94,6 +102,10 @@ export type RuleReducerAction = | { command: CommandType<'setRuleActionAlertsFilter'>; payload: Payload; + } + | { + command: CommandType<'setAlertDelayProperty'>; + payload: AlertDelayPayload; }; export type InitialRuleReducer = Reducer<{ rule: InitialRule }, RuleReducerAction>; @@ -281,5 +293,22 @@ export const ruleReducer = ( }; } } + case 'setAlertDelayProperty': { + const { key, value } = action.payload as Payload; + if (rule.alertDelay && isEqual(rule.alertDelay[key], value)) { + return state; + } else { + return { + ...state, + rule: { + ...rule, + alertDelay: { + ...rule.alertDelay, + [key]: value, + }, + }, + }; + } + } } };