From 1c88fd9a820084d09cc5bbf39d0d979095a7582b Mon Sep 17 00:00:00 2001 From: Allison King Date: Fri, 7 Apr 2023 10:27:43 -0400 Subject: [PATCH] Fix navigating directly to a plus route (#3003) --- clients/admin-ui/cypress/e2e/datamap.cy.ts | 3 +- .../cypress/e2e/privacy-notices.cy.ts | 3 ++ clients/admin-ui/cypress/e2e/routes.cy.ts | 23 ++++++++- .../src/features/auth/ProtectedRoute.tsx | 15 +++--- .../features/common/nav/v2/nav-config.test.ts | 47 +------------------ .../src/features/common/nav/v2/nav-config.ts | 35 -------------- 6 files changed, 35 insertions(+), 91 deletions(-) diff --git a/clients/admin-ui/cypress/e2e/datamap.cy.ts b/clients/admin-ui/cypress/e2e/datamap.cy.ts index d9772abf93..e07e383ba3 100644 --- a/clients/admin-ui/cypress/e2e/datamap.cy.ts +++ b/clients/admin-ui/cypress/e2e/datamap.cy.ts @@ -1,9 +1,10 @@ -import { stubDatamap } from "cypress/support/stubs"; +import { stubDatamap, stubPlus } from "cypress/support/stubs"; describe("Datamap table and spatial view", () => { beforeEach(() => { cy.login(); stubDatamap(); + stubPlus(true); }); it("Can render only render one view at a time", () => { diff --git a/clients/admin-ui/cypress/e2e/privacy-notices.cy.ts b/clients/admin-ui/cypress/e2e/privacy-notices.cy.ts index 491a6f2311..c5a77a9531 100644 --- a/clients/admin-ui/cypress/e2e/privacy-notices.cy.ts +++ b/clients/admin-ui/cypress/e2e/privacy-notices.cy.ts @@ -1,9 +1,12 @@ +import { stubPlus } from "cypress/support/stubs"; + import { PRIVACY_NOTICES_ROUTE } from "~/features/common/nav/v2/routes"; import { RoleRegistryEnum } from "~/types/api"; describe("Privacy notices", () => { beforeEach(() => { cy.login(); + stubPlus(true); }); describe("permissions", () => { diff --git a/clients/admin-ui/cypress/e2e/routes.cy.ts b/clients/admin-ui/cypress/e2e/routes.cy.ts index 9d3b6dcfd1..ee6720e86a 100644 --- a/clients/admin-ui/cypress/e2e/routes.cy.ts +++ b/clients/admin-ui/cypress/e2e/routes.cy.ts @@ -1,6 +1,10 @@ import { stubPlus } from "cypress/support/stubs"; -import { ADD_SYSTEMS_ROUTE } from "~/features/common/nav/v2/routes"; +import { + ADD_SYSTEMS_ROUTE, + DATAMAP_ROUTE, + PRIVACY_NOTICES_ROUTE, +} from "~/features/common/nav/v2/routes"; import { RoleRegistryEnum } from "~/types/api"; describe("Routes", () => { @@ -64,4 +68,21 @@ describe("Routes", () => { }); }); }); + + describe("plus", () => { + it("non-plus cannot access plus routes", () => { + stubPlus(false); + cy.visit(DATAMAP_ROUTE); + cy.getByTestId("home-content"); + cy.getByTestId("cytoscape-graph").should("not.exist"); + cy.visit(PRIVACY_NOTICES_ROUTE); + cy.getByTestId("home-content"); + }); + + it("plus can access plus routes", () => { + stubPlus(true); + cy.visit(DATAMAP_ROUTE); + cy.getByTestId("cytoscape-graph"); + }); + }); }); diff --git a/clients/admin-ui/src/features/auth/ProtectedRoute.tsx b/clients/admin-ui/src/features/auth/ProtectedRoute.tsx index 74e985553d..43666e1002 100644 --- a/clients/admin-ui/src/features/auth/ProtectedRoute.tsx +++ b/clients/admin-ui/src/features/auth/ProtectedRoute.tsx @@ -3,7 +3,8 @@ import { ReactNode } from "react"; import { useAppDispatch, useAppSelector } from "~/app/hooks"; import { LOGIN_ROUTE, VERIFY_AUTH_INTERVAL } from "~/constants"; -import { canAccessRoute } from "~/features/common/nav/v2/nav-config"; +import { useNav } from "~/features/common/nav/v2/hooks"; +import { useGetHealthQuery } from "~/features/plus/plus.slice"; import { useGetUserPermissionsQuery } from "~/features/user-management"; import { logout, selectToken, selectUser } from "./auth.slice"; @@ -18,6 +19,8 @@ const useProtectedRoute = (redirectUrl: string) => { pollingInterval: VERIFY_AUTH_INTERVAL, skip: !userId, }); + const plusQuery = useGetHealthQuery(); + const nav = useNav({ path: router.pathname }); if (!token || !userId || permissionsQuery.isError) { // Reset the user information in redux only if we have stale information @@ -30,16 +33,12 @@ const useProtectedRoute = (redirectUrl: string) => { return { authenticated: false, hasAccess: false }; } - const path = router.pathname; - const userScopes = permissionsQuery.data - ? permissionsQuery.data.total_scopes - : []; - - const hasAccess = canAccessRoute({ path, userScopes }); + const hasAccess = !!nav.active; if ( !hasAccess && permissionsQuery.isSuccess && - typeof window !== "undefined" + typeof window !== "undefined" && + !plusQuery.isLoading ) { router.push("/"); } diff --git a/clients/admin-ui/src/features/common/nav/v2/nav-config.test.ts b/clients/admin-ui/src/features/common/nav/v2/nav-config.test.ts index cbaa9577a7..652c7766be 100644 --- a/clients/admin-ui/src/features/common/nav/v2/nav-config.test.ts +++ b/clients/admin-ui/src/features/common/nav/v2/nav-config.test.ts @@ -2,12 +2,7 @@ import { describe, expect, it } from "@jest/globals"; import { ScopeRegistryEnum } from "~/types/api"; -import { - canAccessRoute, - configureNavGroups, - findActiveNav, - NAV_CONFIG, -} from "./nav-config"; +import { configureNavGroups, findActiveNav, NAV_CONFIG } from "./nav-config"; import * as routes from "./routes"; const ALL_SCOPES = [ @@ -256,44 +251,4 @@ describe("findActiveNav", () => { expect(activeNav?.path).toEqual(expected.path); }); }); - - describe("canAccessRoute", () => { - const accessTestCases = [ - { - path: "/", - expected: true, - userScopes: [], - }, - { - path: routes.ADD_SYSTEMS_ROUTE, - expected: false, - userScopes: [], - }, - { - path: routes.ADD_SYSTEMS_ROUTE, - expected: true, - userScopes: [ScopeRegistryEnum.SYSTEM_CREATE], - }, - { - path: routes.PRIVACY_REQUESTS_ROUTE, - expected: false, - userScopes: [ScopeRegistryEnum.SYSTEM_CREATE], - }, - { - path: routes.PRIVACY_REQUESTS_ROUTE, - expected: true, - userScopes: [ScopeRegistryEnum.PRIVACY_REQUEST_READ], - }, - { - path: `${routes.PRIVACY_REQUESTS_ROUTE}?queryParam`, - expected: true, - userScopes: [ScopeRegistryEnum.PRIVACY_REQUEST_READ], - }, - ]; - accessTestCases.forEach(({ path, expected, userScopes }) => { - it(`${path} is scope restricted`, () => { - expect(canAccessRoute({ path, userScopes })).toBe(expected); - }); - }); - }); }); diff --git a/clients/admin-ui/src/features/common/nav/v2/nav-config.ts b/clients/admin-ui/src/features/common/nav/v2/nav-config.ts index 53f9003808..abf0f97304 100644 --- a/clients/admin-ui/src/features/common/nav/v2/nav-config.ts +++ b/clients/admin-ui/src/features/common/nav/v2/nav-config.ts @@ -282,38 +282,3 @@ export const findActiveNav = ({ path: activePath, }; }; - -/** - * Similar to findActiveNav, but using NavConfig instead of a NavGroup - * This may not be needed once we remove the progressive nav, since then we can - * just check what navs are available (they would all be restricted by scope) - */ -export const canAccessRoute = ({ - path, - userScopes, -}: { - path: string; - userScopes: ScopeRegistryEnum[]; -}) => { - let childMatch: NavConfigRoute | undefined; - const groupMatch = NAV_CONFIG.find((group) => { - childMatch = group.routes.find((child) => - child.exact ? path === child.path : path.startsWith(child.path) - ); - return childMatch; - }); - - if (!(groupMatch && childMatch)) { - return false; - } - - // Special case of empty scopes - if (childMatch.scopes.length === 0) { - return true; - } - - const scopeOverlaps = childMatch.scopes.filter((scope) => - userScopes.includes(scope) - ); - return scopeOverlaps.length > 0; -};