From 1721ec2d812339a56979fb883091b97564af1d54 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Tue, 24 Dec 2019 17:15:18 +0000 Subject: [PATCH] use the SavedObjets updated_at instead of a new field in alerting --- x-pack/legacy/plugins/alerting/mappings.json | 3 -- .../alerting/server/alerts_client.test.ts | 21 +++------ .../plugins/alerting/server/alerts_client.ts | 45 +++++++++---------- .../alerting/server/routes/create.test.ts | 11 +++-- .../alerting/server/routes/get.test.ts | 4 +- .../alerting/server/routes/update.test.ts | 6 ++- .../legacy/plugins/alerting/server/types.ts | 1 - .../tests/alerting/find.ts | 4 +- .../security_and_spaces/tests/alerting/get.ts | 2 +- .../spaces_only/tests/alerting/create.ts | 2 +- .../spaces_only/tests/alerting/find.ts | 2 +- .../spaces_only/tests/alerting/get.ts | 2 +- 12 files changed, 45 insertions(+), 58 deletions(-) diff --git a/x-pack/legacy/plugins/alerting/mappings.json b/x-pack/legacy/plugins/alerting/mappings.json index 91435f55434dd..31733f44e7ce6 100644 --- a/x-pack/legacy/plugins/alerting/mappings.json +++ b/x-pack/legacy/plugins/alerting/mappings.json @@ -57,9 +57,6 @@ "createdAt": { "type": "date" }, - "updatedAt": { - "type": "date" - }, "apiKey": { "type": "binary" }, diff --git a/x-pack/legacy/plugins/alerting/server/alerts_client.test.ts b/x-pack/legacy/plugins/alerting/server/alerts_client.test.ts index 4f0fd3482563d..95fbe4ceb4c57 100644 --- a/x-pack/legacy/plugins/alerting/server/alerts_client.test.ts +++ b/x-pack/legacy/plugins/alerting/server/alerts_client.test.ts @@ -101,7 +101,6 @@ describe('create()', () => { bar: true, }, createdAt: '2019-02-12T21:01:22.479Z', - updatedAt: null, actions: [ { group: 'default', @@ -209,7 +208,6 @@ describe('create()', () => { "foo", ], "throttle": null, - "updatedAt": null, "updatedBy": "elastic", } `); @@ -317,6 +315,7 @@ describe('create()', () => { params: { bar: true, }, + createdAt: new Date().toISOString(), actions: [ { group: 'default', @@ -468,6 +467,7 @@ describe('create()', () => { params: { bar: true, }, + createdAt: new Date().toISOString(), actions: [ { group: 'default', @@ -818,7 +818,6 @@ describe('create()', () => { createdBy: 'elastic', createdAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', - updatedAt: null, enabled: true, schedule: { interval: '10s' }, throttle: null, @@ -877,7 +876,6 @@ describe('enable()', () => { enabled: true, scheduledTaskId: 'task-123', updatedBy: 'elastic', - updatedAt: new Date().toISOString(), apiKey: null, apiKeyOwner: null, }, @@ -961,7 +959,6 @@ describe('enable()', () => { apiKey: Buffer.from('123:abc').toString('base64'), apiKeyOwner: 'elastic', updatedBy: 'elastic', - updatedAt: new Date().toISOString(), }, { version: '123', @@ -1011,7 +1008,6 @@ describe('disable()', () => { enabled: false, scheduledTaskId: null, updatedBy: 'elastic', - updatedAt: new Date().toISOString(), }, { version: '123', @@ -1057,7 +1053,6 @@ describe('muteAll()', () => { muteAll: true, mutedInstanceIds: [], updatedBy: 'elastic', - updatedAt: new Date().toISOString(), }); }); }); @@ -1079,7 +1074,6 @@ describe('unmuteAll()', () => { muteAll: false, mutedInstanceIds: [], updatedBy: 'elastic', - updatedAt: new Date().toISOString(), }); }); }); @@ -1108,7 +1102,6 @@ describe('muteInstance()', () => { { mutedInstanceIds: ['2'], updatedBy: 'elastic', - updatedAt: new Date().toISOString(), }, { version: '123' } ); @@ -1178,7 +1171,6 @@ describe('unmuteInstance()', () => { { mutedInstanceIds: [], updatedBy: 'elastic', - updatedAt: new Date().toISOString(), }, { version: '123' } ); @@ -1500,8 +1492,8 @@ describe('update()', () => { ], scheduledTaskId: 'task-123', createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), }, + updated_at: new Date().toISOString(), references: [ { name: 'action_0', @@ -1586,7 +1578,6 @@ describe('update()', () => { "tags": Array [ "foo", ], - "updatedAt": "2019-02-12T21:01:22.479Z", "updatedBy": "elastic", } `); @@ -1653,7 +1644,6 @@ describe('update()', () => { bar: true, }, createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), actions: [ { group: 'default', @@ -1682,6 +1672,7 @@ describe('update()', () => { ], scheduledTaskId: 'task-123', }, + updated_at: new Date().toISOString(), references: [ { name: 'action_0', @@ -1832,7 +1823,6 @@ describe('update()', () => { bar: true, }, createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), actions: [ { group: 'default', @@ -1846,6 +1836,7 @@ describe('update()', () => { apiKey: Buffer.from('123:abc').toString('base64'), scheduledTaskId: 'task-123', }, + updated_at: new Date().toISOString(), references: [ { name: 'action_0', @@ -1931,7 +1922,6 @@ describe('update()', () => { "tags": Array [ "foo", ], - "updatedAt": "2019-02-12T21:01:22.479Z", "updatedBy": "elastic", } `); @@ -2249,7 +2239,6 @@ describe('updateApiKey()', () => { apiKey: Buffer.from('123:abc').toString('base64'), apiKeyOwner: 'elastic', updatedBy: 'elastic', - updatedAt: new Date().toISOString(), }, { version: '123' } ); diff --git a/x-pack/legacy/plugins/alerting/server/alerts_client.ts b/x-pack/legacy/plugins/alerting/server/alerts_client.ts index 86f0ccbe9d93f..3a27d91006301 100644 --- a/x-pack/legacy/plugins/alerting/server/alerts_client.ts +++ b/x-pack/legacy/plugins/alerting/server/alerts_client.ts @@ -5,7 +5,7 @@ */ import Boom from 'boom'; -import { omit, isEqual, pick } from 'lodash'; +import { omit, isEqual } from 'lodash'; import { i18n } from '@kbn/i18n'; import { Logger, @@ -145,7 +145,6 @@ export class AlertsClient { createdBy: username, updatedBy: username, createdAt: new Date().toISOString(), - updatedAt: null, params: validatedAlertTypeParams, muteAll: false, mutedInstanceIds: [], @@ -175,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, + 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 { @@ -190,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 { @@ -252,14 +256,18 @@ export class AlertsClient { params: validatedAlertTypeParams, actions, updatedBy: username, - updatedAt: new Date().toISOString(), }, { version, references, } ); - return this.getAlertFromRaw(id, updatedObject.attributes, updatedObject.references); + return this.getAlertFromRaw( + id, + updatedObject.attributes, + updatedObject.updated_at, + updatedObject.references + ); } private apiKeyAsAlertAttributes( @@ -288,7 +296,6 @@ export class AlertsClient { ...attributes, ...this.apiKeyAsAlertAttributes(await this.createAPIKey(), username), updatedBy: username, - updatedAt: new Date().toISOString(), }, { version } ); @@ -307,7 +314,7 @@ export class AlertsClient { enabled: true, ...this.apiKeyAsAlertAttributes(await this.createAPIKey(), username), updatedBy: username, - updatedAt: new Date().toISOString(), + scheduledTaskId: scheduledTask.id, }, { version } @@ -328,7 +335,6 @@ export class AlertsClient { apiKey: null, apiKeyOwner: null, updatedBy: await this.getUserName(), - updatedAt: new Date().toISOString(), }, { version } ); @@ -341,7 +347,6 @@ export class AlertsClient { muteAll: true, mutedInstanceIds: [], updatedBy: await this.getUserName(), - updatedAt: new Date().toISOString(), }); } @@ -350,7 +355,6 @@ export class AlertsClient { muteAll: false, mutedInstanceIds: [], updatedBy: await this.getUserName(), - updatedAt: new Date().toISOString(), }); } @@ -371,7 +375,6 @@ export class AlertsClient { { mutedInstanceIds, updatedBy: await this.getUserName(), - updatedAt: new Date().toISOString(), }, { version } ); @@ -393,7 +396,7 @@ export class AlertsClient { alertId, { updatedBy: await this.getUserName(), - updatedAt: new Date().toISOString(), + mutedInstanceIds: mutedInstanceIds.filter((id: string) => id !== alertInstanceId), }, { version } @@ -436,6 +439,7 @@ export class AlertsClient { private getAlertFromRaw( id: string, rawAlert: Partial, + updatedAt: SavedObject['updated_at'], references: SavedObjectReference[] | undefined ) { if (!rawAlert.actions) { @@ -448,7 +452,8 @@ export class AlertsClient { return { id, ...rawAlert, - ...parseDates(pick(rawAlert, 'createdAt', 'updatedAt')), + updatedAt: updatedAt ? new Date(updatedAt) : null, + createdAt: new Date(rawAlert.createdAt!), actions, }; } @@ -508,13 +513,3 @@ export class AlertsClient { }; } } - -function parseDates({ - createdAt, - updatedAt, -}: Pick): Pick { - return { - createdAt: new Date(createdAt), - updatedAt: updatedAt ? new Date(updatedAt) : null, - }; -} diff --git a/x-pack/legacy/plugins/alerting/server/routes/create.test.ts b/x-pack/legacy/plugins/alerting/server/routes/create.test.ts index 02f5644250095..6c18deeee7580 100644 --- a/x-pack/legacy/plugins/alerting/server/routes/create.test.ts +++ b/x-pack/legacy/plugins/alerting/server/routes/create.test.ts @@ -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'; @@ -19,8 +20,6 @@ const mockedAlert = { params: { bar: true, }, - createdAt: new Date(), - updatedAt: null, actions: [ { group: 'default', @@ -41,8 +40,12 @@ test('creates an alert with proper parameters', async () => { payload: mockedAlert, }; + const createdAt = new Date(); + const updatedAt = null; alertsClient.create.mockResolvedValueOnce({ ...mockedAlert, + createdAt, + updatedAt, id: '123', actions: [ { @@ -54,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')).toMatchInlineSnapshot(` Object { "actions": Array [ Object { @@ -79,6 +83,7 @@ test('creates an alert with proper parameters', async () => { "tags": Array [ "foo", ], + "updatedAt": null, } `); expect(alertsClient.create).toHaveBeenCalledTimes(1); diff --git a/x-pack/legacy/plugins/alerting/server/routes/get.test.ts b/x-pack/legacy/plugins/alerting/server/routes/get.test.ts index f3e98ba0e5188..7af3a11ee6d77 100644 --- a/x-pack/legacy/plugins/alerting/server/routes/get.test.ts +++ b/x-pack/legacy/plugins/alerting/server/routes/get.test.ts @@ -42,8 +42,8 @@ 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, ...response } = JSON.parse(payload); + expect({ createdAt: new Date(createdAt), ...response }).toEqual(mockedAlert); expect(alertsClient.get).toHaveBeenCalledTimes(1); expect(alertsClient.get.mock.calls[0]).toMatchInlineSnapshot(` Array [ diff --git a/x-pack/legacy/plugins/alerting/server/routes/update.test.ts b/x-pack/legacy/plugins/alerting/server/routes/update.test.ts index a69e4a968b0fd..83b70b505234f 100644 --- a/x-pack/legacy/plugins/alerting/server/routes/update.test.ts +++ b/x-pack/legacy/plugins/alerting/server/routes/update.test.ts @@ -61,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 [ diff --git a/x-pack/legacy/plugins/alerting/server/types.ts b/x-pack/legacy/plugins/alerting/server/types.ts index 98d43513f8d8e..40490f7b5cf69 100644 --- a/x-pack/legacy/plugins/alerting/server/types.ts +++ b/x-pack/legacy/plugins/alerting/server/types.ts @@ -98,7 +98,6 @@ export interface RawAlert extends SavedObjectAttributes { createdBy: string | null; updatedBy: string | null; createdAt: string; - updatedAt: string | null; apiKey: string | null; apiKeyOwner: string | null; throttle: string | null; diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/find.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/find.ts index a86edaeddcbb0..d99ab794cd28f 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/find.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/find.ts @@ -75,7 +75,7 @@ export default function createFindTests({ getService }: FtrProviderContext) { mutedInstanceIds: [], }); expect(Date.parse(match.createdAt)).to.be.greaterThan(0); - expect(match.updatedAt).to.eql(null); + expect(Date.parse(match.updatedAt)).to.be.greaterThan(0); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); @@ -165,7 +165,7 @@ export default function createFindTests({ getService }: FtrProviderContext) { updatedAt: match.updatedAt, }); expect(Date.parse(match.createdAt)).to.be.greaterThan(0); - expect(match.updatedAt).to.eql(null); + expect(Date.parse(match.updatedAt)).to.be.greaterThan(0); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/get.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/get.ts index 613f96739cffa..20eed4013d7dd 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/get.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/get.ts @@ -69,7 +69,7 @@ export default function createGetTests({ getService }: FtrProviderContext) { mutedInstanceIds: [], }); expect(Date.parse(response.body.createdAt)).to.be.greaterThan(0); - expect(response.body.updatedAt).to.eql(null); + expect(Date.parse(response.body.updatedAt)).to.be.greaterThan(0); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts index 23cf6e9afe41a..be62d541a8412 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts @@ -83,7 +83,7 @@ export default function createAlertTests({ getService }: FtrProviderContext) { updatedAt: response.body.updatedAt, }); expect(Date.parse(response.body.createdAt)).to.be.greaterThan(0); - expect(response.body.updatedAt).to.eql(null); + expect(Date.parse(response.body.updatedAt)).to.be.greaterThan(0); expect(typeof response.body.scheduledTaskId).to.be('string'); const { _source: taskRecord } = await getScheduledTask(response.body.scheduledTaskId); expect(taskRecord.type).to.eql('task'); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/find.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/find.ts index 3b6eb4e865bff..70935a462d03e 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/find.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/find.ts @@ -58,7 +58,7 @@ export default function createFindTests({ getService }: FtrProviderContext) { updatedAt: match.updatedAt, }); expect(Date.parse(match.createdAt)).to.be.greaterThan(0); - expect(match.updatedAt).to.eql(null); + expect(Date.parse(match.updatedAt)).to.be.greaterThan(0); }); it(`shouldn't find alert from another space`, async () => { diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get.ts index 53b08850b81f3..30b5e43aee585 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/get.ts @@ -52,7 +52,7 @@ export default function createGetTests({ getService }: FtrProviderContext) { updatedAt: response.body.updatedAt, }); expect(Date.parse(response.body.createdAt)).to.be.greaterThan(0); - expect(response.body.updatedAt).to.eql(null); + expect(Date.parse(response.body.updatedAt)).to.be.greaterThan(0); }); it(`shouldn't find alert from another space`, async () => {