Skip to content

Commit

Permalink
Allow registered alert types to be non-editable (#65606)
Browse files Browse the repository at this point in the history
* Allow registered alert types to be non-editable

* Fixed isUiEditEnabled values

* Fixed due to comments

* fixed failing tests

* Enable alert type selection per alert consumer, only 'alerting' consumer can display other consumers alert types, but in case if it isEditable

* fixed tests

* Removed consumer property from the client side alert type registry and added server side property producer which purpose is to manage a feature logic

* fixed type check

* Fixed tests and type checks

* Removed error message for non registered plugins

* Fixed failing tests

* Fixed due to comments

* fixed test

* -

* revert logic for requiresAppContext

* Added close toast after saving alert
  • Loading branch information
YulNaumenko authored May 12, 2020
1 parent fd4074f commit 5ed5fda
Show file tree
Hide file tree
Showing 58 changed files with 332 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export function getAlertType(): AlertTypeModel {
}
return validationResult;
},
requiresAppContext: false,
};
}

Expand Down
1 change: 1 addition & 0 deletions examples/alerting_example/public/alert_types/astros.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export function getAlertType(): AlertTypeModel {

return validationResult;
},
requiresAppContext: false,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import uuid from 'uuid';
import { range } from 'lodash';
import { AlertType } from '../../../../x-pack/plugins/alerting/server';
import { DEFAULT_INSTANCES_TO_GENERATE } from '../../common/constants';
import { DEFAULT_INSTANCES_TO_GENERATE, ALERTING_EXAMPLE_APP_ID } from '../../common/constants';

export const alertType: AlertType = {
id: 'example.always-firing',
Expand All @@ -43,4 +43,5 @@ export const alertType: AlertType = {
count,
};
},
producer: ALERTING_EXAMPLE_APP_ID,
};
3 changes: 2 additions & 1 deletion examples/alerting_example/server/alert_types/astros.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import axios from 'axios';
import { AlertType } from '../../../../x-pack/plugins/alerting/server';
import { Operator, Craft } from '../../common/constants';
import { Operator, Craft, ALERTING_EXAMPLE_APP_ID } from '../../common/constants';

interface PeopleInSpace {
people: Array<{
Expand Down Expand Up @@ -79,4 +79,5 @@ export const alertType: AlertType = {
peopleInSpace,
};
},
producer: ALERTING_EXAMPLE_APP_ID,
};
3 changes: 3 additions & 0 deletions x-pack/plugins/alerting/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ The following table describes the properties of the `options` object.
|actionVariables|An explicit list of action variables the alert type makes available via context and state in action parameter templates, and a short human readable description. Alert UI will use this to display prompts for the users for these variables, in action parameter editors. We highly encourage using `kbn-i18n` to translate the descriptions. |{ context: Array<{name:string, description:string}, state: Array<{name:string, description: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|
|producer|The id of the application producing this alert type.|string|

### Executor

Expand Down Expand Up @@ -212,6 +213,7 @@ server.newPlatform.setup.plugins.alerting.registerType({
lastChecked: new Date(),
};
},
producer: 'alerting',
});
```

Expand Down Expand Up @@ -287,6 +289,7 @@ server.newPlatform.setup.plugins.alerting.registerType({
lastChecked: new Date(),
};
},
producer: 'alerting',
});
```

Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/alerting/common/alert_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export interface AlertType {
actionGroups: ActionGroup[];
actionVariables: string[];
defaultActionGroupId: ActionGroup['id'];
producer: string;
}

