From 09d7cfd30cfbcf17c26fa7f125c6fd9d5eb2c5b1 Mon Sep 17 00:00:00 2001 From: Philippe Oberti Date: Fri, 19 Jul 2024 10:41:00 +0200 Subject: [PATCH] [Security Solution][Expandable flyout] - add onClose callback logic to Security Solution application (#188196) --- .../left/components/add_note.test.tsx | 11 +- .../left/components/add_note.tsx | 5 +- .../left/components/tour.test.tsx | 9 +- .../document_details/left/components/tour.tsx | 5 +- .../right/components/tour.test.tsx | 9 +- .../right/components/tour.tsx | 5 +- .../shared/constants/flyouts.ts | 11 ++ .../hooks/use_is_timeline_flyout_open.test.ts | 46 ------- .../shared/hooks/use_which_flyout.test.tsx | 123 ++++++++++++++++++ ...ine_flyout_open.ts => use_which_flyout.ts} | 25 +++- .../security_solution/public/flyout/index.tsx | 41 +++++- .../use_on_expandable_flyout_close.test.tsx | 45 +++++++ .../hooks/use_on_expandable_flyout_close.ts | 41 ++++++ .../query_tab_unified_components.test.tsx | 50 +++++++ .../data_table/index.test.tsx | 8 +- .../unified_components/data_table/index.tsx | 11 +- ...nified_timeline_expandable_flyout.test.tsx | 70 ---------- ...use_unified_timeline_expandable_flyout.tsx | 72 ---------- 18 files changed, 364 insertions(+), 223 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/flyout/document_details/shared/constants/flyouts.ts delete mode 100644 x-pack/plugins/security_solution/public/flyout/document_details/shared/hooks/use_is_timeline_flyout_open.test.ts create mode 100644 x-pack/plugins/security_solution/public/flyout/document_details/shared/hooks/use_which_flyout.test.tsx rename x-pack/plugins/security_solution/public/flyout/document_details/shared/hooks/{use_is_timeline_flyout_open.ts => use_which_flyout.ts} (54%) create mode 100644 x-pack/plugins/security_solution/public/flyout/shared/hooks/use_on_expandable_flyout_close.test.tsx create mode 100644 x-pack/plugins/security_solution/public/flyout/shared/hooks/use_on_expandable_flyout_close.ts delete mode 100644 x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/hooks/use_unified_timeline_expandable_flyout.test.tsx delete mode 100644 x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/hooks/use_unified_timeline_expandable_flyout.tsx diff --git a/x-pack/plugins/security_solution/public/flyout/document_details/left/components/add_note.test.tsx b/x-pack/plugins/security_solution/public/flyout/document_details/left/components/add_note.test.tsx index 980cb97d6edaee..c1929be9325a88 100644 --- a/x-pack/plugins/security_solution/public/flyout/document_details/left/components/add_note.test.tsx +++ b/x-pack/plugins/security_solution/public/flyout/document_details/left/components/add_note.test.tsx @@ -16,11 +16,12 @@ import { ATTACH_TO_TIMELINE_CHECKBOX_TEST_ID, } from './test_ids'; import { ReqStatus } from '../../../../notes/store/notes.slice'; -import { useIsTimelineFlyoutOpen } from '../../shared/hooks/use_is_timeline_flyout_open'; import { TimelineId } from '../../../../../common/types'; import userEvent from '@testing-library/user-event'; +import { useWhichFlyout } from '../../shared/hooks/use_which_flyout'; +import { Flyouts } from '../../shared/constants/flyouts'; -jest.mock('../../shared/hooks/use_is_timeline_flyout_open'); +jest.mock('../../shared/hooks/use_which_flyout'); const mockAddError = jest.fn(); jest.mock('../../../../common/hooks/use_app_toasts', () => ({ @@ -124,7 +125,7 @@ describe('AddNote', () => { }); it('should disable attach to timeline checkbox if flyout is not open from timeline', () => { - (useIsTimelineFlyoutOpen as jest.Mock).mockReturnValue(false); + (useWhichFlyout as jest.Mock).mockReturnValue(Flyouts.securitySolution); const { getByTestId } = renderAddNote(); @@ -132,7 +133,7 @@ describe('AddNote', () => { }); it('should disable attach to timeline checkbox if active timeline is not saved', () => { - (useIsTimelineFlyoutOpen as jest.Mock).mockReturnValue(true); + (useWhichFlyout as jest.Mock).mockReturnValue(Flyouts.timeline); const store = createMockStore({ ...mockGlobalState, @@ -157,7 +158,7 @@ describe('AddNote', () => { }); it('should have attach to timeline checkbox enabled', () => { - (useIsTimelineFlyoutOpen as jest.Mock).mockReturnValue(true); + (useWhichFlyout as jest.Mock).mockReturnValue(Flyouts.timeline); const store = createMockStore({ ...mockGlobalState, diff --git a/x-pack/plugins/security_solution/public/flyout/document_details/left/components/add_note.tsx b/x-pack/plugins/security_solution/public/flyout/document_details/left/components/add_note.tsx index a440ed10a61b48..88c77b5d09160c 100644 --- a/x-pack/plugins/security_solution/public/flyout/document_details/left/components/add_note.tsx +++ b/x-pack/plugins/security_solution/public/flyout/document_details/left/components/add_note.tsx @@ -20,10 +20,11 @@ import { import { css } from '@emotion/react'; import { useDispatch, useSelector } from 'react-redux'; import { i18n } from '@kbn/i18n'; +import { useWhichFlyout } from '../../shared/hooks/use_which_flyout'; +import { Flyouts } from '../../shared/constants/flyouts'; import { useKibana } from '../../../../common/lib/kibana'; import { TimelineId } from '../../../../../common/types'; import { timelineSelectors } from '../../../../timelines/store'; -import { useIsTimelineFlyoutOpen } from '../../shared/hooks/use_is_timeline_flyout_open'; import { ADD_NOTE_BUTTON_TEST_ID, ADD_NOTE_MARKDOWN_TEST_ID, @@ -92,7 +93,7 @@ export const AddNote = memo(({ eventId }: AddNewNoteProps) => { ); // if the flyout is open from a timeline and that timeline is saved, we automatically check the checkbox to associate the note to it - const isTimelineFlyout = useIsTimelineFlyoutOpen(); + const isTimelineFlyout = useWhichFlyout() === Flyouts.timeline; const [checked, setChecked] = useState(true); const onCheckboxChange = useCallback( diff --git a/x-pack/plugins/security_solution/public/flyout/document_details/left/components/tour.test.tsx b/x-pack/plugins/security_solution/public/flyout/document_details/left/components/tour.test.tsx index 9aaffcfe2d71c6..bb288b5c7afaa9 100644 --- a/x-pack/plugins/security_solution/public/flyout/document_details/left/components/tour.test.tsx +++ b/x-pack/plugins/security_solution/public/flyout/document_details/left/components/tour.test.tsx @@ -18,11 +18,12 @@ import { import { useKibana as mockUseKibana } from '../../../../common/lib/kibana/__mocks__'; import { useKibana } from '../../../../common/lib/kibana'; import { FLYOUT_TOUR_CONFIG_ANCHORS } from '../../shared/utils/tour_step_config'; -import { useIsTimelineFlyoutOpen } from '../../shared/hooks/use_is_timeline_flyout_open'; import { FLYOUT_TOUR_TEST_ID } from '../../shared/components/test_ids'; +import { useWhichFlyout } from '../../shared/hooks/use_which_flyout'; +import { Flyouts } from '../../shared/constants/flyouts'; jest.mock('../../../../common/lib/kibana'); -jest.mock('../../shared/hooks/use_is_timeline_flyout_open'); +jest.mock('../../shared/hooks/use_which_flyout'); const mockedUseKibana = mockUseKibana(); @@ -52,7 +53,7 @@ describe('', () => { storage: storageMock, }, }); - (useIsTimelineFlyoutOpen as jest.Mock).mockReturnValue(false); + (useWhichFlyout as jest.Mock).mockReturnValue(Flyouts.securitySolution); storageMock.clear(); }); @@ -105,7 +106,7 @@ describe('', () => { }); it('should not render left panel tour for flyout in timeline', () => { - (useIsTimelineFlyoutOpen as jest.Mock).mockReturnValue(true); + (useWhichFlyout as jest.Mock).mockReturnValue(Flyouts.timeline); storageMock.set('securitySolution.documentDetails.newFeaturesTour.v8.14', { currentTourStep: 3, isTourActive: true, diff --git a/x-pack/plugins/security_solution/public/flyout/document_details/left/components/tour.tsx b/x-pack/plugins/security_solution/public/flyout/document_details/left/components/tour.tsx index 4e3adc140a8aa7..c1bafab10d9a7f 100644 --- a/x-pack/plugins/security_solution/public/flyout/document_details/left/components/tour.tsx +++ b/x-pack/plugins/security_solution/public/flyout/document_details/left/components/tour.tsx @@ -6,12 +6,13 @@ */ import React, { memo, useMemo } from 'react'; +import { useWhichFlyout } from '../../shared/hooks/use_which_flyout'; import { getField } from '../../shared/utils'; import { EventKind } from '../../shared/constants/event_kinds'; import { useDocumentDetailsContext } from '../../shared/context'; import { FlyoutTour } from '../../shared/components/flyout_tour'; import { getLeftSectionTourSteps } from '../../shared/utils/tour_step_config'; -import { useIsTimelineFlyoutOpen } from '../../shared/hooks/use_is_timeline_flyout_open'; +import { Flyouts } from '../../shared/constants/flyouts'; /** * Guided tour for the left panel in details flyout @@ -20,7 +21,7 @@ export const LeftPanelTour = memo(() => { const { getFieldsData, isPreview } = useDocumentDetailsContext(); const eventKind = getField(getFieldsData('event.kind')); const isAlert = eventKind === EventKind.signal; - const isTimelineFlyoutOpen = useIsTimelineFlyoutOpen(); + const isTimelineFlyoutOpen = useWhichFlyout() === Flyouts.timeline; const showTour = isAlert && !isPreview && !isTimelineFlyoutOpen; const tourStepContent = useMemo(() => getLeftSectionTourSteps(), []); diff --git a/x-pack/plugins/security_solution/public/flyout/document_details/right/components/tour.test.tsx b/x-pack/plugins/security_solution/public/flyout/document_details/right/components/tour.test.tsx index f0cc3f1da85593..20540184156b9b 100644 --- a/x-pack/plugins/security_solution/public/flyout/document_details/right/components/tour.test.tsx +++ b/x-pack/plugins/security_solution/public/flyout/document_details/right/components/tour.test.tsx @@ -18,13 +18,14 @@ import { import { useKibana as mockUseKibana } from '../../../../common/lib/kibana/__mocks__'; import { useKibana } from '../../../../common/lib/kibana'; import { FLYOUT_TOUR_CONFIG_ANCHORS } from '../../shared/utils/tour_step_config'; -import { useIsTimelineFlyoutOpen } from '../../shared/hooks/use_is_timeline_flyout_open'; import { FLYOUT_TOUR_TEST_ID } from '../../shared/components/test_ids'; import { useTourContext } from '../../../../common/components/guided_onboarding_tour/tour'; import { casesPluginMock } from '@kbn/cases-plugin/public/mocks'; +import { useWhichFlyout } from '../../shared/hooks/use_which_flyout'; +import { Flyouts } from '../../shared/constants/flyouts'; jest.mock('../../../../common/lib/kibana'); -jest.mock('../../shared/hooks/use_is_timeline_flyout_open'); +jest.mock('../../shared/hooks/use_which_flyout'); jest.mock('../../../../common/components/guided_onboarding_tour/tour'); const mockedUseKibana = mockUseKibana(); @@ -59,7 +60,7 @@ describe('', () => { cases: mockCasesContract, }, }); - (useIsTimelineFlyoutOpen as jest.Mock).mockReturnValue(false); + (useWhichFlyout as jest.Mock).mockReturnValue(Flyouts.securitySolution); (useTourContext as jest.Mock).mockReturnValue({ isTourShown: jest.fn(() => false) }); storageMock.clear(); }); @@ -112,7 +113,7 @@ describe('', () => { }); it('should not render tour for flyout in timeline', () => { - (useIsTimelineFlyoutOpen as jest.Mock).mockReturnValue(true); + (useWhichFlyout as jest.Mock).mockReturnValue(Flyouts.timeline); const { queryByText, queryByTestId } = renderRightPanelTour({ ...mockContextValue, getFieldsData: () => '', diff --git a/x-pack/plugins/security_solution/public/flyout/document_details/right/components/tour.tsx b/x-pack/plugins/security_solution/public/flyout/document_details/right/components/tour.tsx index 621bf90d823c32..093a93149285c8 100644 --- a/x-pack/plugins/security_solution/public/flyout/document_details/right/components/tour.tsx +++ b/x-pack/plugins/security_solution/public/flyout/document_details/right/components/tour.tsx @@ -7,6 +7,8 @@ import React, { memo, useMemo, useCallback } from 'react'; import { useExpandableFlyoutApi } from '@kbn/expandable-flyout'; +import { useWhichFlyout } from '../../shared/hooks/use_which_flyout'; +import { Flyouts } from '../../shared/constants/flyouts'; import { useDocumentDetailsContext } from '../../shared/context'; import { FlyoutTour } from '../../shared/components/flyout_tour'; import { @@ -19,7 +21,6 @@ import { DocumentDetailsRightPanelKey, } from '../../shared/constants/panel_keys'; import { EventKind } from '../../shared/constants/event_kinds'; -import { useIsTimelineFlyoutOpen } from '../../shared/hooks/use_is_timeline_flyout_open'; import { useTourContext } from '../../../../common/components/guided_onboarding_tour/tour'; import { SecurityStepId } from '../../../../common/components/guided_onboarding_tour/tour_config'; import { useKibana } from '../../../../common/lib/kibana'; @@ -39,7 +40,7 @@ export const RightPanelTour = memo(() => { const eventKind = getField(getFieldsData('event.kind')); const isAlert = eventKind === EventKind.signal; - const isTimelineFlyoutOpen = useIsTimelineFlyoutOpen(); + const isTimelineFlyoutOpen = useWhichFlyout() === Flyouts.timeline; const showTour = isAlert && !isPreview && diff --git a/x-pack/plugins/security_solution/public/flyout/document_details/shared/constants/flyouts.ts b/x-pack/plugins/security_solution/public/flyout/document_details/shared/constants/flyouts.ts new file mode 100644 index 00000000000000..c19df7479fc1ef --- /dev/null +++ b/x-pack/plugins/security_solution/public/flyout/document_details/shared/constants/flyouts.ts @@ -0,0 +1,11 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export enum Flyouts { + securitySolution = 'SecuritySolution', + timeline = 'Timeline', +} diff --git a/x-pack/plugins/security_solution/public/flyout/document_details/shared/hooks/use_is_timeline_flyout_open.test.ts b/x-pack/plugins/security_solution/public/flyout/document_details/shared/hooks/use_is_timeline_flyout_open.test.ts deleted file mode 100644 index f0ea59af9bcdb3..00000000000000 --- a/x-pack/plugins/security_solution/public/flyout/document_details/shared/hooks/use_is_timeline_flyout_open.test.ts +++ /dev/null @@ -1,46 +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 - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { renderHook } from '@testing-library/react-hooks'; -import { useIsTimelineFlyoutOpen } from './use_is_timeline_flyout_open'; - -describe('useInvestigationGuide', () => { - beforeAll(() => { - Object.defineProperty(window, 'location', { - value: { - search: '?', - }, - }); - }); - - it('should return false when timeline flyout is not in url', () => { - window.location.search = 'http://app/security/alerts'; - const hookResult = renderHook(() => useIsTimelineFlyoutOpen()); - expect(hookResult.result.current).toEqual(false); - }); - - it('should return false when timeline flyout is in url but params are empty', () => { - window.location.search = - 'http://app/security/alerts&flyout=(right:(id:document-details-right))&timelineFlyout=()'; - const hookResult = renderHook(() => useIsTimelineFlyoutOpen()); - expect(hookResult.result.current).toEqual(false); - }); - - it('should return false when timeline flyout is in url but params are empty preview', () => { - window.location.search = - 'http://app/security/alerts&flyout=(right:(id:document-details-right))&timelineFlyout=(preview:!())'; - const hookResult = renderHook(() => useIsTimelineFlyoutOpen()); - expect(hookResult.result.current).toEqual(false); - }); - - it('should return true when timeline flyout is open', () => { - window.location.search = - 'http://app/security/alerts&flyout=(right:(id:document-details-right))&timelineFlyout=(right:(id:document-details-right,params:(id:id,indexName:index,scopeId:scope)))'; - const hookResult = renderHook(() => useIsTimelineFlyoutOpen()); - expect(hookResult.result.current).toEqual(true); - }); -}); diff --git a/x-pack/plugins/security_solution/public/flyout/document_details/shared/hooks/use_which_flyout.test.tsx b/x-pack/plugins/security_solution/public/flyout/document_details/shared/hooks/use_which_flyout.test.tsx new file mode 100644 index 00000000000000..76277b8da889ba --- /dev/null +++ b/x-pack/plugins/security_solution/public/flyout/document_details/shared/hooks/use_which_flyout.test.tsx @@ -0,0 +1,123 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { RenderHookResult } from '@testing-library/react-hooks'; +import { renderHook } from '@testing-library/react-hooks'; +import { useWhichFlyout } from './use_which_flyout'; +import { Flyouts } from '../constants/flyouts'; + +describe('useWhichFlyout', () => { + let hookResult: RenderHookResult<{}, string | null>; + + beforeEach(() => { + jest.clearAllMocks(); + window.location.search = '?'; + }); + + describe('no flyout open', () => { + it('should return null if only none are the url', () => { + Object.defineProperty(window, 'location', { + value: { + search: '', + }, + }); + + hookResult = renderHook(() => useWhichFlyout()); + + expect(hookResult.result.current).toEqual(null); + }); + + it('should return null if only they are the url but empty', () => { + Object.defineProperty(window, 'location', { + value: { + search: '?flyout=()&timelineFlyout=()', + }, + }); + + hookResult = renderHook(() => useWhichFlyout()); + + expect(hookResult.result.current).toEqual(null); + }); + + it('should return null if only they are the url but params are empty preview', () => { + Object.defineProperty(window, 'location', { + value: { + search: '?flyout=(preview:!())&timelineFlyout=(preview:!())', + }, + }); + + hookResult = renderHook(() => useWhichFlyout()); + + expect(hookResult.result.current).toEqual(null); + }); + }); + + describe('SecuritySolution flyout open', () => { + it('should return SecuritySolution flyout if timelineFlyout is not in the url', () => { + Object.defineProperty(window, 'location', { + value: { + search: + '?flyout=(right:(id:document-details-right,params:(id:id,indexName:indexName,scopeId:scopeId)))', + }, + }); + + hookResult = renderHook(() => useWhichFlyout()); + + expect(hookResult.result.current).toEqual(Flyouts.securitySolution); + }); + + it('should return SecuritySolution flyout if timelineFlyout is in the url but empty', () => { + Object.defineProperty(window, 'location', { + value: { + search: + '?flyout=(right:(id:document-details-right,params:(id:id,indexName:indexName,scopeId:scopeId)))&timelineFlyout=()', + }, + }); + + hookResult = renderHook(() => useWhichFlyout()); + + expect(hookResult.result.current).toEqual(Flyouts.securitySolution); + }); + + it('should return SecuritySolution flyout if timelineFlyout is in the url but params are empty preview', () => { + window.location.search = + 'http://app/security/alerts&flyout=(right:(id:document-details-right))&timelineFlyout=(preview:!())'; + + hookResult = renderHook(() => useWhichFlyout()); + + expect(hookResult.result.current).toEqual(Flyouts.securitySolution); + }); + }); + + describe('Timeline flyout open', () => { + it('should return Timeline flyout if flyout and timelineFlyout are in the url', () => { + Object.defineProperty(window, 'location', { + value: { + search: + '?flyout=(right:(id:document-details-right,params:(id:id,indexName:indexName,scopeId:scopeId)))&timelineFlyout=(right:(id:document-details-right,params:(id:id,indexName:indexName,scopeId:scopeId)))', + }, + }); + + hookResult = renderHook(() => useWhichFlyout()); + + expect(hookResult.result.current).toEqual(Flyouts.timeline); + }); + + it('should return Timeline flyout if only timelineFlyout is in the url', () => { + Object.defineProperty(window, 'location', { + value: { + search: + '?timelineFlyout=(right:(id:document-details-right,params:(id:id,indexName:indexName,scopeId:scopeId)))', + }, + }); + + hookResult = renderHook(() => useWhichFlyout()); + + expect(hookResult.result.current).toEqual(Flyouts.timeline); + }); + }); +}); diff --git a/x-pack/plugins/security_solution/public/flyout/document_details/shared/hooks/use_is_timeline_flyout_open.ts b/x-pack/plugins/security_solution/public/flyout/document_details/shared/hooks/use_which_flyout.ts similarity index 54% rename from x-pack/plugins/security_solution/public/flyout/document_details/shared/hooks/use_is_timeline_flyout_open.ts rename to x-pack/plugins/security_solution/public/flyout/document_details/shared/hooks/use_which_flyout.ts index d886b07333ac06..a5bf69f88fcb13 100644 --- a/x-pack/plugins/security_solution/public/flyout/document_details/shared/hooks/use_is_timeline_flyout_open.ts +++ b/x-pack/plugins/security_solution/public/flyout/document_details/shared/hooks/use_which_flyout.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { Flyouts } from '../constants/flyouts'; import { URL_PARAM_KEY } from '../../../../common/hooks/use_url_state'; /** @@ -12,12 +13,32 @@ import { URL_PARAM_KEY } from '../../../../common/hooks/use_url_state'; * If the url contains timelineFlyout parameter and its value is not empty, we know the timeline flyout is rendered. * As it is always on top of the normal flyout, we can deduce which flyout the user is interacting with. */ -export const useIsTimelineFlyoutOpen = (): boolean => { +export const useWhichFlyout = (): string | null => { const query = new URLSearchParams(window.location.search); + + const queryHasSecuritySolutionFlyout = query.has(URL_PARAM_KEY.flyout); + const securitySolutionFlyoutHasValue = + query.get(URL_PARAM_KEY.flyout) !== '()' && query.get(URL_PARAM_KEY.flyout) !== '(preview:!())'; + const isSecuritySolutionFlyoutOpen = + queryHasSecuritySolutionFlyout && securitySolutionFlyoutHasValue; + const queryHasTimelineFlyout = query.has(URL_PARAM_KEY.timelineFlyout); const timelineFlyoutHasValue = query.get(URL_PARAM_KEY.timelineFlyout) !== '()' && query.get(URL_PARAM_KEY.timelineFlyout) !== '(preview:!())'; + const isTimelineFlyoutOpen = queryHasTimelineFlyout && timelineFlyoutHasValue; + + if (isSecuritySolutionFlyoutOpen && isTimelineFlyoutOpen) { + return Flyouts.timeline; + } + + if (isSecuritySolutionFlyoutOpen) { + return Flyouts.securitySolution; + } + + if (isTimelineFlyoutOpen) { + return Flyouts.timeline; + } - return queryHasTimelineFlyout && timelineFlyoutHasValue; + return null; }; diff --git a/x-pack/plugins/security_solution/public/flyout/index.tsx b/x-pack/plugins/security_solution/public/flyout/index.tsx index 47383d18739c30..e3f2bb8c82d8c0 100644 --- a/x-pack/plugins/security_solution/public/flyout/index.tsx +++ b/x-pack/plugins/security_solution/public/flyout/index.tsx @@ -5,11 +5,12 @@ * 2.0. */ -import React, { memo } from 'react'; +import React, { memo, useCallback } from 'react'; import { ExpandableFlyout, type ExpandableFlyoutProps } from '@kbn/expandable-flyout'; import { useEuiTheme } from '@elastic/eui'; import type { NetworkExpandableFlyoutProps } from './network_details'; import { NetworkPanel, NetworkPanelKey } from './network_details'; +import { Flyouts } from './document_details/shared/constants/flyouts'; import { DocumentDetailsIsolateHostPanelKey, DocumentDetailsLeftPanelKey, @@ -132,28 +133,60 @@ const expandableFlyoutDocumentsPanels: ExpandableFlyoutProps['registeredPanels'] }, ]; +export const SECURITY_SOLUTION_ON_CLOSE_EVENT = `expandable-flyout-on-close-${Flyouts.securitySolution}`; +export const TIMELINE_ON_CLOSE_EVENT = `expandable-flyout-on-close-${Flyouts.timeline}`; + /** * Flyout used for the Security Solution application * We keep the default EUI 1000 z-index to ensure it is always rendered behind Timeline (which has a z-index of 1001) + * We propagate the onClose callback to the rest of Security Solution using a window event 'expandable-flyout-on-close-SecuritySolution' */ -export const SecuritySolutionFlyout = memo(() => ( - -)); +export const SecuritySolutionFlyout = memo(() => { + const onClose = useCallback( + () => + window.dispatchEvent( + new CustomEvent(SECURITY_SOLUTION_ON_CLOSE_EVENT, { + detail: Flyouts.securitySolution, + }) + ), + [] + ); + + return ( + + ); +}); SecuritySolutionFlyout.displayName = 'SecuritySolutionFlyout'; /** * Flyout used in Timeline * We set the z-index to 1002 to ensure it is always rendered above Timeline (which has a z-index of 1001) + * We propagate the onClose callback to the rest of Security Solution using a window event 'expandable-flyout-on-close-Timeline' */ export const TimelineFlyout = memo(() => { const { euiTheme } = useEuiTheme(); + const onClose = useCallback( + () => + window.dispatchEvent( + new CustomEvent(TIMELINE_ON_CLOSE_EVENT, { + detail: Flyouts.timeline, + }) + ), + [] + ); + return ( ); }); diff --git a/x-pack/plugins/security_solution/public/flyout/shared/hooks/use_on_expandable_flyout_close.test.tsx b/x-pack/plugins/security_solution/public/flyout/shared/hooks/use_on_expandable_flyout_close.test.tsx new file mode 100644 index 00000000000000..308c1bcfc6cfc6 --- /dev/null +++ b/x-pack/plugins/security_solution/public/flyout/shared/hooks/use_on_expandable_flyout_close.test.tsx @@ -0,0 +1,45 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { renderHook } from '@testing-library/react-hooks'; +import { useWhichFlyout } from '../../document_details/shared/hooks/use_which_flyout'; +import { useOnExpandableFlyoutClose } from './use_on_expandable_flyout_close'; +import { Flyouts } from '../../document_details/shared/constants/flyouts'; +import { TIMELINE_ON_CLOSE_EVENT } from '../..'; + +jest.mock('../../document_details/shared/hooks/use_which_flyout'); + +describe('useOnExpandableFlyoutClose', () => { + const callbackFct = jest.fn().mockImplementation((id: string) => {}); + + it('should run the callback function and remove the event listener from the window', () => { + (useWhichFlyout as jest.Mock).mockReturnValue(Flyouts.timeline); + + window.removeEventListener = jest.fn().mockImplementationOnce((event, callback) => {}); + + renderHook(() => useOnExpandableFlyoutClose({ callback: callbackFct })); + + window.dispatchEvent( + new CustomEvent(TIMELINE_ON_CLOSE_EVENT, { + detail: Flyouts.timeline, + }) + ); + + expect(callbackFct).toHaveBeenCalledWith(Flyouts.timeline); + expect(window.removeEventListener).toBeCalled(); + }); + + it('should add event listener to window', async () => { + (useWhichFlyout as jest.Mock).mockReturnValue(Flyouts.securitySolution); + + window.addEventListener = jest.fn().mockImplementationOnce((event, callback) => {}); + + renderHook(() => useOnExpandableFlyoutClose({ callback: callbackFct })); + + expect(window.addEventListener).toBeCalled(); + }); +}); diff --git a/x-pack/plugins/security_solution/public/flyout/shared/hooks/use_on_expandable_flyout_close.ts b/x-pack/plugins/security_solution/public/flyout/shared/hooks/use_on_expandable_flyout_close.ts new file mode 100644 index 00000000000000..e763bb222bc7a1 --- /dev/null +++ b/x-pack/plugins/security_solution/public/flyout/shared/hooks/use_on_expandable_flyout_close.ts @@ -0,0 +1,41 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { useCallback, useEffect } from 'react'; +import { useWhichFlyout } from '../../document_details/shared/hooks/use_which_flyout'; +import { Flyouts } from '../../document_details/shared/constants/flyouts'; +import { SECURITY_SOLUTION_ON_CLOSE_EVENT, TIMELINE_ON_CLOSE_EVENT } from '../..'; + +export interface UseOnCloseParams { + /** + * Function to call when the event is dispatched + */ + callback: (id: string) => void; +} + +/** + * Hook to abstract the logic of listening to the onClose event for the Security Solution application. + * The kbn-expandable-flyout package provides the onClose callback, but has there are only 2 instances of the expandable flyout in Security Solution (normal and timeline) + * we need a way to propagate the onClose event to all other components. + * 2 event names are available, we pick the correct one depending on which flyout is open (if the timeline flyout is open, it is always on top, so we choose that one). + */ +export const useOnExpandableFlyoutClose = ({ callback }: UseOnCloseParams): void => { + const flyout = useWhichFlyout(); + + const eventName = + flyout === Flyouts.securitySolution + ? SECURITY_SOLUTION_ON_CLOSE_EVENT + : TIMELINE_ON_CLOSE_EVENT; + + const eventHandler = useCallback((e: CustomEventInit) => callback(e.detail), [callback]); + + useEffect(() => { + window.addEventListener(eventName, eventHandler); + + return () => window.removeEventListener(eventName, eventHandler); + }, [eventHandler, eventName]); +}; diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/query/query_tab_unified_components.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/query/query_tab_unified_components.test.tsx index de6e85a07f8af0..2c5a1687f30aeb 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/query/query_tab_unified_components.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/query/query_tab_unified_components.test.tsx @@ -86,10 +86,12 @@ jest.mock('../../../../../common/lib/kibana'); jest.mock(`@kbn/ebt/client`); const mockOpenFlyout = jest.fn(); +const mockCloseFlyout = jest.fn(); jest.mock('@kbn/expandable-flyout', () => { return { useExpandableFlyoutApi: () => ({ openFlyout: mockOpenFlyout, + closeFlyout: mockCloseFlyout, }), TestProvider: ({ children }: PropsWithChildren<{}>) => <>{children}, }; @@ -781,6 +783,54 @@ describe('query tab with unified timeline', () => { ); }); + describe('Leading actions - expand event', () => { + it( + 'should expand and collapse event correctly', + async () => { + renderTestComponents(); + expect(await screen.findByTestId('discoverDocTable')).toBeVisible(); + + expect(screen.getByTestId('docTableExpandToggleColumn').firstChild).toHaveAttribute( + 'data-euiicon-type', + 'expand' + ); + + // Open Flyout + fireEvent.click(screen.getByTestId('docTableExpandToggleColumn')); + + await waitFor(() => { + expect(mockOpenFlyout).toHaveBeenNthCalledWith(1, { + right: { + id: 'document-details-right', + params: { + id: '1', + indexName: '', + scopeId: TimelineId.test, + }, + }, + }); + }); + + expect(screen.getByTestId('docTableExpandToggleColumn').firstChild).toHaveAttribute( + 'data-euiicon-type', + 'minimize' + ); + + // Close Flyout + fireEvent.click(screen.getByTestId('docTableExpandToggleColumn')); + + await waitFor(() => { + expect(mockCloseFlyout).toHaveBeenNthCalledWith(1); + expect(screen.getByTestId('docTableExpandToggleColumn').firstChild).toHaveAttribute( + 'data-euiicon-type', + 'expand' + ); + }); + }, + SPECIAL_TEST_TIMEOUT + ); + }); + describe('Leading actions - notes', () => { describe('securitySolutionNotesEnabled = true', () => { beforeEach(() => { diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.test.tsx index b9807e08572b4c..33e977f6a2999e 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.test.tsx @@ -17,7 +17,7 @@ import type { ComponentProps } from 'react'; import { getColumnHeaders } from '../../body/column_headers/helpers'; import { mockSourcererScope } from '../../../../../sourcerer/containers/mocks'; import { timelineActions } from '../../../../store'; -import { useUnifiedTableExpandableFlyout } from '../hooks/use_unified_timeline_expandable_flyout'; +import { useExpandableFlyoutApi } from '@kbn/expandable-flyout'; jest.mock('../../../../../sourcerer/containers'); @@ -35,9 +35,8 @@ const onEventClosedMock = jest.fn(); const onChangePageMock = jest.fn(); const openFlyoutMock = jest.fn(); -const closeFlyoutMock = jest.fn(); -jest.mock('../hooks/use_unified_timeline_expandable_flyout'); +jest.mock('@kbn/expandable-flyout'); const initialEnrichedColumns = getColumnHeaders( defaultUdtHeaders, @@ -97,9 +96,8 @@ const getTimelineFromStore = ( describe('unified data table', () => { beforeEach(() => { (useSourcererDataView as jest.Mock).mockReturnValue(mockSourcererScope); - (useUnifiedTableExpandableFlyout as jest.Mock).mockReturnValue({ + (useExpandableFlyoutApi as jest.Mock).mockReturnValue({ openFlyout: openFlyoutMock, - closeFlyout: closeFlyoutMock, }); }); afterEach(() => { diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.tsx index 2eeb4b2b83a6e3..9deca4a332d9ab 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/data_table/index.tsx @@ -13,6 +13,8 @@ import type { UnifiedDataTableProps } from '@kbn/unified-data-table'; import { UnifiedDataTable, DataLoadingState } from '@kbn/unified-data-table'; import type { DataView } from '@kbn/data-views-plugin/public'; import type { EuiDataGridCustomBodyProps, EuiDataGridProps } from '@elastic/eui'; +import { useExpandableFlyoutApi } from '@kbn/expandable-flyout'; +import { useOnExpandableFlyoutClose } from '../../../../../flyout/shared/hooks/use_on_expandable_flyout_close'; import { DocumentDetailsRightPanelKey } from '../../../../../flyout/document_details/shared/constants/panel_keys'; import { selectTimelineById } from '../../../../store/selectors'; import { RowRendererCount } from '../../../../../../common/api/timeline'; @@ -43,7 +45,6 @@ import { transformTimelineItemToUnifiedRows } from '../utils'; import { TimelineEventDetailRow } from './timeline_event_detail_row'; import { CustomTimelineDataGridBody } from './custom_timeline_data_grid_body'; import { TIMELINE_EVENT_DETAIL_ROW_ID } from '../../body/constants'; -import { useUnifiedTableExpandableFlyout } from '../hooks/use_unified_timeline_expandable_flyout'; import type { UnifiedTimelineDataGridCellContext } from '../../types'; export const SAMPLE_SIZE_SETTING = 500; @@ -138,13 +139,12 @@ export const TimelineDataTableComponent: React.FC = memo( const [expandedDoc, setExpandedDoc] = useState(); const [fetchedPage, setFechedPage] = useState(0); - const onCloseExpandableFlyout = useCallback(() => { + const onCloseExpandableFlyout = useCallback((id: string) => { setExpandedDoc((prev) => (!prev ? prev : undefined)); }, []); - const { openFlyout, closeFlyout } = useUnifiedTableExpandableFlyout({ - onClose: onCloseExpandableFlyout, - }); + const { closeFlyout, openFlyout } = useExpandableFlyoutApi(); + useOnExpandableFlyoutClose({ callback: onCloseExpandableFlyout }); const showTimeCol = useMemo(() => !!dataView && !!dataView.timeFieldName, [dataView]); @@ -187,6 +187,7 @@ export const TimelineDataTableComponent: React.FC = memo( } } else { closeFlyout(); + setExpandedDoc(undefined); } }, [tableRows, handleOnEventDetailPanelOpened, closeFlyout] diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/hooks/use_unified_timeline_expandable_flyout.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/hooks/use_unified_timeline_expandable_flyout.test.tsx deleted file mode 100644 index a1b89511de6c37..00000000000000 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/hooks/use_unified_timeline_expandable_flyout.test.tsx +++ /dev/null @@ -1,70 +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 - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { renderHook } from '@testing-library/react-hooks'; -import { useUnifiedTableExpandableFlyout } from './use_unified_timeline_expandable_flyout'; -import { useLocation } from 'react-router-dom'; -import { URL_PARAM_KEY } from '../../../../../common/hooks/use_url_state'; -import { useExpandableFlyoutApi } from '@kbn/expandable-flyout'; - -jest.mock('@kbn/kibana-react-plugin/public'); -jest.mock('react-router-dom', () => { - return { - useLocation: jest.fn(), - }; -}); -jest.mock('@kbn/expandable-flyout'); - -const onFlyoutCloseMock = jest.fn(); - -describe('useUnifiedTimelineExpandableFlyout', () => { - describe('when expandable flyout is enabled', () => { - beforeEach(() => { - (useExpandableFlyoutApi as jest.Mock).mockReturnValue({ - openFlyout: jest.fn(), - closeFlyout: jest.fn(), - }); - - (useLocation as jest.Mock).mockReturnValue({ - search: `${URL_PARAM_KEY.timelineFlyout}=(test:value)`, - }); - }); - - afterEach(() => { - jest.clearAllMocks(); - }); - - it('should mark flyout as close when location has empty `timelineFlyout`', () => { - const { result, rerender } = renderHook(() => - useUnifiedTableExpandableFlyout({ - onClose: onFlyoutCloseMock, - }) - ); - - (useLocation as jest.Mock).mockReturnValue({ - search: `${URL_PARAM_KEY.timelineFlyout}=()`, - }); - - rerender(); - - expect(result.current.isTimelineExpandableFlyoutOpen).toBe(false); - expect(onFlyoutCloseMock).toHaveBeenCalledTimes(1); - }); - - it('should call user provided close handler when flyout is closed', () => { - const { result } = renderHook(() => - useUnifiedTableExpandableFlyout({ - onClose: onFlyoutCloseMock, - }) - ); - - result.current.closeFlyout(); - - expect(onFlyoutCloseMock).toHaveBeenCalledTimes(1); - }); - }); -}); diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/hooks/use_unified_timeline_expandable_flyout.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/hooks/use_unified_timeline_expandable_flyout.tsx deleted file mode 100644 index fb28b6f44f3675..00000000000000 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/hooks/use_unified_timeline_expandable_flyout.tsx +++ /dev/null @@ -1,72 +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 - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { useCallback, useEffect, useMemo, useState } from 'react'; -import { useExpandableFlyoutApi } from '@kbn/expandable-flyout'; -import { useLocation } from 'react-router-dom'; -import { URL_PARAM_KEY } from '../../../../../common/hooks/use_url_state'; - -const EMPTY_TIMELINE_FLYOUT_SEARCH_PARAMS = '()'; - -interface UseUnifiedTableExpandableFlyoutArgs { - onClose?: () => void; -} - -export const useUnifiedTableExpandableFlyout = ({ - onClose, -}: UseUnifiedTableExpandableFlyoutArgs) => { - const location = useLocation(); - - const { openFlyout, closeFlyout } = useExpandableFlyoutApi(); - - const closeFlyoutWithEffect = useCallback(() => { - closeFlyout(); - onClose?.(); - }, [onClose, closeFlyout]); - - const isFlyoutOpen = useMemo(() => { - /** - * Currently, if new expandable flyout is closed, there is no way for - * consumer to trigger an effect `onClose` of flyout. So, we are using - * this hack to know if flyout is open or not. - * - * Raised: https://github.com/elastic/kibana/issues/179520 - * - * */ - const searchParams = new URLSearchParams(location.search); - return ( - searchParams.has(URL_PARAM_KEY.timelineFlyout) && - searchParams.get(URL_PARAM_KEY.timelineFlyout) !== EMPTY_TIMELINE_FLYOUT_SEARCH_PARAMS - ); - }, [location.search]); - - const [isTimelineExpandableFlyoutOpen, setIsTimelineExpandableFlyoutOpen] = - useState(isFlyoutOpen); - - useEffect(() => { - setIsTimelineExpandableFlyoutOpen((prev) => { - if (prev === isFlyoutOpen) { - return prev; - } - if (!isFlyoutOpen && onClose) { - // run onClose only when isFlyoutOpen changed from true to false - // should not be needed when - // https://github.com/elastic/kibana/issues/179520 - // is resolved - - onClose(); - } - return isFlyoutOpen; - }); - }, [isFlyoutOpen, onClose]); - - return { - isTimelineExpandableFlyoutOpen, - openFlyout, - closeFlyout: closeFlyoutWithEffect, - }; -};