Skip to content

Commit fbc142f

Browse files
ycterceroyctercero
authored
[Security Solution][Exceptions] - Update exceptions modal to use existing lists plugin useApi hook (#92348) (#92534)
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.
1 parent b8346bb commit fbc142f

3 files changed

Lines changed: 129 additions & 47 deletions

File tree

‎x-pack/plugins/lists/public/exceptions/hooks/use_api.test.ts‎

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,20 @@
77

88
import { act, renderHook } from '@testing-library/react-hooks';
99

10+
import { getUpdateExceptionListItemSchemaMock } from '../../../common/schemas/request/update_exception_list_item_schema.mock';
1011
import { coreMock } from '../../../../../../src/core/public/mocks';
1112
import * as api from '../api';
1213
import { getExceptionListSchemaMock } from '../../../common/schemas/response/exception_list_schema.mock';
1314
import { getFoundExceptionListItemSchemaMock } from '../../../common/schemas/response/found_exception_list_item_schema.mock';
1415
import { getExceptionListItemSchemaMock } from '../../../common/schemas/response/exception_list_item_schema.mock';
16+
import { getCreateExceptionListItemSchemaMock } from '../../../common/schemas/request/create_exception_list_item_schema.mock';
1517
import { HttpStart } from '../../../../../../src/core/public';
16-
import { ApiCallByIdProps, ApiCallByListIdProps } from '../types';
18+
import {
19+
AddExceptionListItemProps,
20+
ApiCallByIdProps,
21+
ApiCallByListIdProps,
22+
UpdateExceptionListItemProps,
23+
} from '../types';
1724

1825
import { ExceptionsApi, useApi } from './use_api';
1926

@@ -366,4 +373,58 @@ describe('useApi', () => {
366373
expect(onErrorMock).toHaveBeenCalledWith(mockError);
367374
});
368375
});
376+
377+
test('it invokes "addExceptionListItem" when "addExceptionListItem" used', async () => {
378+
const payload = getExceptionListItemSchemaMock();
379+
const itemToCreate = getCreateExceptionListItemSchemaMock();
380+
const spyOnFetchExceptionListItemById = jest
381+
.spyOn(api, 'addExceptionListItem')
382+
.mockResolvedValue(payload);
383+
384+
await act(async () => {
385+
const { result, waitForNextUpdate } = renderHook<HttpStart, ExceptionsApi>(() =>
386+
useApi(mockKibanaHttpService)
387+
);
388+
await waitForNextUpdate();
389+
390+
await result.current.addExceptionListItem({
391+
listItem: itemToCreate,
392+
});
393+
394+
const expected: AddExceptionListItemProps = {
395+
http: mockKibanaHttpService,
396+
listItem: itemToCreate,
397+
signal: new AbortController().signal,
398+
};
399+
400+
expect(spyOnFetchExceptionListItemById).toHaveBeenCalledWith(expected);
401+
});
402+
});
403+
404+
test('it invokes "updateExceptionListItem" when "getExceptionItem" used', async () => {
405+
const payload = getExceptionListItemSchemaMock();
406+
const itemToUpdate = getUpdateExceptionListItemSchemaMock();
407+
const spyOnUpdateExceptionListItem = jest
408+
.spyOn(api, 'updateExceptionListItem')
409+
.mockResolvedValue(payload);
410+
411+
await act(async () => {
412+
const { result, waitForNextUpdate } = renderHook<HttpStart, ExceptionsApi>(() =>
413+
useApi(mockKibanaHttpService)
414+
);
415+
await waitForNextUpdate();
416+
417+
await result.current.updateExceptionListItem({
418+
listItem: itemToUpdate,
419+
});
420+
421+
const expected: UpdateExceptionListItemProps = {
422+
http: mockKibanaHttpService,
423+
listItem: itemToUpdate,
424+
signal: new AbortController().signal,
425+
};
426+
427+
expect(spyOnUpdateExceptionListItem).toHaveBeenCalledWith(expected);
428+
});
429+
});
369430
});

‎x-pack/plugins/lists/public/exceptions/hooks/use_api.ts‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,22 @@ import { useMemo } from 'react';
99

