Skip to content

Commit

Permalink
[Alerting] Add alert.updatedAt field to represent date of last user…
Browse files Browse the repository at this point in the history
… edit (#83578)

* Adding alert.updatedAt field that only updates on user edit

* Updating unit tests

* Functional tests

* Updating alert attributes excluded from AAD

* Fixing test

* PR comments
  • Loading branch information
ymao1 authored Nov 18, 2020
1 parent 4b603da commit acc3e2f
Show file tree
Hide file tree
Showing 22 changed files with 166 additions and 30 deletions.
39 changes: 18 additions & 21 deletions x-pack/plugins/alerts/server/alerts_client/alerts_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,14 +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(),
createdAt: new Date(createTime).toISOString(),
updatedAt: new Date(createTime).toISOString(),
params: validatedAlertTypeParams as RawAlert['params'],
muteAll: false,
mutedInstanceIds: [],
Expand Down Expand Up @@ -289,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<SanitizedAlert> {
Expand All @@ -304,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<AlertTaskState | void> {
Expand Down Expand Up @@ -393,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
);
});
Expand Down Expand Up @@ -585,6 +581,7 @@ export class AlertsClient {
params: validatedAlertTypeParams as RawAlert['params'],
actions,
updatedBy: username,
updatedAt: new Date().toISOString(),
});
try {
updatedObject = await this.unsecuredSavedObjectsClient.create<RawAlert>(
Expand All @@ -607,12 +604,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(
Expand Down Expand Up @@ -677,6 +669,7 @@ export class AlertsClient {
await this.createAPIKey(this.generateAPIKeyName(attributes.alertTypeId, attributes.name)),
username
),
updatedAt: new Date().toISOString(),
updatedBy: username,
});
try {
Expand Down Expand Up @@ -751,6 +744,7 @@ export class AlertsClient {
username
),
updatedBy: username,
updatedAt: new Date().toISOString(),
});
try {
await this.unsecuredSavedObjectsClient.update('alert', id, updateAttributes, { version });
Expand Down Expand Up @@ -829,6 +823,7 @@ export class AlertsClient {
apiKey: null,
apiKeyOwner: null,
updatedBy: await this.getUserName(),
updatedAt: new Date().toISOString(),
}),
{ version }
);
Expand Down Expand Up @@ -875,6 +870,7 @@ export class AlertsClient {
muteAll: true,
mutedInstanceIds: [],
updatedBy: await this.getUserName(),
updatedAt: new Date().toISOString(),
});
const updateOptions = { version };

Expand Down Expand Up @@ -913,6 +909,7 @@ export class AlertsClient {
muteAll: false,
mutedInstanceIds: [],
updatedBy: await this.getUserName(),
updatedAt: new Date().toISOString(),
});
const updateOptions = { version };

