Skip to content

Commit 62c7c75

Browse files
rylndelasticmachine
andcommitted
[Security Solution][Detections] Value Lists Modal supports multiple exports (#73532)
* Remove need for ValueListsTable Modifying columns has revealed that they should be exposed as props, at which point we have no real need for the table component. * Unroll the ActionButton component I thought this was useful when I wrote it! * Handle multiple simultaneous exports on value lists modal Instead of passing our export function to GenericDownloader, we now manage the multiple exports ourselves, and when successful we pass the blob to GenericDownloader. * tracks a list of exporting IDs instead of single ID * chains onto the export promise to set local state * Port useful table tests over to modal tests These verify that we've wired up our table actions to our API calls. A little brittle/tied to implementation, but I'd rather have them than not. * WIP: Simpler version of GenericDownloader * Replace use of GenericDownloader with simpler AutoDownload This component takes a blob and downloads it in a cross-browser-compatible manner. * Handle error when uploading value lists Converts to the try/catch/finally form as well. * Fix failing cypress test We lost this test subj during our refactor, oops * More explicit setting of global DOM function Our component fails due to this method being undefined, so we mock it out for these tests. We do not need to reset the mock as it is assigned fresh on every test. * Fixes jest failures on CI Defines a global static method in a more portable way, as the regular assignment was failing on CI as the property was readonly. * Simplify our export/delete clicks in jest tests The less we assume about the UI, the more robust these'll be. Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
1 parent 6ad54e4 commit 62c7c75

File tree

8 files changed

+220
-246
lines changed

8 files changed

+220
-246
lines changed
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import React from 'react';
8+
import { mount } from 'enzyme';
9+
10+
import { globalNode } from '../../../common/mock';
11+
import { AutoDownload } from './auto_download';
12+
13+
describe('AutoDownload', () => {
14+
beforeEach(() => {
15+
// our DOM environment lacks this function that our component needs
16+
Object.defineProperty(globalNode.window.URL, 'revokeObjectURL', {
17+
writable: true,
18+
value: jest.fn(),
19+
});
20+
});
21+
22+
it('calls onDownload once if a blob is provided', () => {
23+
const onDownload = jest.fn();
24+
mount(<AutoDownload blob={new Blob([''])} onDownload={onDownload} />);
25+
26+
expect(onDownload).toHaveBeenCalledTimes(1);
27+
});
28+
29+
it('does not call onDownload if no blob is provided', () => {
30+
const onDownload = jest.fn();
31+
mount(<AutoDownload blob={undefined} onDownload={onDownload} />);
32+
33+
expect(onDownload).not.toHaveBeenCalled();
34+
});
35+
});
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import React, { useEffect, useRef } from 'react';
8+
import styled from 'styled-components';
9+
10+
const InvisibleAnchor = styled.a`
11+
display: none;
12+
`;
13+
14+
interface AutoDownloadProps {
15+
blob: Blob | undefined;
16+
name?: string;
17+
onDownload?: () => void;
18+
}
19+
20+
export const AutoDownload: React.FC<AutoDownloadProps> = ({ blob, name, onDownload }) => {
21+
const anchorRef = useRef<HTMLAnchorElement>(null);
22+
23+
useEffect(() => {
24+
if (blob && anchorRef?.current) {
25+
if (typeof window.navigator.msSaveOrOpenBlob === 'function') {
26+
window.navigator.msSaveBlob(blob);
27+
} else {
28+
const objectURL = window.URL.createObjectURL(blob);
29+
anchorRef.current.href = objectURL;
30+
anchorRef.current.download = name ?? 'download.txt';
31+
anchorRef.current.click();
32+
window.URL.revokeObjectURL(objectURL);
33+
}
34+
35+
if (onDownload) {
36+
onDownload();
37+
}
38+
}
39+
}, [blob, name, onDownload]);
40+
41+
return <InvisibleAnchor ref={anchorRef} />;
42+
};

x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.test.tsx

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,38 @@
66

77
import React from 'react';
88
import { mount } from 'enzyme';
9+
import { act } from 'react-dom/test-utils';
910

11+
import { getListResponseMock } from '../../../../../lists/common/schemas/response/list_schema.mock';
12+
import { exportList, useDeleteList, useFindLists, ListSchema } from '../../../shared_imports';
1013
import { TestProviders } from '../../../common/mock';
1114
import { ValueListsModal } from './modal';
1215

16+
jest.mock('../../../shared_imports', () => {
17+
const actual = jest.requireActual('../../../shared_imports');
18+
19+
return {
20+
...actual,
21+
exportList: jest.fn(),
22+
useDeleteList: jest.fn(),
23+
useFindLists: jest.fn(),
24+
};
25+
});
26+
1327
describe('ValueListsModal', () => {
28+
beforeEach(() => {
29+
// Do not resolve the export in tests as it causes unexpected state updates
30+
(exportList as jest.Mock).mockImplementation(() => new Promise(() => {}));
31+
(useFindLists as jest.Mock).mockReturnValue({
32+
start: jest.fn(),
33+
result: { data: Array<ListSchema>(3).fill(getListResponseMock()), total: 3 },
34+
});
35+
(useDeleteList as jest.Mock).mockReturnValue({
36+
start: jest.fn(),
37+
result: getListResponseMock(),
38+
});
39+
});
40+
1441
it('renders nothing if showModal is false', () => {
1542
const container = mount(
1643
<TestProviders>
@@ -47,15 +74,58 @@ describe('ValueListsModal', () => {
4774
container.unmount();
4875
});
4976

50-
it('renders ValueListsForm and ValueListsTable', () => {
77+
it('renders ValueListsForm and an EuiTable', () => {
5178
const container = mount(
5279
<TestProviders>
5380
<ValueListsModal showModal={true} onClose={jest.fn()} />
5481
</TestProviders>
5582
);
5683

5784
expect(container.find('ValueListsForm')).toHaveLength(1);
58-
expect(container.find('ValueListsTable')).toHaveLength(1);
85+
expect(container.find('EuiBasicTable')).toHaveLength(1);
5986
container.unmount();
6087
});
88+
89+
describe('modal table actions', () => {
90+
it('calls exportList when export is clicked', () => {
91+
const container = mount(
92+
<TestProviders>
93+
<ValueListsModal showModal={true} onClose={jest.fn()} />
94+
</TestProviders>
95+
);
96+
97+
act(() => {
98+
container
99+
.find('button[data-test-subj="action-export-value-list"]')
100+
.first()
101+
.simulate('click');
102+
container.unmount();
103+
});
104+
105+
expect(exportList).toHaveBeenCalledWith(expect.objectContaining({ listId: 'some-list-id' }));
106+
});
107+
108+
it('calls deleteList when delete is clicked', () => {
109+
const deleteListMock = jest.fn();
110+
(useDeleteList as jest.Mock).mockReturnValue({
111+
start: deleteListMock,
112+
result: getListResponseMock(),
113+
});
114+
const container = mount(
115+
<TestProviders>
116+
<ValueListsModal showModal={true} onClose={jest.fn()} />
117+
</TestProviders>
118+
);
119+
120+
act(() => {
121+
container
122+
.find('button[data-test-subj="action-delete-value-list"]')
123+
.first()
124+
.simulate('click');
125+
container.unmount();
126+
});
127+
128+
expect(deleteListMock).toHaveBeenCalledWith(expect.objectContaining({ id: 'some-list-id' }));
129+
});
130+
});
61131
});

x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx

Lines changed: 41 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,17 @@
66

77
import React, { useCallback, useEffect, useState } from 'react';
88
import {
9+
EuiBasicTable,
910
EuiButton,
1011
EuiModal,
1112
EuiModalBody,
1213
EuiModalFooter,
1314
EuiModalHeader,
1415
EuiModalHeaderTitle,
1516
EuiOverlayMask,
17+
EuiPanel,
1618
EuiSpacer,
19+
EuiText,
1720
} from '@elastic/eui';
1821

1922
import {
@@ -25,10 +28,10 @@ import {
2528
} from '../../../shared_imports';
2629
import { useKibana } from '../../../common/lib/kibana';
2730
import { useAppToasts } from '../../../common/hooks/use_app_toasts';
28-
import { GenericDownloader } from '../../../common/components/generic_downloader';
2931
import * as i18n from './translations';
30-
import { ValueListsTable } from './table';
32+
import { buildColumns } from './table_helpers';
3133
import { ValueListsForm } from './form';
34+
import { AutoDownload } from './auto_download';
3235

3336
interface ValueListsModalProps {
3437
onClose: () => void;
@@ -45,8 +48,9 @@ export const ValueListsModalComponent: React.FC<ValueListsModalProps> = ({
4548
const { http } = useKibana().services;
4649
const { start: findLists, ...lists } = useFindLists();
4750
const { start: deleteList, result: deleteResult } = useDeleteList();
48-
const [exportListId, setExportListId] = useState<string>();
4951
const [deletingListIds, setDeletingListIds] = useState<string[]>([]);
52+
const [exportingListIds, setExportingListIds] = useState<string[]>([]);
53+
const [exportDownload, setExportDownload] = useState<{ name?: string; blob?: Blob }>({});
5054
const { addError, addSuccess } = useAppToasts();
5155

5256
const fetchLists = useCallback(() => {
@@ -62,19 +66,26 @@ export const ValueListsModalComponent: React.FC<ValueListsModalProps> = ({
6266
);
6367

6468
useEffect(() => {
65-
if (deleteResult != null && deletingListIds.length > 0) {
66-
setDeletingListIds([...deletingListIds.filter((id) => id !== deleteResult.id)]);
69+
if (deleteResult != null) {
70+
setDeletingListIds((ids) => [...ids.filter((id) => id !== deleteResult.id)]);
6771
fetchLists();
6872
}
69-
}, [deleteResult, deletingListIds, fetchLists]);
73+
}, [deleteResult, fetchLists]);
7074

7175
const handleExport = useCallback(
72-
async ({ ids }: { ids: string[] }) =>
73-
exportList({ http, listId: ids[0], signal: new AbortController().signal }),
74-
[http]
76+
async ({ id }: { id: string }) => {
77+
try {
78+
setExportingListIds((ids) => [...ids, id]);
79+
const blob = await exportList({ http, listId: id, signal: new AbortController().signal });
80+
setExportDownload({ name: id, blob });
81+
} catch (error) {
82+
addError(error, { title: i18n.EXPORT_ERROR });
83+
} finally {
84+
setExportingListIds((ids) => [...ids.filter((_id) => _id !== id)]);
85+
}
86+
},
87+
[addError, http]
7588
);
76-
const handleExportClick = useCallback(({ id }: { id: string }) => setExportListId(id), []);
77-
const handleExportComplete = useCallback(() => setExportListId(undefined), []);
7889

7990
const handleTableChange = useCallback(
8091
({ page: { index, size } }: { page: { index: number; size: number } }) => {
@@ -121,8 +132,8 @@ export const ValueListsModalComponent: React.FC<ValueListsModalProps> = ({
121132

122133
const tableItems = (lists.result?.data ?? []).map((item) => ({
123134
...item,
124-
isExporting: item.id === exportListId,
125135
isDeleting: deletingListIds.includes(item.id),
136+
isExporting: exportingListIds.includes(item.id),
126137
}));
127138

128139
const pagination = {
@@ -131,6 +142,7 @@ export const ValueListsModalComponent: React.FC<ValueListsModalProps> = ({
131142
totalItemCount: lists.result?.total ?? 0,
132143
hidePerPageOptions: true,
133144
};
145+
const columns = buildColumns(handleExport, handleDelete);
134146

135147
return (
136148
<EuiOverlayMask onClick={onClose}>
@@ -141,27 +153,30 @@ export const ValueListsModalComponent: React.FC<ValueListsModalProps> = ({
141153
<EuiModalBody>
142154
<ValueListsForm onSuccess={handleUploadSuccess} onError={handleUploadError} />
143155
<EuiSpacer />
144-
<ValueListsTable
145-
items={tableItems}
146-
loading={lists.loading}
147-
onDelete={handleDelete}
148-
onExport={handleExportClick}
149-
onChange={handleTableChange}
150-
pagination={pagination}
151-
/>
156+
<EuiPanel>
157+
<EuiText size="s">
158+
<h2>{i18n.TABLE_TITLE}</h2>
159+
</EuiText>
160+
<EuiBasicTable
161+
data-test-subj="value-lists-table"
162+
columns={columns}
163+
items={tableItems}
164+
loading={lists.loading}
165+
onChange={handleTableChange}
166+
pagination={pagination}
167+
/>
168+
</EuiPanel>
152169
</EuiModalBody>
153170
<EuiModalFooter>
154171
<EuiButton data-test-subj="value-lists-modal-close-action" onClick={onClose}>
155172
{i18n.CLOSE_BUTTON}
156173
</EuiButton>
157174
</EuiModalFooter>
158175
</EuiModal>
159-
<GenericDownloader
160-
filename={exportListId ?? 'download.txt'}
161-
ids={exportListId != null ? [exportListId] : undefined}
162-
onExportSuccess={handleExportComplete}
163-
onExportFailure={handleExportComplete}
164-
exportSelectedData={handleExport}
176+
<AutoDownload
177+
blob={exportDownload.blob}
178+
name={exportDownload.name}
179+
onDownload={() => setExportDownload({})}
165180
/>
166181
</EuiOverlayMask>
167182
);

0 commit comments

Comments
 (0)