Skip to content

Commit

Permalink
PR fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
ymao1 committed Dec 10, 2020
1 parent b2f6610 commit 6765bab
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 14 deletions.
8 changes: 6 additions & 2 deletions x-pack/plugins/alerts/common/alert_notify_when_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@
* you may not use this file except in compliance with the Elastic License.
*/

const AlertNotifyWhenTypeValues = ['onActionGroupChange', 'onActiveAlert', 'onThrottleInterval'];
const AlertNotifyWhenTypeValues = [
'onActionGroupChange',
'onActiveAlert',
'onThrottleInterval',
] as const;
export type AlertNotifyWhenType = typeof AlertNotifyWhenTypeValues[number];

export function validateNotifyWhenType(notifyWhen: string) {
if (AlertNotifyWhenTypeValues.includes(notifyWhen)) {
if (AlertNotifyWhenTypeValues.includes(notifyWhen as AlertNotifyWhenType)) {
return;
}
return `string is not a valid AlertNotifyWhenType: ${notifyWhen}`;
Expand Down
19 changes: 7 additions & 12 deletions x-pack/plugins/alerts/server/alerts_client/alerts_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ import {
AlertExecutionStatusValues,
AlertNotifyWhenType,
} from '../types';
import { validateAlertTypeParams, alertExecutionStatusFromRaw } from '../lib';
import {
validateAlertTypeParams,
alertExecutionStatusFromRaw,
getAlertNotifyWhenType,
} from '../lib';
import {
GrantAPIKeyResult as SecurityPluginGrantAPIKeyResult,
InvalidateAPIKeyResult as SecurityPluginInvalidateAPIKeyResult,
Expand Down Expand Up @@ -253,7 +257,7 @@ export class AlertsClient {
const createTime = Date.now();
const { references, actions } = await this.denormalizeActions(data.actions);

const notifyWhen = this.validateAlertNotifyWhenType(data.notifyWhen, data.throttle);
const notifyWhen = getAlertNotifyWhenType(data.notifyWhen, data.throttle);

const rawAlert: RawAlert = {
...data,
Expand Down Expand Up @@ -699,7 +703,7 @@ export class AlertsClient {
? await this.createAPIKey(this.generateAPIKeyName(alertType.id, data.name))
: null;
const apiKeyAttributes = this.apiKeyAsAlertAttributes(createdAPIKey, username);
const notifyWhen = this.validateAlertNotifyWhenType(data.notifyWhen, data.throttle);
const notifyWhen = getAlertNotifyWhenType(data.notifyWhen, data.throttle);

let updatedObject: SavedObject<RawAlert>;
const createAttributes = this.updateMeta({
Expand Down Expand Up @@ -1382,15 +1386,6 @@ export class AlertsClient {
}
}

private validateAlertNotifyWhenType(
notifyWhen: AlertNotifyWhenType | null,
throttle: string | null
): AlertNotifyWhenType {
// We allow notifyWhen to be null for backwards compatibility. If it is null, determine its
// value based on whether the throttle is set to a value or null
return notifyWhen ? notifyWhen! : throttle ? 'onThrottleInterval' : 'onActiveAlert';
}

private async denormalizeActions(
alertActions: NormalizedAlertAction[]
): Promise<{ actions: RawAlert['actions']; references: SavedObjectReference[] }> {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { getAlertNotifyWhenType } from './get_alert_notify_when_type';

test(`should return 'notifyWhen' value if value is set and throttle is null`, () => {
expect(getAlertNotifyWhenType('onActionGroupChange', null)).toEqual('onActionGroupChange');
});

test(`should return 'notifyWhen' value if value is set and throttle is defined`, () => {
expect(getAlertNotifyWhenType('onActionGroupChange', '10m')).toEqual('onActionGroupChange');
});

test(`should return 'onThrottleInterval' value if 'notifyWhen' is null and throttle is defined`, () => {
expect(getAlertNotifyWhenType(null, '10m')).toEqual('onThrottleInterval');
});

test(`should return 'onActiveAlert' value if 'notifyWhen' is null and throttle is null`, () => {
expect(getAlertNotifyWhenType(null, null)).toEqual('onActiveAlert');
});
16 changes: 16 additions & 0 deletions x-pack/plugins/alerts/server/lib/get_alert_notify_when_type.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { AlertNotifyWhenType } from '../types';

export function getAlertNotifyWhenType(
notifyWhen: AlertNotifyWhenType | null,
throttle: string | null
): AlertNotifyWhenType {
// We allow notifyWhen to be null for backwards compatibility. If it is null, determine its
// value based on whether the throttle is set to a value or null
return notifyWhen ? notifyWhen! : throttle ? 'onThrottleInterval' : 'onActiveAlert';
}
1 change: 1 addition & 0 deletions x-pack/plugins/alerts/server/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
export { parseDuration, validateDurationSchema } from '../../common/parse_duration';
export { LicenseState } from './license_state';
export { validateAlertTypeParams } from './validate_alert_type_params';
export { getAlertNotifyWhenType } from './get_alert_notify_when_type';
export { ErrorWithReason, getReasonFromError, isErrorWithReason } from './error_with_reason';
export {
executionStatusFromState,
Expand Down

0 comments on commit 6765bab

Please sign in to comment.