From 0e1896d087a4f652c24a079673ed995af1a6dfcb Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Wed, 14 Oct 2020 11:37:48 -0400 Subject: [PATCH 1/4] Adding check for broken connectors in action form --- .../action_connector_form/action_form.tsx | 21 +++++++++++++++++-- .../sections/alert_form/alert_edit.tsx | 6 +++++- .../sections/alert_form/alert_form.tsx | 3 +++ 3 files changed, 27 insertions(+), 3 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 1b176e0f63dbd..68be5085e7c33 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 @@ -62,6 +62,7 @@ interface ActionAccordionFormProps { messageVariables?: ActionVariable[]; defaultActionMessage?: string; setHasActionsDisabled?: (value: boolean) => void; + setHasActionsWithBrokenConnector?: (value: boolean) => void; capabilities: ApplicationStart['capabilities']; } @@ -83,6 +84,7 @@ export const ActionForm = ({ defaultActionMessage, toastNotifications, setHasActionsDisabled, + setHasActionsWithBrokenConnector, capabilities, docLinks, }: ActionAccordionFormProps) => { @@ -164,6 +166,7 @@ export const ActionForm = ({ if (setHasActionsDisabled) { setHasActionsDisabled(hasActionsDisabled); } + testForBrokenConnector(actions); }; if (connectors.length > 0 && actionTypesIndex) { setActionTypesAvalilability(); @@ -178,6 +181,20 @@ export const ActionForm = ({ } ); + const updateActions = (updatedActions: AlertAction[]) => { + setAlertProperty(updatedActions); + // testForBrokenConnector(updatedActions); + }; + + const testForBrokenConnector = (alertActions: AlertAction[]) => { + const hasActionWithBrokenConnector = alertActions.some( + (action) => !connectors.find((connector) => connector.id === action.id) + ); + if (setHasActionsWithBrokenConnector) { + setHasActionsWithBrokenConnector(hasActionWithBrokenConnector); + } + }; + const getSelectedOptions = (actionItemId: string) => { const selectedConnector = connectors.find((connector) => connector.id === actionItemId); if ( @@ -385,7 +402,7 @@ export const ActionForm = ({ const updatedActions = actions.filter( (_item: AlertAction, i: number) => i !== index ); - setAlertProperty(updatedActions); + updateActions(updatedActions); setIsAddActionPanelOpen( updatedActions.filter((item: AlertAction) => item.id !== actionItem.id).length === 0 @@ -462,7 +479,7 @@ export const ActionForm = ({ const updatedActions = actions.filter( (_item: AlertAction, i: number) => i !== index ); - setAlertProperty(updatedActions); + updateActions(updatedActions); setIsAddActionPanelOpen( updatedActions.filter((item: AlertAction) => item.id !== actionItem.id).length === 0 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 999873a650f07..b60aa04ee9f27 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 @@ -38,6 +38,9 @@ export const AlertEdit = ({ initialAlert, onClose }: AlertEditProps) => { const [{ alert }, dispatch] = useReducer(alertReducer, { alert: initialAlert }); const [isSaving, setIsSaving] = useState(false); const [hasActionsDisabled, setHasActionsDisabled] = useState(false); + const [hasActionsWithBrokenConnector, setHasActionsWithBrokenConnector] = useState( + false + ); const setAlert = (key: string, value: any) => { dispatch({ command: { type: 'setAlert' }, payload: { key, value } }); }; @@ -155,6 +158,7 @@ export const AlertEdit = ({ initialAlert, onClose }: AlertEditProps) => { errors={errors} canChangeTrigger={false} setHasActionsDisabled={setHasActionsDisabled} + setHasActionsWithBrokenConnector={setHasActionsWithBrokenConnector} operation="i18n.translate('xpack.triggersActionsUI.sections.alertEdit.operationName', { defaultMessage: 'edit', })" @@ -176,7 +180,7 @@ export const AlertEdit = ({ initialAlert, onClose }: AlertEditProps) => { data-test-subj="saveEditedAlertButton" type="submit" iconType="check" - isDisabled={hasErrors || hasActionErrors} + isDisabled={hasErrors || hasActionErrors || hasActionsWithBrokenConnector} isLoading={isSaving} onClick={async () => { setIsSaving(true); 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 c69c33c0fe22e..8800f149c033b 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 @@ -81,6 +81,7 @@ interface AlertFormProps { errors: IErrorObject; canChangeTrigger?: boolean; // to hide Change trigger button setHasActionsDisabled?: (value: boolean) => void; + setHasActionsWithBrokenConnector?: (value: boolean) => void; operation: string; } @@ -90,6 +91,7 @@ export const AlertForm = ({ dispatch, errors, setHasActionsDisabled, + setHasActionsWithBrokenConnector, operation, }: AlertFormProps) => { const alertsContext = useAlertsContext(); @@ -260,6 +262,7 @@ export const AlertForm = ({ From 1aae2b67de66d0a6e5d4501cbcc913c0a2ddb5a5 Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Wed, 14 Oct 2020 13:14:54 -0400 Subject: [PATCH 2/4] Adding check for broken connectors in action form --- .../action_connector_form/action_form.tsx | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 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 68be5085e7c33..d9aee4d266927 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 @@ -166,7 +166,6 @@ export const ActionForm = ({ if (setHasActionsDisabled) { setHasActionsDisabled(hasActionsDisabled); } - testForBrokenConnector(actions); }; if (connectors.length > 0 && actionTypesIndex) { setActionTypesAvalilability(); @@ -174,6 +173,16 @@ export const ActionForm = ({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [connectors, actionTypesIndex]); + useEffect(() => { + const hasActionWithBrokenConnector = actions.some( + (action) => !connectors.find((connector) => connector.id === action.id) + ); + if (setHasActionsWithBrokenConnector) { + setHasActionsWithBrokenConnector(hasActionWithBrokenConnector); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [actions, connectors]); + const preconfiguredMessage = i18n.translate( 'xpack.triggersActionsUI.sections.actionForm.preconfiguredTitleMessage', { @@ -183,16 +192,6 @@ export const ActionForm = ({ const updateActions = (updatedActions: AlertAction[]) => { setAlertProperty(updatedActions); - // testForBrokenConnector(updatedActions); - }; - - const testForBrokenConnector = (alertActions: AlertAction[]) => { - const hasActionWithBrokenConnector = alertActions.some( - (action) => !connectors.find((connector) => connector.id === action.id) - ); - if (setHasActionsWithBrokenConnector) { - setHasActionsWithBrokenConnector(hasActionWithBrokenConnector); - } }; const getSelectedOptions = (actionItemId: string) => { From b2cfcd7217763d027da0ab2ce53d513a11e786fa Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Wed, 14 Oct 2020 15:33:53 -0400 Subject: [PATCH 3/4] Adding unit test --- .../action_form.test.tsx | 49 ++++++++++++++----- .../action_connector_form/action_form.tsx | 20 ++++---- 2 files changed, 47 insertions(+), 22 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx index 7ee1e0d3f3fa6..54e793ac51a3c 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx @@ -114,7 +114,7 @@ describe('action_form', () => { describe('action_form in alert', () => { let wrapper: ReactWrapper; - async function setup() { + async function setup(customActions?: AlertAction[]) { const { loadAllActions } = jest.requireMock('../../lib/action_connector_api'); loadAllActions.mockResolvedValueOnce([ { @@ -177,6 +177,7 @@ describe('action_form', () => { show: true, }, }, + setHasActionsWithBrokenConnector: jest.fn(), actionTypeRegistry: actionTypeRegistry as any, docLinks: { ELASTIC_WEBSITE_URL: '', DOC_LINK_VERSION: '' }, }; @@ -198,16 +199,18 @@ describe('action_form', () => { schedule: { interval: '1m', }, - actions: [ - { - group: 'default', - id: 'test', - actionTypeId: actionType.id, - params: { - message: '', - }, - }, - ], + actions: customActions + ? customActions + : [ + { + group: 'default', + id: 'test', + actionTypeId: actionType.id, + params: { + message: '', + }, + }, + ], tags: [], muteAll: false, enabled: false, @@ -229,6 +232,7 @@ describe('action_form', () => { setActionParamsProperty={(key: string, value: any, index: number) => (initialAlert.actions[index] = { ...initialAlert.actions[index], [key]: value }) } + setHasActionsWithBrokenConnector={deps!.setHasActionsWithBrokenConnector} http={deps!.http} actionTypeRegistry={deps!.actionTypeRegistry} defaultActionMessage={'Alert [{{ctx.metadata.name}}] has exceeded the threshold'} @@ -306,6 +310,7 @@ describe('action_form', () => { .find(`EuiToolTip [data-test-subj="${actionType.id}-ActionTypeSelectOption"]`) .exists() ).toBeFalsy(); + expect(deps.setHasActionsWithBrokenConnector).toHaveBeenCalledWith(false); }); it('does not render action types disabled by config', async () => { @@ -392,5 +397,27 @@ describe('action_form', () => { ); expect(actionOption.exists()).toBeFalsy(); }); + + it('recognizes actions with broken connectors', async () => { + await setup([ + { + group: 'default', + id: 'test', + actionTypeId: actionType.id, + params: { + message: '', + }, + }, + { + group: 'default', + id: 'connector-doesnt-exist', + actionTypeId: actionType.id, + params: { + message: 'broken', + }, + }, + ]); + expect(deps.setHasActionsWithBrokenConnector).toHaveBeenCalledWith(true); + }); }); }); 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 d9aee4d266927..36b192cd2c02c 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 @@ -174,11 +174,13 @@ export const ActionForm = ({ }, [connectors, actionTypesIndex]); useEffect(() => { - const hasActionWithBrokenConnector = actions.some( - (action) => !connectors.find((connector) => connector.id === action.id) - ); - if (setHasActionsWithBrokenConnector) { - setHasActionsWithBrokenConnector(hasActionWithBrokenConnector); + if (connectors.length > 0) { + const hasActionWithBrokenConnector = actions.some( + (action) => !connectors.find((connector) => connector.id === action.id) + ); + if (setHasActionsWithBrokenConnector) { + setHasActionsWithBrokenConnector(hasActionWithBrokenConnector); + } } // eslint-disable-next-line react-hooks/exhaustive-deps }, [actions, connectors]); @@ -190,10 +192,6 @@ export const ActionForm = ({ } ); - const updateActions = (updatedActions: AlertAction[]) => { - setAlertProperty(updatedActions); - }; - const getSelectedOptions = (actionItemId: string) => { const selectedConnector = connectors.find((connector) => connector.id === actionItemId); if ( @@ -401,7 +399,7 @@ export const ActionForm = ({ const updatedActions = actions.filter( (_item: AlertAction, i: number) => i !== index ); - updateActions(updatedActions); + setAlertProperty(updatedActions); setIsAddActionPanelOpen( updatedActions.filter((item: AlertAction) => item.id !== actionItem.id).length === 0 @@ -478,7 +476,7 @@ export const ActionForm = ({ const updatedActions = actions.filter( (_item: AlertAction, i: number) => i !== index ); - updateActions(updatedActions); + setAlertProperty(updatedActions); setIsAddActionPanelOpen( updatedActions.filter((item: AlertAction) => item.id !== actionItem.id).length === 0 From e2ba108a215f6ea153362f335b55663030781847 Mon Sep 17 00:00:00 2001 From: Ying Mao Date: Thu, 15 Oct 2020 12:15:34 -0400 Subject: [PATCH 4/4] PR fixes --- .../action_connector_form/action_form.test.tsx | 4 ++-- .../sections/action_connector_form/action_form.tsx | 12 +++++------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx index 54e793ac51a3c..34569dcc75240 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.test.tsx @@ -310,7 +310,7 @@ describe('action_form', () => { .find(`EuiToolTip [data-test-subj="${actionType.id}-ActionTypeSelectOption"]`) .exists() ).toBeFalsy(); - expect(deps.setHasActionsWithBrokenConnector).toHaveBeenCalledWith(false); + expect(deps.setHasActionsWithBrokenConnector).toHaveBeenLastCalledWith(false); }); it('does not render action types disabled by config', async () => { @@ -417,7 +417,7 @@ describe('action_form', () => { }, }, ]); - expect(deps.setHasActionsWithBrokenConnector).toHaveBeenCalledWith(true); + expect(deps.setHasActionsWithBrokenConnector).toHaveBeenLastCalledWith(true); }); }); }); 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 36b192cd2c02c..94571c4eb1e5b 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 @@ -174,13 +174,11 @@ export const ActionForm = ({ }, [connectors, actionTypesIndex]); useEffect(() => { - if (connectors.length > 0) { - const hasActionWithBrokenConnector = actions.some( - (action) => !connectors.find((connector) => connector.id === action.id) - ); - if (setHasActionsWithBrokenConnector) { - setHasActionsWithBrokenConnector(hasActionWithBrokenConnector); - } + const hasActionWithBrokenConnector = actions.some( + (action) => !connectors.find((connector) => connector.id === action.id) + ); + if (setHasActionsWithBrokenConnector) { + setHasActionsWithBrokenConnector(hasActionWithBrokenConnector); } // eslint-disable-next-line react-hooks/exhaustive-deps }, [actions, connectors]);