Expand Down Expand Up @@ -957,6 +954,7 @@ export class AlertsClient {
this.updateMeta({
mutedInstanceIds,
updatedBy: await this.getUserName(),
updatedAt: new Date().toISOString(),
}),
{ version }
);
Expand Down Expand Up @@ -999,6 +997,7 @@ export class AlertsClient {
alertId,
this.updateMeta({
updatedBy: await this.getUserName(),
updatedAt: new Date().toISOString(),
mutedInstanceIds: mutedInstanceIds.filter((id: string) => id !== alertInstanceId),
}),
{ version }
Expand Down Expand Up @@ -1050,19 +1049,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<RawAlert>,
updatedAt: SavedObject['updated_at'] = createdAt,
{ createdAt, updatedAt, meta, scheduledTaskId, ...rawAlert }: Partial<RawAlert>,
references: SavedObjectReference[] | undefined
): PartialAlert {
// Not the prettiest code here, but if we want to use most of the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,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: [
Expand Down Expand Up @@ -330,6 +331,7 @@ describe('create()', () => {
"foo",
],
"throttle": null,
"updatedAt": "2019-02-12T21:01:22.479Z",
"updatedBy": "elastic",
}
`);
Expand Down Expand Up @@ -418,6 +420,7 @@ describe('create()', () => {
bar: true,
},
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
actions: [
{
group: 'default',
Expand Down Expand Up @@ -555,6 +558,7 @@ describe('create()', () => {
bar: true,
},
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
actions: [
{
group: 'default',
Expand Down Expand Up @@ -631,6 +635,7 @@ describe('create()', () => {
bar: true,
},
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
actions: [
{
group: 'default',
Expand Down Expand Up @@ -971,6 +976,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',
Expand Down Expand Up @@ -1092,6 +1098,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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
import { InvalidatePendingApiKey } from '../../types';

const taskManager = taskManagerMock.createStart();
Expand Down Expand Up @@ -45,6 +45,8 @@ beforeEach(() => {
getBeforeSetup(alertsClientParams, taskManager, alertTypeRegistry);
});

setGlobalDate();

describe('disable()', () => {
let alertsClient: AlertsClient;
const existingAlert = {
Expand Down Expand Up @@ -136,6 +138,7 @@ describe('disable()', () => {
scheduledTaskId: null,
apiKey: null,
apiKeyOwner: null,
updatedAt: '2019-02-12T21:01:22.479Z',
updatedBy: 'elastic',
actions: [
{
Expand Down Expand Up @@ -190,6 +193,7 @@ describe('disable()', () => {
scheduledTaskId: null,
apiKey: null,
apiKeyOwner: null,
updatedAt: '2019-02-12T21:01:22.479Z',
updatedBy: 'elastic',
actions: [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
import { InvalidatePendingApiKey } from '../../types';

const taskManager = taskManagerMock.createStart();
Expand Down Expand Up @@ -46,6 +46,8 @@ beforeEach(() => {
getBeforeSetup(alertsClientParams, taskManager, alertTypeRegistry);
});

setGlobalDate();

describe('enable()', () => {
let alertsClient: AlertsClient;
const existingAlert = {
Expand Down Expand Up @@ -186,6 +188,7 @@ describe('enable()', () => {
meta: {
versionApiKeyLastmodified: kibanaVersion,
},
updatedAt: '2019-02-12T21:01:22.479Z',
updatedBy: 'elastic',
apiKey: null,
apiKeyOwner: null,
Expand Down Expand Up @@ -292,6 +295,7 @@ describe('enable()', () => {
apiKey: Buffer.from('123:abc').toString('base64'),
apiKeyOwner: 'elastic',
updatedBy: 'elastic',
updatedAt: '2019-02-12T21:01:22.479Z',
actions: [
{
group: 'default',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ describe('find()', () => {
bar: true,
},
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
actions: [
{
group: 'default',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ describe('get()', () => {
bar: true,
},
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
actions: [
{
group: 'default',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ const BaseAlertInstanceSummarySavedObject: SavedObject<RawAlert> = {
createdBy: null,
updatedBy: null,
createdAt: mockedDateString,
updatedAt: mockedDateString,
apiKey: null,
apiKeyOwner: null,
throttle: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -43,6 +43,8 @@ beforeEach(() => {
getBeforeSetup(alertsClientParams, taskManager, alertTypeRegistry);
});

setGlobalDate();

describe('muteAll()', () => {
test('mutes an alert', async () => {
const alertsClient = new AlertsClient(alertsClientParams);
Expand Down Expand Up @@ -74,6 +76,7 @@ describe('muteAll()', () => {
{
muteAll: true,
mutedInstanceIds: [],
updatedAt: '2019-02-12T21:01:22.479Z',
updatedBy: 'elastic',
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -44,6 +44,8 @@ beforeEach(() => {
getBeforeSetup(alertsClientParams, taskManager, alertTypeRegistry);
});

setGlobalDate();

describe('muteInstance()', () => {
test('mutes an alert instance', async () => {
const alertsClient = new AlertsClient(alertsClientParams);
Expand All @@ -68,6 +70,7 @@ describe('muteInstance()', () => {
'1',
{
mutedInstanceIds: ['2'],
updatedAt: '2019-02-12T21:01:22.479Z',
updatedBy: 'elastic',
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -44,6 +44,8 @@ beforeEach(() => {
getBeforeSetup(alertsClientParams, taskManager, alertTypeRegistry);
});

setGlobalDate();

describe('unmuteAll()', () => {
test('unmutes an alert', async () => {
const alertsClient = new AlertsClient(alertsClientParams);
Expand Down Expand Up @@ -75,6 +77,7 @@ describe('unmuteAll()', () => {
{
muteAll: false,
mutedInstanceIds: [],
updatedAt: '2019-02-12T21:01:22.479Z',
updatedBy: 'elastic',
},
{
Expand Down
Loading

0 comments on commit acc3e2f

Please sign in to comment.