Skip to content

Commit

Permalink
Fix rewriteBodyRes
Browse files Browse the repository at this point in the history
  • Loading branch information
cnasikas committed Sep 18, 2023
1 parent 132a46d commit b6e2578
Show file tree
Hide file tree
Showing 5 changed files with 224 additions and 83 deletions.
196 changes: 153 additions & 43 deletions x-pack/plugins/alerting/server/routes/find_rules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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 },
Expand All @@ -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(`
Expand All @@ -87,12 +165,7 @@ describe('findRulesRoute', () => {
`);

expect(res.ok).toHaveBeenCalledWith({
body: {
page: 1,
per_page: 1,
total: 0,
data: [],
},
body: findResponse,
});
});

Expand All @@ -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 },
Expand Down Expand Up @@ -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 },
{
Expand All @@ -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',
Expand All @@ -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(
Expand All @@ -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,
},
},
]);
});
});
});
14 changes: 3 additions & 11 deletions x-pack/plugins/alerting/server/routes/find_rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -66,11 +61,8 @@ const rewriteQueryReq: RewriteRequestCase<FindOptions> = ({
...(hasReference ? { hasReference } : {}),
...(searchFields ? { searchFields } : {}),
});
const rewriteBodyRes: RewriteResponseCase<FindResult<RuleTypeParams>> = ({
perPage,
data,
...restOfResult
}) => {

const rewriteBodyRes = ({ perPage, data, ...restOfResult }: FindResult<RuleTypeParams>) => {
return {
...restOfResult,
per_page: perPage,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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'),
]);
});
});
});
Loading

0 comments on commit b6e2578

Please sign in to comment.