1010
import * as Api from '../api';
1111
import { HttpStart } from '../../../../../../src/core/public';
12-
import { ExceptionListItemSchema, ExceptionListSchema } from '../../../common/schemas';
12+
import {
13+
CreateExceptionListItemSchema,
14+
ExceptionListItemSchema,
15+
ExceptionListSchema,
16+
UpdateExceptionListItemSchema,
17+
} from '../../../common/schemas';
1318
import { ApiCallFindListsItemsMemoProps, ApiCallMemoProps, ApiListExportProps } from '../types';
1419
import { getIdsAndNamespaces } from '../utils';
1520

1621
export interface ExceptionsApi {
22+
addExceptionListItem: (arg: {
23+
listItem: CreateExceptionListItemSchema;
24+
}) => Promise<ExceptionListItemSchema>;
25+
updateExceptionListItem: (arg: {
26+
listItem: UpdateExceptionListItemSchema;
27+
}) => Promise<ExceptionListItemSchema>;
1728
deleteExceptionItem: (arg: ApiCallMemoProps) => Promise<void>;
1829
deleteExceptionList: (arg: ApiCallMemoProps) => Promise<void>;
1930
getExceptionItem: (
@@ -29,6 +40,19 @@ export interface ExceptionsApi {
2940
export const useApi = (http: HttpStart): ExceptionsApi => {
3041
return useMemo(
3142
(): ExceptionsApi => ({
43+
async addExceptionListItem({
44+
listItem,
45+
}: {
46+
listItem: CreateExceptionListItemSchema;
47+
}): Promise<ExceptionListItemSchema> {
48+
const abortCtrl = new AbortController();
49+
50+
return Api.addExceptionListItem({
51+
http,
52+
listItem,
53+
signal: abortCtrl.signal,
54+
});
55+
},
3256
async deleteExceptionItem({
3357
id,
3458
namespaceType,
@@ -184,6 +208,19 @@ export const useApi = (http: HttpStart): ExceptionsApi => {
184208
onError(error);
185209
}
186210
},
211+
async updateExceptionListItem({
212+
listItem,
213+
}: {
214+
listItem: UpdateExceptionListItemSchema;
215+
}): Promise<ExceptionListItemSchema> {
216+
const abortCtrl = new AbortController();
217+
218+
return Api.updateExceptionListItem({
219+
http,
220+
listItem,
221+
signal: abortCtrl.signal,
222+
});
223+
},
187224
}),
188225
[http]
189226
);

‎x-pack/plugins/security_solution/public/common/components/exceptions/use_add_exception.tsx‎

Lines changed: 29 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,9 @@ import { UpdateDocumentByQueryResponse } from 'elasticsearch';
1010
import { HttpStart } from '../../../../../../../src/core/public';
1111

1212
import {
13-
addExceptionListItem,
14-
updateExceptionListItem,
1513
ExceptionListItemSchema,
1614
CreateExceptionListItemSchema,
17-
UpdateExceptionListItemSchema,
15+
useApi,
1816
} from '../../../lists_plugin_deps';
1917
import { updateAlertStatus } from '../../../detections/containers/detection_engine/alerts/api';
2018
import { getUpdateAlertsQuery } from '../../../detections/components/alerts_table/actions';
@@ -25,6 +23,7 @@ import {
2523
import { getQueryFilter } from '../../../../common/detection_engine/get_query_filter';
2624
import { Index } from '../../../../common/detection_engine/schemas/common/schemas';
2725
import { formatExceptionItemForUpdate, prepareExceptionItemsForBulkClose } from './helpers';
26+
import { useKibana } from '../../lib/kibana';
2827

2928
/**
3029
* Adds exception items to the list. Also optionally closes alerts.
@@ -66,11 +65,13 @@ export const useAddOrUpdateException = ({
6665
onError,
6766
onSuccess,
6867
}: UseAddOrUpdateExceptionProps): ReturnUseAddOrUpdateException => {
68+
const { services } = useKibana();
6969
const [isLoading, setIsLoading] = useState(false);
7070
const addOrUpdateExceptionRef = useRef<AddOrUpdateExceptionItemsFunc | null>(null);
71+
const { addExceptionListItem, updateExceptionListItem } = useApi(services.http);
7172
const addOrUpdateException = useCallback<AddOrUpdateExceptionItemsFunc>(
7273
async (ruleId, exceptionItemsToAddOrUpdate, alertIdToClose, bulkCloseIndex) => {
73-
if (addOrUpdateExceptionRef.current !== null) {
74+
if (addOrUpdateExceptionRef.current != null) {
7475
addOrUpdateExceptionRef.current(
7576
ruleId,
7677
exceptionItemsToAddOrUpdate,
@@ -86,49 +87,33 @@ export const useAddOrUpdateException = ({
8687
let isSubscribed = true;
8788
const abortCtrl = new AbortController();
8889

89-
const addOrUpdateItems = async (
90-
exceptionItemsToAddOrUpdate: Array<ExceptionListItemSchema | CreateExceptionListItemSchema>
91-
): Promise<void> => {
92-
const toAdd: CreateExceptionListItemSchema[] = [];
93-
const toUpdate: UpdateExceptionListItemSchema[] = [];
94-
exceptionItemsToAddOrUpdate.forEach(
95-
(item: ExceptionListItemSchema | CreateExceptionListItemSchema) => {
96-
if ('id' in item && item.id !== undefined) {
97-
toUpdate.push(formatExceptionItemForUpdate(item));
98-
} else {
99-
toAdd.push(item);
100-
}
101-
}
102-
);
103-
104-
const promises: Array<Promise<ExceptionListItemSchema>> = [];
105-
toAdd.forEach((item: CreateExceptionListItemSchema) => {
106-
promises.push(
107-
addExceptionListItem({
108-
http,
109-
listItem: item,
110-
signal: abortCtrl.signal,
111-
})
112-
);
113-
});
114-
toUpdate.forEach((item: UpdateExceptionListItemSchema) => {
115-
promises.push(
116-
updateExceptionListItem({
117-
http,
118-
listItem: item,
119-
signal: abortCtrl.signal,
120-
})
121-
);
122-
});
123-
await Promise.all(promises);
124-
};
125-
126-
const addOrUpdateExceptionItems: AddOrUpdateExceptionItemsFunc = async (
90+
const onUpdateExceptionItemsAndAlertStatus: AddOrUpdateExceptionItemsFunc = async (
12791
ruleId,
12892
exceptionItemsToAddOrUpdate,
12993
alertIdToClose,
13094
bulkCloseIndex
13195
) => {
96+
const addOrUpdateItems = async (
97+
exceptionListItems: Array<ExceptionListItemSchema | CreateExceptionListItemSchema>
98+
): Promise<void> => {
99+
await Promise.all(
100+
exceptionListItems.map(
101+
(item: ExceptionListItemSchema | CreateExceptionListItemSchema) => {
102+
if ('id' in item && item.id != null) {
103+
const formattedExceptionItem = formatExceptionItemForUpdate(item);
104+
return updateExceptionListItem({
105+
listItem: formattedExceptionItem,
106+
});
107+
} else {
108+
return addExceptionListItem({
109+
listItem: item,
110+
});
111+
}
112+
}
113+
)
114+
);
115+
};
116+
132117
try {
133118
setIsLoading(true);
134119
let alertIdResponse: UpdateDocumentByQueryResponse | undefined;
@@ -170,7 +155,6 @@ export const useAddOrUpdateException = ({
170155
const updated = (alertIdResponse?.updated ?? 0) + (bulkResponse?.updated ?? 0);
171156
const conflicts =
172157
alertIdResponse?.version_conflicts ?? 0 + (bulkResponse?.version_conflicts ?? 0);
173-
174158
if (isSubscribed) {
175159
setIsLoading(false);
176160
onSuccess(updated, conflicts);
@@ -187,12 +171,12 @@ export const useAddOrUpdateException = ({
187171
}
188172
};
189173

190-
addOrUpdateExceptionRef.current = addOrUpdateExceptionItems;
174+
addOrUpdateExceptionRef.current = onUpdateExceptionItemsAndAlertStatus;
191175
return (): void => {
192176
isSubscribed = false;
193177
abortCtrl.abort();
194178
};
195-
}, [http, onSuccess, onError]);
179+
}, [http, onSuccess, onError, updateExceptionListItem, addExceptionListItem]);
196180

197181
return [{ isLoading }, addOrUpdateException];
198182
};

0 commit comments

Comments
 (0)