From edf89ca474c8b9fa64966843a6e585944bc3dfcd Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Mon, 16 Nov 2020 15:38:25 -0500 Subject: [PATCH 1/6] Adding alert.updatedAt field that only updates on user edit --- .../server/alerts_client/alerts_client.ts | 34 ++++++++----------- .../alerts/server/saved_objects/mappings.json | 3 ++ x-pack/plugins/alerts/server/types.ts | 1 + 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts index 14bddceb1c03d..26052a238f419 100644 --- a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts @@ -242,6 +242,7 @@ export class AlertsClient { createdBy: username, updatedBy: username, createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), params: validatedAlertTypeParams as RawAlert['params'], muteAll: false, mutedInstanceIds: [], @@ -291,12 +292,7 @@ export class AlertsClient { }); createdAlert.attributes.scheduledTaskId = scheduledTask.id; } - return this.getAlertFromRaw( - createdAlert.id, - createdAlert.attributes, - createdAlert.updated_at, - references - ); + return this.getAlertFromRaw(createdAlert.id, createdAlert.attributes, references); } public async get({ id }: { id: string }): Promise { @@ -306,7 +302,7 @@ export class AlertsClient { result.attributes.consumer, ReadOperations.Get ); - return this.getAlertFromRaw(result.id, result.attributes, result.updated_at, result.references); + return this.getAlertFromRaw(result.id, result.attributes, result.references); } public async getAlertState({ id }: { id: string }): Promise { @@ -395,13 +391,11 @@ export class AlertsClient { type: 'alert', }); - // eslint-disable-next-line @typescript-eslint/naming-convention - const authorizedData = data.map(({ id, attributes, updated_at, references }) => { + const authorizedData = data.map(({ id, attributes, references }) => { ensureAlertTypeIsAuthorized(attributes.alertTypeId, attributes.consumer); return this.getAlertFromRaw( id, fields ? (pick(attributes, fields) as RawAlert) : attributes, - updated_at, references ); }); @@ -577,6 +571,7 @@ export class AlertsClient { params: validatedAlertTypeParams as RawAlert['params'], actions, updatedBy: username, + updatedAt: new Date().toISOString(), }); try { updatedObject = await this.unsecuredSavedObjectsClient.create( @@ -595,12 +590,7 @@ export class AlertsClient { throw e; } - return this.getPartialAlertFromRaw( - id, - updatedObject.attributes, - updatedObject.updated_at, - updatedObject.references - ); + return this.getPartialAlertFromRaw(id, updatedObject.attributes, updatedObject.references); } private apiKeyAsAlertAttributes( @@ -747,6 +737,7 @@ export class AlertsClient { username ), updatedBy: username, + updatedAt: new Date().toISOString(), }); try { await this.unsecuredSavedObjectsClient.update('alert', id, updateAttributes, { version }); @@ -817,6 +808,7 @@ export class AlertsClient { apiKey: null, apiKeyOwner: null, updatedBy: await this.getUserName(), + updatedAt: new Date().toISOString(), }), { version } ); @@ -857,6 +849,7 @@ export class AlertsClient { muteAll: true, mutedInstanceIds: [], updatedBy: await this.getUserName(), + updatedAt: new Date().toISOString(), }); const updateOptions = { version }; @@ -895,6 +888,7 @@ export class AlertsClient { muteAll: false, mutedInstanceIds: [], updatedBy: await this.getUserName(), + updatedAt: new Date().toISOString(), }); const updateOptions = { version }; @@ -939,6 +933,7 @@ export class AlertsClient { this.updateMeta({ mutedInstanceIds, updatedBy: await this.getUserName(), + updatedAt: new Date().toISOString(), }), { version } ); @@ -981,6 +976,7 @@ export class AlertsClient { alertId, this.updateMeta({ updatedBy: await this.getUserName(), + updatedAt: new Date().toISOString(), mutedInstanceIds: mutedInstanceIds.filter((id: string) => id !== alertInstanceId), }), { version } @@ -1032,19 +1028,17 @@ export class AlertsClient { private getAlertFromRaw( id: string, rawAlert: RawAlert, - updatedAt: SavedObject['updated_at'], references: SavedObjectReference[] | undefined ): Alert { // In order to support the partial update API of Saved Objects we have to support // partial updates of an Alert, but when we receive an actual RawAlert, it is safe // to cast the result to an Alert - return this.getPartialAlertFromRaw(id, rawAlert, updatedAt, references) as Alert; + return this.getPartialAlertFromRaw(id, rawAlert, references) as Alert; } private getPartialAlertFromRaw( id: string, - { createdAt, meta, scheduledTaskId, ...rawAlert }: Partial, - updatedAt: SavedObject['updated_at'] = createdAt, + { createdAt, updatedAt, meta, scheduledTaskId, ...rawAlert }: Partial, references: SavedObjectReference[] | undefined ): PartialAlert { // Not the prettiest code here, but if we want to use most of the diff --git a/x-pack/plugins/alerts/server/saved_objects/mappings.json b/x-pack/plugins/alerts/server/saved_objects/mappings.json index a6c92080f18be..f40a7d9075eed 100644 --- a/x-pack/plugins/alerts/server/saved_objects/mappings.json +++ b/x-pack/plugins/alerts/server/saved_objects/mappings.json @@ -62,6 +62,9 @@ "createdAt": { "type": "date" }, + "updatedAt": { + "type": "date" + }, "apiKey": { "type": "binary" }, diff --git a/x-pack/plugins/alerts/server/types.ts b/x-pack/plugins/alerts/server/types.ts index 9226461f6e30a..a23f7ea4036ec 100644 --- a/x-pack/plugins/alerts/server/types.ts +++ b/x-pack/plugins/alerts/server/types.ts @@ -148,6 +148,7 @@ export interface RawAlert extends SavedObjectAttributes { createdBy: string | null; updatedBy: string | null; createdAt: string; + updatedAt: string; apiKey: string | null; apiKeyOwner: string | null; throttle: string | null; From bdf1f67b6101eb98d512cfc5cdf6520849faa442 Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Tue, 17 Nov 2020 11:28:38 -0500 Subject: [PATCH 2/6] Updating unit tests --- .../plugins/alerts/server/alerts_client/alerts_client.ts | 1 + .../alerts/server/alerts_client/tests/create.test.ts | 7 +++++++ .../alerts/server/alerts_client/tests/disable.test.ts | 6 +++++- .../alerts/server/alerts_client/tests/enable.test.ts | 6 +++++- .../plugins/alerts/server/alerts_client/tests/find.test.ts | 1 + .../plugins/alerts/server/alerts_client/tests/get.test.ts | 1 + .../alerts_client/tests/get_alert_instance_summary.test.ts | 1 + .../server/alerts_client/tests/mute_instance.test.ts | 5 ++++- .../server/alerts_client/tests/unmute_instance.test.ts | 5 ++++- .../alerts/server/alerts_client/tests/update.test.ts | 7 ++++++- .../server/alerts_client/tests/update_api_key.test.ts | 6 +++++- 11 files changed, 40 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts index 26052a238f419..ff4fffb1e1d89 100644 --- a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts @@ -655,6 +655,7 @@ export class AlertsClient { await this.createAPIKey(this.generateAPIKeyName(attributes.alertTypeId, attributes.name)), username ), + updatedAt: new Date().toISOString(), updatedBy: username, }); try { diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts index 965ea1949bf3a..e5e6358eb40ab 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/create.test.ts @@ -197,6 +197,7 @@ describe('create()', () => { createdAt: '2019-02-12T21:01:22.479Z', createdBy: 'elastic', updatedBy: 'elastic', + updatedAt: '2019-02-12T21:01:22.479Z', muteAll: false, mutedInstanceIds: [], actions: [ @@ -331,6 +332,7 @@ describe('create()', () => { "foo", ], "throttle": null, + "updatedAt": "2019-02-12T21:01:22.479Z", "updatedBy": "elastic", } `); @@ -419,6 +421,7 @@ describe('create()', () => { bar: true, }, createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), actions: [ { group: 'default', @@ -556,6 +559,7 @@ describe('create()', () => { bar: true, }, createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), actions: [ { group: 'default', @@ -632,6 +636,7 @@ describe('create()', () => { bar: true, }, createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), actions: [ { group: 'default', @@ -958,6 +963,7 @@ describe('create()', () => { createdBy: 'elastic', createdAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', + updatedAt: '2019-02-12T21:01:22.479Z', enabled: true, meta: { versionApiKeyLastmodified: 'v7.10.0', @@ -1079,6 +1085,7 @@ describe('create()', () => { createdBy: 'elastic', createdAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', + updatedAt: '2019-02-12T21:01:22.479Z', enabled: false, meta: { versionApiKeyLastmodified: 'v7.10.0', diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts index d0557df622028..9db4e29f03a7b 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/disable.test.ts @@ -12,7 +12,7 @@ import { encryptedSavedObjectsMock } from '../../../../encrypted_saved_objects/s import { actionsAuthorizationMock } from '../../../../actions/server/mocks'; import { AlertsAuthorization } from '../../authorization/alerts_authorization'; import { ActionsAuthorization } from '../../../../actions/server'; -import { getBeforeSetup } from './lib'; +import { getBeforeSetup, setGlobalDate } from './lib'; const taskManager = taskManagerMock.createStart(); const alertTypeRegistry = alertTypeRegistryMock.create(); @@ -45,6 +45,8 @@ beforeEach(() => { getBeforeSetup(alertsClientParams, taskManager, alertTypeRegistry); }); +setGlobalDate(); + describe('disable()', () => { let alertsClient: AlertsClient; const existingAlert = { @@ -127,6 +129,7 @@ describe('disable()', () => { scheduledTaskId: null, apiKey: null, apiKeyOwner: null, + updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', actions: [ { @@ -170,6 +173,7 @@ describe('disable()', () => { scheduledTaskId: null, apiKey: null, apiKeyOwner: null, + updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', actions: [ { diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts index 215493c71aec7..2f6b283f5192f 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/enable.test.ts @@ -13,7 +13,7 @@ import { actionsAuthorizationMock } from '../../../../actions/server/mocks'; import { AlertsAuthorization } from '../../authorization/alerts_authorization'; import { ActionsAuthorization } from '../../../../actions/server'; import { TaskStatus } from '../../../../task_manager/server'; -import { getBeforeSetup } from './lib'; +import { getBeforeSetup, setGlobalDate } from './lib'; const taskManager = taskManagerMock.createStart(); const alertTypeRegistry = alertTypeRegistryMock.create(); @@ -46,6 +46,8 @@ beforeEach(() => { getBeforeSetup(alertsClientParams, taskManager, alertTypeRegistry); }); +setGlobalDate(); + describe('enable()', () => { let alertsClient: AlertsClient; const existingAlert = { @@ -176,6 +178,7 @@ describe('enable()', () => { meta: { versionApiKeyLastmodified: kibanaVersion, }, + updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', apiKey: null, apiKeyOwner: null, @@ -270,6 +273,7 @@ describe('enable()', () => { apiKey: Buffer.from('123:abc').toString('base64'), apiKeyOwner: 'elastic', updatedBy: 'elastic', + updatedAt: '2019-02-12T21:01:22.479Z', actions: [ { group: 'default', diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/find.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/find.test.ts index c1adaddc80d9e..66760cc289e1b 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/find.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/find.test.ts @@ -80,6 +80,7 @@ describe('find()', () => { bar: true, }, createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), actions: [ { group: 'default', diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/get.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/get.test.ts index 004230403de2e..dd56b2841689e 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/get.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/get.test.ts @@ -60,6 +60,7 @@ describe('get()', () => { bar: true, }, createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), actions: [ { group: 'default', diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_instance_summary.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_instance_summary.test.ts index 9cb2a33222d23..e6f564d72e602 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_instance_summary.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/get_alert_instance_summary.test.ts @@ -77,6 +77,7 @@ const BaseAlertInstanceSummarySavedObject: SavedObject = { createdBy: null, updatedBy: null, createdAt: mockedDateString, + updatedAt: mockedDateString, apiKey: null, apiKeyOwner: null, throttle: null, diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/mute_instance.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/mute_instance.test.ts index cd7112b3551b3..cce950b117f9a 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/mute_instance.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/mute_instance.test.ts @@ -12,7 +12,7 @@ import { encryptedSavedObjectsMock } from '../../../../encrypted_saved_objects/s import { actionsAuthorizationMock } from '../../../../actions/server/mocks'; import { AlertsAuthorization } from '../../authorization/alerts_authorization'; import { ActionsAuthorization } from '../../../../actions/server'; -import { getBeforeSetup } from './lib'; +import { getBeforeSetup, setGlobalDate } from './lib'; const taskManager = taskManagerMock.createStart(); const alertTypeRegistry = alertTypeRegistryMock.create(); @@ -45,6 +45,8 @@ beforeEach(() => { getBeforeSetup(alertsClientParams, taskManager, alertTypeRegistry); }); +setGlobalDate(); + describe('muteInstance()', () => { test('mutes an alert instance', async () => { const alertsClient = new AlertsClient(alertsClientParams); @@ -69,6 +71,7 @@ describe('muteInstance()', () => { '1', { mutedInstanceIds: ['2'], + updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', }, { diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/unmute_instance.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/unmute_instance.test.ts index 97711b8c14579..90809b9a2ea7a 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/unmute_instance.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/unmute_instance.test.ts @@ -12,7 +12,7 @@ import { encryptedSavedObjectsMock } from '../../../../encrypted_saved_objects/s import { actionsAuthorizationMock } from '../../../../actions/server/mocks'; import { AlertsAuthorization } from '../../authorization/alerts_authorization'; import { ActionsAuthorization } from '../../../../actions/server'; -import { getBeforeSetup } from './lib'; +import { getBeforeSetup, setGlobalDate } from './lib'; const taskManager = taskManagerMock.createStart(); const alertTypeRegistry = alertTypeRegistryMock.create(); @@ -45,6 +45,8 @@ beforeEach(() => { getBeforeSetup(alertsClientParams, taskManager, alertTypeRegistry); }); +setGlobalDate(); + describe('unmuteInstance()', () => { test('unmutes an alert instance', async () => { const alertsClient = new AlertsClient(alertsClientParams); @@ -70,6 +72,7 @@ describe('unmuteInstance()', () => { { mutedInstanceIds: [], updatedBy: 'elastic', + updatedAt: '2019-02-12T21:01:22.479Z', }, { version: '123' } ); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts index 1dcde6addb9bf..7120765747413 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update.test.ts @@ -141,8 +141,8 @@ describe('update()', () => { ], scheduledTaskId: 'task-123', createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), }, - updated_at: new Date().toISOString(), references: [ { name: 'action_0', @@ -292,6 +292,7 @@ describe('update()', () => { "foo", ], "throttle": null, + "updatedAt": "2019-02-12T21:01:22.479Z", "updatedBy": "elastic", } `); @@ -354,6 +355,7 @@ describe('update()', () => { bar: true, }, createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), actions: [ { group: 'default', @@ -458,6 +460,7 @@ describe('update()', () => { "foo", ], "throttle": "5m", + "updatedAt": "2019-02-12T21:01:22.479Z", "updatedBy": "elastic", } `); @@ -508,6 +511,7 @@ describe('update()', () => { bar: true, }, createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), actions: [ { group: 'default', @@ -613,6 +617,7 @@ describe('update()', () => { "foo", ], "throttle": "5m", + "updatedAt": "2019-02-12T21:01:22.479Z", "updatedBy": "elastic", } `); diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts index 1f3b567b2c031..1aa6b9c8cae27 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/update_api_key.test.ts @@ -12,7 +12,7 @@ import { encryptedSavedObjectsMock } from '../../../../encrypted_saved_objects/s import { actionsAuthorizationMock } from '../../../../actions/server/mocks'; import { AlertsAuthorization } from '../../authorization/alerts_authorization'; import { ActionsAuthorization } from '../../../../actions/server'; -import { getBeforeSetup } from './lib'; +import { getBeforeSetup, setGlobalDate } from './lib'; const taskManager = taskManagerMock.createStart(); const alertTypeRegistry = alertTypeRegistryMock.create(); @@ -44,6 +44,8 @@ beforeEach(() => { getBeforeSetup(alertsClientParams, taskManager, alertTypeRegistry); }); +setGlobalDate(); + describe('updateApiKey()', () => { let alertsClient: AlertsClient; const existingAlert = { @@ -104,6 +106,7 @@ describe('updateApiKey()', () => { apiKey: Buffer.from('234:abc').toString('base64'), apiKeyOwner: 'elastic', updatedBy: 'elastic', + updatedAt: '2019-02-12T21:01:22.479Z', actions: [ { group: 'default', @@ -142,6 +145,7 @@ describe('updateApiKey()', () => { enabled: true, apiKey: Buffer.from('234:abc').toString('base64'), apiKeyOwner: 'elastic', + updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', actions: [ { From b35af3c69bafeff4219f8c2f2070ed1f921adf9a Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Tue, 17 Nov 2020 14:29:56 -0500 Subject: [PATCH 3/6] Functional tests --- .../server/saved_objects/migrations.test.ts | 43 ++++++++++++++++++- .../alerts/server/saved_objects/migrations.ts | 20 +++++++++ .../spaces_only/tests/alerting/create.ts | 1 + .../tests/alerting/execution_status.ts | 22 ++++++++++ .../spaces_only/tests/alerting/migrations.ts | 9 ++++ 5 files changed, 94 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts index 8c9d10769b18a..a4cbc18e13b47 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.test.ts @@ -261,8 +261,48 @@ describe('7.10.0 migrates with failure', () => { }); }); +describe('7.11.0', () => { + beforeEach(() => { + jest.resetAllMocks(); + encryptedSavedObjectsSetup.createMigration.mockImplementation( + (shouldMigrateWhenPredicate, migration) => migration + ); + }); + + test('add updatedAt field to alert - set to SavedObject updated_at attribute', () => { + const migration711 = getMigrations(encryptedSavedObjectsSetup)['7.11.0']; + const alert = getMockData({}, true); + expect(migration711(alert, { log })).toEqual({ + ...alert, + attributes: { + ...alert.attributes, + updatedAt: alert.updated_at, + }, + }); + }); + + test('add updatedAt field to alert - set to createdAt when SavedObject updated_at is not defined', () => { + const migration711 = getMigrations(encryptedSavedObjectsSetup)['7.11.0']; + const alert = getMockData({}); + expect(migration711(alert, { log })).toEqual({ + ...alert, + attributes: { + ...alert.attributes, + updatedAt: alert.attributes.createdAt, + }, + }); + }); +}); + +function getUpdatedAt(): string { + const updatedAt = new Date(); + updatedAt.setHours(updatedAt.getHours() + 2); + return updatedAt.toISOString(); +} + function getMockData( - overwrites: Record = {} + overwrites: Record = {}, + withSavedObjectUpdatedAt: boolean = false ): SavedObjectUnsanitizedDoc> { return { attributes: { @@ -295,6 +335,7 @@ function getMockData( ], ...overwrites, }, + updated_at: withSavedObjectUpdatedAt ? getUpdatedAt() : undefined, id: uuid.v4(), type: 'alert', }; diff --git a/x-pack/plugins/alerts/server/saved_objects/migrations.ts b/x-pack/plugins/alerts/server/saved_objects/migrations.ts index 0b2c86b84f67b..d8ebced03c5a6 100644 --- a/x-pack/plugins/alerts/server/saved_objects/migrations.ts +++ b/x-pack/plugins/alerts/server/saved_objects/migrations.ts @@ -37,8 +37,15 @@ export function getMigrations( ) ); + const migrationAlertUpdatedAtDate = encryptedSavedObjects.createMigration( + // migrate all documents in 7.11 in order to add the "updatedAt" field + (doc): doc is SavedObjectUnsanitizedDoc => true, + pipeMigrations(setAlertUpdatedAtDate) + ); + return { '7.10.0': executeMigrationWithErrorHandling(migrationWhenRBACWasIntroduced, '7.10.0'), + '7.11.0': executeMigrationWithErrorHandling(migrationAlertUpdatedAtDate, '7.11.0'), }; } @@ -59,6 +66,19 @@ function executeMigrationWithErrorHandling( }; } +const setAlertUpdatedAtDate = ( + doc: SavedObjectUnsanitizedDoc +): SavedObjectUnsanitizedDoc => { + const updatedAt = doc.updated_at || doc.attributes.createdAt; + return { + ...doc, + attributes: { + ...doc.attributes, + updatedAt, + }, + }; +}; + const consumersToChange: Map = new Map( Object.entries({ alerting: 'alerts', 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 41f6b66c30aaf..cf7fc9edd9529 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 @@ -91,6 +91,7 @@ export default function createAlertTests({ getService }: FtrProviderContext) { }); expect(Date.parse(response.body.createdAt)).to.be.greaterThan(0); expect(Date.parse(response.body.updatedAt)).to.be.greaterThan(0); + expect(Date.parse(response.body.updatedAt)).to.eql(Date.parse(response.body.createdAt)); expect(typeof response.body.scheduledTaskId).to.be('string'); const { _source: taskRecord } = await getScheduledTask(response.body.scheduledTaskId); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts index 5ebce8edf6fb7..642173a7c2c6c 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/execution_status.ts @@ -63,6 +63,7 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon ); expect(response.status).to.eql(200); const alertId = response.body.id; + const alertUpdatedAt = response.body.updatedAt; dates.push(response.body.executionStatus.lastExecutionDate); objectRemover.add(Spaces.space1.id, alertId, 'alert', 'alerts'); @@ -70,6 +71,7 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon dates.push(executionStatus.lastExecutionDate); dates.push(Date.now()); ensureDatetimesAreOrdered(dates); + ensureAlertUpdatedAtHasNotChanged(alertId, alertUpdatedAt); // Ensure AAD isn't broken await checkAAD({ @@ -97,6 +99,7 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon ); expect(response.status).to.eql(200); const alertId = response.body.id; + const alertUpdatedAt = response.body.updatedAt; dates.push(response.body.executionStatus.lastExecutionDate); objectRemover.add(Spaces.space1.id, alertId, 'alert', 'alerts'); @@ -104,6 +107,7 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon dates.push(executionStatus.lastExecutionDate); dates.push(Date.now()); ensureDatetimesAreOrdered(dates); + ensureAlertUpdatedAtHasNotChanged(alertId, alertUpdatedAt); // Ensure AAD isn't broken await checkAAD({ @@ -128,6 +132,7 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon ); expect(response.status).to.eql(200); const alertId = response.body.id; + const alertUpdatedAt = response.body.updatedAt; dates.push(response.body.executionStatus.lastExecutionDate); objectRemover.add(Spaces.space1.id, alertId, 'alert', 'alerts'); @@ -135,6 +140,7 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon dates.push(executionStatus.lastExecutionDate); dates.push(Date.now()); ensureDatetimesAreOrdered(dates); + ensureAlertUpdatedAtHasNotChanged(alertId, alertUpdatedAt); // Ensure AAD isn't broken await checkAAD({ @@ -162,12 +168,14 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon ); expect(response.status).to.eql(200); const alertId = response.body.id; + const alertUpdatedAt = response.body.updatedAt; objectRemover.add(Spaces.space1.id, alertId, 'alert', 'alerts'); const executionStatus = await waitForStatus(alertId, new Set(['error'])); expect(executionStatus.error).to.be.ok(); expect(executionStatus.error.reason).to.be('execute'); expect(executionStatus.error.message).to.be('this alert is intended to fail'); + ensureAlertUpdatedAtHasNotChanged(alertId, alertUpdatedAt); }); it('should eventually have error reason "unknown" when appropriate', async () => { @@ -183,6 +191,7 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon ); expect(response.status).to.eql(200); const alertId = response.body.id; + const alertUpdatedAt = response.body.updatedAt; objectRemover.add(Spaces.space1.id, alertId, 'alert', 'alerts'); let executionStatus = await waitForStatus(alertId, new Set(['ok'])); @@ -201,6 +210,7 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon executionStatus = await waitForStatus(alertId, new Set(['error'])); expect(executionStatus.error).to.be.ok(); expect(executionStatus.error.reason).to.be('unknown'); + ensureAlertUpdatedAtHasNotChanged(alertId, alertUpdatedAt); const message = 'params invalid: [param1]: expected value of type [string] but got [number]'; expect(executionStatus.error.message).to.be(message); @@ -306,6 +316,18 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon await delay(WaitForStatusIncrement); return await waitForStatus(id, statuses, waitMillis - WaitForStatusIncrement); } + + async function ensureAlertUpdatedAtHasNotChanged(alertId: string, originalUpdatedAt: string) { + const response = await supertest.get( + `${getUrlPrefix(Spaces.space1.id)}/api/alerts/alert/${alertId}` + ); + const { updatedAt, executionStatus } = response.body; + expect(Date.parse(updatedAt)).to.be.greaterThan(0); + expect(Date.parse(updatedAt)).to.eql(Date.parse(originalUpdatedAt)); + expect(Date.parse(executionStatus.lastExecutionDate)).to.be.greaterThan( + Date.parse(originalUpdatedAt) + ); + } } function expectErrorExecutionStatus(executionStatus: Record, startDate: number) { diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/migrations.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/migrations.ts index 17070a14069ce..bd6afacf206d9 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/migrations.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/migrations.ts @@ -82,5 +82,14 @@ export default function createGetTests({ getService }: FtrProviderContext) { }, ]); }); + + it('7.11.0 migrates alerts to contain `updatedAt` field', async () => { + const response = await supertest.get( + `${getUrlPrefix(``)}/api/alerts/alert/74f3e6d7-b7bb-477d-ac28-92ee22728e6e` + ); + + expect(response.status).to.eql(200); + expect(response.body.updatedAt).to.eql('2020-06-17T15:35:39.839Z'); + }); }); } From dc1dc4b2ac8c82c6dc3bd5474b4769eb7fcff3de Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Tue, 17 Nov 2020 16:04:58 -0500 Subject: [PATCH 4/6] Updating alert attributes excluded from AAD --- x-pack/plugins/alerts/server/saved_objects/index.ts | 2 ++ .../alerts/server/saved_objects/partially_update_alert.test.ts | 1 + 2 files changed, 3 insertions(+) diff --git a/x-pack/plugins/alerts/server/saved_objects/index.ts b/x-pack/plugins/alerts/server/saved_objects/index.ts index da30273e93c6b..dfe122f56bc48 100644 --- a/x-pack/plugins/alerts/server/saved_objects/index.ts +++ b/x-pack/plugins/alerts/server/saved_objects/index.ts @@ -16,6 +16,7 @@ export const AlertAttributesExcludedFromAAD = [ 'muteAll', 'mutedInstanceIds', 'updatedBy', + 'updatedAt', 'executionStatus', ]; @@ -28,6 +29,7 @@ export type AlertAttributesExcludedFromAADType = | 'muteAll' | 'mutedInstanceIds' | 'updatedBy' + | 'updatedAt' | 'executionStatus'; export function setupSavedObjects( diff --git a/x-pack/plugins/alerts/server/saved_objects/partially_update_alert.test.ts b/x-pack/plugins/alerts/server/saved_objects/partially_update_alert.test.ts index 50815c797e399..8041ec551bb0d 100644 --- a/x-pack/plugins/alerts/server/saved_objects/partially_update_alert.test.ts +++ b/x-pack/plugins/alerts/server/saved_objects/partially_update_alert.test.ts @@ -95,6 +95,7 @@ const DefaultAttributes = { muteAll: true, mutedInstanceIds: ['muted-instance-id-1', 'muted-instance-id-2'], updatedBy: 'someone', + updatedAt: '2019-02-12T21:01:22.479Z', }; const InvalidAttributes = { ...DefaultAttributes, foo: 'bar' }; From 5a3aa75932cf1b19d29c3528085a495eff1485aa Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Tue, 17 Nov 2020 17:17:49 -0500 Subject: [PATCH 5/6] Fixing test --- .../alerts/server/alerts_client/tests/mute_all.test.ts | 5 ++++- .../alerts/server/alerts_client/tests/unmute_all.test.ts | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/mute_all.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/mute_all.test.ts index 868fa3d8c6aa2..14ebca2135587 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/mute_all.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/mute_all.test.ts @@ -12,7 +12,7 @@ import { encryptedSavedObjectsMock } from '../../../../encrypted_saved_objects/s import { actionsAuthorizationMock } from '../../../../actions/server/mocks'; import { AlertsAuthorization } from '../../authorization/alerts_authorization'; import { ActionsAuthorization } from '../../../../actions/server'; -import { getBeforeSetup } from './lib'; +import { getBeforeSetup, setGlobalDate } from './lib'; const taskManager = taskManagerMock.createStart(); const alertTypeRegistry = alertTypeRegistryMock.create(); @@ -43,6 +43,8 @@ beforeEach(() => { getBeforeSetup(alertsClientParams, taskManager, alertTypeRegistry); }); +setGlobalDate(); + describe('muteAll()', () => { test('mutes an alert', async () => { const alertsClient = new AlertsClient(alertsClientParams); @@ -74,6 +76,7 @@ describe('muteAll()', () => { { muteAll: true, mutedInstanceIds: [], + updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', }, { diff --git a/x-pack/plugins/alerts/server/alerts_client/tests/unmute_all.test.ts b/x-pack/plugins/alerts/server/alerts_client/tests/unmute_all.test.ts index 5ef1af9b6f0ee..d92304ab873be 100644 --- a/x-pack/plugins/alerts/server/alerts_client/tests/unmute_all.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client/tests/unmute_all.test.ts @@ -12,7 +12,7 @@ import { encryptedSavedObjectsMock } from '../../../../encrypted_saved_objects/s import { actionsAuthorizationMock } from '../../../../actions/server/mocks'; import { AlertsAuthorization } from '../../authorization/alerts_authorization'; import { ActionsAuthorization } from '../../../../actions/server'; -import { getBeforeSetup } from './lib'; +import { getBeforeSetup, setGlobalDate } from './lib'; const taskManager = taskManagerMock.createStart(); const alertTypeRegistry = alertTypeRegistryMock.create(); @@ -44,6 +44,8 @@ beforeEach(() => { getBeforeSetup(alertsClientParams, taskManager, alertTypeRegistry); }); +setGlobalDate(); + describe('unmuteAll()', () => { test('unmutes an alert', async () => { const alertsClient = new AlertsClient(alertsClientParams); @@ -75,6 +77,7 @@ describe('unmuteAll()', () => { { muteAll: false, mutedInstanceIds: [], + updatedAt: '2019-02-12T21:01:22.479Z', updatedBy: 'elastic', }, { From 345ac766c05111f19c5003b6f421f9d29a661d18 Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Wed, 18 Nov 2020 13:42:17 -0500 Subject: [PATCH 6/6] PR comments --- x-pack/plugins/alerts/server/alerts_client/alerts_client.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts index ffbac558194d6..c08ff9449d151 100644 --- a/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client/alerts_client.ts @@ -228,15 +228,17 @@ export class AlertsClient { this.validateActions(alertType, data.actions); + const createTime = Date.now(); const { references, actions } = await this.denormalizeActions(data.actions); + const rawAlert: RawAlert = { ...data, ...this.apiKeyAsAlertAttributes(createdAPIKey, username), actions, createdBy: username, updatedBy: username, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), + createdAt: new Date(createTime).toISOString(), + updatedAt: new Date(createTime).toISOString(), params: validatedAlertTypeParams as RawAlert['params'], muteAll: false, mutedInstanceIds: [],