[Alert Summaries] [FE] Move “Notify When” and throttle from rule to action

x-pack/plugins/alerting/common/rule_notify_when_type.ts

@@ -12,6 +12,12 @@ export const RuleNotifyWhenTypeValues = [
 ] as const;
 export type RuleNotifyWhenType = typeof RuleNotifyWhenTypeValues[number];
 
+export enum RuleNotifyWhen {
+  CHANGE = 'onActionGroupChange',
+  ACTIVE = 'onActiveAlert',
+  THROTTLE = 'onThrottleInterval',
+}
+
 export function validateNotifyWhenType(notifyWhen: string) {
   if (RuleNotifyWhenTypeValues.includes(notifyWhen as RuleNotifyWhenType)) {
     return;

x-pack/plugins/alerting/server/index.ts

@@ -30,6 +30,7 @@ export type {
   RuleParamsAndRefs,
   GetSummarizedAlertsFnOpts,
 } from './types';
+export { RuleNotifyWhen } from '../common';
 export { DEFAULT_MAX_EPHEMERAL_ACTIONS_PER_ALERT } from './config';
 export type { PluginSetupContract, PluginStartContract } from './plugin';
 export type {

x-pack/plugins/alerting/server/lib/get_rule_notify_when_type.test.ts

@@ -19,6 +19,6 @@ test(`should return 'onThrottleInterval' value if 'notifyWhen' is null and throt
   expect(getRuleNotifyWhenType(null, '10m')).toEqual('onThrottleInterval');
 });
 
-test(`should return 'onActiveAlert' value if 'notifyWhen' is null and throttle is null`, () => {
-  expect(getRuleNotifyWhenType(null, null)).toEqual('onActiveAlert');
+test(`should return null value if 'notifyWhen' is null and throttle is null`, () => {
+  expect(getRuleNotifyWhenType(null, null)).toEqual(null);
 });

x-pack/plugins/alerting/server/lib/get_rule_notify_when_type.ts

@@ -10,8 +10,8 @@ import { RuleNotifyWhenType } from '../types';
 export function getRuleNotifyWhenType(
   notifyWhen: RuleNotifyWhenType | null,
   throttle: string | null
-): RuleNotifyWhenType {
+): RuleNotifyWhenType | null {
   // We allow notifyWhen to be null for backwards compatibility. If it is null, determine its
   // value based on whether the throttle is set to a value or null
-  return notifyWhen ? notifyWhen! : throttle ? 'onThrottleInterval' : 'onActiveAlert';
+  return notifyWhen ? notifyWhen! : throttle ? 'onThrottleInterval' : null;
 }

x-pack/plugins/alerting/server/routes/clone_rule.ts

@@ -69,11 +69,12 @@ const rewriteBodyRes: RewriteResponseCase<PartialRule<RuleTypeParams>> = ({
     : {}),
   ...(actions
     ? {
-        actions: actions.map(({ group, id, actionTypeId, params }) => ({
+        actions: actions.map(({ group, id, actionTypeId, params, frequency }) => ({
           group,
           id,
           params,
           connector_type_id: actionTypeId,
+          frequency,
         })),
       }
     : {}),

x-pack/plugins/alerting/server/routes/get_rule.ts

@@ -59,11 +59,12 @@ const rewriteBodyRes: RewriteResponseCase<SanitizedRule<RuleTypeParams>> = ({
     last_execution_date: executionStatus.lastExecutionDate,
     last_duration: executionStatus.lastDuration,
   },
-  actions: actions.map(({ group, id, actionTypeId, params }) => ({
+  actions: actions.map(({ group, id, actionTypeId, params, frequency }) => ({
     group,
     id,
     params,
     connector_type_id: actionTypeId,
+    frequency,
   })),
   ...(lastRun ? { last_run: rewriteRuleLastRun(lastRun) } : {}),
   ...(nextRun ? { next_run: nextRun } : {}),

x-pack/plugins/alerting/server/routes/legacy/update.test.ts

@@ -13,7 +13,7 @@ import { verifyApiAccess } from '../../lib/license_api_access';
 import { mockHandlerArguments } from '../_mock_handler_arguments';
 import { rulesClientMock } from '../../rules_client.mock';
 import { RuleTypeDisabledError } from '../../lib/errors/rule_type_disabled';
-import { RuleNotifyWhenType } from '../../../common';
+import { RuleNotifyWhen } from '../../../common';
 import { trackLegacyRouteUsage } from '../../lib/track_legacy_route_usage';
 
 const rulesClient = rulesClientMock.create();
@@ -50,7 +50,7 @@ describe('updateAlertRoute', () => {
         },
       },
     ],
-    notifyWhen: 'onActionGroupChange' as RuleNotifyWhenType,
+    notifyWhen: RuleNotifyWhen.CHANGE,
   };
 
   it('updates an alert with proper parameters', async () => {

x-pack/plugins/alerting/server/routes/lib/rewrite_rule.ts

@@ -63,10 +63,7 @@ export const rewriteRule = ({
     connector_type_id: actionTypeId,
     ...(frequency
       ? {
-          frequency: {
-            ...frequency,
-            notify_when: frequency.notifyWhen,
-          },
+          frequency,
         }
       : {}),
   })),

x-pack/plugins/alerting/server/routes/resolve_rule.ts

@@ -54,11 +54,12 @@ const rewriteBodyRes: RewriteResponseCase<ResolvedSanitizedRule<RuleTypeParams>>
     last_execution_date: executionStatus.lastExecutionDate,
     last_duration: executionStatus.lastDuration,
   },
-  actions: actions.map(({ group, id, actionTypeId, params }) => ({
+  actions: actions.map(({ group, id, actionTypeId, params, frequency }) => ({
     group,
     id,
     params,
     connector_type_id: actionTypeId,
+    frequency,
   })),
   ...(lastRun ? { last_run: rewriteRuleLastRun(lastRun) } : {}),
   ...(nextRun ? { next_run: nextRun } : {}),

x-pack/plugins/alerting/server/routes/update_rule.test.ts

@@ -14,7 +14,7 @@ import { mockHandlerArguments } from './_mock_handler_arguments';
 import { UpdateOptions } from '../rules_client';
 import { rulesClientMock } from '../rules_client.mock';
 import { RuleTypeDisabledError } from '../lib/errors/rule_type_disabled';
-import { RuleNotifyWhenType } from '../../common';
+import { RuleNotifyWhen } from '../../common';
 import { AsApiContract } from './lib';
 import { PartialRule } from '../types';
 
@@ -50,7 +50,7 @@ describe('updateRuleRoute', () => {
         },
       },
     ],
-    notifyWhen: 'onActionGroupChange' as RuleNotifyWhenType,
+    notifyWhen: RuleNotifyWhen.CHANGE,
   };
 
   const updateRequest: AsApiContract<UpdateOptions<{ otherField: boolean }>['data']> = {

x-pack/plugins/alerting/server/routes/update_rule.ts

@@ -96,11 +96,12 @@ const rewriteBodyRes: RewriteResponseCase<PartialRule<RuleTypeParams>> = ({
     : {}),
   ...(actions
     ? {
-        actions: actions.map(({ group, id, actionTypeId, params }) => ({
+        actions: actions.map(({ group, id, actionTypeId, params, frequency }) => ({
           group,
           id,
           params,
           connector_type_id: actionTypeId,
+          frequency,
         })),
       }
     : {}),

x-pack/plugins/alerting/server/rules_client/lib/validate_actions.ts

@@ -19,24 +19,8 @@ export async function validateActions(
   data: Pick<RawRule, 'notifyWhen' | 'throttle'> & { actions: NormalizedAlertAction[] }
 ): Promise<void> {
   const { actions, notifyWhen, throttle } = data;
-  const hasNotifyWhen = typeof notifyWhen !== 'undefined';
-  const hasThrottle = typeof throttle !== 'undefined';
-  let usesRuleLevelFreqParams;
-  // I removed the below ` && hasThrottle` check temporarily.
-  // Currently the UI sends "throttle" as undefined but schema converts it to null, so they never become both undefined
-  // I changed the schema too, but as the UI (and tests) sends "notifyWhen" as string and "throttle" as undefined, they never become both defined.
-  // We should add it back when the UI is changed (https://github.com/elastic/kibana/issues/143369)
-  if (hasNotifyWhen) usesRuleLevelFreqParams = true;
-  else if (!hasNotifyWhen && !hasThrottle) usesRuleLevelFreqParams = false;
-  else {
-    throw Boom.badRequest(
-      i18n.translate('xpack.alerting.rulesClient.usesValidGlobalFreqParams.oneUndefined', {
-        defaultMessage:
-          'Rule-level notifyWhen and throttle must both be defined or both be undefined',
-      })
-    );
-  }
-
+  const hasRuleLevelNotifyWhen = typeof notifyWhen !== 'undefined';
+  const hasRuleLevelThrottle = Boolean(throttle);
   if (actions.length === 0) {
     return;
   }
@@ -81,13 +65,13 @@ export async function validateActions(
   }
 
   // check for actions using frequency params if the rule has rule-level frequency params defined
-  if (usesRuleLevelFreqParams) {
+  if (hasRuleLevelNotifyWhen || hasRuleLevelThrottle) {
     const actionsWithFrequency = actions.filter((action) => Boolean(action.frequency));
     if (actionsWithFrequency.length) {
       throw Boom.badRequest(
         i18n.translate('xpack.alerting.rulesClient.validateActions.mixAndMatchFreqParams', {
           defaultMessage:
-            'Cannot specify per-action frequency params when notify_when and throttle are defined at the rule level: {groups}',
+            'Cannot specify per-action frequency params when notify_when or throttle are defined at the rule level: {groups}',
           values: {
             groups: actionsWithFrequency.map((a) => a.group).join(', '),
           },

x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts

@@ -7,7 +7,7 @@
 
 import pMap from 'p-map';
 import Boom from '@hapi/boom';
-import { cloneDeep } from 'lodash';
+import { cloneDeep, omit } from 'lodash';
 import { AlertConsumers } from '@kbn/rule-data-utils';
 import { KueryNode, nodeBuilder } from '@kbn/es-query';
 import {
@@ -540,7 +540,20 @@ async function getUpdatedAttributesFromOperations(
     // the `isAttributesUpdateSkipped` flag to false.
     switch (operation.field) {
       case 'actions': {
-        await validateActions(context, ruleType, { ...attributes, actions: operation.value });
+        try {
+          await validateActions(context, ruleType, {
+            ...attributes,
+            actions: operation.value,
+          });
+        } catch (e) {
+          // If validateActions fails on the first attempt, it may be because of legacy rule-level frequency params
+          attributes = await attemptToMigrateLegacyFrequency(
+            context,
+            operation,
+            attributes,
+            ruleType
+          );
+        }
 
         const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation(
           operation,
@@ -550,6 +563,18 @@ async function getUpdatedAttributesFromOperations(
           ruleActions = modifiedAttributes;
           isAttributesUpdateSkipped = false;
         }
+
+        // TODO https://github.com/elastic/kibana/issues/148414
+        // If any action-level frequencies get pushed into a SIEM rule, strip their frequencies
+        const firstFrequency = operation.value[0]?.frequency;
+        if (rule.attributes.consumer === AlertConsumers.SIEM && firstFrequency) {
+          ruleActions.actions = ruleActions.actions.map((action) => omit(action, 'frequency'));
+          if (!attributes.notifyWhen) {
+            attributes.notifyWhen = firstFrequency.notifyWhen;
+            attributes.throttle = firstFrequency.throttle;
+          }
+        }
+
         break;
       }
       case 'snoozeSchedule': {
@@ -754,3 +779,21 @@ async function saveBulkUpdatedRules(
 
   return { result, apiKeysToInvalidate };
 }
+
+async function attemptToMigrateLegacyFrequency(
+  context: RulesClientContext,
+  operation: BulkEditOperation,
+  attributes: SavedObjectsFindResult<RawRule>['attributes'],
+  ruleType: RuleType
+) {
+  if (operation.field !== 'actions')
+    throw new Error('Can only perform frequency migration on an action operation');
+  // Try to remove the rule-level frequency params, and then validate actions
+  if (typeof attributes.notifyWhen !== 'undefined') attributes.notifyWhen = undefined;
+  if (attributes.throttle) attributes.throttle = undefined;
+  await validateActions(context, ruleType, {
+    ...attributes,
+    actions: operation.value,
+  });
+  return attributes;
+}

x-pack/plugins/alerting/server/rules_client/methods/create.ts

@@ -6,6 +6,8 @@
  */
 import Semver from 'semver';
 import Boom from '@hapi/boom';
+import { omit } from 'lodash';
+import { AlertConsumers } from '@kbn/rule-data-utils';
 import { SavedObjectsUtils } from '@kbn/core/server';
 import { withSpan } from '@kbn/apm-utils';
 import { parseDuration } from '../../../common/parse_duration';
@@ -91,6 +93,17 @@ export async function create<Params extends RuleTypeParams = never>(
     throw Boom.badRequest(`Error creating rule: could not create API key - ${error.message}`);
   }
 
+  // TODO https://github.com/elastic/kibana/issues/148414
+  // If any action-level frequencies get pushed into a SIEM rule, strip their frequencies
+  const firstFrequency = data.actions[0]?.frequency;
+  if (data.consumer === AlertConsumers.SIEM && firstFrequency) {
+    data.actions = data.actions.map((action) => omit(action, 'frequency'));
+    if (!data.notifyWhen) {
+      data.notifyWhen = firstFrequency.notifyWhen;
+      data.throttle = firstFrequency.throttle;
+    }
+  }
+
   await validateActions(context, ruleType, data);
   await withSpan({ name: 'validateActions', type: 'rules' }, () =>
     validateActions(context, ruleType, data)

x-pack/plugins/alerting/server/rules_client/methods/update.ts

@@ -6,8 +6,9 @@
  */
 
 import Boom from '@hapi/boom';
-import { isEqual } from 'lodash';
+import { isEqual, omit } from 'lodash';
 import { SavedObject } from '@kbn/core/server';
+import { AlertConsumers } from '@kbn/rule-data-utils';
 import {
   PartialRule,
   RawRule,
@@ -142,6 +143,17 @@ async function updateAlert<Params extends RuleTypeParams>(
 ): Promise<PartialRule<Params>> {
   const ruleType = context.ruleTypeRegistry.get(attributes.alertTypeId);
 
+  // TODO https://github.com/elastic/kibana/issues/148414
+  // If any action-level frequencies get pushed into a SIEM rule, strip their frequencies
+  const firstFrequency = data.actions[0]?.frequency;
+  if (attributes.consumer === AlertConsumers.SIEM && firstFrequency) {
+    data.actions = data.actions.map((action) => omit(action, 'frequency'));
+    if (!attributes.notifyWhen) {
+      attributes.notifyWhen = firstFrequency.notifyWhen;
+      attributes.throttle = firstFrequency.throttle;
+    }
+  }
+
   // Validate
   const validatedAlertTypeParams = validateRuleTypeParams(data.params, ruleType.validate?.params);
   await validateActions(context, ruleType, data);