export interface ActionGroup {
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/alerting/public/alert_api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ describe('loadAlertTypes', () => {
actionVariables: ['var1'],
actionGroups: [{ id: 'default', name: 'Default' }],
defaultActionGroupId: 'default',
producer: 'alerting',
},
];
http.get.mockResolvedValueOnce(resolvedValue);
Expand All @@ -44,6 +45,7 @@ describe('loadAlertType', () => {
actionVariables: ['var1'],
actionGroups: [{ id: 'default', name: 'Default' }],
defaultActionGroupId: 'default',
producer: 'alerting',
};
http.get.mockResolvedValueOnce([alertType]);

Expand All @@ -63,6 +65,7 @@ describe('loadAlertType', () => {
actionVariables: [],
actionGroups: [{ id: 'default', name: 'Default' }],
defaultActionGroupId: 'default',
producer: 'alerting',
};
http.get.mockResolvedValueOnce([alertType]);

Expand All @@ -77,6 +80,7 @@ describe('loadAlertType', () => {
actionVariables: [],
actionGroups: [{ id: 'default', name: 'Default' }],
defaultActionGroupId: 'default',
producer: 'alerting',
},
]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const mockAlertType = (id: string): AlertType => ({
actionGroups: [],
actionVariables: [],
defaultActionGroupId: 'default',
producer: 'alerting',
});

describe('AlertNavigationRegistry', () => {
Expand Down
10 changes: 10 additions & 0 deletions x-pack/plugins/alerting/server/alert_type_registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe('has()', () => {
],
defaultActionGroupId: 'default',
executor: jest.fn(),
producer: 'alerting',
});
expect(registry.has('foo')).toEqual(true);
});
Expand All @@ -54,6 +55,7 @@ describe('register()', () => {
],
defaultActionGroupId: 'default',
executor: jest.fn(),
producer: 'alerting',
};
// eslint-disable-next-line @typescript-eslint/no-var-requires
const registry = new AlertTypeRegistry(alertTypeRegistryParams);
Expand Down Expand Up @@ -84,6 +86,7 @@ describe('register()', () => {
],
defaultActionGroupId: 'default',
executor: jest.fn(),
producer: 'alerting',
};
const registry = new AlertTypeRegistry(alertTypeRegistryParams);
registry.register(alertType);
Expand All @@ -104,6 +107,7 @@ describe('register()', () => {
],
defaultActionGroupId: 'default',
executor: jest.fn(),
producer: 'alerting',
});
expect(() =>
registry.register({
Expand All @@ -117,6 +121,7 @@ describe('register()', () => {
],
defaultActionGroupId: 'default',
executor: jest.fn(),
producer: 'alerting',
})
).toThrowErrorMatchingInlineSnapshot(`"Alert type \\"test\\" is already registered."`);
});
Expand All @@ -136,6 +141,7 @@ describe('get()', () => {
],
defaultActionGroupId: 'default',
executor: jest.fn(),
producer: 'alerting',
});
const alertType = registry.get('test');
expect(alertType).toMatchInlineSnapshot(`
Expand All @@ -154,6 +160,7 @@ describe('get()', () => {
"executor": [MockFunction],
"id": "test",
"name": "Test",
"producer": "alerting",
}
`);
});
Expand Down Expand Up @@ -186,6 +193,7 @@ describe('list()', () => {
],
defaultActionGroupId: 'testActionGroup',
executor: jest.fn(),
producer: 'alerting',
});
const result = registry.list();
expect(result).toMatchInlineSnapshot(`
Expand All @@ -204,6 +212,7 @@ describe('list()', () => {
"defaultActionGroupId": "testActionGroup",
"id": "test",
"name": "Test",
"producer": "alerting",
},
]
`);
Expand Down Expand Up @@ -251,6 +260,7 @@ function alertTypeWithVariables(id: string, context: string, state: string): Ale
actionGroups: [],
defaultActionGroupId: id,
async executor() {},
producer: 'alerting',
};

if (!context && !state) {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/alerting/server/alert_type_registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export class AlertTypeRegistry {
actionGroups: alertType.actionGroups,
defaultActionGroupId: alertType.defaultActionGroupId,
actionVariables: alertType.actionVariables,
producer: alertType.producer,
}));
}
}
Expand Down
5 changes: 5 additions & 0 deletions x-pack/plugins/alerting/server/alerts_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ describe('create()', () => {
actionGroups: [{ id: 'default', name: 'Default' }],
defaultActionGroupId: 'default',
async executor() {},
producer: 'alerting',
});
});

Expand Down Expand Up @@ -539,6 +540,7 @@ describe('create()', () => {
}),
},
async executor() {},
producer: 'alerting',
});
await expect(alertsClient.create({ data })).rejects.toThrowErrorMatchingInlineSnapshot(
`"params invalid: [param1]: expected value of type [string] but got [undefined]"`
Expand Down Expand Up @@ -1896,6 +1898,7 @@ describe('update()', () => {
actionGroups: [{ id: 'default', name: 'Default' }],
defaultActionGroupId: 'default',
async executor() {},
producer: 'alerting',
});
});

