[Security Solution][Exceptions] - Update exceptions modal to use existing lists plugin useApi hook (#92348) (#92534)

yctercero · web-flow · commit fbc142f63245 · 2021-02-23T20:06:23.000-05:00
Doing a quick refactor to help with #90634 . While working on #90634 found that I would introduce a circular dependency if I didn't refactor the hook used by the exception builder component to make use of the existing useApi hook in the lists plugin. #90634 adds temporary ids to item entries to mitigate some React key requirements and the logic to remove/add these id's isn't at the boundary but in the hooks (so as not to pollute the data for everyone wanting to use the api. An upside is that it removed some of the looping and seemed to speed things up a bit. I briefly considered adding the bulk SO endpoints for exception items, but they are still experimental.
diff --git a/x-pack/plugins/lists/public/exceptions/hooks/use_api.test.ts b/x-pack/plugins/lists/public/exceptions/hooks/use_api.test.ts
@@ -7,13 +7,20 @@
 
 import { act, renderHook } from '@testing-library/react-hooks';
 
+import { getUpdateExceptionListItemSchemaMock } from '../../../common/schemas/request/update_exception_list_item_schema.mock';
 import { coreMock } from '../../../../../../src/core/public/mocks';
 import * as api from '../api';
 import { getExceptionListSchemaMock } from '../../../common/schemas/response/exception_list_schema.mock';
 import { getFoundExceptionListItemSchemaMock } from '../../../common/schemas/response/found_exception_list_item_schema.mock';
 import { getExceptionListItemSchemaMock } from '../../../common/schemas/response/exception_list_item_schema.mock';
+import { getCreateExceptionListItemSchemaMock } from '../../../common/schemas/request/create_exception_list_item_schema.mock';
 import { HttpStart } from '../../../../../../src/core/public';
-import { ApiCallByIdProps, ApiCallByListIdProps } from '../types';
+import {
+  AddExceptionListItemProps,
+  ApiCallByIdProps,
+  ApiCallByListIdProps,
+  UpdateExceptionListItemProps,
+} from '../types';
 
 import { ExceptionsApi, useApi } from './use_api';
 
@@ -366,4 +373,58 @@ describe('useApi', () => {
       expect(onErrorMock).toHaveBeenCalledWith(mockError);
     });
   });
+
+  test('it invokes "addExceptionListItem" when "addExceptionListItem" used', async () => {
+    const payload = getExceptionListItemSchemaMock();
+    const itemToCreate = getCreateExceptionListItemSchemaMock();
+    const spyOnFetchExceptionListItemById = jest
+      .spyOn(api, 'addExceptionListItem')
+      .mockResolvedValue(payload);
+
+    await act(async () => {
+      const { result, waitForNextUpdate } = renderHook<HttpStart, ExceptionsApi>(() =>
+        useApi(mockKibanaHttpService)
+      );
+      await waitForNextUpdate();
+
+      await result.current.addExceptionListItem({
+        listItem: itemToCreate,
+      });
+
+      const expected: AddExceptionListItemProps = {
+        http: mockKibanaHttpService,
+        listItem: itemToCreate,
+        signal: new AbortController().signal,
+      };
+
+      expect(spyOnFetchExceptionListItemById).toHaveBeenCalledWith(expected);
+    });
+  });
+
+  test('it invokes "updateExceptionListItem" when "getExceptionItem" used', async () => {
+    const payload = getExceptionListItemSchemaMock();
+    const itemToUpdate = getUpdateExceptionListItemSchemaMock();
+    const spyOnUpdateExceptionListItem = jest
+      .spyOn(api, 'updateExceptionListItem')
+      .mockResolvedValue(payload);
+
+    await act(async () => {
+      const { result, waitForNextUpdate } = renderHook<HttpStart, ExceptionsApi>(() =>
+        useApi(mockKibanaHttpService)
+      );
+      await waitForNextUpdate();
+
+      await result.current.updateExceptionListItem({
+        listItem: itemToUpdate,
+      });
+
+      const expected: UpdateExceptionListItemProps = {
+        http: mockKibanaHttpService,
+        listItem: itemToUpdate,
+        signal: new AbortController().signal,
+      };
+
+      expect(spyOnUpdateExceptionListItem).toHaveBeenCalledWith(expected);
+    });
+  });
 });
