diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/value_validators.test.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/value_validators.test.ts index e531a0da2f1fd..4ada6f4fc8992 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/value_validators.test.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/value_validators.test.ts @@ -13,7 +13,7 @@ import { getAlertWithInvalidatedFields, } from './value_validators'; import uuid from 'uuid'; -import { Alert, UserConfiguredActionConnector } from '../../types'; +import { Alert, IErrorObject, UserConfiguredActionConnector } from '../../types'; describe('throwIfAbsent', () => { test('throws if value is absent', () => { @@ -156,7 +156,7 @@ describe('getConnectorWithInvalidatedFields', () => { }); describe('getAlertWithInvalidatedFields', () => { - test('set nulls to all required undefined fields in alert', () => { + test('sets to null all fields that are required but undefined in alert', () => { const alert: Alert = { params: {}, consumer: 'test', @@ -169,14 +169,40 @@ describe('getAlertWithInvalidatedFields', () => { enabled: false, mutedInstanceIds: [], } as any; - const baseAlertErrors = { name: ['Name is required.'] }; - const actionsErrors = {}; + const baseAlertErrors = { + name: ['Name is required.'], + alertTypeId: ['Rule type is required.'], + }; + const actionsErrors: IErrorObject[] = []; const paramsErrors = {}; getAlertWithInvalidatedFields(alert, paramsErrors, baseAlertErrors, actionsErrors); expect(alert.name).toBeNull(); + expect(alert.alertTypeId).toBeNull(); + }); + + test('does not set to null any fields that are required and defined but invalid in alert', () => { + const alert: Alert = { + name: 'test', + id: '123', + params: {}, + consumer: '@@@@', + schedule: { + interval: '1m', + }, + actions: [], + tags: [], + muteAll: false, + enabled: false, + mutedInstanceIds: [], + } as any; + const baseAlertErrors = { consumer: ['Consumer is invalid.'] }; + const actionsErrors: IErrorObject[] = []; + const paramsErrors = {}; + getAlertWithInvalidatedFields(alert, paramsErrors, baseAlertErrors, actionsErrors); + expect(alert.consumer).toEqual('@@@@'); }); - test('set nulls to all required undefined fields in alert params', () => { + test('set to null all fields that are required but undefined in alert params', () => { const alert: Alert = { name: 'test', alertTypeId: '.threshold', @@ -204,18 +230,23 @@ describe('getAlertWithInvalidatedFields', () => { updatedBy: '', }; const baseAlertErrors = {}; - const actionsErrors = {}; - const paramsErrors = { index: ['Index is required.'] }; + const actionsErrors: IErrorObject[] = []; + const paramsErrors = { index: ['Index is required.'], timeField: ['Time field is required.'] }; getAlertWithInvalidatedFields(alert, paramsErrors, baseAlertErrors, actionsErrors); expect(alert.params.index).toBeNull(); + expect(alert.params.timeField).toBeNull(); }); - test('do not set nulls to the invalid fields with values in the connector properties, config and secrets', () => { + test('does not set to null any fields that are required and defined but invalid in alert params', () => { const alert: Alert = { name: 'test', + alertTypeId: '.threshold', id: '123', - params: {}, - consumer: '@@@@', + params: { + aggField: 'foo', + termSize: 'big', + }, + consumer: 'test', schedule: { interval: '1m', }, @@ -224,15 +255,75 @@ describe('getAlertWithInvalidatedFields', () => { muteAll: false, enabled: false, mutedInstanceIds: [], - } as any; - const baseAlertErrors = { consumer: ['Consumer is invalid.'] }; - const actionsErrors = {}; + createdBy: '', + apiKeyOwner: '', + createdAt: new Date(), + executionStatus: { + status: 'ok', + lastExecutionDate: new Date(), + }, + notifyWhen: 'onActionGroupChange', + throttle: '', + updatedAt: new Date(), + updatedBy: '', + }; + const baseAlertErrors = {}; + const actionsErrors: IErrorObject[] = []; + const paramsErrors = { + aggField: ['Aggregation field is invalid.'], + termSize: ['Term size is invalid.'], + }; + getAlertWithInvalidatedFields(alert, paramsErrors, baseAlertErrors, actionsErrors); + expect(alert.params.aggField).toEqual('foo'); + expect(alert.params.termSize).toEqual('big'); + }); + + test('set to null all fields that are required but undefined in alert actions', () => { + const alert: Alert = { + name: 'test', + alertTypeId: '.threshold', + id: '123', + params: {}, + consumer: 'test', + schedule: { + interval: '1m', + }, + actions: [ + { + actionTypeId: 'test', + group: 'qwer', + id: '123', + params: { + incident: { + field: {}, + }, + }, + }, + ], + tags: [], + muteAll: false, + enabled: false, + mutedInstanceIds: [], + createdBy: '', + apiKeyOwner: '', + createdAt: new Date(), + executionStatus: { + status: 'ok', + lastExecutionDate: new Date(), + }, + notifyWhen: 'onActionGroupChange', + throttle: '', + updatedAt: new Date(), + updatedBy: '', + }; + const baseAlertErrors = {}; + const actionsErrors = [{ 'incident.field.name': ['Name is required.'] }]; const paramsErrors = {}; getAlertWithInvalidatedFields(alert, paramsErrors, baseAlertErrors, actionsErrors); - expect(alert.consumer).toEqual('@@@@'); + expect((alert.actions[0].params as any).incident.field.name).toBeNull(); }); - test('if complex alert action fields which is required is set to nulls if it is undefined', () => { + test('validates multiple alert actions with the same connector id', () => { const alert: Alert = { name: 'test', alertTypeId: '.threshold', @@ -253,6 +344,18 @@ describe('getAlertWithInvalidatedFields', () => { }, }, }, + { + actionTypeId: 'test', + group: 'qwer', + id: '123', + params: { + incident: { + field: { + name: 'myIncident', + }, + }, + }, + }, ], tags: [], muteAll: false, @@ -271,9 +374,13 @@ describe('getAlertWithInvalidatedFields', () => { updatedBy: '', }; const baseAlertErrors = {}; - const actionsErrors = { '123': { 'incident.field.name': ['Name is required.'] } }; + const actionsErrors = [ + { 'incident.field.name': ['Name is required.'] }, + { 'incident.field.name': ['Name is invalid.'] }, + ]; const paramsErrors = {}; getAlertWithInvalidatedFields(alert, paramsErrors, baseAlertErrors, actionsErrors); expect((alert.actions[0].params as any).incident.field.name).toBeNull(); + expect((alert.actions[1].params as any).incident.field.name).toEqual('myIncident'); }); }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/value_validators.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/value_validators.ts index e873a34da1730..8ff561adc211f 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/lib/value_validators.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/lib/value_validators.ts @@ -74,7 +74,7 @@ export function getAlertWithInvalidatedFields( alert: Alert, paramsErrors: IErrorObject, baseAlertErrors: IErrorObject, - actionsErrors: Record + actionsErrors: IErrorObject[] ) { Object.keys(paramsErrors).forEach((errorKey) => { if (paramsErrors[errorKey].length >= 1 && get(alert.params, errorKey) === undefined) { @@ -86,17 +86,15 @@ export function getAlertWithInvalidatedFields( set(alert, errorKey, null); } }); - Object.keys(actionsErrors).forEach((actionId) => { - const actionToValidate = alert.actions.find((action) => action.id === actionId); - Object.keys(actionsErrors[actionId]).forEach((errorKey) => { - if ( - actionToValidate && - actionsErrors[actionId][errorKey].length >= 1 && - get(actionToValidate!.params, errorKey) === undefined - ) { - set(actionToValidate!.params, errorKey, null); - } - }); + actionsErrors.forEach((error: IErrorObject, index: number) => { + const actionToValidate = alert.actions.length > index ? alert.actions[index] : null; + if (actionToValidate) { + Object.keys(error).forEach((errorKey) => { + if (error[errorKey].length >= 1 && get(actionToValidate!.params, errorKey) === undefined) { + set(actionToValidate!.params, errorKey, null); + } + }); + } }); return alert; } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx index fdf398003ab03..6bb485bc7fbb8 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_form.tsx @@ -135,14 +135,10 @@ export function getAlertErrors( ...alertBaseErrors, } as IErrorObject; - const alertActionsErrors = alert.actions.reduce((prev, alertAction: AlertAction) => { - return { - ...prev, - [alertAction.id]: actionTypeRegistry - .get(alertAction.actionTypeId) - ?.validateParams(alertAction.params).errors, - }; - }, {}) as Record; + const alertActionsErrors = alert.actions.map((alertAction: AlertAction) => { + return actionTypeRegistry.get(alertAction.actionTypeId)?.validateParams(alertAction.params) + .errors; + }); return { alertParamsErrors, alertBaseErrors, @@ -160,13 +156,11 @@ export const hasObjectErrors: (errors: IErrorObject) => boolean = (errors) => export function isValidAlert( alertObject: InitialAlert | Alert, validationResult: IErrorObject, - actionsErrors: Record + actionsErrors: IErrorObject[] ): alertObject is Alert { return ( !hasObjectErrors(validationResult) && - Object.keys(actionsErrors).find((actionErrorsKey) => - hasObjectErrors(actionsErrors[actionErrorsKey]) - ) === undefined + actionsErrors.every((error: IErrorObject) => !hasObjectErrors(error)) ); }