From 6a40cfaaf935b7ba6e2c3a822539cd78d428c625 Mon Sep 17 00:00:00 2001 From: Nick De Villiers Date: Wed, 19 Jul 2023 15:42:01 +0100 Subject: [PATCH 1/3] fix(navigation): Prevent secondary navigation from displaying without auth MAASENG-1986 --- .../PageContent/PageContent.test.tsx | 87 ++++++++++++++++++- .../components/PageContent/PageContent.tsx | 7 +- 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/src/app/base/components/PageContent/PageContent.test.tsx b/src/app/base/components/PageContent/PageContent.test.tsx index 3707efd3c7..02cdfd9a7b 100644 --- a/src/app/base/components/PageContent/PageContent.test.tsx +++ b/src/app/base/components/PageContent/PageContent.test.tsx @@ -1,6 +1,15 @@ import PageContent from "./PageContent"; -import { renderWithBrowserRouter, screen, within } from "testing/utils"; +import { preferencesNavItems } from "app/preferences/constants"; +import { settingsNavItems } from "app/settings/constants"; +import { + getTestState, + renderWithBrowserRouter, + screen, + within, +} from "testing/utils"; + +const state = getTestState(); it("displays sidebar with provided content", () => { renderWithBrowserRouter( @@ -29,3 +38,79 @@ it("displays hidden sidebar when no content provided", () => { ); expect(screen.queryByRole("complementary")).toHaveClass("is-collapsed"); }); + +it("shows the secondary navigation for settings", () => { + state.status.authenticated = true; + state.status.connected = true; + renderWithBrowserRouter( + + content + , + { route: "/settings/configuration/general", state } + ); + + expect(screen.getByRole("navigation")).toBeInTheDocument(); + + settingsNavItems.forEach((item) => { + expect(screen.getByText(item.label)).toBeInTheDocument(); + }); +}); + +it("shows the secondary navigation for preferences", () => { + state.status.authenticated = true; + state.status.connected = true; + renderWithBrowserRouter( + + content + , + { route: "/account/prefs/details", state } + ); + + expect(screen.getByRole("navigation")).toBeInTheDocument(); + + preferencesNavItems.forEach((item) => { + expect(screen.getByText(item.label)).toBeInTheDocument(); + }); +}); + +it("doesn't show the side nav if not authenticated", () => { + state.status.authenticated = false; + state.status.connected = true; + renderWithBrowserRouter( + + content + , + { route: "/account/prefs/details", state } + ); + + expect(screen.queryByRole("navigation")).not.toBeInTheDocument(); +}); + +it("doesn't show the side nav if not connected", () => { + state.status.authenticated = true; + state.status.connected = false; + renderWithBrowserRouter( + + content + , + { route: "/account/prefs/details", state } + ); + + expect(screen.queryByRole("navigation")).not.toBeInTheDocument(); +}); diff --git a/src/app/base/components/PageContent/PageContent.tsx b/src/app/base/components/PageContent/PageContent.tsx index eaa224f9b4..75d514f12f 100644 --- a/src/app/base/components/PageContent/PageContent.tsx +++ b/src/app/base/components/PageContent/PageContent.tsx @@ -1,6 +1,7 @@ import type { HTMLProps, ReactNode } from "react"; import classNames from "classnames"; +import { useSelector } from "react-redux"; import { matchPath, useLocation } from "react-router-dom-v5-compat"; import AppSidePanel from "../AppSidePanel"; @@ -11,6 +12,7 @@ import SecondaryNavigation from "../SecondaryNavigation"; import { useThemeContext } from "app/base/theme-context"; import { preferencesNavItems } from "app/preferences/constants"; import { settingsNavItems } from "app/settings/constants"; +import status from "app/store/status/selectors"; export type Props = { children?: ReactNode; @@ -35,7 +37,10 @@ const PageContent = ({ const { pathname } = useLocation(); const isSettingsPage = matchPath("settings/*", pathname); const isPreferencesPage = matchPath("account/prefs/*", pathname); - const isSideNavVisible = isSettingsPage || isPreferencesPage; + const authenticated = useSelector(status.authenticated); + const connected = useSelector(status.connected); + const isSideNavVisible = + (isSettingsPage || isPreferencesPage) && authenticated && connected; const { theme } = useThemeContext(); return ( From 7f08b9c59c77e5bf2d8b0660c80302ca62a951df Mon Sep 17 00:00:00 2001 From: Nick De Villiers Date: Thu, 20 Jul 2023 10:01:41 +0100 Subject: [PATCH 2/3] Update src/app/base/components/PageContent/PageContent.tsx Co-authored-by: Peter Makowski --- src/app/base/components/PageContent/PageContent.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/app/base/components/PageContent/PageContent.tsx b/src/app/base/components/PageContent/PageContent.tsx index 75d514f12f..40a0a94e9e 100644 --- a/src/app/base/components/PageContent/PageContent.tsx +++ b/src/app/base/components/PageContent/PageContent.tsx @@ -39,8 +39,9 @@ const PageContent = ({ const isPreferencesPage = matchPath("account/prefs/*", pathname); const authenticated = useSelector(status.authenticated); const connected = useSelector(status.connected); - const isSideNavVisible = - (isSettingsPage || isPreferencesPage) && authenticated && connected; + const hasSecondaryNav = isSettingsPage || isPreferencesPage; + const isSecondaryNavVisible = + hasSecondaryNav && authenticated && connected; const { theme } = useThemeContext(); return ( From 679a4f9f07a1c91d08c1de6c6315d6e32ad1cee5 Mon Sep 17 00:00:00 2001 From: Nick De Villiers Date: Thu, 20 Jul 2023 10:20:55 +0100 Subject: [PATCH 3/3] fix broken stuff from last commit --- src/app/base/components/PageContent/PageContent.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/app/base/components/PageContent/PageContent.tsx b/src/app/base/components/PageContent/PageContent.tsx index 40a0a94e9e..e7d217ae9d 100644 --- a/src/app/base/components/PageContent/PageContent.tsx +++ b/src/app/base/components/PageContent/PageContent.tsx @@ -40,19 +40,18 @@ const PageContent = ({ const authenticated = useSelector(status.authenticated); const connected = useSelector(status.connected); const hasSecondaryNav = isSettingsPage || isPreferencesPage; - const isSecondaryNavVisible = - hasSecondaryNav && authenticated && connected; + const isSecondaryNavVisible = hasSecondaryNav && authenticated && connected; const { theme } = useThemeContext(); return ( <>
- {isSideNavVisible ? ( + {isSecondaryNavVisible ? (