diff --git a/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts b/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts
@@ -9,11 +9,22 @@ import { useMemo } from 'react';
 
 import * as Api from '../api';
 import { HttpStart } from '../../../../../../src/core/public';
-import { ExceptionListItemSchema, ExceptionListSchema } from '../../../common/schemas';
+import {
+  CreateExceptionListItemSchema,
+  ExceptionListItemSchema,
+  ExceptionListSchema,
+  UpdateExceptionListItemSchema,
+} from '../../../common/schemas';
 import { ApiCallFindListsItemsMemoProps, ApiCallMemoProps, ApiListExportProps } from '../types';
 import { getIdsAndNamespaces } from '../utils';
 
 export interface ExceptionsApi {
+  addExceptionListItem: (arg: {
+    listItem: CreateExceptionListItemSchema;
+  }) => Promise<ExceptionListItemSchema>;
+  updateExceptionListItem: (arg: {
+    listItem: UpdateExceptionListItemSchema;
+  }) => Promise<ExceptionListItemSchema>;
   deleteExceptionItem: (arg: ApiCallMemoProps) => Promise<void>;
   deleteExceptionList: (arg: ApiCallMemoProps) => Promise<void>;
   getExceptionItem: (
@@ -29,6 +40,19 @@ export interface ExceptionsApi {
 export const useApi = (http: HttpStart): ExceptionsApi => {
   return useMemo(
     (): ExceptionsApi => ({
+      async addExceptionListItem({
+        listItem,
+      }: {
+        listItem: CreateExceptionListItemSchema;
+      }): Promise<ExceptionListItemSchema> {
+        const abortCtrl = new AbortController();
+
+        return Api.addExceptionListItem({
+          http,
+          listItem,
+          signal: abortCtrl.signal,
+        });
+      },
       async deleteExceptionItem({
         id,
         namespaceType,
@@ -184,6 +208,19 @@ export const useApi = (http: HttpStart): ExceptionsApi => {
           onError(error);
         }
       },
+      async updateExceptionListItem({
+        listItem,
+      }: {
+        listItem: UpdateExceptionListItemSchema;
+      }): Promise<ExceptionListItemSchema> {
+        const abortCtrl = new AbortController();
+
+        return Api.updateExceptionListItem({
+          http,
+          listItem,
+          signal: abortCtrl.signal,
+        });
+      },
     }),
     [http]
   );
diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/use_add_exception.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/use_add_exception.tsx
@@ -10,11 +10,9 @@ import { UpdateDocumentByQueryResponse } from 'elasticsearch';
 import { HttpStart } from '../../../../../../../src/core/public';
 
 import {
-  addExceptionListItem,
-  updateExceptionListItem,
   ExceptionListItemSchema,
   CreateExceptionListItemSchema,
-  UpdateExceptionListItemSchema,
+  useApi,
 } from '../../../lists_plugin_deps';
 import { updateAlertStatus } from '../../../detections/containers/detection_engine/alerts/api';
 import { getUpdateAlertsQuery } from '../../../detections/components/alerts_table/actions';
@@ -25,6 +23,7 @@ import {
 import { getQueryFilter } from '../../../../common/detection_engine/get_query_filter';
 import { Index } from '../../../../common/detection_engine/schemas/common/schemas';
 import { formatExceptionItemForUpdate, prepareExceptionItemsForBulkClose } from './helpers';
+import { useKibana } from '../../lib/kibana';
 
 /**
  * Adds exception items to the list. Also optionally closes alerts.
@@ -66,11 +65,13 @@ export const useAddOrUpdateException = ({
   onError,
   onSuccess,
 }: UseAddOrUpdateExceptionProps): ReturnUseAddOrUpdateException => {
+  const { services } = useKibana();
   const [isLoading, setIsLoading] = useState(false);
   const addOrUpdateExceptionRef = useRef<AddOrUpdateExceptionItemsFunc | null>(null);
+  const { addExceptionListItem, updateExceptionListItem } = useApi(services.http);
   const addOrUpdateException = useCallback<AddOrUpdateExceptionItemsFunc>(
     async (ruleId, exceptionItemsToAddOrUpdate, alertIdToClose, bulkCloseIndex) => {
-      if (addOrUpdateExceptionRef.current !== null) {
+      if (addOrUpdateExceptionRef.current != null) {
         addOrUpdateExceptionRef.current(
           ruleId,
           exceptionItemsToAddOrUpdate,
@@ -86,49 +87,33 @@ export const useAddOrUpdateException = ({
     let isSubscribed = true;
     const abortCtrl = new AbortController();
 
-    const addOrUpdateItems = async (
-      exceptionItemsToAddOrUpdate: Array<ExceptionListItemSchema | CreateExceptionListItemSchema>
-    ): Promise<void> => {
-      const toAdd: CreateExceptionListItemSchema[] = [];
-      const toUpdate: UpdateExceptionListItemSchema[] = [];
-      exceptionItemsToAddOrUpdate.forEach(
-        (item: ExceptionListItemSchema | CreateExceptionListItemSchema) => {
-          if ('id' in item && item.id !== undefined) {
-            toUpdate.push(formatExceptionItemForUpdate(item));
-          } else {
-            toAdd.push(item);
-          }
-        }
-      );
-
-      const promises: Array<Promise<ExceptionListItemSchema>> = [];
-      toAdd.forEach((item: CreateExceptionListItemSchema) => {
-        promises.push(
-          addExceptionListItem({
-            http,
-            listItem: item,
-            signal: abortCtrl.signal,
-          })
-        );
-      });
-      toUpdate.forEach((item: UpdateExceptionListItemSchema) => {
-        promises.push(
-          updateExceptionListItem({
-            http,
-            listItem: item,
-            signal: abortCtrl.signal,
-          })
-        );
-      });
-      await Promise.all(promises);
-    };
-
-    const addOrUpdateExceptionItems: AddOrUpdateExceptionItemsFunc = async (
+    const onUpdateExceptionItemsAndAlertStatus: AddOrUpdateExceptionItemsFunc = async (
       ruleId,
       exceptionItemsToAddOrUpdate,
       alertIdToClose,
       bulkCloseIndex
     ) => {
+      const addOrUpdateItems = async (
+        exceptionListItems: Array<ExceptionListItemSchema | CreateExceptionListItemSchema>
+      ): Promise<void> => {
+        await Promise.all(
+          exceptionListItems.map(
+            (item: ExceptionListItemSchema | CreateExceptionListItemSchema) => {
+              if ('id' in item && item.id != null) {
+                const formattedExceptionItem = formatExceptionItemForUpdate(item);
+                return updateExceptionListItem({
+                  listItem: formattedExceptionItem,
+                });
+              } else {
+                return addExceptionListItem({
+                  listItem: item,
+                });
+              }
+            }
+          )
+        );
+      };
+
       try {
         setIsLoading(true);
         let alertIdResponse: UpdateDocumentByQueryResponse | undefined;
@@ -170,7 +155,6 @@ export const useAddOrUpdateException = ({
         const updated = (alertIdResponse?.updated ?? 0) + (bulkResponse?.updated ?? 0);
         const conflicts =
           alertIdResponse?.version_conflicts ?? 0 + (bulkResponse?.version_conflicts ?? 0);
-
         if (isSubscribed) {
           setIsLoading(false);
           onSuccess(updated, conflicts);
@@ -187,12 +171,12 @@ export const useAddOrUpdateException = ({
       }
     };
 
-    addOrUpdateExceptionRef.current = addOrUpdateExceptionItems;
+    addOrUpdateExceptionRef.current = onUpdateExceptionItemsAndAlertStatus;
     return (): void => {
       isSubscribed = false;
       abortCtrl.abort();
     };
-  }, [http, onSuccess, onError]);
+  }, [http, onSuccess, onError, updateExceptionListItem, addExceptionListItem]);
 
   return [{ isLoading }, addOrUpdateException];
 };
