Skip to content

Commit

Permalink
[Alerting] make actionGroup name's i18n-able (#57404)
Browse files Browse the repository at this point in the history
We want to make the Action Group i18n-able for display in the AlertDetails page, so instead of just a list of ids, the AlertType now registers an object where key is the id and value is the human readable, and translatable, value.
  • Loading branch information
gmmorris authored Feb 12, 2020
1 parent d00adca commit fd193fd
Show file tree
Hide file tree
Showing 20 changed files with 101 additions and 48 deletions.
2 changes: 1 addition & 1 deletion x-pack/legacy/plugins/alerting/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ The following table describes the properties of the `options` object.
|---|---|---|
|id|Unique identifier for the alert type. For convention purposes, ids starting with `.` are reserved for built in alert types. We recommend using a convention like `<plugin_id>.mySpecialAlert` for your alert types to avoid conflicting with another plugin.|string|
|name|A user-friendly name for the alert type. These will be displayed in dropdowns when choosing alert types.|string|
|actionGroups|An explicit list of groups the alert type may schedule actions for. Alert `actions` validation will use this array to ensure groups are valid.|string[]|
|actionGroups|An explicit list of groups the alert type may schedule actions for, each specifying the ActionGroup's unique ID and human readable name. Alert `actions` validation will use this configuartion to ensure groups are valid. We highly encourage using `kbn-i18n` to translate the names of actionGroup when registering the AlertType. |Array<{id:string, name:string}>|
|validate.params|When developing an alert type, you can choose to accept a series of parameters. You may also have the parameters validated before they are passed to the `executor` function or created as an alert saved object. In order to do this, provide a `@kbn/config-schema` schema that we will use to validate the `params` attribute.|@kbn/config-schema|
|executor|This is where the code of the alert type lives. This is a function to be called when executing an alert on an interval basis. For full details, see executor section below.|Function|

