Skip to content

Commit

Permalink
[SIEM][Exceptions] - ExceptionsViewer cleanup (#68739)
Browse files Browse the repository at this point in the history
### Summary

- Adds missing unit tests for relevant files missing them
- Changes filter search to fire request on 'Enter'
- Breaks out the main ExceptionViewer component into smaller components to make more readable and better tested
- Updates utility bar to have the specific list description text next to it as proposed by @spong in #68294 (comment)
- Adds loading state any time async request occurs
- Now fetches list on list type toggle (if user selects to view either only detections or endpoint items), before was simply filtering already fetched items
  • Loading branch information
yctercero committed Jun 10, 2020
1 parent 524e0d1 commit 2861f99
Show file tree
Hide file tree
Showing 24 changed files with 1,083 additions and 351 deletions.
15 changes: 15 additions & 0 deletions x-pack/plugins/lists/public/exceptions/__mocks__/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,18 @@ export const fetchExceptionListItemById = async ({
signal,
}: ApiCallByIdProps): Promise<ExceptionListItemSchema> =>
Promise.resolve(getExceptionListItemSchemaMock());

export const deleteExceptionListById = async ({
http,
id,
namespaceType,
signal,
}: ApiCallByIdProps): Promise<ExceptionListSchema> => Promise.resolve(getExceptionListSchemaMock());

export const deleteExceptionListItemById = async ({
http,
id,
namespaceType,
signal,
}: ApiCallByIdProps): Promise<ExceptionListItemSchema> =>
Promise.resolve(getExceptionListItemSchemaMock());
257 changes: 257 additions & 0 deletions x-pack/plugins/lists/public/exceptions/hooks/use_api.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,257 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

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

import * as api from '../api';
import { createKibanaCoreStartMock } from '../../common/mocks/kibana_core';
import { getExceptionListSchemaMock } from '../../../common/schemas/response/exception_list_schema.mock';
import { getExceptionListItemSchemaMock } from '../../../common/schemas/response/exception_list_item_schema.mock';
import { HttpStart } from '../../../../../../src/core/public';
import { ApiCallByIdProps } from '../types';

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

jest.mock('../api');

const mockKibanaHttpService = createKibanaCoreStartMock().http;

describe('useApi', () => {
const onErrorMock = jest.fn();

afterEach(() => {
onErrorMock.mockClear();
jest.clearAllMocks();
});

test('it invokes "deleteExceptionListItemById" when "deleteExceptionItem" used', async () => {
const payload = getExceptionListItemSchemaMock();
const onSuccessMock = jest.fn();
const spyOnDeleteExceptionListItemById = jest
.spyOn(api, 'deleteExceptionListItemById')
.mockResolvedValue(payload);

await act(async () => {
const { result, waitForNextUpdate } = renderHook<HttpStart, ExceptionsApi>(() =>
useApi(mockKibanaHttpService)
);
await waitForNextUpdate();

const { id, namespace_type: namespaceType } = payload;

await result.current.deleteExceptionItem({
id,
namespaceType,
onError: jest.fn(),
onSuccess: onSuccessMock,
});

const expected: ApiCallByIdProps = {
http: mockKibanaHttpService,
id,
namespaceType,
signal: new AbortController().signal,
};

expect(spyOnDeleteExceptionListItemById).toHaveBeenCalledWith(expected);
expect(onSuccessMock).toHaveBeenCalled();
});
});

test('invokes "onError" callback if "deleteExceptionListItemById" fails', async () => {
const mockError = new Error('failed to delete item');
jest.spyOn(api, 'deleteExceptionListItemById').mockRejectedValue(mockError);

await act(async () => {
const { result, waitForNextUpdate } = renderHook<HttpStart, ExceptionsApi>(() =>
useApi(mockKibanaHttpService)
);
await waitForNextUpdate();

const { id, namespace_type: namespaceType } = getExceptionListItemSchemaMock();

await result.current.deleteExceptionItem({
id,
namespaceType,
onError: onErrorMock,
onSuccess: jest.fn(),
});

expect(onErrorMock).toHaveBeenCalledWith(mockError);
});
});

test('it invokes "deleteExceptionListById" when "deleteExceptionList" used', async () => {
const payload = getExceptionListSchemaMock();
const onSuccessMock = jest.fn();
const spyOnDeleteExceptionListById = jest
.spyOn(api, 'deleteExceptionListById')
.mockResolvedValue(payload);

await act(async () => {
const { result, waitForNextUpdate } = renderHook<HttpStart, ExceptionsApi>(() =>
useApi(mockKibanaHttpService)
);
await waitForNextUpdate();

const { id, namespace_type: namespaceType } = payload;

await result.current.deleteExceptionList({
id,
namespaceType,
onError: jest.fn(),
onSuccess: onSuccessMock,
});

const expected: ApiCallByIdProps = {
http: mockKibanaHttpService,
id,
namespaceType,
signal: new AbortController().signal,
};

expect(spyOnDeleteExceptionListById).toHaveBeenCalledWith(expected);
expect(onSuccessMock).toHaveBeenCalled();
});
});

test('invokes "onError" callback if "deleteExceptionListById" fails', async () => {
const mockError = new Error('failed to delete item');
jest.spyOn(api, 'deleteExceptionListById').mockRejectedValue(mockError);

await act(async () => {
const { result, waitForNextUpdate } = renderHook<HttpStart, ExceptionsApi>(() =>
useApi(mockKibanaHttpService)
);
await waitForNextUpdate();

const { id, namespace_type: namespaceType } = getExceptionListSchemaMock();

await result.current.deleteExceptionList({
id,
namespaceType,
onError: onErrorMock,
onSuccess: jest.fn(),
});

expect(onErrorMock).toHaveBeenCalledWith(mockError);
});
});

test('it invokes "fetchExceptionListItemById" when "getExceptionItem" used', async () => {
const payload = getExceptionListItemSchemaMock();
const onSuccessMock = jest.fn();
const spyOnFetchExceptionListItemById = jest
.spyOn(api, 'fetchExceptionListItemById')
.mockResolvedValue(payload);

await act(async () => {
const { result, waitForNextUpdate } = renderHook<HttpStart, ExceptionsApi>(() =>
useApi(mockKibanaHttpService)
);
await waitForNextUpdate();

const { id, namespace_type: namespaceType } = payload;

await result.current.getExceptionItem({
id,
namespaceType,
onError: jest.fn(),
onSuccess: onSuccessMock,
});

const expected: ApiCallByIdProps = {
http: mockKibanaHttpService,
id,
namespaceType,
signal: new AbortController().signal,
};

expect(spyOnFetchExceptionListItemById).toHaveBeenCalledWith(expected);
expect(onSuccessMock).toHaveBeenCalled();
});
});

test('invokes "onError" callback if "fetchExceptionListItemById" fails', async () => {
const mockError = new Error('failed to delete item');
jest.spyOn(api, 'fetchExceptionListItemById').mockRejectedValue(mockError);

await act(async () => {
const { result, waitForNextUpdate } = renderHook<HttpStart, ExceptionsApi>(() =>
useApi(mockKibanaHttpService)
);
await waitForNextUpdate();

const { id, namespace_type: namespaceType } = getExceptionListSchemaMock();

await result.current.getExceptionItem({
id,
namespaceType,
onError: onErrorMock,
onSuccess: jest.fn(),
});

expect(onErrorMock).toHaveBeenCalledWith(mockError);
});
});

test('it invokes "fetchExceptionListById" when "getExceptionList" used', async () => {
const payload = getExceptionListSchemaMock();
const onSuccessMock = jest.fn();
const spyOnFetchExceptionListById = jest
.spyOn(api, 'fetchExceptionListById')
.mockResolvedValue(payload);

await act(async () => {
const { result, waitForNextUpdate } = renderHook<HttpStart, ExceptionsApi>(() =>
useApi(mockKibanaHttpService)
);
await waitForNextUpdate();

const { id, namespace_type: namespaceType } = payload;

await result.current.getExceptionList({
id,
namespaceType,
onError: jest.fn(),
onSuccess: onSuccessMock,
});

const expected: ApiCallByIdProps = {
http: mockKibanaHttpService,
id,
namespaceType,
signal: new AbortController().signal,
};

expect(spyOnFetchExceptionListById).toHaveBeenCalledWith(expected);
expect(onSuccessMock).toHaveBeenCalled();
});
});

test('invokes "onError" callback if "fetchExceptionListById" fails', async () => {
const mockError = new Error('failed to delete item');
jest.spyOn(api, 'fetchExceptionListById').mockRejectedValue(mockError);

await act(async () => {
const { result, waitForNextUpdate } = renderHook<HttpStart, ExceptionsApi>(() =>
useApi(mockKibanaHttpService)
);
await waitForNextUpdate();

const { id, namespace_type: namespaceType } = getExceptionListSchemaMock();

await result.current.getExceptionList({
id,
namespaceType,
onError: onErrorMock,
onSuccess: jest.fn(),
});

expect(onErrorMock).toHaveBeenCalledWith(mockError);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { createKibanaCoreStartMock } from '../../common/mocks/kibana_core';
import { getExceptionListSchemaMock } from '../../../common/schemas/response/exception_list_schema.mock';
import { getExceptionListItemSchemaMock } from '../../../common/schemas/response/exception_list_item_schema.mock';
import { ExceptionListItemSchema } from '../../../common/schemas';
import { ExceptionList, UseExceptionListProps } from '../types';
import { ExceptionList, UseExceptionListProps, UseExceptionListSuccess } from '../types';

import { ReturnExceptionListAndItems, useExceptionList } from './use_exception_list';

Expand Down Expand Up @@ -57,6 +57,7 @@ describe('useExceptionList', () => {

test('fetch exception list and items', async () => {
await act(async () => {
const onSuccessMock = jest.fn();
const { result, waitForNextUpdate } = renderHook<
UseExceptionListProps,
ReturnExceptionListAndItems
Expand All @@ -65,6 +66,7 @@ describe('useExceptionList', () => {
http: mockKibanaHttpService,
lists: [{ id: 'myListId', namespaceType: 'single' }],
onError: onErrorMock,
onSuccess: onSuccessMock,
})
);
await waitForNextUpdate();
Expand All @@ -78,6 +80,12 @@ describe('useExceptionList', () => {
{ ...getExceptionListItemSchemaMock() },
];

const expectedResult: UseExceptionListSuccess = {
exceptions: expectedListItemsResult,
lists: expectedListResult,
pagination: { page: 1, perPage: 20, total: 1 },
};

expect(result.current).toEqual([
false,
expectedListResult,
Expand All @@ -89,6 +97,7 @@ describe('useExceptionList', () => {
},
result.current[4],
]);
expect(onSuccessMock).toHaveBeenCalledWith(expectedResult);
});
});

Expand All @@ -100,13 +109,14 @@ describe('useExceptionList', () => {
UseExceptionListProps,
ReturnExceptionListAndItems
>(
({ filterOptions, http, lists, pagination, onError }) =>
useExceptionList({ filterOptions, http, lists, onError, pagination }),
({ filterOptions, http, lists, pagination, onError, onSuccess }) =>
useExceptionList({ filterOptions, http, lists, onError, onSuccess, pagination }),
{
initialProps: {
http: mockKibanaHttpService,
lists: [{ id: 'myListId', namespaceType: 'single' }],
onError: onErrorMock,
onSuccess: jest.fn(),
},
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ export type ReturnExceptionListAndItems = [
* Hook for using to get an ExceptionList and it's ExceptionListItems
*
* @param http Kibana http service
* @param id desired ExceptionList ID (not list_id)
* @param namespaceType list namespaceType determines list space
* @param lists array of ExceptionIdentifiers for all lists to fetch
* @param onError error callback
* @param onSuccess callback when all lists fetched successfully
* @param filterOptions optional - filter by fields or tags
* @param pagination optional
*
Expand All @@ -43,7 +43,7 @@ export const useExceptionList = ({
tags: [],
},
onError,
dispatchListsInReducer,
onSuccess,
}: UseExceptionListProps): ReturnExceptionListAndItems => {
const [exceptionLists, setExceptionLists] = useState<ExceptionList[]>([]);
const [exceptionItems, setExceptionListItems] = useState<ExceptionListItemSchema[]>([]);
Expand Down Expand Up @@ -116,8 +116,8 @@ export const useExceptionList = ({
exceptions = [...exceptions, ...fetchListItemsResult.data];
setExceptionListItems(exceptions);

if (dispatchListsInReducer != null) {
dispatchListsInReducer({
if (onSuccess != null) {
onSuccess({
exceptions,
lists: exceptionListsReturned,
pagination: {
Expand Down
Loading

0 comments on commit 2861f99

Please sign in to comment.