From 22a9a46dbaa0f0d9a3987721d734506394b5fcf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Thu, 19 Sep 2024 14:00:28 +0100 Subject: [PATCH 1/3] chore: Drop search feature flag (#3667) --- editor.planx.uk/src/lib/featureFlags.ts | 2 +- .../src/pages/FlowEditor/components/Sidebar/index.tsx | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/editor.planx.uk/src/lib/featureFlags.ts b/editor.planx.uk/src/lib/featureFlags.ts index b1e478c760..5fd7349747 100644 --- a/editor.planx.uk/src/lib/featureFlags.ts +++ b/editor.planx.uk/src/lib/featureFlags.ts @@ -1,5 +1,5 @@ // add/edit/remove feature flags in array below -const AVAILABLE_FEATURE_FLAGS = ["SEARCH", "ADD_NEW_EDITOR"] as const; +const AVAILABLE_FEATURE_FLAGS = [] as const; type FeatureFlag = (typeof AVAILABLE_FEATURE_FLAGS)[number]; diff --git a/editor.planx.uk/src/pages/FlowEditor/components/Sidebar/index.tsx b/editor.planx.uk/src/pages/FlowEditor/components/Sidebar/index.tsx index 51c1522901..bdfa6aba31 100644 --- a/editor.planx.uk/src/pages/FlowEditor/components/Sidebar/index.tsx +++ b/editor.planx.uk/src/pages/FlowEditor/components/Sidebar/index.tsx @@ -8,7 +8,6 @@ import Link from "@mui/material/Link"; import { styled } from "@mui/material/styles"; import Tabs from "@mui/material/Tabs"; import Tooltip from "@mui/material/Tooltip"; -import { hasFeatureFlag } from "lib/featureFlags"; import React, { useState } from "react"; import { rootFlowPath } from "routes/utils"; import Permission from "ui/editor/Permission"; @@ -175,9 +174,7 @@ const Sidebar: React.FC = React.memo(() => { - {hasFeatureFlag("SEARCH") && ( - - )} + From fbe014fd8c6832efa005fb6145c521c33fc3a215 Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Fri, 20 Sep 2024 08:56:58 +0200 Subject: [PATCH 2/3] fix: hide "Add a new editor" button from non-admin users (#3713) --- .../Team/components/MembersTable.tsx | 26 +++++++------ ...rs.addNewEditor.errors.serverSide.test.tsx | 3 +- ...ewEditor.errors.userAlreadyExists.test.tsx | 3 +- ...rs.addNewEditor.noExistingMembers.test.tsx | 6 ++- .../tests/TeamMembers.addNewEditor.test.tsx | 38 +++++++++++++++++-- .../tests/TeamMembers.updateEditor.test.tsx | 1 + 6 files changed, 59 insertions(+), 18 deletions(-) diff --git a/editor.planx.uk/src/pages/FlowEditor/components/Team/components/MembersTable.tsx b/editor.planx.uk/src/pages/FlowEditor/components/Team/components/MembersTable.tsx index cda4de0aad..45d32eed65 100644 --- a/editor.planx.uk/src/pages/FlowEditor/components/Team/components/MembersTable.tsx +++ b/editor.planx.uk/src/pages/FlowEditor/components/Team/components/MembersTable.tsx @@ -148,18 +148,20 @@ export const MembersTable = ({ ))} {showAddMemberButton && ( - - - { - setInitialValues(undefined); - setShowAddModal(true); - }} - > - Add a new editor - - - + + + + { + setInitialValues(undefined); + setShowAddModal(true); + }} + > + Add a new editor + + + + )} diff --git a/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.addNewEditor.errors.serverSide.test.tsx b/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.addNewEditor.errors.serverSide.test.tsx index 22fd4e5db2..7d0fb98a28 100644 --- a/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.addNewEditor.errors.serverSide.test.tsx +++ b/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.addNewEditor.errors.serverSide.test.tsx @@ -5,7 +5,7 @@ import { vi } from "vitest"; import { setupTeamMembersScreen } from "./helpers/setupTeamMembersScreen"; import { userTriesToAddNewEditor } from "./helpers/userTriesToAddNewEditor"; import { mockTeamMembersData } from "./mocks/mockTeamMembersData"; -import { alreadyExistingUser } from "./mocks/mockUsers"; +import { alreadyExistingUser, mockPlatformAdminUser } from "./mocks/mockUsers"; let initialState: FullStore; vi.mock( @@ -22,6 +22,7 @@ describe("when a user fills in the 'add a new editor' form correctly but there i beforeEach(async () => { useStore.setState({ teamMembers: [...mockTeamMembersData, alreadyExistingUser], + user: mockPlatformAdminUser, }); const { user } = await setupTeamMembersScreen(); diff --git a/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.addNewEditor.errors.userAlreadyExists.test.tsx b/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.addNewEditor.errors.userAlreadyExists.test.tsx index 11f53bc288..d2ac094305 100644 --- a/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.addNewEditor.errors.userAlreadyExists.test.tsx +++ b/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.addNewEditor.errors.userAlreadyExists.test.tsx @@ -5,7 +5,7 @@ import { vi } from "vitest"; import { setupTeamMembersScreen } from "./helpers/setupTeamMembersScreen"; import { userTriesToAddNewEditor } from "./helpers/userTriesToAddNewEditor"; import { mockTeamMembersData } from "./mocks/mockTeamMembersData"; -import { alreadyExistingUser } from "./mocks/mockUsers"; +import { alreadyExistingUser, mockPlatformAdminUser } from "./mocks/mockUsers"; vi.mock( "pages/FlowEditor/components/Team/queries/createAndAddUserToTeam.tsx", @@ -23,6 +23,7 @@ describe("when a user fills in the 'add a new editor' form correctly but the use beforeEach(async () => { useStore.setState({ teamMembers: [...mockTeamMembersData, alreadyExistingUser], + user: mockPlatformAdminUser, }); const { user } = await setupTeamMembersScreen(); diff --git a/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.addNewEditor.noExistingMembers.test.tsx b/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.addNewEditor.noExistingMembers.test.tsx index c3f5e64a4d..59a9aff7f2 100644 --- a/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.addNewEditor.noExistingMembers.test.tsx +++ b/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.addNewEditor.noExistingMembers.test.tsx @@ -3,6 +3,7 @@ import { useStore } from "pages/FlowEditor/lib/store"; import { TeamMember } from "../types"; import { setupTeamMembersScreen } from "./helpers/setupTeamMembersScreen"; +import { mockPlatformAdminUser } from "./mocks/mockUsers"; const mockTeamMembersDataWithNoTeamEditors: TeamMember[] = [ { @@ -16,7 +17,10 @@ const mockTeamMembersDataWithNoTeamEditors: TeamMember[] = [ describe("when a user views the 'Team members' screen but there are no existing team editors listed", () => { beforeEach(async () => { - useStore.setState({ teamMembers: mockTeamMembersDataWithNoTeamEditors }); + useStore.setState({ + teamMembers: mockTeamMembersDataWithNoTeamEditors, + user: mockPlatformAdminUser, + }); const { getByText } = await setupTeamMembersScreen(); getByText("No members found"); }); diff --git a/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.addNewEditor.test.tsx b/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.addNewEditor.test.tsx index c82b41cf71..cbf0765d4f 100644 --- a/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.addNewEditor.test.tsx +++ b/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.addNewEditor.test.tsx @@ -11,7 +11,11 @@ import { EditorUpsertModal } from "../components/EditorUpsertModal"; import { setupTeamMembersScreen } from "./helpers/setupTeamMembersScreen"; import { userTriesToAddNewEditor } from "./helpers/userTriesToAddNewEditor"; import { mockTeamMembersData } from "./mocks/mockTeamMembersData"; -import { emptyTeamMemberObj } from "./mocks/mockUsers"; +import { + emptyTeamMemberObj, + mockPlainUser, + mockPlatformAdminUser, +} from "./mocks/mockUsers"; vi.mock( "pages/FlowEditor/components/Team/queries/createAndAddUserToTeam.tsx", @@ -27,7 +31,11 @@ let initialState: FullStore; describe("when a user presses 'add a new editor'", () => { beforeEach(async () => { - useStore.setState({ teamMembers: mockTeamMembersData, teamSlug: "planx" }); + useStore.setState({ + teamMembers: mockTeamMembersData, + user: mockPlatformAdminUser, + teamSlug: "planx", + }); const { user } = await setupTeamMembersScreen(); const teamEditorsTable = screen.getByTestId("team-editors"); @@ -45,8 +53,13 @@ describe("when a user presses 'add a new editor'", () => { describe("when a user fills in the 'add a new editor' form correctly", () => { afterAll(() => useStore.setState(initialState)); + beforeEach(async () => { - useStore.setState({ teamMembers: mockTeamMembersData, teamSlug: "planx" }); + useStore.setState({ + teamMembers: mockTeamMembersData, + user: mockPlatformAdminUser, + teamSlug: "planx", + }); const { user } = await setupTeamMembersScreen(); await userTriesToAddNewEditor(user); }); @@ -97,6 +110,7 @@ describe("'add a new editor' button is hidden from Templates team", () => { beforeEach(async () => { useStore.setState({ teamMembers: mockTeamMembersData, + user: mockPlatformAdminUser, teamSlug: "templates", }); }); @@ -109,3 +123,21 @@ describe("'add a new editor' button is hidden from Templates team", () => { expect(addEditorButton).not.toBeInTheDocument(); }); }); + +describe("when a user is not a platform admin", () => { + beforeEach(async () => { + useStore.setState({ + teamMembers: mockTeamMembersData, + user: mockPlainUser, + teamSlug: "templates", + }); + }); + + it("hides the button from non-admin users", async () => { + const { user: _user } = await setupTeamMembersScreen(); + const teamEditorsTable = screen.getByTestId("team-editors"); + const addEditorButton = + within(teamEditorsTable).queryByText("Add a new editor"); + expect(addEditorButton).not.toBeInTheDocument(); + }); +}); diff --git a/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.updateEditor.test.tsx b/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.updateEditor.test.tsx index 7527470705..77c05d3c5e 100644 --- a/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.updateEditor.test.tsx +++ b/editor.planx.uk/src/pages/FlowEditor/components/Team/tests/TeamMembers.updateEditor.test.tsx @@ -192,6 +192,7 @@ describe("when a user is not a platform admin", () => { await setupTeamMembersScreen(); }); + it("does not show an edit button", async () => { const teamEditorsTable = screen.getByTestId("team-editors"); const addEditorButton = From 3549e615a232617157d4c8fc20065e6efc21bbac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Fri, 20 Sep 2024 08:26:19 +0100 Subject: [PATCH 3/3] fix: Add loading indicator to public pages (#3714) --- .../src/pages/layout/LoadingLayout.tsx | 10 ++++ editor.planx.uk/src/routes/index.tsx | 52 +++++++++++-------- 2 files changed, 41 insertions(+), 21 deletions(-) create mode 100644 editor.planx.uk/src/pages/layout/LoadingLayout.tsx diff --git a/editor.planx.uk/src/pages/layout/LoadingLayout.tsx b/editor.planx.uk/src/pages/layout/LoadingLayout.tsx new file mode 100644 index 0000000000..3198b87999 --- /dev/null +++ b/editor.planx.uk/src/pages/layout/LoadingLayout.tsx @@ -0,0 +1,10 @@ +import DelayedLoadingIndicator from "components/DelayedLoadingIndicator"; +import React from "react"; +import { useLoadingRoute, View } from "react-navi"; + +export const loadingView = () => ; + +export const LoadingLayout = () => { + const isLoading = useLoadingRoute(); + return isLoading ? : ; +}; diff --git a/editor.planx.uk/src/routes/index.tsx b/editor.planx.uk/src/routes/index.tsx index 831a73be98..c1cad20ee9 100644 --- a/editor.planx.uk/src/routes/index.tsx +++ b/editor.planx.uk/src/routes/index.tsx @@ -1,4 +1,5 @@ -import { lazy, map, mount, redirect, route } from "navi"; +import { compose, lazy, map, mount, redirect, route, withView } from "navi"; +import { loadingView } from "pages/layout/LoadingLayout"; import * as React from "react"; import { client } from "../lib/graphql"; @@ -58,27 +59,36 @@ const editorRoutes = mount({ const mountPayRoutes = () => map(async () => { + compose(withView(loadingView)); return lazy(() => import("./pay")); }); export default isPreviewOnlyDomain - ? mount({ - "/:team/:flow/published": lazy(() => import("./published")), // XXX: keeps old URL working, but only for the team listed in the domain. - "/:flow": lazy(() => import("./published")), - "/:flow/pay": mountPayRoutes(), - // XXX: We're not sure where to redirect `/` to so for now we'll just return the default 404 - // "/": redirect("somewhere?"), - }) - : mount({ - "/:team/:flow/published": lazy(() => import("./published")), // loads current published flow if exists, or throws Not Found if unpublished - "/canterbury/find-out-if-you-need-planning-permission/preview": map( - async (req) => - redirect( - `/canterbury/find-out-if-you-need-planning-permission/published${req?.search}`, - ), - ), // temporary redirect while Canterbury works with internal IT to update advertised service links - "/:team/:flow/preview": lazy(() => import("./preview")), // loads current draft flow and latest published external portals, or throws Not Found if any external portal is unpublished - "/:team/:flow/draft": lazy(() => import("./draft")), // loads current draft flow and draft external portals - "/:team/:flow/pay": mountPayRoutes(), - "*": editorRoutes, - }); + ? compose( + withView(loadingView), + + mount({ + "/:team/:flow/published": lazy(() => import("./published")), // XXX: keeps old URL working, but only for the team listed in the domain. + "/:flow": lazy(() => import("./published")), + "/:flow/pay": mountPayRoutes(), + // XXX: We're not sure where to redirect `/` to so for now we'll just return the default 404 + // "/": redirect("somewhere?"), + }), + ) + : compose( + withView(loadingView), + + mount({ + "/:team/:flow/published": lazy(() => import("./published")), // loads current published flow if exists, or throws Not Found if unpublished + "/canterbury/find-out-if-you-need-planning-permission/preview": map( + async (req) => + redirect( + `/canterbury/find-out-if-you-need-planning-permission/published${req?.search}`, + ), + ), // temporary redirect while Canterbury works with internal IT to update advertised service links + "/:team/:flow/preview": lazy(() => import("./preview")), // loads current draft flow and latest published external portals, or throws Not Found if any external portal is unpublished + "/:team/:flow/draft": lazy(() => import("./draft")), // loads current draft flow and draft external portals + "/:team/:flow/pay": mountPayRoutes(), + "*": editorRoutes, + }), + );