[Alerting V2] Fix stale throttle.interval on strategy change (elastic#268764)

kdelemme · web-flow · commit 37dc569dfd4b · 2026-05-13T20:25:09.000-05:00
diff --git a/x-pack/platform/packages/shared/response-ops/alerting-v2-schemas/src/action_policy_data_schema.ts b/x-pack/platform/packages/shared/response-ops/alerting-v2-schemas/src/action_policy_data_schema.ts
@@ -65,7 +65,11 @@ export type ThrottleStrategy = z.infer<typeof throttleStrategySchema>;
 
 const throttleSchema = z.object({
   strategy: throttleStrategySchema.optional().describe('The throttle strategy.'),
-  interval: durationSchema.optional().describe('The throttle interval duration (e.g. 5m, 1h).'),
+  interval: durationSchema
+    .nullish()
+    .describe(
+      'The throttle interval duration (e.g. 5m, 1h), or null when the strategy is intervalless.'
+    ),
 });
 
 const PER_EPISODE_STRATEGIES = new Set<string>([
@@ -76,10 +80,13 @@ const PER_EPISODE_STRATEGIES = new Set<string>([
 const AGGREGATE_STRATEGIES = new Set<string>(['time_interval', 'every_time']);
 const STRATEGIES_REQUIRING_INTERVAL = new Set<string>(['per_status_interval', 'time_interval']);
 
+export const needsInterval = (strategy: string | undefined): boolean =>
+  strategy != null && STRATEGIES_REQUIRING_INTERVAL.has(strategy);
+
 interface ValidationPayload {
   value: {
     groupingMode?: string | null;
-    throttle?: { strategy?: string; interval?: string } | null;
+    throttle?: { strategy?: string; interval?: string | null } | null;
     type?: string;
     ruleId?: string;
   };
@@ -91,7 +98,7 @@ const validateStrategyInterval = (payload: ValidationPayload) => {
   const strategy = data.throttle?.strategy;
   if (!strategy) return;
 
-  if (STRATEGIES_REQUIRING_INTERVAL.has(strategy) && !data.throttle?.interval) {
+  if (needsInterval(strategy) && !data.throttle?.interval) {
     issues.push({
       code: 'custom',
       message: `Strategy "${strategy}" requires an interval to be defined`,
diff --git a/x-pack/platform/packages/shared/response-ops/alerting-v2-schemas/src/action_policy_response_schema.test.ts b/x-pack/platform/packages/shared/response-ops/alerting-v2-schemas/src/action_policy_response_schema.test.ts
@@ -24,7 +24,7 @@ const validResponse = {
   groupBy: ['host.name'],
   tags: ['production'],
   groupingMode: 'per_episode' as const,
-  throttle: { strategy: 'on_status_change' as const },
+  throttle: { strategy: 'on_status_change' as const, interval: null },
   snoozedUntil: null,
   auth: { owner: 'user-1', createdByUser: true },
   createdBy: 'user-1',
diff --git a/x-pack/platform/packages/shared/response-ops/alerting-v2-schemas/src/action_policy_response_schema.ts b/x-pack/platform/packages/shared/response-ops/alerting-v2-schemas/src/action_policy_response_schema.ts
@@ -38,7 +38,11 @@ export const actionPolicyResponseSchema = z.object({
   throttle: z
     .object({
       strategy: throttleStrategySchema.optional().describe('The throttle strategy.'),
-      interval: durationSchema.optional().describe('The throttle interval duration (e.g. 5m, 1h).'),
+      interval: durationSchema
+        .nullable()
+        .describe(
+          'The throttle interval duration (e.g. 5m, 1h), or null when the strategy is intervalless.'
+        ),
     })
     .nullable()
     .describe('The throttle configuration for notifications.'),
diff --git a/x-pack/platform/plugins/shared/alerting_v2/public/components/action_policy/details_flyout/action_policy_details_flyout.test.tsx b/x-pack/platform/plugins/shared/alerting_v2/public/components/action_policy/details_flyout/action_policy_details_flyout.test.tsx
@@ -195,7 +195,7 @@ describe('ActionPolicyDetailsFlyout', () => {
         policy: createPolicy({
           groupingMode: 'per_episode',
           groupBy: null,
-          throttle: { strategy: 'on_status_change' },
+          throttle: { strategy: 'on_status_change', interval: null },
         }),
       });
 
diff --git a/x-pack/platform/plugins/shared/alerting_v2/public/components/action_policy/form/form_utils.test.ts b/x-pack/platform/plugins/shared/alerting_v2/public/components/action_policy/form/form_utils.test.ts
@@ -28,7 +28,7 @@ describe('action policy form utils', () => {
         description: 'Description',
         type: 'global',
         groupingMode: 'per_episode',
-        throttle: { strategy: 'on_status_change' },
+        throttle: { strategy: 'on_status_change', interval: null },
         destinations: [{ type: 'workflow', id: 'workflow-1' }],
       });
     });
@@ -53,13 +53,23 @@ describe('action policy form utils', () => {
       });
     });
 
-    it('omits throttle interval for strategies that do not require it', () => {
+    it('emits interval: null for strategies that do not require it', () => {
       const payload = toCreatePayload({
         ...state,
         throttleStrategy: 'every_time',
       });
 
-      expect(payload.throttle).toEqual({ strategy: 'every_time' });
+      expect(payload.throttle).toEqual({ strategy: 'every_time', interval: null });
+    });
+
+    it('emits interval: null when strategy does not need interval, even if state holds a stale value', () => {
+      const payload = toCreatePayload({
+        ...state,
+        throttleStrategy: 'on_status_change',
+        throttleInterval: '5m',
+      });
+
+      expect(payload.throttle).toEqual({ strategy: 'on_status_change', interval: null });
     });
   });
 
@@ -73,7 +83,7 @@ describe('action policy form utils', () => {
         tags: null,
         matcher: null,
         groupBy: null,
-        throttle: { strategy: 'on_status_change' },
+        throttle: { strategy: 'on_status_change', interval: null },
         destinations: [{ type: 'workflow', id: 'workflow-1' }],
       });
     });
diff --git a/x-pack/platform/plugins/shared/alerting_v2/public/components/action_policy/form/form_utils.ts b/x-pack/platform/plugins/shared/alerting_v2/public/components/action_policy/form/form_utils.ts
@@ -8,18 +8,17 @@
 import type {
   CreateActionPolicyData,
   ActionPolicyResponse,
-  ThrottleStrategy,
   UpdateActionPolicyBody,
 } from '@kbn/alerting-v2-schemas';
+import { needsInterval } from '@kbn/alerting-v2-schemas';
 import { DEFAULT_STRATEGY_FOR_MODE } from './constants';
 import type { ActionPolicyFormState } from './types';
 
-export const needsInterval = (strategy: ThrottleStrategy): boolean =>
-  strategy === 'per_status_interval' || strategy === 'time_interval';
+export { needsInterval };
 
 const buildThrottle = (state: ActionPolicyFormState) => ({
   strategy: state.throttleStrategy,
-  ...(needsInterval(state.throttleStrategy) ? { interval: state.throttleInterval } : {}),
+  interval: needsInterval(state.throttleStrategy) ? state.throttleInterval : null,
 });
 
 export const toFormState = (response: ActionPolicyResponse): ActionPolicyFormState => {
diff --git a/x-pack/platform/plugins/shared/alerting_v2/public/components/action_policy/form/use_action_policy_form.test.ts b/x-pack/platform/plugins/shared/alerting_v2/public/components/action_policy/form/use_action_policy_form.test.ts
@@ -85,7 +85,7 @@ describe('useActionPolicyForm', () => {
         description: 'A description',
         type: 'global',
         groupingMode: 'per_episode',
-        throttle: { strategy: 'on_status_change' },
+        throttle: { strategy: 'on_status_change', interval: null },
         destinations: [],
       });
     });
@@ -114,7 +114,7 @@ describe('useActionPolicyForm', () => {
       expect(payload).not.toHaveProperty('matcher');
       expect(payload).not.toHaveProperty('groupBy');
       expect(payload.groupingMode).toBe('per_episode');
-      expect(payload.throttle).toEqual({ strategy: 'on_status_change' });
+      expect(payload.throttle).toEqual({ strategy: 'on_status_change', interval: null });
     });
   });
 
diff --git a/x-pack/platform/plugins/shared/alerting_v2/public/components/action_policy/form_flyout/action_policy_form_flyout.test.tsx b/x-pack/platform/plugins/shared/alerting_v2/public/components/action_policy/form_flyout/action_policy_form_flyout.test.tsx
@@ -151,7 +151,7 @@ describe('ActionPolicyFormFlyout', () => {
       description: 'Description from test',
       type: 'global',
       groupingMode: 'per_episode',
-      throttle: { strategy: 'on_status_change' },
+      throttle: { strategy: 'on_status_change', interval: null },
       destinations: [{ type: 'workflow', id: 'wf-1' }],
     });
   });
diff --git a/x-pack/platform/plugins/shared/alerting_v2/public/pages/action_policy_form_page/action_policy_form_page.test.tsx b/x-pack/platform/plugins/shared/alerting_v2/public/pages/action_policy_form_page/action_policy_form_page.test.tsx
@@ -209,7 +209,7 @@ describe('ActionPolicyFormPage', () => {
             description: 'Description from test',
             type: 'global',
             groupingMode: 'per_episode',
-            throttle: { strategy: 'on_status_change' },
+            throttle: { strategy: 'on_status_change', interval: null },
             destinations: [{ type: 'workflow', id: 'workflow-1' }],
           },
           expect.objectContaining({ onSuccess: expect.any(Function) })
diff --git a/x-pack/platform/plugins/shared/alerting_v2/public/pages/list_action_policies_page/list_action_policies_page.test.tsx b/x-pack/platform/plugins/shared/alerting_v2/public/pages/list_action_policies_page/list_action_policies_page.test.tsx
@@ -164,7 +164,7 @@ const createPolicy = (overrides: Partial<ActionPolicyResponse> = {}): ActionPoli
   groupBy: null,
   tags: null,
   groupingMode: null,
-  throttle: { strategy: undefined, interval: undefined },
+  throttle: { strategy: undefined, interval: null },
   snoozedUntil: null,
   auth: {
     owner: 'elastic',
diff --git a/x-pack/platform/plugins/shared/alerting_v2/server/lib/action_policy_client/action_policy_client.test.ts b/x-pack/platform/plugins/shared/alerting_v2/server/lib/action_policy_client/action_policy_client.test.ts
@@ -480,33 +480,37 @@ describe('ActionPolicyClient', () => {
       });
     });
 
-    it('defaults legacy attributes (no type) to type "global" with null ruleId', async () => {
-      const existingAttributes = {
-        name: 'legacy',
-        description: 'd',
+    it('heals pre-fix documents: reads throttle.interval as null for intervalless strategy', async () => {
+      const existingAttributes: ActionPolicySavedObjectAttributes = {
+        name: 'stale-policy',
+        description: 'stale-policy description',
         type: 'global',
         enabled: true,
-        destinations: [{ type: 'workflow' as const, id: 'w' }],
-        auth: { apiKey: 'k', owner: 'u', createdByUser: false },
-        createdBy: null,
-        createdByUsername: null,
+        destinations: [{ type: 'workflow', id: 'test-workflow' }],
+        throttle: { strategy: 'on_status_change', interval: '5m' }, // stale pre-fix state
+        auth: {
+          apiKey: 'encrypted-api-key',
+          owner: 'test-user',
+          createdByUser: false,
+        },
+        createdBy: 'elastic_profile_uid',
+        createdByUsername: 'elastic',
         createdAt: '2025-01-01T00:00:00.000Z',
-        updatedBy: null,
-        updatedByUsername: null,
+        updatedBy: 'elastic_profile_uid',
+        updatedByUsername: 'elastic',
         updatedAt: '2025-01-01T00:00:00.000Z',
-      } as ActionPolicySavedObjectAttributes;
+      };
       mockSavedObjectsClient.get.mockResolvedValueOnce({
-        id: 'p-legacy',
+        id: 'policy-id-get-stale',
         type: ACTION_POLICY_SAVED_OBJECT_TYPE,
         attributes: existingAttributes,
         references: [],
         version: 'WzEsMV0=',
       });
 
-      const res = await client.getActionPolicy({ id: 'p-legacy' });
+      const res = await client.getActionPolicy({ id: 'policy-id-get-stale' });
 
-      expect(res.type).toBe('global');
-      expect(res.ruleId).toBeNull();
+      expect(res.throttle).toEqual({ strategy: 'on_status_change', interval: null });
     });
 
     it('returns ruleId for a single_rule policy', async () => {
@@ -525,6 +529,7 @@ describe('ActionPolicyClient', () => {
         updatedByUsername: null,
         updatedAt: '2025-01-01T00:00:00.000Z',
       } as ActionPolicySavedObjectAttributes;
+
       mockSavedObjectsClient.get.mockResolvedValueOnce({
         id: 'p-single',
         type: ACTION_POLICY_SAVED_OBJECT_TYPE,
@@ -1081,6 +1086,116 @@ describe('ActionPolicyClient', () => {
       expect(res.snoozedUntil).toBeNull();
     });
 
+    it('nulls throttle.interval when transitioning to an intervalless strategy', async () => {
+      const existingAttributes: ActionPolicySavedObjectAttributes = {
+        name: 'transition-policy',
+        description: 'transition-policy description',
+        type: 'global',
+        enabled: true,
+        destinations: [{ type: 'workflow', id: 'wf-1' }],
+        groupingMode: 'per_episode',
+        throttle: { strategy: 'per_status_interval', interval: '10m' },
+        auth: {
+          apiKey: 'old-api-key',
+          owner: 'old-user',
+          createdByUser: false,
+        },
+        createdBy: 'creator_profile_uid',
+        createdByUsername: 'creator',
+        createdAt: '2024-12-01T00:00:00.000Z',
+        updatedBy: 'updater_profile_uid',
+        updatedByUsername: 'updater',
+        updatedAt: '2024-12-01T00:00:00.000Z',
+      };
+      mockSavedObjectsClient.get.mockResolvedValueOnce({
+        id: 'policy-id-update-1',
+        type: ACTION_POLICY_SAVED_OBJECT_TYPE,
+        references: [],
+        version: 'WzEsMV0=',
+        attributes: existingAttributes,
+      });
+      mockSavedObjectsClient.update.mockResolvedValueOnce({
+        id: 'policy-id-update-1',
+        type: ACTION_POLICY_SAVED_OBJECT_TYPE,
+        attributes: {
+          ...existingAttributes,
+          throttle: { strategy: 'on_status_change', interval: null },
+        },
+        references: [],
+        version: 'WzIsMV0=',
+      });
+
+      const res = await client.updateActionPolicy({
+        data: { throttle: { strategy: 'on_status_change' } },
+        options: { id: 'policy-id-update-1', version: 'WzEsMV0=' },
+      });
+
+      expect(mockSavedObjectsClient.update).toHaveBeenCalledWith(
+        ACTION_POLICY_SAVED_OBJECT_TYPE,
+        'policy-id-update-1',
+        expect.objectContaining({
+          throttle: { strategy: 'on_status_change', interval: null },
+        }),
+        { version: 'WzEsMV0=' }
+      );
+      expect(res.throttle).toEqual({ strategy: 'on_status_change', interval: null });
+    });
+
+    it('preserves throttle.interval for interval-requiring strategies', async () => {
+      const existingAttributes: ActionPolicySavedObjectAttributes = {
+        name: 'keep-interval-policy',
+        description: 'keep-interval-policy description',
+        type: 'global',
+        enabled: true,
+        destinations: [{ type: 'workflow', id: 'wf-1' }],
+        groupingMode: 'per_episode',
+        throttle: { strategy: 'on_status_change', interval: null },
+        auth: {
+          apiKey: 'old-api-key',
+          owner: 'old-user',
+          createdByUser: false,
+        },
+        createdBy: 'creator_profile_uid',
+        createdByUsername: 'creator',
+        createdAt: '2024-12-01T00:00:00.000Z',
+        updatedBy: 'updater_profile_uid',
+        updatedByUsername: 'updater',
+        updatedAt: '2024-12-01T00:00:00.000Z',
+      };
+      mockSavedObjectsClient.get.mockResolvedValueOnce({
+        id: 'policy-id-update-1',
+        type: ACTION_POLICY_SAVED_OBJECT_TYPE,
+        references: [],
+        version: 'WzEsMV0=',
+        attributes: existingAttributes,
+      });
+      mockSavedObjectsClient.update.mockResolvedValueOnce({
+        id: 'policy-id-update-1',
+        type: ACTION_POLICY_SAVED_OBJECT_TYPE,
+        attributes: {
+          ...existingAttributes,
+          throttle: { strategy: 'per_status_interval', interval: '5m' },
+        },
+        references: [],
+        version: 'WzIsMV0=',
+      });
+
+      const res = await client.updateActionPolicy({
+        data: { throttle: { strategy: 'per_status_interval', interval: '5m' } },
+        options: { id: 'policy-id-update-1', version: 'WzEsMV0=' },
+      });
+
+      expect(mockSavedObjectsClient.update).toHaveBeenCalledWith(
+        ACTION_POLICY_SAVED_OBJECT_TYPE,
+        'policy-id-update-1',
+        expect.objectContaining({
+          throttle: { strategy: 'per_status_interval', interval: '5m' },
+        }),
+        { version: 'WzEsMV0=' }
+      );
+      expect(res.throttle).toEqual({ strategy: 'per_status_interval', interval: '5m' });
+    });
+
     it('updates a action policy and rotates the API key', async () => {
       const existingAttributes: ActionPolicySavedObjectAttributes = {
         name: 'original-policy',
@@ -1557,7 +1672,7 @@ describe('ActionPolicyClient', () => {
         );
       });
 
-      it('normalizes a legacy attributes (no type) to "global" with null ruleId on update', async () => {
+      it('preserves type and ruleId from the existing global policy on partial update', async () => {
         setupSinglePolicyMocks({ ...baseExisting });
 
         await client.updateActionPolicy({
diff --git a/x-pack/platform/plugins/shared/alerting_v2/server/lib/action_policy_client/utils.ts b/x-pack/platform/plugins/shared/alerting_v2/server/lib/action_policy_client/utils.ts
diff --git a/x-pack/platform/plugins/shared/alerting_v2/server/lib/dispatcher/types.ts b/x-pack/platform/plugins/shared/alerting_v2/server/lib/dispatcher/types.ts
diff --git a/x-pack/platform/plugins/shared/alerting_v2/server/saved_objects/schemas/action_policy_saved_object_attributes/v1.ts b/x-pack/platform/plugins/shared/alerting_v2/server/saved_objects/schemas/action_policy_saved_object_attributes/v1.ts
diff --git a/x-pack/platform/test/api_integration_deployment_agnostic/apis/alerting_v2/action_policy/create_action_policy.ts b/x-pack/platform/test/api_integration_deployment_agnostic/apis/alerting_v2/action_policy/create_action_policy.ts
diff --git a/x-pack/platform/test/api_integration_deployment_agnostic/apis/alerting_v2/action_policy/update_action_policy.ts b/x-pack/platform/test/api_integration_deployment_agnostic/apis/alerting_v2/action_policy/update_action_policy.ts
