From 1482dd41c4b930536aced7f16b0ee36cddb68048 Mon Sep 17 00:00:00 2001 From: Gidi Meir Morris Date: Mon, 2 Nov 2020 10:53:14 +0000 Subject: [PATCH] [Actions & Connectors] removes Connector flyouts after usage (#82126) We've had several bugs over the past few months due to flyouts retaining state between renders. This ensure we remove these flyouts entirely in between usage to avoid such issues recurring. --- .../components/configure_cases/index.test.tsx | 41 ++++++++----------- .../components/configure_cases/index.tsx | 20 ++++----- .../connector_add_flyout.test.tsx | 12 ++---- .../connector_add_flyout.tsx | 14 ++----- .../connector_edit_flyout.test.tsx | 12 +----- .../connector_edit_flyout.tsx | 14 ++----- .../components/actions_connectors_list.tsx | 21 ++++++---- .../settings/add_connector_flyout.tsx | 7 ++-- 8 files changed, 56 insertions(+), 85 deletions(-) diff --git a/x-pack/plugins/security_solution/public/cases/components/configure_cases/index.test.tsx b/x-pack/plugins/security_solution/public/cases/components/configure_cases/index.test.tsx index fba352d3ee582..e7a4e7091180a 100644 --- a/x-pack/plugins/security_solution/public/cases/components/configure_cases/index.test.tsx +++ b/x-pack/plugins/security_solution/public/cases/components/configure_cases/index.test.tsx @@ -67,9 +67,9 @@ describe('ConfigureCases', () => { expect(wrapper.find(ActionsConnectorsContextProvider).exists()).toBeTruthy(); }); - test('it renders the ConnectorAddFlyout', () => { + test('it does NOT render the ConnectorAddFlyout', () => { // Components from triggersActionsUi do not have a data-test-subj - expect(wrapper.find(ConnectorAddFlyout).exists()).toBeTruthy(); + expect(wrapper.find(ConnectorAddFlyout).exists()).toBeFalsy(); }); test('it does NOT render the ConnectorEditFlyout', () => { @@ -157,10 +157,6 @@ describe('ConfigureCases', () => { wrapper = mount(, { wrappingComponent: TestProviders }); }); - test('it renders the ConnectorEditFlyout', () => { - expect(wrapper.find(ConnectorEditFlyout).exists()).toBeTruthy(); - }); - test('it renders with correct props', () => { // Connector expect(wrapper.find(Connectors).prop('connectors')).toEqual(connectors); @@ -173,21 +169,8 @@ describe('ConfigureCases', () => { expect(wrapper.find(ClosureOptions).prop('closureTypeSelected')).toBe('close-by-user'); // Flyouts - expect(wrapper.find(ConnectorAddFlyout).prop('addFlyoutVisible')).toBe(false); - expect(wrapper.find(ConnectorAddFlyout).prop('actionTypes')).toEqual([ - expect.objectContaining({ - id: '.servicenow', - }), - expect.objectContaining({ - id: '.jira', - }), - expect.objectContaining({ - id: '.resilient', - }), - ]); - - expect(wrapper.find(ConnectorEditFlyout).prop('editFlyoutVisible')).toBe(false); - expect(wrapper.find(ConnectorEditFlyout).prop('initialConnector')).toEqual(connectors[0]); + expect(wrapper.find(ConnectorAddFlyout).exists()).toBe(false); + expect(wrapper.find(ConnectorEditFlyout).exists()).toBe(false); }); test('it disables correctly when the user cannot crud', () => { @@ -518,7 +501,18 @@ describe('user interactions', () => { wrapper.find('button[data-test-subj="dropdown-connector-add-connector"]').simulate('click'); wrapper.update(); - expect(wrapper.find(ConnectorAddFlyout).prop('addFlyoutVisible')).toBe(true); + expect(wrapper.find(ConnectorAddFlyout).exists()).toBe(true); + expect(wrapper.find(ConnectorAddFlyout).prop('actionTypes')).toEqual([ + expect.objectContaining({ + id: '.servicenow', + }), + expect.objectContaining({ + id: '.jira', + }), + expect.objectContaining({ + id: '.resilient', + }), + ]); }); test('it show the edit flyout when pressing the update connector button', () => { @@ -528,7 +522,8 @@ describe('user interactions', () => { .simulate('click'); wrapper.update(); - expect(wrapper.find(ConnectorEditFlyout).prop('editFlyoutVisible')).toBe(true); + expect(wrapper.find(ConnectorEditFlyout).exists()).toBe(true); + expect(wrapper.find(ConnectorEditFlyout).prop('initialConnector')).toEqual(connectors[1]); expect( wrapper.find('[data-test-subj="case-configure-action-bottom-bar"]').exists() ).toBeFalsy(); diff --git a/x-pack/plugins/security_solution/public/cases/components/configure_cases/index.tsx b/x-pack/plugins/security_solution/public/cases/components/configure_cases/index.tsx index 9418eabaec352..a660ac0c097bf 100644 --- a/x-pack/plugins/security_solution/public/cases/components/configure_cases/index.tsx +++ b/x-pack/plugins/security_solution/public/cases/components/configure_cases/index.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { useCallback, useEffect, useState, Dispatch, SetStateAction } from 'react'; +import React, { useCallback, useEffect, useState } from 'react'; import styled, { css } from 'styled-components'; import { EuiCallOut } from '@elastic/eui'; @@ -204,19 +204,17 @@ const ConfigureCasesComponent: React.FC = ({ userC consumer: 'case', }} > - >} - actionTypes={actionTypes} - /> - {editedConnectorItem && ( + {addFlyoutVisible && ( + handleSetAddFlyoutVisibility(false)} + actionTypes={actionTypes} + /> + )} + {editedConnectorItem && editFlyoutVisible && ( > - } + onClose={() => handleSetEditFlyoutVisibility(false)} /> )} diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_flyout.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_flyout.test.tsx index b32a8ed4161d6..551bcf658e87f 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_flyout.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_flyout.test.tsx @@ -58,8 +58,7 @@ describe('connector_add_flyout', () => { }} > {}} + onClose={() => {}} actionTypes={[ { id: actionType.id, @@ -100,8 +99,7 @@ describe('connector_add_flyout', () => { }} > {}} + onClose={() => {}} actionTypes={[ { id: actionType.id, @@ -160,8 +158,7 @@ describe('connector_add_flyout', () => { }} > {}} + onClose={() => {}} actionTypes={[ { id: actionType.id, @@ -208,8 +205,7 @@ describe('connector_add_flyout', () => { }} > {}} + onClose={() => {}} actionTypes={[ { id: actionType.id, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_flyout.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_flyout.tsx index 2e222884dab50..00ff6fc132cdc 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_flyout.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_add_flyout.tsx @@ -32,15 +32,13 @@ import { useActionsConnectorsContext } from '../../context/actions_connectors_co import { VIEW_LICENSE_OPTIONS_LINK } from '../../../common/constants'; export interface ConnectorAddFlyoutProps { - addFlyoutVisible: boolean; - setAddFlyoutVisibility: React.Dispatch>; + onClose: () => void; actionTypes?: ActionType[]; onTestConnector?: (connector: ActionConnector) => void; } export const ConnectorAddFlyout = ({ - addFlyoutVisible, - setAddFlyoutVisibility, + onClose, actionTypes, onTestConnector, }: ConnectorAddFlyoutProps) => { @@ -74,17 +72,13 @@ export const ConnectorAddFlyout = ({ const [isSaving, setIsSaving] = useState(false); const closeFlyout = useCallback(() => { - setAddFlyoutVisibility(false); setActionType(undefined); setConnector(initialConnector); - }, [setAddFlyoutVisibility, initialConnector]); + onClose(); + }, [onClose, initialConnector]); const canSave = hasSaveActionsCapability(capabilities); - if (!addFlyoutVisible) { - return null; - } - function onActionTypeChange(newActionType: ActionType) { setActionType(newActionType); setActionProperty('actionTypeId', newActionType.id); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_edit_flyout.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_edit_flyout.test.tsx index ac379e279f7f2..ab58b2430fa1a 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_edit_flyout.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_edit_flyout.test.tsx @@ -84,11 +84,7 @@ describe('connector_edit_flyout', () => { docLinks: deps.docLinks, }} > - {}} - /> + {}} /> ); @@ -141,11 +137,7 @@ describe('connector_edit_flyout', () => { docLinks: deps.docLinks, }} > - {}} - /> + {}} /> ); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_edit_flyout.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_edit_flyout.tsx index e89eb8c95fbab..d81f30e4f3647 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_edit_flyout.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/connector_edit_flyout.tsx @@ -39,8 +39,7 @@ import './connector_edit_flyout.scss'; export interface ConnectorEditProps { initialConnector: ActionConnector; - editFlyoutVisible: boolean; - setEditFlyoutVisibility: React.Dispatch>; + onClose: () => void; tab?: EditConectorTabs; } @@ -51,8 +50,7 @@ export enum EditConectorTabs { export const ConnectorEditFlyout = ({ initialConnector, - editFlyoutVisible, - setEditFlyoutVisibility, + onClose, tab = EditConectorTabs.Configuration, }: ConnectorEditProps) => { const { @@ -86,16 +84,12 @@ export const ConnectorEditFlyout = ({ const [isExecutingAction, setIsExecutinAction] = useState(false); const closeFlyout = useCallback(() => { - setEditFlyoutVisibility(false); setConnector('connector', { ...initialConnector, secrets: {} }); setHasChanges(false); setTestExecutionResult(none); + onClose(); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [setEditFlyoutVisibility]); - - if (!editFlyoutVisible) { - return null; - } + }, [onClose]); const actionTypeModel = actionTypeRegistry.get(connector.actionTypeId); const errorsInConnectorConfig = (!connector.isPreconfigured diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/actions_connectors_list/components/actions_connectors_list.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/actions_connectors_list/components/actions_connectors_list.tsx index da833c3495b4a..ff5585cf04dbe 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/actions_connectors_list/components/actions_connectors_list.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/actions_connectors_list/components/actions_connectors_list.tsx @@ -20,6 +20,7 @@ import { EuiEmptyPrompt, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; +import { omit } from 'lodash'; import { FormattedMessage } from '@kbn/i18n/react'; import { useAppDependencies } from '../../../app_context'; import { loadAllActions, loadActionTypes, deleteActions } from '../../../lib/action_connector_api'; @@ -56,7 +57,6 @@ export const ActionsConnectorsList: React.FunctionComponent = () => { const [selectedItems, setSelectedItems] = useState([]); const [isLoadingActionTypes, setIsLoadingActionTypes] = useState(false); const [isLoadingActions, setIsLoadingActions] = useState(false); - const [editFlyoutVisible, setEditFlyoutVisibility] = useState(false); const [addFlyoutVisible, setAddFlyoutVisibility] = useState(false); const [editConnectorProps, setEditConnectorProps] = useState<{ initialConnector?: ActionConnector; @@ -134,7 +134,6 @@ export const ActionsConnectorsList: React.FunctionComponent = () => { async function editItem(actionConnector: ActionConnector, tab: EditConectorTabs) { setEditConnectorProps({ initialConnector: actionConnector, tab }); - setEditFlyoutVisibility(true); } const actionsTableColumns = [ @@ -367,11 +366,14 @@ export const ActionsConnectorsList: React.FunctionComponent = () => { docLinks, }} > - editItem(connector, EditConectorTabs.Test)} - /> + {addFlyoutVisible ? ( + { + setAddFlyoutVisibility(false); + }} + onTestConnector={(connector) => editItem(connector, EditConectorTabs.Test)} + /> + ) : null} {editConnectorProps.initialConnector ? ( { }`} initialConnector={editConnectorProps.initialConnector} tab={editConnectorProps.tab} - editFlyoutVisible={editFlyoutVisible} - setEditFlyoutVisibility={setEditFlyoutVisibility} + onClose={() => { + setEditConnectorProps(omit(editConnectorProps, 'initialConnector')); + }} /> ) : null} diff --git a/x-pack/plugins/uptime/public/components/settings/add_connector_flyout.tsx b/x-pack/plugins/uptime/public/components/settings/add_connector_flyout.tsx index a87748e62b513..9baacaf21acd0 100644 --- a/x-pack/plugins/uptime/public/components/settings/add_connector_flyout.tsx +++ b/x-pack/plugins/uptime/public/components/settings/add_connector_flyout.tsx @@ -69,10 +69,9 @@ export const AddConnectorFlyout = ({ focusInput }: Props) => { capabilities: application?.capabilities, }} > - + {addFlyoutVisible ? ( + setAddFlyoutVisibility(false)} /> + ) : null} );