[Actions & Connectors] removes Connector flyouts after usage (elastic…

…#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.
gmmorris · Nov 2, 2020 · 96d191d · 96d191d
1 parent aae4273
commit 96d191d
Show file tree

Hide file tree

Showing 8 changed files with 56 additions and 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
@@ -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(<ConfigureCases userCanCrud />, { 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
@@ -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<ConfigureCasesComponentProps> = ({ userC
           consumer: 'case',
         }}
       >
-        <ConnectorAddFlyout
-          addFlyoutVisible={addFlyoutVisible}
-          setAddFlyoutVisibility={handleSetAddFlyoutVisibility as Dispatch<SetStateAction<boolean>>}
-          actionTypes={actionTypes}
-        />
-        {editedConnectorItem && (
+        {addFlyoutVisible && (
+          <ConnectorAddFlyout
+            onClose={() => handleSetAddFlyoutVisibility(false)}
+            actionTypes={actionTypes}
+          />
+        )}
+        {editedConnectorItem && editFlyoutVisible && (
           <ConnectorEditFlyout
             key={editedConnectorItem.id}
             initialConnector={editedConnectorItem}
-            editFlyoutVisible={editFlyoutVisible}
-            setEditFlyoutVisibility={
-              handleSetEditFlyoutVisibility as Dispatch<SetStateAction<boolean>>
-            }
+            onClose={() => handleSetEditFlyoutVisibility(false)}
           />
         )}
       </ActionsConnectorsContextProvider>

diff --git a/...ctions_ui/public/application/sections/action_connector_form/connector_add_flyout.test.tsx b/...ctions_ui/public/application/sections/action_connector_form/connector_add_flyout.test.tsx
@@ -58,8 +58,7 @@ describe('connector_add_flyout', () => {
         }}
       >
         <ConnectorAddFlyout
-          addFlyoutVisible={true}
-          setAddFlyoutVisibility={() => {}}
+          onClose={() => {}}
           actionTypes={[
             {
               id: actionType.id,
@@ -100,8 +99,7 @@ describe('connector_add_flyout', () => {
         }}
       >
         <ConnectorAddFlyout
-          addFlyoutVisible={true}
-          setAddFlyoutVisibility={() => {}}
+          onClose={() => {}}
           actionTypes={[
             {
               id: actionType.id,
@@ -160,8 +158,7 @@ describe('connector_add_flyout', () => {
         }}
       >
         <ConnectorAddFlyout
-          addFlyoutVisible={true}
-          setAddFlyoutVisibility={() => {}}
+          onClose={() => {}}
           actionTypes={[
             {
               id: actionType.id,
@@ -208,8 +205,7 @@ describe('connector_add_flyout', () => {
         }}
       >
         <ConnectorAddFlyout
-          addFlyoutVisible={true}
-          setAddFlyoutVisibility={() => {}}
+          onClose={() => {}}
           actionTypes={[
             {
               id: actionType.id,

diff --git a/...ers_actions_ui/public/application/sections/action_connector_form/connector_add_flyout.tsx b/...ers_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<React.SetStateAction<boolean>>;
+  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<boolean>(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/...tions_ui/public/application/sections/action_connector_form/connector_edit_flyout.test.tsx b/...tions_ui/public/application/sections/action_connector_form/connector_edit_flyout.test.tsx
@@ -84,11 +84,7 @@ describe('connector_edit_flyout', () => {
             docLinks: deps.docLinks,
           }}
         >
-          <ConnectorEditFlyout
-            initialConnector={connector}
-            editFlyoutVisible={true}
-            setEditFlyoutVisibility={(state) => {}}
-          />
+          <ConnectorEditFlyout initialConnector={connector} onClose={() => {}} />
         </ActionsConnectorsContextProvider>
       </AppContextProvider>
     );
@@ -141,11 +137,7 @@ describe('connector_edit_flyout', () => {
             docLinks: deps.docLinks,
           }}
         >
-          <ConnectorEditFlyout
-            initialConnector={connector}
-            editFlyoutVisible={true}
-            setEditFlyoutVisibility={(state) => {}}
-          />
+          <ConnectorEditFlyout initialConnector={connector} onClose={() => {}} />
         </ActionsConnectorsContextProvider>
       </AppContextProvider>
     );

diff --git a/...rs_actions_ui/public/application/sections/action_connector_form/connector_edit_flyout.tsx b/...rs_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<React.SetStateAction<boolean>>;
+  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<boolean>(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/...ublic/application/sections/actions_connectors_list/components/actions_connectors_list.tsx b/...ublic/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<ActionConnectorTableItem[]>([]);
   const [isLoadingActionTypes, setIsLoadingActionTypes] = useState<boolean>(false);
   const [isLoadingActions, setIsLoadingActions] = useState<boolean>(false);
-  const [editFlyoutVisible, setEditFlyoutVisibility] = useState<boolean>(false);
   const [addFlyoutVisible, setAddFlyoutVisibility] = useState<boolean>(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,20 +366,24 @@ export const ActionsConnectorsList: React.FunctionComponent = () => {
           docLinks,
         }}
       >
-        <ConnectorAddFlyout
-          addFlyoutVisible={addFlyoutVisible}
-          setAddFlyoutVisibility={setAddFlyoutVisibility}
-          onTestConnector={(connector) => editItem(connector, EditConectorTabs.Test)}
-        />
+        {addFlyoutVisible ? (
+          <ConnectorAddFlyout
+            onClose={() => {
+              setAddFlyoutVisibility(false);
+            }}
+            onTestConnector={(connector) => editItem(connector, EditConectorTabs.Test)}
+          />
+        ) : null}
         {editConnectorProps.initialConnector ? (
           <ConnectorEditFlyout
             key={`${editConnectorProps.initialConnector.id}${
               editConnectorProps.tab ? `:${editConnectorProps.tab}` : ``
             }`}
             initialConnector={editConnectorProps.initialConnector}
             tab={editConnectorProps.tab}
-            editFlyoutVisible={editFlyoutVisible}
-            setEditFlyoutVisibility={setEditFlyoutVisibility}
+            onClose={() => {
+              setEditConnectorProps(omit(editConnectorProps, 'initialConnector'));
+            }}
           />
         ) : null}
       </ActionsConnectorsContextProvider>

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
@@ -71,10 +71,9 @@ export const AddConnectorFlyout = ({ focusInput }: Props) => {
           capabilities: application?.capabilities,
         }}
       >
-        <ConnectorAddFlyout
-          addFlyoutVisible={addFlyoutVisible}
-          setAddFlyoutVisibility={setAddFlyoutVisibility}
-        />
+        {addFlyoutVisible ? (
+          <ConnectorAddFlyout onClose={() => setAddFlyoutVisibility(false)} />
+        ) : null}
       </ActionsConnectorsContextProvider>
     </>
   );