Fix rules management in list details (#145334)

## Summary Fix rules management in list details, by updating the rules references in the Exceptions cards when the List' rules get updated - Disable the Save button in Rule management, until the user changes the rules - Add `Refresh exceptions` to trigger fetching the Exceptions in any hook - Apply Docs comments - Prevent calling the Backend if the added items equal to the removed items
elastic · Nov 16, 2022 · 74e068c · 74e068c
1 parent c825321
commit 74e068c
Show file tree

Hide file tree

Showing 9 changed files with 77 additions and 28 deletions.
diff --git a/x-pack/plugins/security_solution/public/exceptions/api/list_api.ts b/x-pack/plugins/security_solution/public/exceptions/api/list_api.ts
@@ -10,6 +10,7 @@ import { fetchExceptionLists, updateExceptionList } from '@kbn/securitysolution-
 import type { HttpSetup } from '@kbn/core-http-browser';
 import { getFilters } from '@kbn/securitysolution-list-utils';
 import type { List, ListArray } from '@kbn/securitysolution-io-ts-list-types';
+import { asyncForEach } from '@kbn/std';
 import { ALL_ENDPOINT_ARTIFACT_LIST_IDS } from '../../../common/endpoint/service/artifacts/constants';
 import type {
   FetchListById,
@@ -80,12 +81,13 @@ export const updateList = async ({ list, http }: UpdateExceptionList) => {
 
 export const unlinkListFromRules = async ({ rules, listId }: UnlinkListFromRules) => {
   try {
+    if (!rules.length) return;
     const abortCtrl = new AbortController();
-    rules.map((rule) => {
+    await asyncForEach(rules, async (rule) => {
       const exceptionLists: ListArray | [] = (rule.exceptions_list ?? []).filter(
         ({ list_id: id }) => id !== listId
       );
-      return patchRule({
+      await patchRule({
         ruleProperties: {
           rule_id: rule.rule_id,
           exceptions_list: exceptionLists,
@@ -106,16 +108,17 @@ export const linkListToRules = async ({
   listNamespaceType,
 }: LinkListToRules) => {
   try {
+    if (!rules.length) return;
     const abortCtrl = new AbortController();
-    rules.map((rule) => {
+    await asyncForEach(rules, async (rule) => {
       const newExceptionList: List = {
         list_id: listId,
         id,
         type: listType,
         namespace_type: listNamespaceType,
       };
       const exceptionLists: ListArray | [] = [...(rule.exceptions_list ?? []), newExceptionList];
-      return patchRule({
+      await patchRule({
         ruleProperties: {
           rule_id: rule.rule_id,
           exceptions_list: exceptionLists,

diff --git a/x-pack/plugins/security_solution/public/exceptions/components/list_with_search/index.tsx b/x-pack/plugins/security_solution/public/exceptions/components/list_with_search/index.tsx
@@ -26,9 +26,14 @@ import { ListExceptionItems } from '..';
 interface ListWithSearchComponentProps {
   list: ExceptionListSchema;
   isReadOnly: boolean;
+  refreshExceptions?: boolean;
 }
 
-const ListWithSearchComponent: FC<ListWithSearchComponentProps> = ({ list, isReadOnly }) => {
+const ListWithSearchComponent: FC<ListWithSearchComponentProps> = ({
+  list,
+  isReadOnly,
+  refreshExceptions,
+}) => {
   const {
     listName,
     exceptions,
@@ -50,7 +55,7 @@ const ListWithSearchComponent: FC<ListWithSearchComponentProps> = ({ list, isRea
     onPaginationChange,
     handleCancelExceptionItemFlyout,
     handleConfirmExceptionFlyout,
-  } = useListWithSearchComponent(list);
+  } = useListWithSearchComponent(list, refreshExceptions);
   return (
     <>
       {showAddExceptionFlyout ? (

diff --git a/x-pack/plugins/security_solution/public/exceptions/components/manage_rules/index.tsx b/x-pack/plugins/security_solution/public/exceptions/components/manage_rules/index.tsx
@@ -27,13 +27,21 @@ import * as i18n from '../../translations';
 interface ManageRulesProps {
   linkedRules: Rule[];
   showButtonLoader?: boolean;
+  saveIsDisabled?: boolean;
   onSave: () => void;
   onCancel: () => void;
   onRuleSelectionChange: (rulesSelectedToAdd: Rule[]) => void;
 }
 
 export const ManageRules: FC<ManageRulesProps> = memo(
-  ({ linkedRules, showButtonLoader, onSave, onCancel, onRuleSelectionChange }) => {
+  ({
+    linkedRules,
+    showButtonLoader,
+    saveIsDisabled = true,
+    onSave,
+    onCancel,
+    onRuleSelectionChange,
+  }) => {
     const complicatedFlyoutTitleId = useGeneratedHtmlId({
       prefix: 'complicatedFlyoutTitle',
     });
@@ -67,7 +75,12 @@ export const ManageRules: FC<ManageRulesProps> = memo(
               </EuiButtonEmpty>
             </EuiFlexItem>
             <EuiFlexItem grow={false}>
-              <EuiButton isLoading={showButtonLoader} onClick={onSave} fill>
+              <EuiButton
+                isLoading={showButtonLoader}
+                disabled={saveIsDisabled}
+                onClick={onSave}
+                fill
+              >
                 {i18n.MANAGE_RULES_SAVE}
               </EuiButton>
             </EuiFlexItem>

diff --git a/x-pack/plugins/security_solution/public/exceptions/hooks/use_list_detail_view/index.ts b/x-pack/plugins/security_solution/public/exceptions/hooks/use_list_detail_view/index.ts
@@ -14,6 +14,7 @@ import { ViewerStatus } from '@kbn/securitysolution-exception-list-components';
 import { useParams } from 'react-router-dom';
 import type { ExceptionListSchema, NamespaceType } from '@kbn/securitysolution-io-ts-list-types';
 import { useApi } from '@kbn/securitysolution-list-hooks';
+import { isEqual } from 'lodash';
 import { ALL_ENDPOINT_ARTIFACT_LIST_IDS } from '../../../../common/endpoint/service/artifacts/constants';
 import { useUserData } from '../../../detections/components/user_info';
 import { APP_UI_ID, SecurityPageName } from '../../../../common/constants';
@@ -71,6 +72,8 @@ export const useExceptionListDetails = () => {
   const [referenceModalState, setReferenceModalState] = useState<ReferenceModalState>(
     exceptionReferenceModalInitialState
   );
+  const [disableManageButton, setDisableManageButton] = useState(true);
+  const [refreshExceptions, setRefreshExceptions] = useState(false);
 
   const headerBackOptions: BackOptions = useMemo(
     () => ({
@@ -254,6 +257,13 @@ export const useExceptionListDetails = () => {
 
   // #region Manage Rules
 
+  const resetManageRulesAfterSaving = useCallback(() => {
+    setLinkedRules(newLinkedRules);
+    setNewLinkedRules(newLinkedRules);
+    setShowManageRulesFlyout(false);
+    setShowManageButtonLoader(false);
+    setDisableManageButton(true);
+  }, [newLinkedRules]);
   const onManageRules = useCallback(() => {
     setShowManageRulesFlyout(true);
   }, []);
@@ -268,17 +278,21 @@ export const useExceptionListDetails = () => {
 
   const onRuleSelectionChange = useCallback((value) => {
     setNewLinkedRules(value);
+    setDisableManageButton(false);
   }, []);
 
   const onSaveManageRules = useCallback(async () => {
-    setShowManageButtonLoader(true);
     try {
-      if (!list) return;
+      if (!list) return setShowManageRulesFlyout(false);
+
+      setShowManageButtonLoader(true);
       const rulesToAdd = getRulesToAdd();
       const rulesToRemove = getRulesToRemove();
-      if (!rulesToAdd.length && !rulesToRemove.length) return;
 
-      await Promise.all([
+      if ((!rulesToAdd.length && !rulesToRemove.length) || isEqual(rulesToAdd, rulesToRemove))
+        return resetManageRulesAfterSaving();
+
+      Promise.all([
         unlinkListFromRules({ rules: rulesToRemove, listId: exceptionListId }),
         linkListToRules({
           rules: rulesToAdd,
@@ -287,15 +301,23 @@ export const useExceptionListDetails = () => {
           listType: list.type,
           listNamespaceType: list.namespace_type,
         }),
-      ]);
-      setShowManageButtonLoader(false);
-      setNewLinkedRules([]);
-      setLinkedRules(newLinkedRules);
-      setShowManageRulesFlyout(false);
+      ])
+        .then(() => {
+          setRefreshExceptions(true);
+          resetManageRulesAfterSaving();
+        })
+        .then(() => setRefreshExceptions(false));
     } catch (err) {
       handleErrorStatus(err);
     }
-  }, [list, getRulesToAdd, getRulesToRemove, exceptionListId, newLinkedRules, handleErrorStatus]);
+  }, [
+    list,
+    getRulesToAdd,
+    getRulesToRemove,
+    exceptionListId,
+    resetManageRulesAfterSaving,
+    handleErrorStatus,
+  ]);
   const onCancelManageRules = useCallback(() => {
     setShowManageRulesFlyout(false);
   }, []);
@@ -319,6 +341,8 @@ export const useExceptionListDetails = () => {
     referenceModalState,
     showReferenceErrorModal,
     showManageButtonLoader,
+    refreshExceptions,
+    disableManageButton,
     handleDelete,
     onEditListDetails,
     onExportList,

diff --git a/x-pack/plugins/security_solution/public/exceptions/hooks/use_list_exception_items/index.ts b/x-pack/plugins/security_solution/public/exceptions/hooks/use_list_exception_items/index.ts
@@ -133,7 +133,6 @@ export const useListExceptionItems = ({
     lastUpdated,
     pagination,
     exceptionViewerStatus: viewerStatus,
-
     ruleReferences: exceptionListReferences,
     fetchItems,
     onDeleteException,

diff --git a/x-pack/plugins/security_solution/public/exceptions/hooks/use_list_with_search/index.ts b/x-pack/plugins/security_solution/public/exceptions/hooks/use_list_with_search/index.ts
@@ -18,10 +18,12 @@ import { ViewerStatus } from '@kbn/securitysolution-exception-list-components';
 import * as i18n from '../../translations';
 import { useListExceptionItems } from '..';
 
-export const useListWithSearchComponent = (list: ExceptionListSchema) => {
+export const useListWithSearchComponent = (
+  list: ExceptionListSchema,
+  refreshExceptions?: boolean
+) => {
   const [showAddExceptionFlyout, setShowAddExceptionFlyout] = useState(false);
   const [showEditExceptionFlyout, setShowEditExceptionFlyout] = useState(false);
-
   const [exceptionToEdit, setExceptionToEdit] = useState<ExceptionListItemSchema>();
   const [viewerStatus, setViewerStatus] = useState<ViewerStatus | string>(ViewerStatus.LOADING);
 
@@ -54,7 +56,7 @@ export const useListWithSearchComponent = (list: ExceptionListSchema) => {
 
   useEffect(() => {
     fetchItems(null, ViewerStatus.LOADING);
-  }, [fetchItems]);
+  }, [fetchItems, refreshExceptions]);
 
   const emptyViewerTitle = useMemo(() => {
     return viewerStatus === ViewerStatus.EMPTY ? i18n.EXCEPTION_LIST_EMPTY_VIEWER_TITLE : '';

diff --git a/x-pack/plugins/security_solution/public/exceptions/pages/list_detail_view/index.tsx b/x-pack/plugins/security_solution/public/exceptions/pages/list_detail_view/index.tsx
@@ -40,6 +40,8 @@ export const ListsDetailViewComponent: FC = () => {
     showReferenceErrorModal,
     referenceModalState,
     showManageButtonLoader,
+    refreshExceptions,
+    disableManageButton,
     onEditListDetails,
     onExportList,
     onManageRules,
@@ -76,7 +78,7 @@ export const ListsDetailViewComponent: FC = () => {
       />
 
       <AutoDownload blob={exportedList} name={listId} />
-      <ListWithSearch list={list} isReadOnly={isReadOnly} />
+      <ListWithSearch list={list} refreshExceptions={refreshExceptions} isReadOnly={isReadOnly} />
       <ReferenceErrorModal
         cancelText={i18n.REFERENCE_MODAL_CANCEL_BUTTON}
         confirmText={i18n.REFERENCE_MODAL_CONFIRM_BUTTON}
@@ -91,10 +93,11 @@ export const ListsDetailViewComponent: FC = () => {
       {showManageRulesFlyout ? (
         <ManageRules
           linkedRules={linkedRules as Rule[]}
+          showButtonLoader={showManageButtonLoader}
+          saveIsDisabled={disableManageButton}
           onSave={onSaveManageRules}
           onCancel={onCancelManageRules}
           onRuleSelectionChange={onRuleSelectionChange}
-          showButtonLoader={showManageButtonLoader}
         />
       ) : null}
     </>

diff --git a/x-pack/plugins/security_solution/public/exceptions/translations/list_details_view.ts b/x-pack/plugins/security_solution/public/exceptions/translations/list_details_view.ts
@@ -121,13 +121,13 @@ export const MANAGE_RULES_DESCRIPTION = i18n.translate(
 export const DELETE_EXCEPTION_LIST = i18n.translate(
   'xpack.securitySolution.exceptionsTable.deleteExceptionList',
   {
-    defaultMessage: 'Delete Exception List',
+    defaultMessage: 'Delete exception list',
   }
 );
 
 export const EXPORT_EXCEPTION_LIST = i18n.translate(
   'xpack.securitySolution.exceptionsTable.exportExceptionList',
   {
-    defaultMessage: 'Export Exception List',
+    defaultMessage: 'Export exception list',
   }
 );
diff --git a/x-pack/plugins/security_solution/public/exceptions/translations/shared_list.ts b/x-pack/plugins/security_solution/public/exceptions/translations/shared_list.ts
@@ -282,14 +282,14 @@ export const EXCEPTIONS = i18n.translate(
 export const CREATE_SHARED_LIST_BUTTON = i18n.translate(
   'xpack.securitySolution.exceptions.manageExceptions.createSharedListButton',
   {
-    defaultMessage: 'create shared list',
+    defaultMessage: 'Create shared list',
   }
 );
 
 export const CREATE_BUTTON_ITEM_BUTTON = i18n.translate(
   'xpack.securitySolution.exceptions.manageExceptions.createItemButton',
   {
-    defaultMessage: 'create exception item',
+    defaultMessage: 'Create exception item',
   }
 );