Skip to content

Commit

Permalink
[SIEM][Detections] Value Lists Management Modal (#67068)
Browse files Browse the repository at this point in the history
* Add Frontend components for Value Lists Management Modal

Imports and uses the hooks provided by the lists plugin. Tests coming
next.

* Update value list components to use newest Lists API

* uses useEffect on a task's state instead of promise chaining
* handles the fact that API calls can be rejected with strings
* uses exportList function instead of hook

* Close modal on outside click

* Add hook for using a cursor with paged API calls.

For e.g. findLists, we can send along a cursor to optimize our query. On
the backend, this cursor is used as part of a search_after query.

* Better implementation of useCursor

* Does not require args for setCursor as they're already passed to the
hook
* Finds nearest cursor for the same page size

Eventually this logic will also include sortField as part of the
hash/lookup, but we do not currently use that on the frontend.

* Fixes useCursor hook functionality

We were previously storing the cursor on the _current_ page, when it's
only truly valid for the _next_ page (and beyond).

This was causing a few issues, but now that it's fixed everything works
great.

* Add cursor to lists query

This allows us to search_after a previous page's search, if available.

* Do not validate response of export

This is just a blob, so we have nothing to validate.

* Fix double callback post-import

After uploading a list, the modal was being shown twice. Declaring the
constituent state dependencies separately fixed the issue.

* Update ValueListsForm to manually abort import request

These hooks no longer care about/expose an abort function. In this one
case where we need that functionality, we can do it ourselves relatively
simply.

* Default modal table to five rows

* Update translation keys following plugin rename

* Try to fit table contents on a single row

Dates were wrapping (and raw), and so were wrapped in a FormattedDate
component. However, since this component didn't wrap, we needed to
shrink/truncate the uploaded_by field as well as allow the fileName to
truncate.

* Add helper function to prevent tests from logging errors

enzymejs/enzyme#2073 seems to be an ongoing
issue, and causes components with useEffect to update after the test is
completed.

waitForUpdates ensures that updates have completed within an act()
before continuing on.

* Add jest tests for our form, table, and modal components

* Fix translation conflict

* Add more waitForUpdates to new overview page tests

Each of these logs a console.error without them.

* Fix bad merge resolution

That resulted in duplicate exports.

* Make cursor an optional parameter to findLists

This param is an optimization and not required for basic functionality.

* Tweaking Table column sizes

Makes actions column smaller, leaving more room for everything else.

* Fix bug where onSuccess is called upon pagination change

Because fetchLists changes when pagination does, and handleUploadSuccess
changes with fetchLists, our useEffect in Form was being fired on every
pagination change due to its onSuccess changing.

The solution in this instance is to remove fetchLists from
handleUploadSuccess's dependencies, as we merely want to invoke
fetchLists from it, not change our reference.

* Fix failing test

It looks like this broke because EuiTable's pagination changed from a
button to an anchor tag.

* Hide page size options on ValueLists modal table

These have style issues, and anything above 5 rows causes the modal to
scroll, so we're going to disable it for now.

* Update error callbacks now that we have Errors

We don't display the nice errors in the case of an ApiError right now,
but this is better than it was.

* Synchronize delete with the subsequent fetch

Our start() no longer resolves in a meaningful way, so we instead need
to perform the refetch in an effect watching the result of our delete.

* Cast our unknown error to an Error

useAsync generally does not know how what its tasks are going to be
rejected with, hence the unknown.

For these API calls we know that it will be an Error, but I don't
currently have a way to type that generally. For now, we'll cast it
where we use it.

* Import lists code from our new, standardized modules

Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
rylnd and elasticmachine committed Jul 14, 2020
1 parent 16dee4a commit 1f64c32
Show file tree
Hide file tree
Showing 22 changed files with 1,157 additions and 71 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/lists/common/shared_exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,5 @@ export {
entriesList,
namespaceType,
ExceptionListType,
Type,
} from './schemas';
118 changes: 118 additions & 0 deletions x-pack/plugins/lists/public/common/hooks/use_cursor.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/*
* 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 { UseCursorProps, useCursor } from './use_cursor';

describe('useCursor', () => {
it('returns undefined cursor if no values have been set', () => {
const { result } = renderHook((props: UseCursorProps) => useCursor(props), {
initialProps: { pageIndex: 0, pageSize: 0 },
});

expect(result.current[0]).toBeUndefined();
});

it('retrieves a cursor for the next page of a given page size', () => {
const { rerender, result } = renderHook((props: UseCursorProps) => useCursor(props), {
initialProps: { pageIndex: 0, pageSize: 0 },
});
rerender({ pageIndex: 1, pageSize: 1 });
act(() => {
result.current[1]('new_cursor');
});

expect(result.current[0]).toBeUndefined();

rerender({ pageIndex: 2, pageSize: 1 });
expect(result.current[0]).toEqual('new_cursor');
});

it('returns undefined cursor for an unknown search', () => {
const { rerender, result } = renderHook((props: UseCursorProps) => useCursor(props), {
initialProps: { pageIndex: 0, pageSize: 0 },
});
act(() => {
result.current[1]('new_cursor');
});

rerender({ pageIndex: 1, pageSize: 2 });
expect(result.current[0]).toBeUndefined();
});

it('remembers cursor through rerenders', () => {
const { rerender, result } = renderHook((props: UseCursorProps) => useCursor(props), {
initialProps: { pageIndex: 0, pageSize: 0 },
});

rerender({ pageIndex: 1, pageSize: 1 });
act(() => {
result.current[1]('new_cursor');
});

rerender({ pageIndex: 2, pageSize: 1 });
expect(result.current[0]).toEqual('new_cursor');

rerender({ pageIndex: 0, pageSize: 0 });
expect(result.current[0]).toBeUndefined();

rerender({ pageIndex: 2, pageSize: 1 });
expect(result.current[0]).toEqual('new_cursor');
});

it('remembers multiple cursors', () => {
const { rerender, result } = renderHook((props: UseCursorProps) => useCursor(props), {
initialProps: { pageIndex: 0, pageSize: 0 },
});

rerender({ pageIndex: 1, pageSize: 1 });
act(() => {
result.current[1]('new_cursor');
});
rerender({ pageIndex: 2, pageSize: 2 });
act(() => {
result.current[1]('another_cursor');
});

rerender({ pageIndex: 2, pageSize: 1 });
expect(result.current[0]).toEqual('new_cursor');

rerender({ pageIndex: 3, pageSize: 2 });
expect(result.current[0]).toEqual('another_cursor');
});

it('returns the "nearest" cursor for the given page size', () => {
const { rerender, result } = renderHook((props: UseCursorProps) => useCursor(props), {
initialProps: { pageIndex: 0, pageSize: 0 },
});

rerender({ pageIndex: 1, pageSize: 2 });
act(() => {
result.current[1]('cursor1');
});
rerender({ pageIndex: 2, pageSize: 2 });
act(() => {
result.current[1]('cursor2');
});
rerender({ pageIndex: 3, pageSize: 2 });
act(() => {
result.current[1]('cursor3');
});

rerender({ pageIndex: 2, pageSize: 2 });
expect(result.current[0]).toEqual('cursor1');

rerender({ pageIndex: 3, pageSize: 2 });
expect(result.current[0]).toEqual('cursor2');

rerender({ pageIndex: 4, pageSize: 2 });
expect(result.current[0]).toEqual('cursor3');

rerender({ pageIndex: 6, pageSize: 2 });
expect(result.current[0]).toEqual('cursor3');
});
});
43 changes: 43 additions & 0 deletions x-pack/plugins/lists/public/common/hooks/use_cursor.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* 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 { useCallback, useState } from 'react';

export interface UseCursorProps {
pageIndex: number;
pageSize: number;
}
type Cursor = string | undefined;
type SetCursor = (cursor: Cursor) => void;
type UseCursor = (props: UseCursorProps) => [Cursor, SetCursor];

const hash = (props: UseCursorProps): string => JSON.stringify(props);

export const useCursor: UseCursor = ({ pageIndex, pageSize }) => {
const [cache, setCache] = useState<Record<string, Cursor>>({});

const setCursor = useCallback<SetCursor>(
(cursor) => {
setCache({
...cache,
[hash({ pageIndex: pageIndex + 1, pageSize })]: cursor,
});
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[pageIndex, pageSize]
);

let cursor: Cursor;
for (let i = pageIndex; i >= 0; i--) {
const currentProps = { pageIndex: i, pageSize };
cursor = cache[hash(currentProps)];
if (cursor) {
break;
}
}

return [cursor, setCursor];
};
100 changes: 47 additions & 53 deletions x-pack/plugins/lists/public/lists/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ describe('Value Lists API', () => {
it('sends pagination as query parameters', async () => {
const abortCtrl = new AbortController();
await findLists({
cursor: 'cursor',
http: httpMock,
pageIndex: 1,
pageSize: 10,
Expand All @@ -123,14 +124,21 @@ describe('Value Lists API', () => {
expect(httpMock.fetch).toHaveBeenCalledWith(
'/api/lists/_find',
expect.objectContaining({
query: { page: 1, per_page: 10 },
query: {
cursor: 'cursor',
page: 1,
per_page: 10,
},
})
);
});

it('rejects with an error if request payload is invalid (and does not make API call)', async () => {
const abortCtrl = new AbortController();
const payload: ApiPayload<FindListsParams> = { pageIndex: 10, pageSize: 0 };
const payload: ApiPayload<FindListsParams> = {
pageIndex: 10,
pageSize: 0,
};

await expect(
findLists({
Expand All @@ -144,7 +152,10 @@ describe('Value Lists API', () => {

it('rejects with an error if response payload is invalid', async () => {
const abortCtrl = new AbortController();
const payload: ApiPayload<FindListsParams> = { pageIndex: 1, pageSize: 10 };
const payload: ApiPayload<FindListsParams> = {
pageIndex: 1,
pageSize: 10,
};
const badResponse = { ...getFoundListSchemaMock(), cursor: undefined };
httpMock.fetch.mockResolvedValue(badResponse);

Expand Down Expand Up @@ -269,7 +280,7 @@ describe('Value Lists API', () => {

describe('exportList', () => {
beforeEach(() => {
httpMock.fetch.mockResolvedValue(getListResponseMock());
httpMock.fetch.mockResolvedValue({});
});

it('POSTs to the export endpoint', async () => {
Expand Down Expand Up @@ -319,66 +330,49 @@ describe('Value Lists API', () => {
).rejects.toEqual(new Error('Invalid value "23" supplied to "list_id"'));
expect(httpMock.fetch).not.toHaveBeenCalled();
});
});

describe('readListIndex', () => {
beforeEach(() => {
httpMock.fetch.mockResolvedValue(getListItemIndexExistSchemaResponseMock());
});

it('rejects with an error if response payload is invalid', async () => {
it('GETs the list index', async () => {
const abortCtrl = new AbortController();
const payload: ApiPayload<ExportListParams> = {
listId: 'list-id',
};
const badResponse = { ...getListResponseMock(), id: undefined };
httpMock.fetch.mockResolvedValue(badResponse);
await readListIndex({
http: httpMock,
signal: abortCtrl.signal,
});

await expect(
exportList({
http: httpMock,
...payload,
signal: abortCtrl.signal,
expect(httpMock.fetch).toHaveBeenCalledWith(
'/api/lists/index',
expect.objectContaining({
method: 'GET',
})
).rejects.toEqual(new Error('Invalid value "undefined" supplied to "id"'));
);
});

describe('readListIndex', () => {
beforeEach(() => {
httpMock.fetch.mockResolvedValue(getListItemIndexExistSchemaResponseMock());
it('returns the response when valid', async () => {
const abortCtrl = new AbortController();
const result = await readListIndex({
http: httpMock,
signal: abortCtrl.signal,
});

it('GETs the list index', async () => {
const abortCtrl = new AbortController();
await readListIndex({
http: httpMock,
signal: abortCtrl.signal,
});

expect(httpMock.fetch).toHaveBeenCalledWith(
'/api/lists/index',
expect.objectContaining({
method: 'GET',
})
);
});
expect(result).toEqual(getListItemIndexExistSchemaResponseMock());
});

it('rejects with an error if response payload is invalid', async () => {
const abortCtrl = new AbortController();
const badResponse = { ...getListItemIndexExistSchemaResponseMock(), list_index: undefined };
httpMock.fetch.mockResolvedValue(badResponse);

it('returns the response when valid', async () => {
const abortCtrl = new AbortController();
const result = await readListIndex({
await expect(
readListIndex({
http: httpMock,
signal: abortCtrl.signal,
});

expect(result).toEqual(getListItemIndexExistSchemaResponseMock());
});

it('rejects with an error if response payload is invalid', async () => {
const abortCtrl = new AbortController();
const badResponse = { ...getListItemIndexExistSchemaResponseMock(), list_index: undefined };
httpMock.fetch.mockResolvedValue(badResponse);

await expect(
readListIndex({
http: httpMock,
signal: abortCtrl.signal,
})
).rejects.toEqual(new Error('Invalid value "undefined" supplied to "list_index"'));
});
})
).rejects.toEqual(new Error('Invalid value "undefined" supplied to "list_index"'));
});
});

Expand Down
7 changes: 4 additions & 3 deletions x-pack/plugins/lists/public/lists/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,17 @@ const findLists = async ({
};

const findListsWithValidation = async ({
cursor,
http,
pageIndex,
pageSize,
signal,
}: FindListsParams): Promise<FoundListSchema> =>
pipe(
{
page: String(pageIndex),
per_page: String(pageSize),
cursor: cursor?.toString(),
page: pageIndex?.toString(),
per_page: pageSize?.toString(),
},
(payload) => fromEither(validateEither(findListSchema, payload)),
chain((payload) => tryCatch(() => findLists({ http, signal, ...payload }), toError)),
Expand Down Expand Up @@ -170,7 +172,6 @@ const exportListWithValidation = async ({
{ list_id: listId },
(payload) => fromEither(validateEither(exportListItemQuerySchema, payload)),
chain((payload) => tryCatch(() => exportList({ http, signal, ...payload }), toError)),
chain((response) => fromEither(validateEither(listSchema, response))),
flow(toPromise)
);

Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/lists/public/lists/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export interface ApiParams {
export type ApiPayload<T extends ApiParams> = Omit<T, 'http' | 'signal'>;

export interface FindListsParams extends ApiParams {
cursor?: string | undefined;
pageSize: number | undefined;
pageIndex: number | undefined;
}
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/lists/public/shared_exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ export { useExceptionList } from './exceptions/hooks/use_exception_list';
export { useFindLists } from './lists/hooks/use_find_lists';
export { useImportList } from './lists/hooks/use_import_list';
export { useDeleteList } from './lists/hooks/use_delete_list';
export { exportList } from './lists/api';
export { useCursor } from './common/hooks/use_cursor';
export { useExportList } from './lists/hooks/use_export_list';
export { useReadListIndex } from './lists/hooks/use_read_list_index';
export { useCreateListIndex } from './lists/hooks/use_create_list_index';
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/security_solution/common/shared_imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,5 @@ export {
entriesList,
namespaceType,
ExceptionListType,
Type,
} from '../../lists/common';
Loading

0 comments on commit 1f64c32

Please sign in to comment.