Expand Down
12 changes: 10 additions & 2 deletions x-pack/legacy/plugins/alerting/server/alert_type_registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,23 @@ describe('list()', () => {
registry.register({
id: 'test',
name: 'Test',
actionGroups: ['testActionGroup'],
actionGroups: [
{
id: 'testActionGroup',
name: 'Test Action Group',
},
],
executor: jest.fn(),
});
const result = registry.list();
expect(result).toMatchInlineSnapshot(`
Array [
Object {
"actionGroups": Array [
"testActionGroup",
Object {
"id": "testActionGroup",
"name": "Test Action Group",
},
],
"id": "test",
"name": "Test",
Expand Down
8 changes: 4 additions & 4 deletions x-pack/legacy/plugins/alerting/server/alerts_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ describe('create()', () => {
alertTypeRegistry.get.mockReturnValue({
id: '123',
name: 'Test',
actionGroups: ['default'],
actionGroups: [{ id: 'default', name: 'Default' }],
async executor() {},
});
});
Expand Down Expand Up @@ -1884,7 +1884,7 @@ describe('update()', () => {
alertTypeRegistry.get.mockReturnValue({
id: '123',
name: 'Test',
actionGroups: ['default'],
actionGroups: [{ id: 'default', name: 'Default' }],
async executor() {},
});
});
Expand Down Expand Up @@ -2414,7 +2414,7 @@ describe('update()', () => {
alertTypeRegistry.get.mockReturnValueOnce({
id: '123',
name: 'Test',
actionGroups: ['default'],
actionGroups: [{ id: 'default', name: 'Default' }],
validate: {
params: schema.object({
param1: schema.string(),
Expand Down Expand Up @@ -2646,7 +2646,7 @@ describe('update()', () => {
alertTypeRegistry.get.mockReturnValueOnce({
id: '123',
name: 'Test',
actionGroups: ['default'],
actionGroups: [{ id: 'default', name: 'Default' }],
async executor() {},
});
savedObjectsClient.bulkGet.mockResolvedValueOnce({
Expand Down
5 changes: 3 additions & 2 deletions x-pack/legacy/plugins/alerting/server/alerts_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import Boom from 'boom';
import { omit, isEqual } from 'lodash';
import { omit, isEqual, pluck } from 'lodash';
import { i18n } from '@kbn/i18n';
import {
Logger,
Expand Down Expand Up @@ -639,8 +639,9 @@ export class AlertsClient {
private validateActions(alertType: AlertType, actions: NormalizedAlertAction[]): void {
const { actionGroups: alertTypeActionGroups } = alertType;
const usedAlertActionGroups = actions.map(action => action.group);
const availableAlertTypeActionGroups = new Set(pluck(alertTypeActionGroups, 'id'));
const invalidActionGroups = usedAlertActionGroups.filter(
group => !alertTypeActionGroups.includes(group)
group => !availableAlertTypeActionGroups.has(group)
);
if (invalidActionGroups.length) {
throw Boom.badRequest(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ import { loggingServiceMock } from '../../../../../../src/core/server/mocks';
const alertType: AlertType = {
id: 'test',
name: 'Test',
actionGroups: ['default', 'other-group'],
actionGroups: [
{ id: 'default', name: 'Default' },
{ id: 'other-group', name: 'Other Group' },
],
executor: jest.fn(),
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { pluck } from 'lodash';
import { AlertAction, State, Context, AlertType } from '../types';
import { Logger } from '../../../../../../src/core/server';
import { transformActionParams } from './transform_action_params';
Expand Down Expand Up @@ -35,8 +36,9 @@ export function createExecutionHandler({
apiKey,
alertType,
}: CreateExecutionHandlerOptions) {
const alertTypeActionGroups = new Set(pluck(alertType.actionGroups, 'id'));
return async ({ actionGroup, context, state, alertInstanceId }: ExecutionHandlerOptions) => {
if (!alertType.actionGroups.includes(actionGroup)) {
if (!alertTypeActionGroups.has(actionGroup)) {
logger.error(`Invalid action group "${actionGroup}" for alert "${alertType.id}".`);
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
const alertType = {
id: 'test',
name: 'My test alert',
actionGroups: ['default'],
actionGroups: [{ id: 'default', name: 'Default' }],
executor: jest.fn(),
};
let fakeTimer: sinon.SinonFakeTimers;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
const alertType = {
id: 'test',
name: 'My test alert',
actionGroups: ['default'],
actionGroups: [{ id: 'default', name: 'Default' }],
executor: jest.fn(),
};
let fakeTimer: sinon.SinonFakeTimers;
Expand Down
7 changes: 6 additions & 1 deletion x-pack/legacy/plugins/alerting/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,18 @@ export interface AlertExecutorOptions {
updatedBy: string | null;
}

export interface ActionGroup {
id: string;
name: string;
}

export interface AlertType {
id: string;
name: string;
validate?: {
params?: { validate: (object: any) => any };
};
actionGroups: string[];
actionGroups: ActionGroup[];
executor: ({ services, params, state }: AlertExecutorOptions) => Promise<State | void>;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ describe('getLicenseExpiration', () => {
it('should have the right id and actionGroups', () => {
const alert = getLicenseExpiration(server, getMonitoringCluster, getLogger, ccrEnabled);
expect(alert.id).toBe(ALERT_TYPE_LICENSE_EXPIRATION);
expect(alert.actionGroups).toEqual(['default']);
expect(alert.actionGroups).toEqual([{ id: 'default', name: 'Default' }]);
});

it('should return the state if no license is provided', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import moment from 'moment-timezone';
import { get } from 'lodash';
import { Legacy } from 'kibana';
import { Logger } from 'src/core/server';
import { i18n } from '@kbn/i18n';
import { ALERT_TYPE_LICENSE_EXPIRATION, INDEX_PATTERN_ELASTICSEARCH } from '../../common/constants';
import { AlertType } from '../../../alerting';
import { fetchLicenses } from '../lib/alerts/fetch_licenses';
Expand Down Expand Up @@ -45,7 +46,14 @@ export const getLicenseExpiration = (
return {
id: ALERT_TYPE_LICENSE_EXPIRATION,
name: 'Monitoring Alert - License Expiration',
actionGroups: ['default'],
actionGroups: [
{
id: 'default',
name: i18n.translate('xpack.monitoring.alerts.licenseExpiration.actionGroups.default', {
defaultMessage: 'Default',
}),
},
],
async executor({
services,
params,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { schema } from '@kbn/config-schema';
import { Logger } from 'src/core/server';
import moment from 'moment';
import { i18n } from '@kbn/i18n';
import {
SIGNALS_ID,
DEFAULT_MAX_SIGNALS,
Expand All @@ -32,7 +33,14 @@ export const signalRulesAlertType = ({
return {
id: SIGNALS_ID,
name: 'SIEM Signals',
actionGroups: ['default'],
actionGroups: [
{
id: 'default',
name: i18n.translate('xpack.siem.detectionEngine.signalRuleAlert.actionGroups.default', {
defaultMessage: 'Default',
}),
},
],
validate: {
params: schema.object({
description: schema.string(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe('loadAlertTypes', () => {
id: 'test',
name: 'Test',
actionVariables: ['var1'],
actionGroups: ['default'],
actionGroups: [{ id: 'default', name: 'Default' }],
},
];
http.get.mockResolvedValueOnce(resolvedValue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
ActionTypeIndex,
ActionConnector,
AlertTypeIndex,
ActionGroup,
} from '../../../types';
import { SectionLoading } from '../../components/section_loading';
import { ConnectorAddModal } from '../action_connector_form/connector_add_modal';
Expand Down Expand Up @@ -118,7 +119,7 @@ export const AlertForm = ({
const [alertThrottleUnit, setAlertThrottleUnit] = useState<string>('m');
const [isAddActionPanelOpen, setIsAddActionPanelOpen] = useState<boolean>(true);
const [connectors, setConnectors] = useState<ActionConnector[]>([]);
const [defaultActionGroup, setDefaultActionGroup] = useState<string | undefined>(undefined);
const [defaultActionGroup, setDefaultActionGroup] = useState<ActionGroup | undefined>(undefined);
const [activeActionItem, setActiveActionItem] = useState<ActiveActionConnectorState | undefined>(
undefined
);
Expand Down Expand Up @@ -158,7 +159,11 @@ export const AlertForm = ({
// temp hack of API result
alertTypes.push({
id: 'threshold',
actionGroups: ['Alert', 'Warning', 'If unacknowledged'],
actionGroups: [
{ id: 'alert', name: 'Alert' },
{ id: 'warning', name: 'Warning' },
{ id: 'ifUnacknowledged', name: 'If unacknowledged' },
],
name: 'threshold',
actionVariables: ['ctx.metadata.name', 'ctx.metadata.test'],
});
Expand Down Expand Up @@ -261,7 +266,7 @@ export const AlertForm = ({
alert.actions.push({
id: '',
actionTypeId: actionTypeModel.id,
group: defaultActionGroup ?? 'Alert',
group: defaultActionGroup?.id ?? 'Alert',
params: {},
});
setActionProperty('id', freeConnectors[0].id, alert.actions.length - 1);
Expand All @@ -273,7 +278,7 @@ export const AlertForm = ({
alert.actions.push({
id: '',
actionTypeId: actionTypeModel.id,
group: defaultActionGroup ?? 'Alert',
group: defaultActionGroup?.id ?? 'Alert',
params: {},
});
setActionProperty('id', alert.actions.length, alert.actions.length - 1);
Expand Down Expand Up @@ -351,7 +356,7 @@ export const AlertForm = ({
id,
}));
const actionTypeRegisterd = actionTypeRegistry.get(actionConnector.actionTypeId);
if (actionTypeRegisterd === null || actionItem.group !== defaultActionGroup) return null;
if (actionTypeRegisterd === null || actionItem.group !== defaultActionGroup?.id) return null;
const ParamsFieldsComponent = actionTypeRegisterd.actionParamsFields;
const actionParamsErrors: { errors: IErrorObject } =
Object.keys(actionsErrors).length > 0 ? actionsErrors[actionItem.id] : { errors: {} };
Expand Down Expand Up @@ -474,7 +479,7 @@ export const AlertForm = ({
? actionTypesIndex[actionItem.actionTypeId].name
: actionItem.actionTypeId;
const actionTypeRegisterd = actionTypeRegistry.get(actionItem.actionTypeId);
if (actionTypeRegisterd === null || actionItem.group !== defaultActionGroup) return null;
if (actionTypeRegisterd === null || actionItem.group !== defaultActionGroup?.id) return null;
return (
<EuiAccordion
initialIsOpen={true}
Expand Down
Loading

0 comments on commit fd193fd

Please sign in to comment.