From c74615c059d60e5a6d7967e08774f13686395b39 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Fri, 20 Mar 2020 14:12:01 -0700 Subject: [PATCH] Removed restriction on adding multiple connectors of the same action type to an alert (#60720) (#60797) * Allows multiple action under the same connector for alert * Fixed due to comments * fixed ui issue --- .../action_connector_form/action_form.tsx | 63 ++++++------------- .../connector_add_modal.scss | 4 ++ .../sections/alert_form/alert_add.tsx | 29 ++++----- .../sections/alert_form/alert_edit.tsx | 29 ++++----- .../public/application/type_registry.ts | 2 +- 5 files changed, 47 insertions(+), 80 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx index 18bc6ad8810a0..4dcbfeaaf1d3a 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx @@ -139,19 +139,6 @@ export const ActionForm = ({ }); } } - - const actionsErrors = actions.reduce( - (acc: Record, alertAction: AlertAction) => { - const actionType = actionTypeRegistry.get(alertAction.actionTypeId); - if (!actionType) { - return { ...acc }; - } - const actionValidationErrors = actionType.validateParams(alertAction.params); - return { ...acc, [alertAction.id]: actionValidationErrors }; - }, - {} - ) as Record; - const getSelectedOptions = (actionItemId: string) => { const val = connectors.find(connector => connector.id === actionItemId); if (!val) { @@ -169,17 +156,16 @@ export const ActionForm = ({ const getActionTypeForm = ( actionItem: AlertAction, actionConnector: ActionConnector, + actionParamsErrors: { + errors: IErrorObject; + }, index: number ) => { const optionsList = connectors .filter( connectorItem => connectorItem.actionTypeId === actionItem.actionTypeId && - (connectorItem.id === actionItem.id || - !actions.find( - (existingAction: AlertAction) => - existingAction.id === connectorItem.id && existingAction.group === actionItem.group - )) + connectorItem.id === actionItem.id ) .map(({ name, id }) => ({ label: name, @@ -189,8 +175,6 @@ export const ActionForm = ({ const actionTypeRegistered = actionTypeRegistry.get(actionConnector.actionTypeId); if (!actionTypeRegistered || actionItem.group !== defaultActionGroupId) return null; const ParamsFieldsComponent = actionTypeRegistered.actionParamsFields; - const actionParamsErrors: { errors: IErrorObject } = - Object.keys(actionsErrors).length > 0 ? actionsErrors[actionItem.id] : { errors: {} }; const checkEnabledResult = checkActionTypeEnabled( actionTypesIndex && actionTypesIndex[actionConnector.actionTypeId] ); @@ -317,9 +301,7 @@ export const ActionForm = ({ } )} onClick={() => { - const updatedActions = actions.filter( - (item: AlertAction) => item.id !== actionItem.id - ); + const updatedActions = actions.filter((_item: AlertAction, i: number) => i !== index); setAlertProperty(updatedActions); setIsAddActionPanelOpen( updatedActions.filter((item: AlertAction) => item.id !== actionItem.id).length === 0 @@ -381,9 +363,7 @@ export const ActionForm = ({ } )} onClick={() => { - const updatedActions = actions.filter( - (item: AlertAction) => item.id !== actionItem.id - ); + const updatedActions = actions.filter((_item: AlertAction, i: number) => i !== index); setAlertProperty(updatedActions); setIsAddActionPanelOpen( updatedActions.filter((item: AlertAction) => item.id !== actionItem.id).length === 0 @@ -441,24 +421,16 @@ export const ActionForm = ({ const actionTypeConnectors = connectors.filter( field => field.actionTypeId === actionTypeModel.id ); - let freeConnectors; if (actionTypeConnectors.length > 0) { - // Should we allow adding multiple actions to the same connector under the alert? - freeConnectors = actionTypeConnectors.filter( - (actionConnector: ActionConnector) => - !actions.find((actionItem: AlertAction) => actionItem.id === actionConnector.id) - ); - if (freeConnectors.length > 0) { - actions.push({ - id: '', - actionTypeId: actionTypeModel.id, - group: defaultActionGroupId, - params: {}, - }); - setActionIdByIndex(freeConnectors[0].id, actions.length - 1); - } + actions.push({ + id: '', + actionTypeId: actionTypeModel.id, + group: defaultActionGroupId, + params: {}, + }); + setActionIdByIndex(actionTypeConnectors[0].id, actions.length - 1); } - if (actionTypeConnectors.length === 0 || !freeConnectors || freeConnectors.length === 0) { + if (actionTypeConnectors.length === 0) { // if no connectors exists or all connectors is already assigned an action under current alert // set actionType as id to be able to create new connector within the alert form actions.push({ @@ -520,7 +492,12 @@ export const ActionForm = ({ if (!actionConnector) { return getAddConnectorsForm(actionItem, index); } - return getActionTypeForm(actionItem, actionConnector, index); + + const actionErrors: { errors: IErrorObject } = actionTypeRegistry + .get(actionItem.actionTypeId) + ?.validateParams(actionItem.params); + + return getActionTypeForm(actionItem, actionConnector, actionErrors, index); })} {isAddActionPanelOpen === false ? ( diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_modal.scss b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_modal.scss index f8fa882cd617d..7f56c220db4bb 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_modal.scss +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_modal.scss @@ -1,3 +1,7 @@ .actConnectorModal { z-index: 9000; } + +.euiComboBoxOptionsList { + z-index: 9001; +} diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_add.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_add.tsx index 2ab59a9d10ab9..4e6d63e97ec45 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_add.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_add.tsx @@ -82,25 +82,18 @@ export const AlertAdd = ({ } as IErrorObject; const hasErrors = !!Object.keys(errors).find(errorKey => errors[errorKey].length >= 1); - const actionsErrors = alert.actions.reduce( - (acc: Record, alertAction: AlertAction) => { - const actionType = actionTypeRegistry.get(alertAction.actionTypeId); - if (!actionType) { - return { ...acc }; - } - const actionValidationErrors = actionType.validateParams(alertAction.params); - return { ...acc, [alertAction.id]: actionValidationErrors }; - }, - {} - ) as Record; + const actionsErrors: Array<{ + errors: IErrorObject; + }> = alert.actions.map((alertAction: AlertAction) => + actionTypeRegistry.get(alertAction.actionTypeId)?.validateParams(alertAction.params) + ); - const hasActionErrors = !!Object.entries(actionsErrors) - .map(([, actionErrors]) => actionErrors) - .find((actionErrors: { errors: IErrorObject }) => { - return !!Object.keys(actionErrors.errors).find( - errorKey => actionErrors.errors[errorKey].length >= 1 - ); - }); + const hasActionErrors = + actionsErrors.find( + (errorObj: { errors: IErrorObject }) => + errorObj && + !!Object.keys(errorObj.errors).find(errorKey => errorObj.errors[errorKey].length >= 1) + ) !== undefined; async function onSaveAlert(): Promise { try { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.tsx index cd368193e5fa4..3feceb42e6ddc 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_form/alert_edit.tsx @@ -66,25 +66,18 @@ export const AlertEdit = ({ } as IErrorObject; const hasErrors = !!Object.keys(errors).find(errorKey => errors[errorKey].length >= 1); - const actionsErrors = alert.actions.reduce( - (acc: Record, alertAction: AlertAction) => { - const actionType = actionTypeRegistry.get(alertAction.actionTypeId); - if (!actionType) { - return { ...acc }; - } - const actionValidationErrors = actionType.validateParams(alertAction.params); - return { ...acc, [alertAction.id]: actionValidationErrors }; - }, - {} - ) as Record; + const actionsErrors: Array<{ + errors: IErrorObject; + }> = alert.actions.map((alertAction: AlertAction) => + actionTypeRegistry.get(alertAction.actionTypeId)?.validateParams(alertAction.params) + ); - const hasActionErrors = !!Object.entries(actionsErrors) - .map(([, actionErrors]) => actionErrors) - .find((actionErrors: { errors: IErrorObject }) => { - return !!Object.keys(actionErrors.errors).find( - errorKey => actionErrors.errors[errorKey].length >= 1 - ); - }); + const hasActionErrors = + actionsErrors.find( + (errorObj: { errors: IErrorObject }) => + errorObj && + !!Object.keys(errorObj.errors).find(errorKey => errorObj.errors[errorKey].length >= 1) + ) !== undefined; async function onSaveAlert(): Promise { try { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/type_registry.ts b/x-pack/plugins/triggers_actions_ui/public/application/type_registry.ts index 8eaa9638d0806..f5fe6f96ca8c0 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/type_registry.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/type_registry.ts @@ -41,7 +41,7 @@ export class TypeRegistry { } /** - * Returns an object type, null if not registered + * Returns an object type, throw error if not registered */ public get(id: string): T { if (!this.has(id)) {