Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adds createdAt and updatedAt fields to alerting #53793

Merged
merged 16 commits into from
Jan 3, 2020
Merged

adds createdAt and updatedAt fields to alerting #53793

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
dcd574e
added createdAt and updatedAt fields in alerting
gmmorris Dec 24, 2019
1721ec2
use the SavedObjets updated_at instead of a new field in alerting
gmmorris Dec 24, 2019
430fe9b
Merge branch 'master' into alerting/created_at-and-updated_at
gmmorris Dec 27, 2019
b6c9385
Merge branch 'master' into alerting/created_at-and-updated_at
elasticmachine Dec 30, 2019
e8dbc15
added missing assetion
gmmorris Jan 2, 2020
27f54c0
Merge branch 'alerting/created_at-and-updated_at' of github.com:gmmor…
gmmorris Jan 2, 2020
fe1902e
Merge branch 'master' into alerting/created_at-and-updated_at
gmmorris Jan 2, 2020
1fb5735
used createdAt in place of empty updatedAt
gmmorris Jan 3, 2020
2f50541
fixed misuse of null on updatedAt
gmmorris Jan 3, 2020
5a4057d
fixed tests
gmmorris Jan 3, 2020
954a71a
fixed tests
gmmorris Jan 3, 2020
db0d578
Merge branch 'master' into alerting/created_at-and-updated_at
elasticmachine Jan 3, 2020
924393f
updatedAt should equal createdAt on creation
gmmorris Jan 3, 2020
dbd61a7
Merge branch 'alerting/created_at-and-updated_at' of github.com:gmmor…
gmmorris Jan 3, 2020
cf24b03
Merge branch 'master' into alerting/created_at-and-updated_at
gmmorris Jan 3, 2020
6dff233
removed assertions for updatedAt in non-update operations
gmmorris Jan 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions x-pack/legacy/plugins/alerting/mappings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@
"updatedBy": {
"type": "keyword"
},
"createdAt": {
"type": "date"
},
"apiKey": {
"type": "binary"
},
Expand Down
27 changes: 27 additions & 0 deletions x-pack/legacy/plugins/alerting/server/alerts_client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ describe('create()', () => {
params: {
bar: true,
},
createdAt: '2019-02-12T21:01:22.479Z',
actions: [
{
group: 'default',
Expand Down Expand Up @@ -160,6 +161,7 @@ describe('create()', () => {
},
],
"alertTypeId": "123",
"createdAt": 2019-02-12T21:01:22.479Z,
"id": "1",
"params": Object {
"bar": true,
Expand All @@ -168,6 +170,7 @@ describe('create()', () => {
"interval": "10s",
},
"scheduledTaskId": "task-123",
"updatedAt": 2019-02-12T21:01:22.479Z,
}
`);
expect(savedObjectsClient.create).toHaveBeenCalledTimes(1);
Expand All @@ -189,6 +192,7 @@ describe('create()', () => {
"apiKey": null,
"apiKeyOwner": null,
"consumer": "bar",
"createdAt": "2019-02-12T21:01:22.479Z",
"createdBy": "elastic",
"enabled": true,
"muteAll": false,
Expand Down Expand Up @@ -311,6 +315,7 @@ describe('create()', () => {
params: {
bar: true,
},
createdAt: new Date().toISOString(),
actions: [
{
group: 'default',
Expand Down Expand Up @@ -407,6 +412,7 @@ describe('create()', () => {
},
],
"alertTypeId": "123",
"createdAt": 2019-02-12T21:01:22.479Z,
"id": "1",
"params": Object {
"bar": true,
Expand All @@ -415,6 +421,7 @@ describe('create()', () => {
"interval": "10s",
},
"scheduledTaskId": "task-123",
"updatedAt": 2019-02-12T21:01:22.479Z,
}
`);
expect(savedObjectsClient.bulkGet).toHaveBeenCalledWith([
Expand Down Expand Up @@ -460,6 +467,7 @@ describe('create()', () => {
params: {
bar: true,
},
createdAt: new Date().toISOString(),
actions: [
{
group: 'default',
Expand Down Expand Up @@ -493,6 +501,7 @@ describe('create()', () => {
},
],
"alertTypeId": "123",
"createdAt": 2019-02-12T21:01:22.479Z,
"enabled": false,
"id": "1",
"params": Object {
Expand All @@ -501,6 +510,7 @@ describe('create()', () => {
"schedule": Object {
"interval": 10000,
},
"updatedAt": 2019-02-12T21:01:22.479Z,
}
`);
expect(savedObjectsClient.create).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -806,6 +816,7 @@ describe('create()', () => {
apiKey: Buffer.from('123:abc').toString('base64'),
apiKeyOwner: 'elastic',
createdBy: 'elastic',
createdAt: '2019-02-12T21:01:22.479Z',
updatedBy: 'elastic',
enabled: true,
schedule: { interval: '10s' },
Expand Down Expand Up @@ -1248,13 +1259,15 @@ describe('get()', () => {
},
],
"alertTypeId": "123",
"createdAt": 2019-02-12T21:01:22.479Z,
"id": "1",
"params": Object {
"bar": true,
},
"schedule": Object {
"interval": "10s",
},
"updatedAt": 2019-02-12T21:01:22.479Z,
}
`);
expect(savedObjectsClient.get).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -1347,13 +1360,15 @@ describe('find()', () => {
},
],
"alertTypeId": "123",
"createdAt": 2019-02-12T21:01:22.479Z,
"id": "1",
"params": Object {
"bar": true,
},
"schedule": Object {
"interval": "10s",
},
"updatedAt": 2019-02-12T21:01:22.479Z,
},
],
"page": 1,
Expand Down Expand Up @@ -1476,7 +1491,9 @@ describe('update()', () => {
},
],
scheduledTaskId: 'task-123',
createdAt: new Date().toISOString(),
},
updated_at: new Date().toISOString(),
references: [
{
name: 'action_0',
Expand Down Expand Up @@ -1517,6 +1534,7 @@ describe('update()', () => {
},
},
],
"createdAt": 2019-02-12T21:01:22.479Z,
"enabled": true,
"id": "1",
"params": Object {
Expand All @@ -1526,6 +1544,7 @@ describe('update()', () => {
"interval": "10s",
},
"scheduledTaskId": "task-123",
"updatedAt": 2019-02-12T21:01:22.479Z,
}
`);
expect(savedObjectsClient.update).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -1624,6 +1643,7 @@ describe('update()', () => {
params: {
bar: true,
},
createdAt: new Date().toISOString(),
actions: [
{
group: 'default',
Expand Down Expand Up @@ -1652,6 +1672,7 @@ describe('update()', () => {
],
scheduledTaskId: 'task-123',
},
updated_at: new Date().toISOString(),
references: [
{
name: 'action_0',
Expand Down Expand Up @@ -1732,6 +1753,7 @@ describe('update()', () => {
},
},
],
"createdAt": 2019-02-12T21:01:22.479Z,
"enabled": true,
"id": "1",
"params": Object {
Expand All @@ -1741,6 +1763,7 @@ describe('update()', () => {
"interval": "10s",
},
"scheduledTaskId": "task-123",
"updatedAt": 2019-02-12T21:01:22.479Z,
}
`);
expect(savedObjectsClient.bulkGet).toHaveBeenCalledWith([
Expand Down Expand Up @@ -1799,6 +1822,7 @@ describe('update()', () => {
params: {
bar: true,
},
createdAt: new Date().toISOString(),
actions: [
{
group: 'default',
Expand All @@ -1812,6 +1836,7 @@ describe('update()', () => {
apiKey: Buffer.from('123:abc').toString('base64'),
scheduledTaskId: 'task-123',
},
updated_at: new Date().toISOString(),
references: [
{
name: 'action_0',
Expand Down Expand Up @@ -1853,6 +1878,7 @@ describe('update()', () => {
},
],
"apiKey": "MTIzOmFiYw==",
"createdAt": 2019-02-12T21:01:22.479Z,
"enabled": true,
"id": "1",
"params": Object {
Expand All @@ -1862,6 +1888,7 @@ describe('update()', () => {
"interval": "10s",
},
"scheduledTaskId": "task-123",
"updatedAt": 2019-02-12T21:01:22.479Z,
}
`);
expect(savedObjectsClient.update).toHaveBeenCalledTimes(1);
Expand Down
28 changes: 23 additions & 5 deletions x-pack/legacy/plugins/alerting/server/alerts_client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ interface CreateOptions {
keyof Alert,
| 'createdBy'
| 'updatedBy'
| 'createdAt'
gmmorris marked this conversation as resolved.
Show resolved Hide resolved
| 'updatedAt'
| 'apiKey'
| 'apiKeyOwner'
| 'muteAll'
Expand Down Expand Up @@ -142,6 +144,7 @@ export class AlertsClient {
actions,
createdBy: username,
updatedBy: username,
createdAt: new Date().toISOString(),
params: validatedAlertTypeParams,
muteAll: false,
mutedInstanceIds: [],
Expand Down Expand Up @@ -171,12 +174,17 @@ export class AlertsClient {
});
createdAlert.attributes.scheduledTaskId = scheduledTask.id;
}
return this.getAlertFromRaw(createdAlert.id, createdAlert.attributes, references);
return this.getAlertFromRaw(
createdAlert.id,
createdAlert.attributes,
createdAlert.updated_at,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing worth noting in regards to comment #52738 (comment): This won't allow users to sort on the last time the alert was updated.

I'm not sure if this is a requirement but if ever it becomes, a separate PR to add updated_at to the following file is all we'd have to do:

kibana/src/core/server/saved_objects/service/lib/search_dsl/sorting_params.ts

Line 23 in e233e41

const TOP_LEVEL_FIELDS = ['_id', '_score'];

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's an interesting point.
Thank's for pointing it out.

references
);
}

public async get({ id }: { id: string }) {
const result = await this.savedObjectsClient.get('alert', id);
return this.getAlertFromRaw(result.id, result.attributes, result.references);
return this.getAlertFromRaw(result.id, result.attributes, result.updated_at, result.references);
}

public async find({ options = {} }: FindOptions = {}): Promise<FindResult> {
Expand All @@ -186,7 +194,7 @@ export class AlertsClient {
});

const data = results.saved_objects.map(result =>
this.getAlertFromRaw(result.id, result.attributes, result.references)
this.getAlertFromRaw(result.id, result.attributes, result.updated_at, result.references)
);

return {
Expand Down Expand Up @@ -214,7 +222,7 @@ export class AlertsClient {
updateResult.scheduledTaskId &&
!isEqual(alert.attributes.schedule, updateResult.schedule)
) {
this.taskManager.runNow(updateResult.scheduledTaskId).catch(err => {
this.taskManager.runNow(updateResult.scheduledTaskId).catch((err: Error) => {
this.logger.error(
`Alert update failed to run its underlying task. TaskManager runNow failed with Error: ${err.message}`
);
Expand Down Expand Up @@ -254,7 +262,12 @@ export class AlertsClient {
references,
}
);
return this.getAlertFromRaw(id, updatedObject.attributes, updatedObject.references);
return this.getAlertFromRaw(
id,
updatedObject.attributes,
updatedObject.updated_at,
updatedObject.references
);
}

private apiKeyAsAlertAttributes(
Expand Down Expand Up @@ -301,6 +314,7 @@ export class AlertsClient {
enabled: true,
...this.apiKeyAsAlertAttributes(await this.createAPIKey(), username),
updatedBy: username,

scheduledTaskId: scheduledTask.id,
},
{ version }
Expand Down Expand Up @@ -382,6 +396,7 @@ export class AlertsClient {
alertId,
{
updatedBy: await this.getUserName(),

mutedInstanceIds: mutedInstanceIds.filter((id: string) => id !== alertInstanceId),
},
{ version }
Expand Down Expand Up @@ -424,6 +439,7 @@ export class AlertsClient {
private getAlertFromRaw(
id: string,
rawAlert: Partial<RawAlert>,
updatedAt: SavedObject['updated_at'],
references: SavedObjectReference[] | undefined
) {
if (!rawAlert.actions) {
Expand All @@ -436,6 +452,8 @@ export class AlertsClient {
return {
id,
...rawAlert,
updatedAt: updatedAt ? new Date(updatedAt) : new Date(rawAlert.createdAt!),
createdAt: new Date(rawAlert.createdAt!),
gmmorris marked this conversation as resolved.
Show resolved Hide resolved
actions,
};
}
Expand Down
8 changes: 7 additions & 1 deletion x-pack/legacy/plugins/alerting/server/routes/create.test.ts
View file
Open in desktop
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 { omit } from 'lodash';
import { createMockServer } from './_mock_server';
import { createAlertRoute } from './create';

Expand Down Expand Up @@ -39,8 +40,12 @@ test('creates an alert with proper parameters', async () => {
payload: mockedAlert,
};

const createdAt = new Date();
const updatedAt = new Date();
alertsClient.create.mockResolvedValueOnce({
...mockedAlert,
createdAt,
updatedAt,
id: '123',
actions: [
{
Expand All @@ -52,7 +57,8 @@ test('creates an alert with proper parameters', async () => {
const { payload, statusCode } = await server.inject(request);
expect(statusCode).toBe(200);
const response = JSON.parse(payload);
expect(response).toMatchInlineSnapshot(`
expect(new Date(response.createdAt)).toEqual(createdAt);
expect(omit(response, 'createdAt', 'updatedAt')).toMatchInlineSnapshot(`
Object {
"actions": Array [
Object {
Expand Down
8 changes: 6 additions & 2 deletions x-pack/legacy/plugins/alerting/server/routes/get.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ const mockedAlert = {
params: {
bar: true,
},
createdAt: new Date(),
updatedAt: new Date(),
actions: [
{
group: 'default',
Expand All @@ -40,8 +42,10 @@ test('calls get with proper parameters', async () => {
alertsClient.get.mockResolvedValueOnce(mockedAlert);
const { payload, statusCode } = await server.inject(request);
expect(statusCode).toBe(200);
const response = JSON.parse(payload);
expect(response).toEqual(mockedAlert);
const { createdAt, updatedAt, ...response } = JSON.parse(payload);
expect({ createdAt: new Date(createdAt), updatedAt: new Date(updatedAt), ...response }).toEqual(
mockedAlert
);
expect(alertsClient.get).toHaveBeenCalledTimes(1);
expect(alertsClient.get.mock.calls[0]).toMatchInlineSnapshot(`
Array [
Expand Down
8 changes: 6 additions & 2 deletions x-pack/legacy/plugins/alerting/server/routes/update.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ const mockedResponse = {
params: {
otherField: false,
},
createdAt: new Date(),
updatedAt: new Date(),
actions: [
{
group: 'default',
Expand Down Expand Up @@ -59,8 +61,10 @@ test('calls the update function with proper parameters', async () => {
alertsClient.update.mockResolvedValueOnce(mockedResponse);
const { payload, statusCode } = await server.inject(request);
expect(statusCode).toBe(200);
const response = JSON.parse(payload);
expect(response).toEqual(mockedResponse);
const { createdAt, updatedAt, ...response } = JSON.parse(payload);
expect({ createdAt: new Date(createdAt), updatedAt: new Date(updatedAt), ...response }).toEqual(
mockedResponse
);
expect(alertsClient.update).toHaveBeenCalledTimes(1);
expect(alertsClient.update.mock.calls[0]).toMatchInlineSnapshot(`
Array [
Expand Down
Loading