Expand Down Expand Up @@ -2438,6 +2441,7 @@ describe('update()', () => {
}),
},
async executor() {},
producer: 'alerting',
});
await expect(
alertsClient.update({
Expand Down Expand Up @@ -2669,6 +2673,7 @@ describe('update()', () => {
actionGroups: [{ id: 'default', name: 'Default' }],
defaultActionGroupId: 'default',
async executor() {},
producer: 'alerting',
});
savedObjectsClient.bulkGet.mockResolvedValueOnce({
saved_objects: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ test('should return passed in params when validation not defined', () => {
],
defaultActionGroupId: 'default',
async executor() {},
producer: 'alerting',
},
{
foo: true,
Expand Down Expand Up @@ -47,6 +48,7 @@ test('should validate and apply defaults when params is valid', () => {
}),
},
async executor() {},
producer: 'alerting',
},
{ param1: 'value' }
);
Expand Down Expand Up @@ -75,6 +77,7 @@ test('should validate and throw error when params is invalid', () => {
}),
},
async executor() {},
producer: 'alerting',
},
{}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ describe('listAlertTypesRoute', () => {
],
defaultActionGroupId: 'default',
actionVariables: [],
producer: 'test',
},
];

Expand All @@ -67,6 +68,7 @@ describe('listAlertTypesRoute', () => {
"defaultActionGroupId": "default",
"id": "1",
"name": "name",
"producer": "test",
},
],
}
Expand Down Expand Up @@ -109,6 +111,7 @@ describe('listAlertTypesRoute', () => {
],
defaultActionGroupId: 'default',
actionVariables: [],
producer: 'alerting',
},
];

Expand Down Expand Up @@ -158,6 +161,7 @@ describe('listAlertTypesRoute', () => {
],
defaultActionGroupId: 'default',
actionVariables: [],
producer: 'alerting',
},
];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const alertType: AlertType = {
],
defaultActionGroupId: 'default',
executor: jest.fn(),
producer: 'alerting',
};

const createExecutionHandlerParams = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const alertType = {
actionGroups: [{ id: 'default', name: 'Default' }],
defaultActionGroupId: 'default',
executor: jest.fn(),
producer: 'alerting',
};
let fakeTimer: sinon.SinonFakeTimers;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const alertType = {
actionGroups: [{ id: 'default', name: 'Default' }],
defaultActionGroupId: 'default',
executor: jest.fn(),
producer: 'alerting',
};
let fakeTimer: sinon.SinonFakeTimers;

Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/alerting/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export interface AlertType {
actionGroups: ActionGroup[];
defaultActionGroupId: ActionGroup['id'];
executor: ({ services, params, state }: AlertExecutorOptions) => Promise<State | void>;
producer: string;
actionVariables?: {
context?: ActionVariable[];
state?: ActionVariable[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ export function getAlertType(service: Service): AlertType {
],
},
executor,
producer: 'alerting',
};

async function executor(options: AlertExecutorOptions) {
Expand Down
6 changes: 4 additions & 2 deletions x-pack/plugins/apm/common/alert_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ export const ALERT_TYPES_CONFIG = {
})
}
],
defaultActionGroupId: 'threshold_met'
defaultActionGroupId: 'threshold_met',
producer: 'apm'
},
[AlertType.TransactionDuration]: {
name: i18n.translate('xpack.apm.transactionDurationAlert.name', {
Expand All @@ -41,7 +42,8 @@ export const ALERT_TYPES_CONFIG = {
)
}
],
defaultActionGroupId: 'threshold_met'
defaultActionGroupId: 'threshold_met',
producer: 'apm'
}
};

Expand Down
6 changes: 4 additions & 2 deletions x-pack/plugins/apm/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ export class ApmPlugin implements Plugin<ApmPluginSetup, ApmPluginStart> {
alertParamsExpression: ErrorRateAlertTrigger,
validate: () => ({
errors: []
})
}),
requiresAppContext: true
});

plugins.triggers_actions_ui.alertTypeRegistry.register({
Expand All @@ -127,7 +128,8 @@ export class ApmPlugin implements Plugin<ApmPluginSetup, ApmPluginStart> {
alertParamsExpression: TransactionDurationAlertTrigger,
validate: () => ({
errors: []
})
}),
requiresAppContext: true
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export function registerErrorRateAlertType({
}
]
},
producer: 'apm',
executor: async ({ services, params }) => {
const config = await config$.pipe(take(1)).toPromise();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export function registerTransactionDurationAlertType({
}
]
},
producer: 'apm',
executor: async ({ services, params }) => {
const config = await config$.pipe(take(1)).toPromise();

Expand Down
Loading

0 comments on commit 5ed5fda

Please sign in to comment.