From 2f4ba1bde898445361434c407886b3828c9c2228 Mon Sep 17 00:00:00 2001 From: semd Date: Wed, 2 Feb 2022 17:48:10 +0100 Subject: [PATCH 1/4] close field editor when context is lost --- .../common/components/events_viewer/index.tsx | 15 +- .../components/create_field_button/index.tsx | 20 +- .../__snapshots__/index.test.tsx.snap | 1 + .../body/column_headers/index.test.tsx | 172 ++++++++---------- .../timeline/body/column_headers/index.tsx | 21 ++- .../components/timeline/body/index.test.tsx | 1 + .../components/timeline/body/index.tsx | 7 +- 7 files changed, 124 insertions(+), 113 deletions(-) diff --git a/x-pack/plugins/security_solution/public/common/components/events_viewer/index.tsx b/x-pack/plugins/security_solution/public/common/components/events_viewer/index.tsx index 97b0424168f0a..34a1843a701d9 100644 --- a/x-pack/plugins/security_solution/public/common/components/events_viewer/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/events_viewer/index.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React, { useCallback, useMemo, useEffect } from 'react'; +import React, { useRef, useCallback, useMemo, useEffect } from 'react'; import { connect, ConnectedProps, useDispatch } from 'react-redux'; import deepEqual from 'fast-deep-equal'; import styled from 'styled-components'; @@ -29,7 +29,10 @@ import { CellValueElementProps } from '../../../timelines/components/timeline/ce import { FIELDS_WITHOUT_CELL_ACTIONS } from '../../lib/cell_actions/constants'; import { useKibana } from '../../lib/kibana'; import { GraphOverlay } from '../../../timelines/components/graph_overlay'; -import { useCreateFieldButton } from '../../../timelines/components/create_field_button'; +import { + CreateFieldEditorActions, + useCreateFieldButton, +} from '../../../timelines/components/create_field_button'; const EMPTY_CONTROL_COLUMNS: ControlColumnProps[] = []; @@ -106,6 +109,8 @@ const StatefulEventsViewerComponent: React.FC = ({ unit, }) => { const dispatch = useDispatch(); + const editorActionsRef = useRef(null); + const { timelines: timelinesUi } = useKibana().services; const { browserFields, @@ -137,6 +142,10 @@ const StatefulEventsViewerComponent: React.FC = ({ } return () => { deleteEventQuery({ id, inputId: 'global' }); + if (editorActionsRef.current) { + // eslint-disable-next-line react-hooks/exhaustive-deps + editorActionsRef.current.closeEditor(); + } }; // eslint-disable-next-line react-hooks/exhaustive-deps }, []); @@ -167,7 +176,7 @@ const StatefulEventsViewerComponent: React.FC = ({ }, [id, timelineQuery, globalQuery]); const bulkActions = useMemo(() => ({ onAlertStatusActionSuccess }), [onAlertStatusActionSuccess]); - const createFieldComponent = useCreateFieldButton(scopeId, id); + const createFieldComponent = useCreateFieldButton(scopeId, id, editorActionsRef); return ( <> diff --git a/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.tsx b/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.tsx index 04f23605efac5..42f870a263d48 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import React, { MutableRefObject, useCallback, useEffect, useMemo, useState } from 'react'; import { EuiButton } from '@elastic/eui'; import styled from 'styled-components'; @@ -23,17 +23,21 @@ import { useDeepEqualSelector } from '../../../common/hooks/use_selector'; import { DEFAULT_COLUMN_MIN_WIDTH } from '../timeline/body/constants'; import { defaultColumnHeaderType } from '../timeline/body/column_headers/default_headers'; +export type CreateFieldEditorActions = { closeEditor: () => void } | null; +export type CreateFieldEditorActionsRef = MutableRefObject; + interface CreateFieldButtonProps { selectedDataViewId: string; onClick: () => void; timelineId: TimelineId; + editorActionsRef?: CreateFieldEditorActionsRef; } const StyledButton = styled(EuiButton)` margin-left: ${({ theme }) => theme.eui.paddingSizes.m}; `; export const CreateFieldButton = React.memo( - ({ selectedDataViewId, onClick: onClickParam, timelineId }) => { + ({ selectedDataViewId, onClick: onClickParam, timelineId, editorActionsRef }) => { const [dataView, setDataView] = useState(null); const dispatch = useDispatch(); @@ -52,7 +56,7 @@ export const CreateFieldButton = React.memo( const onClick = useCallback(() => { if (dataView) { - dataViewFieldEditor?.openEditor({ + const closeEditor = dataViewFieldEditor?.openEditor({ ctx: { dataView }, onSave: async (field: DataViewField) => { // Fetch the updated list of fields @@ -72,6 +76,9 @@ export const CreateFieldButton = React.memo( ); }, }); + if (editorActionsRef) { + editorActionsRef.current = { closeEditor }; + } } onClickParam(); }, [ @@ -82,6 +89,7 @@ export const CreateFieldButton = React.memo( selectedDataViewId, dispatch, timelineId, + editorActionsRef, ]); if ( @@ -116,7 +124,8 @@ CreateFieldButton.displayName = 'CreateFieldButton'; */ export const useCreateFieldButton = ( sourcererScope: SourcererScopeName, - timelineId: TimelineId + timelineId: TimelineId, + editorActionsRef?: CreateFieldEditorActionsRef ) => { const scopeIdSelector = useMemo(() => sourcererSelectors.scopeIdSelector(), []); const { missingPatterns, selectedDataViewId } = useDeepEqualSelector((state) => @@ -133,9 +142,10 @@ export const useCreateFieldButton = ( selectedDataViewId={selectedDataViewId} onClick={onClick} timelineId={timelineId} + editorActionsRef={editorActionsRef} /> ); return CreateFieldButtonComponent; - }, [missingPatterns.length, selectedDataViewId, timelineId]); + }, [missingPatterns.length, selectedDataViewId, timelineId, editorActionsRef]); }; diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/__snapshots__/index.test.tsx.snap b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/__snapshots__/index.test.tsx.snap index 644a3c95baf08..2d625f678721b 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/__snapshots__/index.test.tsx.snap +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/__snapshots__/index.test.tsx.snap @@ -658,6 +658,7 @@ exports[`ColumnHeaders rendering renders correctly against snapshot 1`] = ` ] } onSelectAll={[Function]} + show={true} showEventsSelect={false} showSelectAllCheckbox={false} sort={ diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.test.tsx index 59bdcf808ca42..957a504b659b7 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.test.tsx @@ -16,7 +16,7 @@ import { Sort } from '../sort'; import { TestProviders } from '../../../../../common/mock/test_providers'; import { useMountAppended } from '../../../../../common/utils/use_mount_appended'; -import { ColumnHeadersComponent } from '.'; +import { ColumnHeadersComponent, ColumnHeadersComponentProps } from '.'; import { cloneDeep } from 'lodash/fp'; import { timelineActions } from '../../../../store/timeline'; import { TimelineTabs } from '../../../../../../common/types/timeline'; @@ -24,9 +24,15 @@ import { Direction } from '../../../../../../common/search_strategy'; import { getDefaultControlColumn } from '../control_columns'; import { testTrailingControlColumns } from '../../../../../common/mock/mock_timeline_control_columns'; import { HeaderActions } from '../actions/header_actions'; +import { act } from 'react-dom/test-utils'; jest.mock('../../../../../common/lib/kibana'); +const mockUseCreateFieldButton = jest.fn().mockReturnValue(<>); +jest.mock('../../../create_field_button', () => ({ + useCreateFieldButton: (...params: unknown[]) => mockUseCreateFieldButton(...params), +})); + const mockDispatch = jest.fn(); jest.mock('react-redux', () => { const original = jest.requireActual('react-redux'); @@ -46,33 +52,34 @@ describe('ColumnHeaders', () => { ...x, headerCellRender: HeaderActions, })); + const sort: Sort[] = [ + { + columnId: '@timestamp', + columnType: 'number', + sortDirection: Direction.desc, + }, + ]; + const defaultProps: ColumnHeadersComponentProps = { + actionsColumnWidth, + browserFields: mockBrowserFields, + columnHeaders: defaultHeaders, + isSelectAllChecked: false, + onSelectAll: jest.fn, + show: true, + showEventsSelect: false, + showSelectAllCheckbox: false, + sort, + tabType: TimelineTabs.query, + timelineId, + leadingControlColumns, + trailingControlColumns: [], + }; describe('rendering', () => { - const sort: Sort[] = [ - { - columnId: '@timestamp', - columnType: 'number', - sortDirection: Direction.desc, - }, - ]; - test('renders correctly against snapshot', () => { const wrapper = shallow( - + ); expect(wrapper.find('ColumnHeadersComponent')).toMatchSnapshot(); @@ -81,20 +88,7 @@ describe('ColumnHeaders', () => { test('it renders the field browser', () => { const wrapper = mount( - + ); @@ -104,20 +98,7 @@ describe('ColumnHeaders', () => { test('it renders every column header', () => { const wrapper = mount( - + ); @@ -166,18 +147,7 @@ describe('ColumnHeaders', () => { const wrapper = mount( ); @@ -210,18 +180,7 @@ describe('ColumnHeaders', () => { const wrapper = mount( ); @@ -249,18 +208,11 @@ describe('ColumnHeaders', () => { const wrapper = mount( ); @@ -287,18 +239,13 @@ describe('ColumnHeaders', () => { const wrapper = mount( ); @@ -307,4 +254,31 @@ describe('ColumnHeaders', () => { expect(wrapper.exists('[data-test-subj="test-header-action-cell"]')).toBeTruthy(); }); }); + + describe('hiding', () => { + beforeEach(() => { + mockUseCreateFieldButton.mockClear(); + }); + + test('Calls closeEditor ref function when the timeline is closed', () => { + const mockCloseEditor = jest.fn(); + mockUseCreateFieldButton.mockImplementation((_, __, fieldEditorActionsRef) => { + fieldEditorActionsRef.current = { closeEditor: mockCloseEditor }; + }); + + const wrapper = mount( + + + + ); + act(() => { + wrapper.setProps({ ...defaultProps, show: false }); + wrapper.update(); + }); + + setTimeout(() => { + expect(mockCloseEditor).toHaveBeenCalled(); + }, 0); + }); + }); }); diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.tsx index 80a9022105d2c..7190737e078e7 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.tsx @@ -5,7 +5,7 @@ * 2.0. */ import deepEqual from 'fast-deep-equal'; -import React, { useState, useEffect, useCallback, useMemo } from 'react'; +import React, { useState, useEffect, useCallback, useMemo, useRef } from 'react'; import { Droppable, DraggableChildrenFn } from 'react-beautiful-dnd'; import { DragEffects } from '../../../../../common/components/drag_and_drop/draggable_wrapper'; @@ -34,15 +34,16 @@ import { Sort } from '../sort'; import { ColumnHeader } from './column_header'; import { SourcererScopeName } from '../../../../../common/store/sourcerer/model'; -import { useCreateFieldButton } from '../../../create_field_button'; +import { CreateFieldEditorActions, useCreateFieldButton } from '../../../create_field_button'; -interface Props { +export interface ColumnHeadersComponentProps { actionsColumnWidth: number; browserFields: BrowserFields; columnHeaders: ColumnHeaderOptions[]; isEventViewer?: boolean; isSelectAllChecked: boolean; onSelectAll: OnSelectAll; + show: boolean; showEventsSelect: boolean; showSelectAllCheckbox: boolean; sort: Sort[]; @@ -92,6 +93,7 @@ export const ColumnHeadersComponent = ({ isEventViewer = false, isSelectAllChecked, onSelectAll, + show, showEventsSelect, showSelectAllCheckbox, sort, @@ -99,8 +101,15 @@ export const ColumnHeadersComponent = ({ timelineId, leadingControlColumns, trailingControlColumns, -}: Props) => { +}: ColumnHeadersComponentProps) => { const [draggingIndex, setDraggingIndex] = useState(null); + const fieldEditorActionsRef = useRef(null); + + useEffect(() => { + if (!show && fieldEditorActionsRef.current) { + fieldEditorActionsRef.current.closeEditor(); + } + }, [show]); const renderClone: DraggableChildrenFn = useCallback( (dragProvided, _dragSnapshot, rubric) => { @@ -174,7 +183,8 @@ export const ColumnHeadersComponent = ({ const createFieldComponent = useCreateFieldButton( SourcererScopeName.timeline, - timelineId as TimelineId + timelineId as TimelineId, + fieldEditorActionsRef ); const LeadingHeaderActions = useMemo(() => { @@ -300,6 +310,7 @@ export const ColumnHeaders = React.memo( prevProps.isEventViewer === nextProps.isEventViewer && prevProps.isSelectAllChecked === nextProps.isSelectAllChecked && prevProps.onSelectAll === nextProps.onSelectAll && + prevProps.show === nextProps.show && prevProps.showEventsSelect === nextProps.showEventsSelect && prevProps.showSelectAllCheckbox === nextProps.showSelectAllCheckbox && deepEqual(prevProps.sort, nextProps.sort) && diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/index.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/index.test.tsx index 5467dbab9845c..db927e67ccc67 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/index.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/index.test.tsx @@ -146,6 +146,7 @@ describe('Body', () => { selectedEventIds: {}, setSelected: jest.fn() as unknown as StatefulBodyProps['setSelected'], sort: mockSort, + show: true, showCheckboxes: false, tabType: TimelineTabs.query, totalPages: 1, diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/index.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/index.tsx index 7e7192610a222..7257d4246f6fe 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/index.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/index.tsx @@ -89,6 +89,7 @@ export const BodyComponent = React.memo( setSelected, clearSelected, onRuleChange, + show, showCheckboxes, refetch, renderCellValue, @@ -244,6 +245,7 @@ export const BodyComponent = React.memo( isEventViewer={isEventViewer} isSelectAllChecked={isSelectAllChecked} onSelectAll={onSelectAll} + show={show} showEventsSelect={false} showSelectAllCheckbox={showCheckboxes} sort={sort} @@ -298,7 +300,8 @@ export const BodyComponent = React.memo( prevProps.renderCellValue === nextProps.renderCellValue && prevProps.rowRenderers === nextProps.rowRenderers && prevProps.showCheckboxes === nextProps.showCheckboxes && - prevProps.tabType === nextProps.tabType + prevProps.tabType === nextProps.tabType && + prevProps.show === nextProps.show ); BodyComponent.displayName = 'BodyComponent'; @@ -321,6 +324,7 @@ const makeMapStateToProps = () => { pinnedEventIds, selectedEventIds, showCheckboxes, + show, } = timeline; return { @@ -333,6 +337,7 @@ const makeMapStateToProps = () => { pinnedEventIds, selectedEventIds, showCheckboxes, + show, }; }; return mapStateToProps; From 7bf8aba18276b041dd66dccf8a08374ad7bb8789 Mon Sep 17 00:00:00 2001 From: semd Date: Wed, 2 Feb 2022 19:00:33 +0100 Subject: [PATCH 2/4] tests added --- .../components/events_viewer/index.test.tsx | 22 ++++++++++++++ .../common/components/events_viewer/index.tsx | 4 +-- .../create_field_button/index.test.tsx | 29 +++++++++++++++++-- .../components/create_field_button/index.tsx | 2 +- .../body/column_headers/index.test.tsx | 15 ++++------ 5 files changed, 57 insertions(+), 15 deletions(-) diff --git a/x-pack/plugins/security_solution/public/common/components/events_viewer/index.test.tsx b/x-pack/plugins/security_solution/public/common/components/events_viewer/index.test.tsx index 9dd8bf59893c0..3b4cf95b0f60c 100644 --- a/x-pack/plugins/security_solution/public/common/components/events_viewer/index.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/events_viewer/index.test.tsx @@ -33,6 +33,11 @@ jest.mock('../../../timelines/containers', () => ({ jest.mock('../../components/url_state/normalize_time_range.ts'); +const mockUseCreateFieldButton = jest.fn().mockReturnValue(<>); +jest.mock('../../../timelines/components/create_field_button', () => ({ + useCreateFieldButton: (...params: unknown[]) => mockUseCreateFieldButton(...params), +})); + const mockUseResizeObserver: jest.Mock = useResizeObserver as jest.Mock; jest.mock('use-resize-observer/polyfilled'); mockUseResizeObserver.mockImplementation(() => ({})); @@ -87,4 +92,21 @@ describe('StatefulEventsViewer', () => { expect(wrapper.find(`InspectButtonContainer`).exists()).toBe(true); }); }); + + test('it closes field editor when unmounted', async () => { + const mockCloseEditor = jest.fn(); + mockUseCreateFieldButton.mockImplementation((_, __, fieldEditorActionsRef) => { + fieldEditorActionsRef.current = { closeEditor: mockCloseEditor }; + return <>; + }); + + const wrapper = mount( + + + + ); + wrapper.unmount(); + + expect(mockCloseEditor).toHaveBeenCalled(); + }); }); diff --git a/x-pack/plugins/security_solution/public/common/components/events_viewer/index.tsx b/x-pack/plugins/security_solution/public/common/components/events_viewer/index.tsx index 34a1843a701d9..9fa91ed25c995 100644 --- a/x-pack/plugins/security_solution/public/common/components/events_viewer/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/events_viewer/index.tsx @@ -109,8 +109,6 @@ const StatefulEventsViewerComponent: React.FC = ({ unit, }) => { const dispatch = useDispatch(); - const editorActionsRef = useRef(null); - const { timelines: timelinesUi } = useKibana().services; const { browserFields, @@ -126,6 +124,8 @@ const StatefulEventsViewerComponent: React.FC = ({ const tGridEventRenderedViewEnabled = useIsExperimentalFeatureEnabled( 'tGridEventRenderedViewEnabled' ); + const editorActionsRef = useRef(null); + useEffect(() => { if (createTimeline != null) { createTimeline({ diff --git a/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.test.tsx index 6f3f3e8b87bc8..fbc72d57b0466 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.test.tsx @@ -6,8 +6,8 @@ */ import { render, fireEvent, act, screen } from '@testing-library/react'; -import React from 'react'; -import { CreateFieldButton } from './index'; +import React, { MutableRefObject } from 'react'; +import { CreateFieldButton, CreateFieldEditorActions } from './index'; import { indexPatternFieldEditorPluginMock, Start, @@ -108,4 +108,29 @@ describe('CreateFieldButton', () => { fireEvent.click(screen.getByRole('button')); expect(onClickParam).toHaveBeenCalled(); }); + + it("stores 'closeEditor' in the actions ref when editor is open", async () => { + const closeEditorDummyFn = () => {}; + useKibanaMock().services.data.dataViews.get = () => Promise.resolve({} as DataView); + useKibanaMock().services.dataViewFieldEditor.openEditor = () => closeEditorDummyFn; + + const editorActionsRef: MutableRefObject = React.createRef(); + await act(async () => { + render( + undefined} + timelineId={TimelineId.detectionsPage} + editorActionsRef={editorActionsRef} + />, + { + wrapper: TestProviders, + } + ); + await runAllPromises(); + }); + + fireEvent.click(screen.getByRole('button')); + expect(editorActionsRef?.current?.closeEditor).toBe(closeEditorDummyFn); + }); }); diff --git a/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.tsx b/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.tsx index 42f870a263d48..3d48a2b798ee8 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.tsx @@ -24,7 +24,7 @@ import { DEFAULT_COLUMN_MIN_WIDTH } from '../timeline/body/constants'; import { defaultColumnHeaderType } from '../timeline/body/column_headers/default_headers'; export type CreateFieldEditorActions = { closeEditor: () => void } | null; -export type CreateFieldEditorActionsRef = MutableRefObject; +type CreateFieldEditorActionsRef = MutableRefObject; interface CreateFieldButtonProps { selectedDataViewId: string; diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.test.tsx index 957a504b659b7..58177d285a0d1 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.test.tsx @@ -255,15 +255,12 @@ describe('ColumnHeaders', () => { }); }); - describe('hiding', () => { - beforeEach(() => { - mockUseCreateFieldButton.mockClear(); - }); - - test('Calls closeEditor ref function when the timeline is closed', () => { + describe('Fields', () => { + test('Closes field editor when the timeline is closed', () => { const mockCloseEditor = jest.fn(); mockUseCreateFieldButton.mockImplementation((_, __, fieldEditorActionsRef) => { fieldEditorActionsRef.current = { closeEditor: mockCloseEditor }; + return <>; }); const wrapper = mount( @@ -271,11 +268,9 @@ describe('ColumnHeaders', () => { ); - act(() => { - wrapper.setProps({ ...defaultProps, show: false }); - wrapper.update(); - }); + wrapper.setProps({ ...defaultProps, show: false }); + // let useEffect hook execute setTimeout(() => { expect(mockCloseEditor).toHaveBeenCalled(); }, 0); From e25c441c4a1434b0d84c9deb5bd40d2decb287b2 Mon Sep 17 00:00:00 2001 From: semd Date: Wed, 2 Feb 2022 19:42:27 +0100 Subject: [PATCH 3/4] typecheck clean --- .../components/timeline/body/column_headers/index.test.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.test.tsx index 58177d285a0d1..27e90e1c01b66 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.test.tsx @@ -24,7 +24,6 @@ import { Direction } from '../../../../../../common/search_strategy'; import { getDefaultControlColumn } from '../control_columns'; import { testTrailingControlColumns } from '../../../../../common/mock/mock_timeline_control_columns'; import { HeaderActions } from '../actions/header_actions'; -import { act } from 'react-dom/test-utils'; jest.mock('../../../../../common/lib/kibana'); From 1c071a5fa372c8b2ed3c7da532e2994691d3ec0d Mon Sep 17 00:00:00 2001 From: semd Date: Thu, 3 Feb 2022 15:26:57 +0100 Subject: [PATCH 4/4] close editor when timeline is unmounted --- .../components/events_viewer/index.test.tsx | 3 +- .../create_field_button/index.test.tsx | 15 +++++++-- .../components/create_field_button/index.tsx | 9 ++++-- .../body/column_headers/index.test.tsx | 31 ++++++++++++++----- .../timeline/body/column_headers/index.tsx | 9 ++++++ 5 files changed, 54 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/security_solution/public/common/components/events_viewer/index.test.tsx b/x-pack/plugins/security_solution/public/common/components/events_viewer/index.test.tsx index 3b4cf95b0f60c..2ecae44487908 100644 --- a/x-pack/plugins/security_solution/public/common/components/events_viewer/index.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/events_viewer/index.test.tsx @@ -105,8 +105,9 @@ describe('StatefulEventsViewer', () => { ); - wrapper.unmount(); + expect(mockCloseEditor).not.toHaveBeenCalled(); + wrapper.unmount(); expect(mockCloseEditor).toHaveBeenCalled(); }); }); diff --git a/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.test.tsx index fbc72d57b0466..0afb2bf641351 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.test.tsx @@ -110,9 +110,9 @@ describe('CreateFieldButton', () => { }); it("stores 'closeEditor' in the actions ref when editor is open", async () => { - const closeEditorDummyFn = () => {}; + const mockCloseEditor = jest.fn(); useKibanaMock().services.data.dataViews.get = () => Promise.resolve({} as DataView); - useKibanaMock().services.dataViewFieldEditor.openEditor = () => closeEditorDummyFn; + useKibanaMock().services.dataViewFieldEditor.openEditor = () => mockCloseEditor; const editorActionsRef: MutableRefObject = React.createRef(); await act(async () => { @@ -130,7 +130,16 @@ describe('CreateFieldButton', () => { await runAllPromises(); }); + expect(editorActionsRef?.current).toBeNull(); + fireEvent.click(screen.getByRole('button')); - expect(editorActionsRef?.current?.closeEditor).toBe(closeEditorDummyFn); + + expect(mockCloseEditor).not.toHaveBeenCalled(); + expect(editorActionsRef?.current?.closeEditor).toBeDefined(); + + editorActionsRef!.current!.closeEditor(); + + expect(mockCloseEditor).toHaveBeenCalled(); + expect(editorActionsRef!.current).toBeNull(); }); }); diff --git a/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.tsx b/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.tsx index 3d48a2b798ee8..8979a78d7aa46 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/create_field_button/index.tsx @@ -56,7 +56,7 @@ export const CreateFieldButton = React.memo( const onClick = useCallback(() => { if (dataView) { - const closeEditor = dataViewFieldEditor?.openEditor({ + const closeFieldEditor = dataViewFieldEditor?.openEditor({ ctx: { dataView }, onSave: async (field: DataViewField) => { // Fetch the updated list of fields @@ -77,7 +77,12 @@ export const CreateFieldButton = React.memo( }, }); if (editorActionsRef) { - editorActionsRef.current = { closeEditor }; + editorActionsRef.current = { + closeEditor: () => { + editorActionsRef.current = null; + closeFieldEditor(); + }, + }; } } onClickParam(); diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.test.tsx index 27e90e1c01b66..aec28732f38af 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.test.tsx @@ -254,8 +254,8 @@ describe('ColumnHeaders', () => { }); }); - describe('Fields', () => { - test('Closes field editor when the timeline is closed', () => { + describe('Field Editor', () => { + test('Closes field editor when the timeline is unmounted', () => { const mockCloseEditor = jest.fn(); mockUseCreateFieldButton.mockImplementation((_, __, fieldEditorActionsRef) => { fieldEditorActionsRef.current = { closeEditor: mockCloseEditor }; @@ -267,12 +267,29 @@ describe('ColumnHeaders', () => { ); - wrapper.setProps({ ...defaultProps, show: false }); + expect(mockCloseEditor).not.toHaveBeenCalled(); + + wrapper.unmount(); + expect(mockCloseEditor).toHaveBeenCalled(); + }); + + test('Closes field editor when the timeline is closed', () => { + const mockCloseEditor = jest.fn(); + mockUseCreateFieldButton.mockImplementation((_, __, fieldEditorActionsRef) => { + fieldEditorActionsRef.current = { closeEditor: mockCloseEditor }; + return <>; + }); + + const Proxy = (props: ColumnHeadersComponentProps) => ( + + + + ); + const wrapper = mount(); + expect(mockCloseEditor).not.toHaveBeenCalled(); - // let useEffect hook execute - setTimeout(() => { - expect(mockCloseEditor).toHaveBeenCalled(); - }, 0); + wrapper.setProps({ ...defaultProps, show: false }); + expect(mockCloseEditor).toHaveBeenCalled(); }); }); }); diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.tsx index 7190737e078e7..ca1cdef903de8 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/column_headers/index.tsx @@ -105,6 +105,15 @@ export const ColumnHeadersComponent = ({ const [draggingIndex, setDraggingIndex] = useState(null); const fieldEditorActionsRef = useRef(null); + useEffect(() => { + return () => { + if (fieldEditorActionsRef.current) { + // eslint-disable-next-line react-hooks/exhaustive-deps + fieldEditorActionsRef.current.closeEditor(); + } + }; + }, []); + useEffect(() => { if (!show && fieldEditorActionsRef.current) { fieldEditorActionsRef.current.closeEditor();