From 0ebcd17ed4907df7d5b700e3565628a124bc0788 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 28 Jul 2020 01:59:03 -0500 Subject: [PATCH 01/11] 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. --- .../value_lists_management_modal/modal.tsx | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx index dc72260439090..f8caeae995d62 100644 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx @@ -6,6 +6,7 @@ import React, { useCallback, useEffect, useState } from 'react'; import { + EuiBasicTable, EuiButton, EuiModal, EuiModalBody, @@ -13,7 +14,9 @@ import { EuiModalHeader, EuiModalHeaderTitle, EuiOverlayMask, + EuiPanel, EuiSpacer, + EuiText, } from '@elastic/eui'; import { @@ -27,7 +30,7 @@ import { useKibana } from '../../../common/lib/kibana'; import { useAppToasts } from '../../../common/hooks/use_app_toasts'; import { GenericDownloader } from '../../../common/components/generic_downloader'; import * as i18n from './translations'; -import { ValueListsTable } from './table'; +import { buildColumns } from './table_helpers'; import { ValueListsForm } from './form'; interface ValueListsModalProps { @@ -131,6 +134,7 @@ export const ValueListsModalComponent: React.FC = ({ totalItemCount: lists.result?.total ?? 0, hidePerPageOptions: true, }; + const columns = buildColumns(handleExportClick, handleDelete); return ( @@ -141,14 +145,18 @@ export const ValueListsModalComponent: React.FC = ({ - + + +

{i18n.TABLE_TITLE}

+ + + From df3385d0995d5f3c2c973da6c3d04506bf6558e5 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 28 Jul 2020 02:22:42 -0500 Subject: [PATCH 02/11] Unroll the ActionButton component I thought this was useful when I wrote it! --- .../table_helpers.tsx | 66 ++++++++----------- 1 file changed, 26 insertions(+), 40 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/table_helpers.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/table_helpers.tsx index e90d106636e63..bb3a97749a11a 100644 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/table_helpers.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/table_helpers.tsx @@ -8,40 +8,18 @@ import React from 'react'; import styled from 'styled-components'; -import { EuiButtonIcon, IconType, EuiLoadingSpinner, EuiToolTip } from '@elastic/eui'; +import { EuiButtonIcon, EuiLoadingSpinner, EuiToolTip } from '@elastic/eui'; import { ListSchema } from '../../../../../lists/common/schemas/response'; import { FormattedDate } from '../../../common/components/formatted_date'; import * as i18n from './translations'; -import { TableItem, TableItemCallback, TableProps } from './types'; +import { TableItemCallback, TableProps } from './types'; const AlignedSpinner = styled(EuiLoadingSpinner)` margin: ${({ theme }) => theme.eui.euiSizeXS}; vertical-align: middle; `; -const ActionButton: React.FC<{ - content: string; - dataTestSubj: string; - icon: IconType; - isLoading: boolean; - item: TableItem; - onClick: TableItemCallback; -}> = ({ content, dataTestSubj, icon, item, onClick, isLoading }) => ( - - {isLoading ? ( - - ) : ( - onClick(item)} - /> - )} - -); - export const buildColumns = ( onExport: TableItemCallback, onDelete: TableItemCallback @@ -70,26 +48,34 @@ export const buildColumns = ( actions: [ { render: (item) => ( - + + {item.isExporting ? ( + + ) : ( + onExport(item)} + /> + )} + ), }, { render: (item) => ( - + + {item.isDeleting ? ( + + ) : ( + onDelete(item)} + /> + )} + ), }, ], From 15a777690e12bdfdac723997475c7d890acaae82 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 28 Jul 2020 12:18:32 -0500 Subject: [PATCH 03/11] 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 --- .../value_lists_management_modal/modal.tsx | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx index f8caeae995d62..bee0c00445c6f 100644 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx @@ -48,8 +48,9 @@ export const ValueListsModalComponent: React.FC = ({ const { http } = useKibana().services; const { start: findLists, ...lists } = useFindLists(); const { start: deleteList, result: deleteResult } = useDeleteList(); - const [exportListId, setExportListId] = useState(); const [deletingListIds, setDeletingListIds] = useState([]); + const [exportingListIds, setExportingListIds] = useState([]); + const [exportDownload, setExportDownload] = useState<{ name?: string; blob?: Blob }>({}); const { addError, addSuccess } = useAppToasts(); const fetchLists = useCallback(() => { @@ -65,19 +66,23 @@ export const ValueListsModalComponent: React.FC = ({ ); useEffect(() => { - if (deleteResult != null && deletingListIds.length > 0) { - setDeletingListIds([...deletingListIds.filter((id) => id !== deleteResult.id)]); + if (deleteResult != null) { + setDeletingListIds((ids) => [...ids.filter((id) => id !== deleteResult.id)]); fetchLists(); } - }, [deleteResult, deletingListIds, fetchLists]); + }, [deleteResult, fetchLists]); const handleExport = useCallback( - async ({ ids }: { ids: string[] }) => - exportList({ http, listId: ids[0], signal: new AbortController().signal }), + ({ id }: { id: string }) => { + setExportingListIds((ids) => [...ids, id]); + exportList({ http, listId: id, signal: new AbortController().signal }) + .then((blob) => { + setExportDownload({ name: id, blob }); + }) + .finally(() => setExportingListIds((ids) => [...ids.filter((_id) => _id !== id)])); + }, [http] ); - const handleExportClick = useCallback(({ id }: { id: string }) => setExportListId(id), []); - const handleExportComplete = useCallback(() => setExportListId(undefined), []); const handleTableChange = useCallback( ({ page: { index, size } }: { page: { index: number; size: number } }) => { @@ -124,8 +129,8 @@ export const ValueListsModalComponent: React.FC = ({ const tableItems = (lists.result?.data ?? []).map((item) => ({ ...item, - isExporting: item.id === exportListId, isDeleting: deletingListIds.includes(item.id), + isExporting: exportingListIds.includes(item.id), })); const pagination = { @@ -134,7 +139,7 @@ export const ValueListsModalComponent: React.FC = ({ totalItemCount: lists.result?.total ?? 0, hidePerPageOptions: true, }; - const columns = buildColumns(handleExportClick, handleDelete); + const columns = buildColumns(handleExport, handleDelete); return ( @@ -164,13 +169,15 @@ export const ValueListsModalComponent: React.FC = ({ - + {exportDownload.name && exportDownload.blob && ( + setExportDownload({})} + onExportFailure={() => setExportDownload({})} + exportSelectedData={async () => exportDownload.blob!} + /> + )} ); }; From 6c24872dbb3e33f23282329083d63229c9c13b6d Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 28 Jul 2020 13:38:26 -0500 Subject: [PATCH 04/11] 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. --- .../modal.test.tsx | 76 ++++++++++- .../table.test.tsx | 125 ------------------ .../value_lists_management_modal/table.tsx | 53 -------- 3 files changed, 74 insertions(+), 180 deletions(-) delete mode 100644 x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/table.test.tsx delete mode 100644 x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/table.tsx diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.test.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.test.tsx index 175882de551cb..89a86e4f0eac9 100644 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.test.tsx @@ -6,11 +6,38 @@ import React from 'react'; import { mount } from 'enzyme'; +import { act } from 'react-dom/test-utils'; +import { getListResponseMock } from '../../../../../lists/common/schemas/response/list_schema.mock'; +import { exportList, useDeleteList, useFindLists, ListSchema } from '../../../shared_imports'; import { TestProviders } from '../../../common/mock'; import { ValueListsModal } from './modal'; +jest.mock('../../../shared_imports', () => { + const actual = jest.requireActual('../../../shared_imports'); + + return { + ...actual, + exportList: jest.fn(), + useDeleteList: jest.fn(), + useFindLists: jest.fn(), + }; +}); + describe('ValueListsModal', () => { + beforeEach(() => { + // Do not resolve the export in tests as it causes unexpected state updates + (exportList as jest.Mock).mockImplementation(() => new Promise(() => {})); + (useFindLists as jest.Mock).mockReturnValue({ + start: jest.fn(), + result: { data: Array(3).fill(getListResponseMock()), total: 3 }, + }); + (useDeleteList as jest.Mock).mockReturnValue({ + start: jest.fn(), + result: getListResponseMock(), + }); + }); + it('renders nothing if showModal is false', () => { const container = mount( @@ -47,7 +74,7 @@ describe('ValueListsModal', () => { container.unmount(); }); - it('renders ValueListsForm and ValueListsTable', () => { + it('renders ValueListsForm and an EuiTable', () => { const container = mount( @@ -55,7 +82,52 @@ describe('ValueListsModal', () => { ); expect(container.find('ValueListsForm')).toHaveLength(1); - expect(container.find('ValueListsTable')).toHaveLength(1); + expect(container.find('EuiBasicTable')).toHaveLength(1); container.unmount(); }); + + describe('modal table actions', () => { + it('calls exportList when export is clicked', () => { + const container = mount( + + + + ); + + act(() => { + container + .find('tbody tr') + .first() + .find('button[data-test-subj="action-export-value-list"]') + .simulate('click'); + container.unmount(); + }); + + expect(exportList).toHaveBeenCalledWith(expect.objectContaining({ listId: 'some-list-id' })); + }); + + it('calls deleteList when delete is clicked', () => { + const deleteListMock = jest.fn(); + (useDeleteList as jest.Mock).mockReturnValue({ + start: deleteListMock, + result: getListResponseMock(), + }); + const container = mount( + + + + ); + + act(() => { + container + .find('tbody tr') + .first() + .find('button[data-test-subj="action-delete-value-list"]') + .simulate('click'); + container.unmount(); + }); + + expect(deleteListMock).toHaveBeenCalledWith(expect.objectContaining({ id: 'some-list-id' })); + }); + }); }); diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/table.test.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/table.test.tsx deleted file mode 100644 index 2724c0a0696b6..0000000000000 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/table.test.tsx +++ /dev/null @@ -1,125 +0,0 @@ -/* - * 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 React from 'react'; -import { mount } from 'enzyme'; -import { act } from 'react-dom/test-utils'; - -import { getListResponseMock } from '../../../../../lists/common/schemas/response/list_schema.mock'; -import { ListSchema } from '../../../../../lists/common/schemas/response'; -import { TestProviders } from '../../../common/mock'; -import { ValueListsTable } from './table'; -import { TableItem } from './types'; - -const buildItems = (lists: ListSchema[]): TableItem[] => - lists.map((list) => ({ - ...list, - isDeleting: false, - isExporting: false, - })); - -describe('ValueListsTable', () => { - it('renders a row for each list', () => { - const lists = Array(3).fill(getListResponseMock()); - const items = buildItems(lists); - const container = mount( - - - - ); - - expect(container.find('tbody tr')).toHaveLength(3); - }); - - it('calls onChange when pagination is modified', () => { - const lists = Array(6).fill(getListResponseMock()); - const items = buildItems(lists); - const onChange = jest.fn(); - const container = mount( - - - - ); - - act(() => { - container.find('a[data-test-subj="pagination-button-next"]').simulate('click'); - }); - - expect(onChange).toHaveBeenCalledWith( - expect.objectContaining({ page: expect.objectContaining({ index: 1 }) }) - ); - }); - - it('calls onExport when export is clicked', () => { - const lists = Array(3).fill(getListResponseMock()); - const items = buildItems(lists); - const onExport = jest.fn(); - const container = mount( - - - - ); - - act(() => { - container - .find('tbody tr') - .first() - .find('button[data-test-subj="action-export-value-list"]') - .simulate('click'); - }); - - expect(onExport).toHaveBeenCalledWith(expect.objectContaining({ id: 'some-list-id' })); - }); - - it('calls onDelete when delete is clicked', () => { - const lists = Array(3).fill(getListResponseMock()); - const items = buildItems(lists); - const onDelete = jest.fn(); - const container = mount( - - - - ); - - act(() => { - container - .find('tbody tr') - .first() - .find('button[data-test-subj="action-delete-value-list"]') - .simulate('click'); - }); - - expect(onDelete).toHaveBeenCalledWith(expect.objectContaining({ id: 'some-list-id' })); - }); -}); diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/table.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/table.tsx deleted file mode 100644 index a2e3b73a0abf0..0000000000000 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/table.tsx +++ /dev/null @@ -1,53 +0,0 @@ -/* - * 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 React from 'react'; -import { EuiBasicTable, EuiText, EuiPanel } from '@elastic/eui'; - -import * as i18n from './translations'; -import { buildColumns } from './table_helpers'; -import { TableProps, TableItemCallback } from './types'; - -export interface ValueListsTableProps { - items: TableProps['items']; - loading: boolean; - onChange: TableProps['onChange']; - onExport: TableItemCallback; - onDelete: TableItemCallback; - pagination: Exclude; -} - -export const ValueListsTableComponent: React.FC = ({ - items, - loading, - onChange, - onExport, - onDelete, - pagination, -}) => { - const columns = buildColumns(onExport, onDelete); - return ( - - -

{i18n.TABLE_TITLE}

- - - - ); -}; - -ValueListsTableComponent.displayName = 'ValueListsTableComponent'; - -export const ValueListsTable = React.memo(ValueListsTableComponent); - -ValueListsTable.displayName = 'ValueListsTable'; From 00140d0c720e8b216c2b9e21d621d91baba8bb4a Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 28 Jul 2020 13:40:02 -0500 Subject: [PATCH 05/11] WIP: Simpler version of GenericDownloader --- .../auto_download.tsx | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/auto_download.tsx diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/auto_download.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/auto_download.tsx new file mode 100644 index 0000000000000..9c8280bebe4fd --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/auto_download.tsx @@ -0,0 +1,42 @@ +/* + * 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 React, { useEffect, useRef } from 'react'; +import styled from 'styled-components'; + +const InvisibleAnchor = styled.a` + display: none; +`; + +interface AutoDownloadProps { + blob: Blob | undefined; + name?: string; + onDownload?: () => void; +} + +export const AutoDownload: React.FC = ({ blob, name, onDownload }) => { + const anchorRef = useRef(null); + + useEffect(() => { + if (blob && anchorRef?.current) { + if (typeof window.navigator.msSaveOrOpenBlob === 'function') { + window.navigator.msSaveBlob(blob); + } else { + const objectURL = window.URL.createObjectURL(blob); + anchorRef.current.href = objectURL; + anchorRef.current.download = name ?? 'download.txt'; + anchorRef.current.click(); + window.URL.revokeObjectURL(objectURL); + } + + if (onDownload) { + onDownload(); + } + } + }, [blob, name, onDownload]); + + return ; +}; From 7b817740680113a7630912d189950b7d78d2101c Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 28 Jul 2020 19:01:36 -0500 Subject: [PATCH 06/11] Replace use of GenericDownloader with simpler AutoDownload This component takes a blob and downloads it in a cross-browser-compatible manner. --- .../auto_download.test.tsx | 34 +++++++++++++++++++ .../value_lists_management_modal/modal.tsx | 16 ++++----- 2 files changed, 40 insertions(+), 10 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/auto_download.test.tsx diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/auto_download.test.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/auto_download.test.tsx new file mode 100644 index 0000000000000..16863ce3a30df --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/auto_download.test.tsx @@ -0,0 +1,34 @@ +/* + * 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 React from 'react'; +import { mount } from 'enzyme'; + +import { AutoDownload } from './auto_download'; + +describe('AutoDownload', () => { + beforeEach(() => { + window.URL.revokeObjectURL = jest.fn(); + }); + + afterEach(() => { + (window.URL.revokeObjectURL as jest.Mock).mockReset(); + }); + + it('calls onDownload once if a blob is provided', () => { + const onDownload = jest.fn(); + mount(); + + expect(onDownload).toHaveBeenCalledTimes(1); + }); + + it('does not call onDownload if no blob is provided', () => { + const onDownload = jest.fn(); + mount(); + + expect(onDownload).not.toHaveBeenCalled(); + }); +}); diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx index bee0c00445c6f..cf71029389210 100644 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx @@ -28,10 +28,10 @@ import { } from '../../../shared_imports'; import { useKibana } from '../../../common/lib/kibana'; import { useAppToasts } from '../../../common/hooks/use_app_toasts'; -import { GenericDownloader } from '../../../common/components/generic_downloader'; import * as i18n from './translations'; import { buildColumns } from './table_helpers'; import { ValueListsForm } from './form'; +import { AutoDownload } from './auto_download'; interface ValueListsModalProps { onClose: () => void; @@ -169,15 +169,11 @@ export const ValueListsModalComponent: React.FC = ({ - {exportDownload.name && exportDownload.blob && ( - setExportDownload({})} - onExportFailure={() => setExportDownload({})} - exportSelectedData={async () => exportDownload.blob!} - /> - )} + setExportDownload({})} + /> ); }; From 1a7f82b0b7ac988a6b0490f6e8c51a9f20adff3d Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 28 Jul 2020 19:10:40 -0500 Subject: [PATCH 07/11] Handle error when uploading value lists Converts to the try/catch/finally form as well. --- .../value_lists_management_modal/modal.tsx | 19 +++++++++++-------- .../translations.ts | 4 ++++ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx index cf71029389210..b6af28dfc4006 100644 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx @@ -73,15 +73,18 @@ export const ValueListsModalComponent: React.FC = ({ }, [deleteResult, fetchLists]); const handleExport = useCallback( - ({ id }: { id: string }) => { - setExportingListIds((ids) => [...ids, id]); - exportList({ http, listId: id, signal: new AbortController().signal }) - .then((blob) => { - setExportDownload({ name: id, blob }); - }) - .finally(() => setExportingListIds((ids) => [...ids.filter((_id) => _id !== id)])); + async ({ id }: { id: string }) => { + try { + setExportingListIds((ids) => [...ids, id]); + const blob = await exportList({ http, listId: id, signal: new AbortController().signal }); + setExportDownload({ name: id, blob }); + } catch (error) { + addError(error, { title: i18n.EXPORT_ERROR }); + } finally { + setExportingListIds((ids) => [...ids.filter((_id) => _id !== id)]); + } }, - [http] + [addError, http] ); const handleTableChange = useCallback( diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/translations.ts b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/translations.ts index b7b2cae7b0ad6..7063dca2341ca 100644 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/translations.ts +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/translations.ts @@ -65,6 +65,10 @@ export const uploadSuccessMessage = (fileName: string) => values: { fileName }, }); +export const EXPORT_ERROR = i18n.translate('xpack.securitySolution.lists.valueListsExportError', { + defaultMessage: 'There was an error exporting the value list.', +}); + export const COLUMN_FILE_NAME = i18n.translate( 'xpack.securitySolution.lists.valueListsTable.fileNameColumn', { From 4191176553d0ccf17cc54b6b3fe21219eb42bf17 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 28 Jul 2020 19:19:35 -0500 Subject: [PATCH 08/11] Fix failing cypress test We lost this test subj during our refactor, oops --- .../detections/components/value_lists_management_modal/modal.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx index b6af28dfc4006..4921a98b38bd1 100644 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx @@ -158,6 +158,7 @@ export const ValueListsModalComponent: React.FC = ({

{i18n.TABLE_TITLE}

Date: Tue, 28 Jul 2020 19:44:50 -0500 Subject: [PATCH 09/11] 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. --- .../value_lists_management_modal/auto_download.test.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/auto_download.test.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/auto_download.test.tsx index 16863ce3a30df..f45b572f28bee 100644 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/auto_download.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/auto_download.test.tsx @@ -11,13 +11,10 @@ import { AutoDownload } from './auto_download'; describe('AutoDownload', () => { beforeEach(() => { + // our DOM environment lacks this function that our component needs window.URL.revokeObjectURL = jest.fn(); }); - afterEach(() => { - (window.URL.revokeObjectURL as jest.Mock).mockReset(); - }); - it('calls onDownload once if a blob is provided', () => { const onDownload = jest.fn(); mount(); From 65f3953ae99ed20ef0d9050989c469c88b34831e Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 28 Jul 2020 21:17:11 -0500 Subject: [PATCH 10/11] 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. --- .../value_lists_management_modal/auto_download.test.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/auto_download.test.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/auto_download.test.tsx index f45b572f28bee..53dcf986d395c 100644 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/auto_download.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/auto_download.test.tsx @@ -7,12 +7,16 @@ import React from 'react'; import { mount } from 'enzyme'; +import { globalNode } from '../../../common/mock'; import { AutoDownload } from './auto_download'; describe('AutoDownload', () => { beforeEach(() => { // our DOM environment lacks this function that our component needs - window.URL.revokeObjectURL = jest.fn(); + Object.defineProperty(globalNode.window.URL, 'revokeObjectURL', { + writable: true, + value: jest.fn(), + }); }); it('calls onDownload once if a blob is provided', () => { From 3653c465c00d10afebad0d6edacf559a96f0fb9c Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 28 Jul 2020 21:20:39 -0500 Subject: [PATCH 11/11] Simplify our export/delete clicks in jest tests The less we assume about the UI, the more robust these'll be. --- .../components/value_lists_management_modal/modal.test.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.test.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.test.tsx index 89a86e4f0eac9..ff743d1d5090a 100644 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.test.tsx @@ -96,9 +96,8 @@ describe('ValueListsModal', () => { act(() => { container - .find('tbody tr') - .first() .find('button[data-test-subj="action-export-value-list"]') + .first() .simulate('click'); container.unmount(); }); @@ -120,9 +119,8 @@ describe('ValueListsModal', () => { act(() => { container - .find('tbody tr') - .first() .find('button[data-test-subj="action-delete-value-list"]') + .first() .simulate('click'); container.unmount(); });