diff --git a/x-pack/plugins/alerting/server/routes/find_rules.test.ts b/x-pack/plugins/alerting/server/routes/find_rules.test.ts index 6e8f4e5474dbf..38e0497a876d2 100644 --- a/x-pack/plugins/alerting/server/routes/find_rules.test.ts +++ b/x-pack/plugins/alerting/server/routes/find_rules.test.ts @@ -4,6 +4,8 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ + +import { pick } from 'lodash'; import { usageCountersServiceMock } from '@kbn/usage-collection-plugin/server/usage_counters/usage_counters_service.mock'; import { findRulesRoute } from './find_rules'; import { httpServiceMock } from '@kbn/core/server/mocks'; @@ -12,6 +14,7 @@ import { verifyApiAccess } from '../lib/license_api_access'; import { mockHandlerArguments } from './_mock_handler_arguments'; import { rulesClientMock } from '../rules_client.mock'; import { trackLegacyTerminology } from './lib/track_legacy_terminology'; +import { RuleActionTypes } from '../types'; const rulesClient = rulesClientMock.create(); const mockUsageCountersSetup = usageCountersServiceMock.createSetupContract(); @@ -30,6 +33,96 @@ beforeEach(() => { }); describe('findRulesRoute', () => { + const action = { + actionTypeId: 'test', + group: 'default', + id: '2', + params: { + foo: true, + }, + type: RuleActionTypes.DEFAULT, + }; + + const systemAction = { + actionTypeId: 'test-2', + id: 'system_action-id', + params: { + foo: true, + }, + type: RuleActionTypes.SYSTEM, + }; + + const rule = { + alertTypeId: '1', + consumer: 'bar', + name: 'abc', + schedule: { interval: '10s' }, + tags: ['foo'], + params: { + bar: true, + }, + throttle: '30s', + actions: [action], + enabled: true, + muteAll: false, + createdBy: '', + updatedBy: '', + apiKeyOwner: '', + mutedInstanceIds: [], + notifyWhen: 'onActionGroupChange' as const, + createdAt: new Date(), + updatedAt: new Date(), + id: '123', + executionStatus: { + status: 'unknown' as const, + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), + }, + revision: 0, + }; + + const rulesClientResponse = { + page: 1, + perPage: 1, + total: 0, + data: [rule], + }; + + const findResponse = { + page: 1, + per_page: 1, + total: 0, + data: [ + { + ...pick(rule, 'consumer', 'name', 'schedule', 'tags', 'params', 'throttle', 'enabled'), + rule_type_id: rule.alertTypeId, + notify_when: rule.notifyWhen, + mute_all: rule.muteAll, + created_by: rule.createdBy, + updated_by: rule.updatedBy, + api_key_owner: rule.apiKeyOwner, + muted_alert_ids: rule.mutedInstanceIds, + created_at: rule.createdAt, + updated_at: rule.updatedAt, + id: rule.id, + revision: rule.revision, + execution_status: { + status: rule.executionStatus.status, + last_execution_date: rule.executionStatus.lastExecutionDate, + }, + actions: [ + { + connector_type_id: 'test', + group: 'default', + id: '2', + params: { + foo: true, + }, + }, + ], + }, + ], + }; + it('finds rules with proper parameters', async () => { const licenseState = licenseStateMock.create(); const router = httpServiceMock.createRouter(); @@ -40,13 +133,7 @@ describe('findRulesRoute', () => { expect(config.path).toMatchInlineSnapshot(`"/api/alerting/rules/_find"`); - const findResult = { - page: 1, - perPage: 1, - total: 0, - data: [], - }; - rulesClient.find.mockResolvedValueOnce(findResult); + rulesClient.find.mockResolvedValueOnce(rulesClientResponse); const [context, req, res] = mockHandlerArguments( { rulesClient }, @@ -60,16 +147,7 @@ describe('findRulesRoute', () => { ['ok'] ); - expect(await handler(context, req, res)).toMatchInlineSnapshot(` - Object { - "body": Object { - "data": Array [], - "page": 1, - "per_page": 1, - "total": 0, - }, - } - `); + await handler(context, req, res); expect(rulesClient.find).toHaveBeenCalledTimes(1); expect(rulesClient.find.mock.calls[0]).toMatchInlineSnapshot(` @@ -87,12 +165,7 @@ describe('findRulesRoute', () => { `); expect(res.ok).toHaveBeenCalledWith({ - body: { - page: 1, - per_page: 1, - total: 0, - data: [], - }, + body: findResponse, }); }); @@ -104,12 +177,7 @@ describe('findRulesRoute', () => { const [, handler] = router.get.mock.calls[0]; - rulesClient.find.mockResolvedValueOnce({ - page: 1, - perPage: 1, - total: 0, - data: [], - }); + rulesClient.find.mockResolvedValueOnce(rulesClientResponse); const [context, req, res] = mockHandlerArguments( { rulesClient }, @@ -160,14 +228,11 @@ describe('findRulesRoute', () => { const router = httpServiceMock.createRouter(); findRulesRoute(router, licenseState, mockUsageCounter); - const findResult = { - page: 1, - perPage: 1, - total: 0, - data: [], - }; - rulesClient.find.mockResolvedValueOnce(findResult); + + rulesClient.find.mockResolvedValueOnce(rulesClientResponse); + const [, handler] = router.get.mock.calls[0]; + const [context, req, res] = mockHandlerArguments( { rulesClient }, { @@ -180,7 +245,9 @@ describe('findRulesRoute', () => { }, ['ok'] ); + await handler(context, req, res); + expect(trackLegacyTerminology).toHaveBeenCalledTimes(1); expect((trackLegacyTerminology as jest.Mock).mock.calls[0][0]).toStrictEqual([ 'alertTypeId:2', @@ -195,13 +262,7 @@ describe('findRulesRoute', () => { findRulesRoute(router, licenseState, mockUsageCounter); - const findResult = { - page: 1, - perPage: 1, - total: 0, - data: [], - }; - rulesClient.find.mockResolvedValueOnce(findResult); + rulesClient.find.mockResolvedValueOnce(rulesClientResponse); const [, handler] = router.get.mock.calls[0]; const [context, req, res] = mockHandlerArguments( @@ -217,11 +278,60 @@ describe('findRulesRoute', () => { }, ['ok'] ); + await handler(context, req, res); + expect(mockUsageCounter.incrementCounter).toHaveBeenCalledWith({ counterName: `alertingFieldsUsage`, counterType: 'alertingFieldsUsage', incrementBy: 1, }); }); + + describe('actions', () => { + it('removes the type from the actions correctly before sending the response', async () => { + const licenseState = licenseStateMock.create(); + const router = httpServiceMock.createRouter(); + + findRulesRoute(router, licenseState, mockUsageCounter); + + rulesClient.find.mockResolvedValueOnce({ + ...rulesClientResponse, + data: [{ ...rule, actions: [action, systemAction] }], + }); + + const [, handler] = router.get.mock.calls[0]; + const [context, req, res] = mockHandlerArguments( + { rulesClient }, + { + params: {}, + query: { + fields: ['foo', 'bar'], + }, + }, + ['ok'] + ); + + const routeRes = await handler(context, req, res); + + // @ts-expect-error: body exists + expect(routeRes.body.data[0].actions).toEqual([ + { + connector_type_id: 'test', + group: 'default', + id: '2', + params: { + foo: true, + }, + }, + { + connector_type_id: 'test-2', + id: 'system_action-id', + params: { + foo: true, + }, + }, + ]); + }); + }); }); diff --git a/x-pack/plugins/alerting/server/routes/find_rules.ts b/x-pack/plugins/alerting/server/routes/find_rules.ts index 04b18da1a1b0c..3b134045245cf 100644 --- a/x-pack/plugins/alerting/server/routes/find_rules.ts +++ b/x-pack/plugins/alerting/server/routes/find_rules.ts @@ -10,12 +10,7 @@ import { UsageCounter } from '@kbn/usage-collection-plugin/server'; import { schema } from '@kbn/config-schema'; import { ILicenseState } from '../lib'; import { FindOptions, FindResult } from '../rules_client'; -import { - RewriteRequestCase, - RewriteResponseCase, - verifyAccessAndContext, - rewriteRule, -} from './lib'; +import { RewriteRequestCase, verifyAccessAndContext, rewriteRule } from './lib'; import { RuleTypeParams, AlertingRequestHandlerContext, @@ -66,11 +61,8 @@ const rewriteQueryReq: RewriteRequestCase = ({ ...(hasReference ? { hasReference } : {}), ...(searchFields ? { searchFields } : {}), }); -const rewriteBodyRes: RewriteResponseCase> = ({ - perPage, - data, - ...restOfResult -}) => { + +const rewriteBodyRes = ({ perPage, data, ...restOfResult }: FindResult) => { return { ...restOfResult, per_page: perPage, diff --git a/x-pack/plugins/alerting/server/routes/lib/rewrite_actions.test.ts b/x-pack/plugins/alerting/server/routes/lib/rewrite_actions.test.ts index 24cb7f120ba84..df56554d77ef7 100644 --- a/x-pack/plugins/alerting/server/routes/lib/rewrite_actions.test.ts +++ b/x-pack/plugins/alerting/server/routes/lib/rewrite_actions.test.ts @@ -5,8 +5,9 @@ * 2.0. */ +import { omit } from 'lodash'; import { RuleActionTypes } from '../../../common'; -import { rewriteActionsReq, rewriteActionsRes } from './rewrite_actions'; +import { rewriteActionsReq, rewriteActionsRes, rewriteActionsResLegacy } from './rewrite_actions'; describe('rewrite Actions', () => { const isSystemAction = (id: string) => id === 'system-action-id'; @@ -181,4 +182,32 @@ describe('rewrite Actions', () => { expect(rewriteActionsReq([], isSystemAction)).toEqual([]); }); }); + + describe('rewriteActionsResLegacy', () => { + const action = { + actionTypeId: 'test', + group: 'default', + id: '2', + params: { + foo: true, + }, + type: RuleActionTypes.DEFAULT, + }; + + const systemAction = { + actionTypeId: 'test-2', + id: 'system_action-id', + params: { + foo: true, + }, + type: RuleActionTypes.SYSTEM, + }; + + it('removes the type from the actions', () => { + expect(rewriteActionsResLegacy([action, systemAction])).toEqual([ + omit(action, 'type'), + omit(systemAction, 'type'), + ]); + }); + }); }); 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 065762c2e4191..d453ab7e6cd70 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 @@ -60,7 +60,7 @@ const sampleRule: SanitizedRule & { activeSnoozes?: string[] } = lastExecutionDate: DATE_2020, lastDuration: 1000, }, - actions: [defaultAction], + actions: [defaultAction, systemAction], scheduledTaskId: 'xyz456', snoozeSchedule: [], isSnoozedUntil: null, @@ -92,27 +92,35 @@ describe('rewriteRule', () => { } }); - it('should rewrite default actions correctly', () => { - const rewritten = rewriteRule(sampleRule); - for (const rewrittenAction of rewritten.actions) { - expect(Object.keys(rewrittenAction)).toEqual( - expect.arrayContaining(['group', 'id', 'connector_type_id', 'params', 'frequency']) - ); - - expect( - Object.keys((rewrittenAction as Omit).frequency!) - ).toEqual(expect.arrayContaining(['summary', 'notify_when', 'throttle'])); - } - }); - - it('should rewrite system actions correctly', () => { - const rewritten = rewriteRule({ ...sampleRule, actions: [systemAction] }); - expect(Object.keys(rewritten.actions[0])).toEqual( - expect.arrayContaining(['id', 'connector_type_id', 'params', 'uuid', 'type']) - ); - - expect(Object.keys(rewritten.actions[0])).not.toEqual( - expect.arrayContaining(['group', 'frequency', 'alertsFilter']) - ); + it('should rewrite actions correctly', () => { + const res = rewriteRule(sampleRule); + expect(res.actions).toMatchInlineSnapshot(` + Array [ + Object { + "alerts_filter": Object { + "query": Object { + "dsl": "{}", + "filters": Array [], + "kql": "test:1", + }, + }, + "connector_type_id": "bbb", + "frequency": Object { + "notify_when": "onThrottleInterval", + "summary": false, + "throttle": "1m", + }, + "group": "default", + "id": "aaa", + "params": Object {}, + }, + Object { + "connector_type_id": "bbb", + "id": "system-action", + "params": Object {}, + "uuid": "123", + }, + ] + `); }); }); 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 609614b38da45..a7d4be2f0a2e5 100644 --- a/x-pack/plugins/alerting/server/routes/lib/rewrite_rule.ts +++ b/x-pack/plugins/alerting/server/routes/lib/rewrite_rule.ts @@ -7,7 +7,8 @@ import { omit } from 'lodash'; import { isSystemAction } from '../../../common/system_actions/is_system_action'; -import { RuleTypeParams, SanitizedRule, RuleLastRun } from '../../types'; +import { RuleTypeParams, SanitizedRule, RuleLastRun, SanitizedRuleResponse } from '../../types'; +import { AsApiContract } from './rewrite_request_case'; export const rewriteRuleLastRun = (lastRun: RuleLastRun) => { const { outcomeMsg, outcomeOrder, alertsCount, ...rest } = lastRun; @@ -39,7 +40,9 @@ export const rewriteRule = ({ lastRun, nextRun, ...rest -}: SanitizedRule & { activeSnoozes?: string[] }) => ({ +}: SanitizedRule & { + activeSnoozes?: string[]; +}): AsApiContract> => ({ ...rest, rule_type_id: alertTypeId, created_by: createdBy, @@ -61,18 +64,17 @@ export const rewriteRule = ({ }, actions: actions.map((action) => { if (isSystemAction(action)) { - const { actionTypeId, ...restSystemAction } = action; + const { actionTypeId, type, ...restSystemAction } = action; return { ...restSystemAction, connector_type_id: action.actionTypeId }; } - const { group, id, actionTypeId, params, frequency, uuid, alertsFilter, type } = action; + const { group, id, actionTypeId, params, frequency, uuid, alertsFilter } = action; return { group, id, params, connector_type_id: actionTypeId, - ...(type && { type }), ...(frequency ? { frequency: {