From d3ee297d9fc5e007d7333816074b02463dd0c646 Mon Sep 17 00:00:00 2001 From: Sergi Massaneda Date: Mon, 25 Nov 2024 18:56:28 +0100 Subject: [PATCH] [SecuritySolution][Navigation] Prevent initial re-render using project nav (#201431) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Prevents an initial re-render when the `project` navigation style is set (serverless and new solution nav for stateful). This re-render was affecting all the top-level pages in Security Solution. --------- Co-authored-by: Michael Olorunnisola --- .../app/home/template_wrapper/index.test.tsx | 64 +++++++++++-------- .../app/home/template_wrapper/index.tsx | 8 +-- .../use_security_solution_navigation.test.tsx | 18 +++++- .../use_security_solution_navigation.tsx | 12 ++-- 4 files changed, 66 insertions(+), 36 deletions(-) diff --git a/x-pack/plugins/security_solution/public/app/home/template_wrapper/index.test.tsx b/x-pack/plugins/security_solution/public/app/home/template_wrapper/index.test.tsx index c1d3760813a15..49508dd79b1e1 100644 --- a/x-pack/plugins/security_solution/public/app/home/template_wrapper/index.test.tsx +++ b/x-pack/plugins/security_solution/public/app/home/template_wrapper/index.test.tsx @@ -22,32 +22,11 @@ jest.mock('./timeline', () => ({ Timeline: () =>
{'Timeline'}
, })); -jest.mock('../../../common/components/navigation/use_security_solution_navigation', () => { - return { - useSecuritySolutionNavigation: () => ({ - icon: 'logoSecurity', - items: [ - { - id: 'investigate', - name: 'Investigate', - items: [ - { - 'data-href': 'some-data-href', - 'data-test-subj': 'navigation-cases', - disabled: false, - href: 'some-href', - id: 'cases', - isSelected: true, - name: 'Cases', - }, - ], - tabIndex: undefined, - }, - ], - name: 'Security', - }), - }; -}); +const navProps = { icon: 'logoSecurity', items: [], name: 'Security' }; +const mockUseSecuritySolutionNavigation = jest.fn(); +jest.mock('../../../common/components/navigation/use_security_solution_navigation', () => ({ + useSecuritySolutionNavigation: () => mockUseSecuritySolutionNavigation(), +})); const mockUseRouteSpy = jest.fn((): [{ pageName: string }] => [ { pageName: SecurityPageName.alerts }, @@ -69,6 +48,39 @@ const renderComponent = ({ describe('SecuritySolutionTemplateWrapper', () => { beforeEach(() => { jest.clearAllMocks(); + mockUseSecuritySolutionNavigation.mockReturnValue(navProps); + }); + + describe('when navigation props are defined (classic nav)', () => { + beforeEach(() => { + mockUseSecuritySolutionNavigation.mockReturnValue(navProps); + }); + it('should render the children', async () => { + const { queryByText } = renderComponent(); + expect(queryByText('child of wrapper')).toBeInTheDocument(); + }); + }); + + describe('when navigation props are null (project nav)', () => { + beforeEach(() => { + mockUseSecuritySolutionNavigation.mockReturnValue(null); + }); + + it('should render the children', async () => { + const { queryByText } = renderComponent(); + expect(queryByText('child of wrapper')).toBeInTheDocument(); + }); + }); + + describe('when navigation props are undefined (loading)', () => { + beforeEach(() => { + mockUseSecuritySolutionNavigation.mockReturnValue(undefined); + }); + + it('should not render the children', async () => { + const { queryByText } = renderComponent(); + expect(queryByText('child of wrapper')).not.toBeInTheDocument(); + }); }); it('Should render with bottom bar when user allowed', async () => { diff --git a/x-pack/plugins/security_solution/public/app/home/template_wrapper/index.tsx b/x-pack/plugins/security_solution/public/app/home/template_wrapper/index.tsx index 19e8d55aa2dd5..f547d128ab54b 100644 --- a/x-pack/plugins/security_solution/public/app/home/template_wrapper/index.tsx +++ b/x-pack/plugins/security_solution/public/app/home/template_wrapper/index.tsx @@ -69,8 +69,8 @@ export const SecuritySolutionTemplateWrapper: React.FC - {isNotEmpty && ( + {renderChildren && ( <> { beforeEach(() => { jest.clearAllMocks(); }); + describe('while chrome style is undefined', () => { + beforeAll(() => { + mockGetChromeStyle$.mockReturnValue(of()); + }); + + it('should return proper navigation props', async () => { + const { result } = renderHook(useSecuritySolutionNavigation); + expect(result.current).toEqual(undefined); + + // check rendering of SecuritySideNav children + expect(mockSecuritySideNav).not.toHaveBeenCalled(); + }); + }); + describe('when classic navigation is enabled', () => { beforeAll(() => { mockGetChromeStyle$.mockReturnValue(of('classic')); @@ -77,9 +91,9 @@ describe('Security Solution Navigation', () => { mockGetChromeStyle$.mockReturnValue(of('project')); }); - it('should return undefined props when disabled', () => { + it('should return null props when disabled', () => { const { result } = renderHook(useSecuritySolutionNavigation); - expect(result.current).toEqual(undefined); + expect(result.current).toEqual(null); }); it('should initialize breadcrumbs', () => { diff --git a/x-pack/plugins/security_solution/public/common/components/navigation/use_security_solution_navigation/use_security_solution_navigation.tsx b/x-pack/plugins/security_solution/public/common/components/navigation/use_security_solution_navigation/use_security_solution_navigation.tsx index a2f54c04ca467..9abebcbb359ce 100644 --- a/x-pack/plugins/security_solution/public/common/components/navigation/use_security_solution_navigation/use_security_solution_navigation.tsx +++ b/x-pack/plugins/security_solution/public/common/components/navigation/use_security_solution_navigation/use_security_solution_navigation.tsx @@ -23,16 +23,20 @@ const translatedNavTitle = i18n.translate('xpack.securitySolution.navigation.mai defaultMessage: 'Security', }); -export const useSecuritySolutionNavigation = (): KibanaPageTemplateProps['solutionNav'] => { +export const useSecuritySolutionNavigation = (): KibanaPageTemplateProps['solutionNav'] | null => { const { chrome } = useKibana().services; const chromeStyle$ = useMemo(() => chrome.getChromeStyle$(), [chrome]); - const chromeStyle = useObservable(chromeStyle$, 'classic'); + const chromeStyle = useObservable(chromeStyle$, undefined); useBreadcrumbsNav(); + if (chromeStyle === undefined) { + return undefined; // wait for chromeStyle to be initialized + } + if (chromeStyle === 'project') { - // new shared-ux 'project' navigation enabled, return undefined to disable the 'classic' navigation - return undefined; + // new shared-ux 'project' navigation enabled, return null to disable the 'classic' navigation + return null; } return {