From 5c87e30e70d8041932bdd9c4ad327a4c16363673 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Thu, 21 Nov 2024 10:23:12 +0000 Subject: [PATCH 1/9] docs: Add mermaid diagrams explaining `.env` file setup (#3994) --- doc/how-to/how-to-add-a-secret.md | 51 ++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/doc/how-to/how-to-add-a-secret.md b/doc/how-to/how-to-add-a-secret.md index 72332e6f66..5e6a8a07e0 100644 --- a/doc/how-to/how-to-add-a-secret.md +++ b/doc/how-to/how-to-add-a-secret.md @@ -14,6 +14,7 @@ This guide will demonstrate how to - 2. Add to your local `.env` file for local development - Note: This file is never checked into our public repository and is listed in our `.gitignore` config 3. Document the secret in `.env.example` +4. If a secret is required for a unit test, add dummy values to corresponding `.env.test` files e.g `YOUR_NEW_SECRET=test` ### Docker Environments (Local development + Pizza environments) To pass a secret into our Docker Compose setup you will need to map it into the relevant container in `docker-compose.yml`. For example - @@ -35,6 +36,39 @@ When building Pizza environments for testing, GitHub actions access secrets via > Please be aware that if you are rotating secrets this may affect existing Pizzas which will need to be rebuilt. This can be done manually in GitHub by re-running the latest action associated with affected PRs. +## Diagram - Docker environments +```mermaid +flowchart LR + subgraph "Local Environment" + localEnv[".env file(s)"] --> Docker["Docker Compose"] + Docker --> API + Docker --> Hasura + Docker --> Frontend + end + + subgraph Staging AWS S3 bucket + S3 + end + + subgraph GitHub actions + pizzaEnv[".env file"] + end + + subgraph "Pizza Environment" + pizzaEnv[".env file"] --> PizzaDocker["Docker Compose"] + PizzaDocker --> PizzaAPI["API"] + PizzaDocker --> PizzaHasura["Hasura"] + PizzaDocker --> PizzaFrontend["Frontend"] + end + + %% Scripts reading and writing + S3 --"Pull/Push scripts"--> localEnv + + %% CI + S3 --> pizzaEnv +``` + + ### AWS / Pulumi Environments (Staging + Production environments) Secrets for Staging and Production environment are not handled in `.env` files, and are set directly in Pulumi, our Infrastruture as Code (IaC) platform. @@ -68,4 +102,19 @@ const apiService = new awsx.ecs.FargateService("api", { }); ``` -> Pulumi uses our Docker images to construct Fargate services. This means that the "name" value above must match that used in Docker. \ No newline at end of file +> Pulumi uses our Docker images to construct Fargate services. This means that the "name" value above must match that used in Docker. + +## Diagram - AWS / Pulumi Environments +```mermaid +flowchart LR + subgraph "Staging & Production" + direction LR + + PulumiService["Pulumi Service"] --"Private key"--> Pulumi + PulumiFile["pulumi.staging.yaml"] --"Public key"--> Pulumi + Pulumi["Pulumi IaC code"] --Decrypted--> Fargate + Fargate --> FargateAPI["API"] + Fargate --> FargateHasura["Hasura"] + Fargate --> FargateFrontend["Frontend"] + end +``` \ No newline at end of file From 06e01756676637d0fbb433a15c859d17454b5e5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Thu, 21 Nov 2024 10:53:03 +0000 Subject: [PATCH 2/9] chore: Save applicant research opt in query as view (#3996) --- hasura.planx.uk/metadata/tables.yaml | 3 ++ .../down.sql | 3 ++ .../up.sql | 29 +++++++++++++++++++ 3 files changed, 35 insertions(+) create mode 100644 hasura.planx.uk/migrations/1732184010002_applicant_research_opt_in_view/down.sql create mode 100644 hasura.planx.uk/migrations/1732184010002_applicant_research_opt_in_view/up.sql diff --git a/hasura.planx.uk/metadata/tables.yaml b/hasura.planx.uk/metadata/tables.yaml index ea03c776ed..3a24978694 100644 --- a/hasura.planx.uk/metadata/tables.yaml +++ b/hasura.planx.uk/metadata/tables.yaml @@ -94,6 +94,9 @@ - table: name: analytics_summary schema: public +- table: + name: applicant_research_opt_in_view + schema: public - table: name: blpu_codes schema: public diff --git a/hasura.planx.uk/migrations/1732184010002_applicant_research_opt_in_view/down.sql b/hasura.planx.uk/migrations/1732184010002_applicant_research_opt_in_view/down.sql new file mode 100644 index 0000000000..3da145bdbc --- /dev/null +++ b/hasura.planx.uk/migrations/1732184010002_applicant_research_opt_in_view/down.sql @@ -0,0 +1,3 @@ +comment on view "public"."applicant_research_opt_in_view" is NULL; + +DROP VIEW applicant_research_opt_in_view; \ No newline at end of file diff --git a/hasura.planx.uk/migrations/1732184010002_applicant_research_opt_in_view/up.sql b/hasura.planx.uk/migrations/1732184010002_applicant_research_opt_in_view/up.sql new file mode 100644 index 0000000000..17e0b04db8 --- /dev/null +++ b/hasura.planx.uk/migrations/1732184010002_applicant_research_opt_in_view/up.sql @@ -0,0 +1,29 @@ +CREATE OR REPLACE VIEW applicant_research_opt_in_view AS +WITH data AS ( + SELECT + ls.id, + ls.email, + ls.created_at AS application_started_at, + ls.submitted_at, + ls.data->'passport'->'data'->>'applicant.name.last' AS last_name, + ls.data->'passport'->'data'->>'applicant.name.first' AS first_name, + f.name AS flow_name, + t.name AS team_name + FROM + lowcal_sessions ls + JOIN + flows f ON ls.flow_id = f.id + JOIN + teams t ON t.id = f.team_id + WHERE + ls.data->'passport'->'data'->>'applicant.researchOptIn' = '["true"]' +) +SELECT + * +FROM + data +WHERE + last_name IS NOT NULL + AND first_name IS NOT NULL; + +comment on view "public"."applicant_research_opt_in_view" is E'Temporary view to expose a list of applicants to opt in to user research during the 2024/25 pilot. Used to generate a CSV report bi-weekly.'; \ No newline at end of file From 9660b8eeb9e0c513c5e441bbc34e7c8cd7176052 Mon Sep 17 00:00:00 2001 From: Rory Doak <138574807+RODO94@users.noreply.github.com> Date: Thu, 21 Nov 2024 14:32:31 +0000 Subject: [PATCH 3/9] chore: playwright bump to 1.49.0 (#3991) --- e2e/tests/ui-driven/package.json | 2 +- e2e/tests/ui-driven/pnpm-lock.yaml | 27 +++++++++++++-------------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/e2e/tests/ui-driven/package.json b/e2e/tests/ui-driven/package.json index 03e2203caf..91c2c8e9db 100644 --- a/e2e/tests/ui-driven/package.json +++ b/e2e/tests/ui-driven/package.json @@ -21,7 +21,7 @@ }, "packageManager": "pnpm@8.6.6", "devDependencies": { - "@playwright/test": "^1.40.1", + "@playwright/test": "^1.49.0", "@types/node": "18.16.1", "eslint-plugin-playwright": "^0.20.0" } diff --git a/e2e/tests/ui-driven/pnpm-lock.yaml b/e2e/tests/ui-driven/pnpm-lock.yaml index 9620816eb7..4e3fc3e0fe 100644 --- a/e2e/tests/ui-driven/pnpm-lock.yaml +++ b/e2e/tests/ui-driven/pnpm-lock.yaml @@ -38,8 +38,8 @@ dependencies: devDependencies: '@playwright/test': - specifier: ^1.40.1 - version: 1.40.1 + specifier: ^1.49.0 + version: 1.49.0 '@types/node': specifier: 18.16.1 version: 18.16.1 @@ -635,13 +635,12 @@ packages: '@nodelib/fs.scandir': 2.1.5 fastq: 1.17.1 - /@playwright/test@1.40.1: - resolution: {integrity: sha512-EaaawMTOeEItCRvfmkI9v6rBkF1svM8wjl/YPRrg2N2Wmp+4qJYkWtJsbew1szfKKDm6fPLy4YAanBhIlf9dWw==} - engines: {node: '>=16'} - deprecated: Please update to the latest version of Playwright to test up-to-date browsers. + /@playwright/test@1.49.0: + resolution: {integrity: sha512-DMulbwQURa8rNIQrf94+jPJQ4FmOVdpE5ZppRNvWVjvhC+6sOeo28r8MgIpQRYouXRtt/FCCXU7zn20jnHR4Qw==} + engines: {node: '>=18'} hasBin: true dependencies: - playwright: 1.40.1 + playwright: 1.49.0 dev: true /@popperjs/core@2.11.8: @@ -2046,18 +2045,18 @@ packages: engines: {node: '>=12'} dev: false - /playwright-core@1.40.1: - resolution: {integrity: sha512-+hkOycxPiV534c4HhpfX6yrlawqVUzITRKwHAmYfmsVreltEl6fAZJ3DPfLMOODw0H3s1Itd6MDCWmP1fl/QvQ==} - engines: {node: '>=16'} + /playwright-core@1.49.0: + resolution: {integrity: sha512-R+3KKTQF3npy5GTiKH/T+kdhoJfJojjHESR1YEWhYuEKRVfVaxH3+4+GvXE5xyCngCxhxnykk0Vlah9v8fs3jA==} + engines: {node: '>=18'} hasBin: true dev: true - /playwright@1.40.1: - resolution: {integrity: sha512-2eHI7IioIpQ0bS1Ovg/HszsN/XKNwEG1kbzSDDmADpclKc7CyqkHw7Mg2JCz/bbCxg25QUPcjksoMW7JcIFQmw==} - engines: {node: '>=16'} + /playwright@1.49.0: + resolution: {integrity: sha512-eKpmys0UFDnfNb3vfsf8Vx2LEOtflgRebl0Im2eQQnYMA4Aqd+Zw8bEOB+7ZKvN76901mRnqdsiOGKxzVTbi7A==} + engines: {node: '>=18'} hasBin: true dependencies: - playwright-core: 1.40.1 + playwright-core: 1.49.0 optionalDependencies: fsevents: 2.3.2 dev: true From c3cac3c18f9f69bb7063f23a560bef6fcb6d1bef Mon Sep 17 00:00:00 2001 From: ollie <107874766+zz-hh-aa@users.noreply.github.com> Date: Thu, 21 Nov 2024 14:40:36 +0000 Subject: [PATCH 4/9] feat: setup Metabase env variables (#3992) --- api.planx.uk/.env.test.example | 4 ++++ docker-compose.yml | 2 ++ infrastructure/application/Pulumi.production.yaml | 2 ++ infrastructure/application/Pulumi.staging.yaml | 2 ++ infrastructure/application/index.ts | 8 ++++++++ 5 files changed, 18 insertions(+) diff --git a/api.planx.uk/.env.test.example b/api.planx.uk/.env.test.example index b2356fd300..ec234e54f9 100644 --- a/api.planx.uk/.env.test.example +++ b/api.planx.uk/.env.test.example @@ -45,3 +45,7 @@ ORDNANCE_SURVEY_API_KEY=👻 IDOX_NEXUS_TOKEN_URL=👻 IDOX_NEXUS_SUBMISSION_URL=👻 + +# Analytics +METABASE_API_KEY=👻 +METABASE_URL_EXT=metabase.example.com \ No newline at end of file diff --git a/docker-compose.yml b/docker-compose.yml index 8569cc75dd..ad5c1ee0ac 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -145,6 +145,8 @@ services: IDOX_NEXUS_TOKEN_URL: ${IDOX_NEXUS_TOKEN_URL} JWT_SECRET: ${JWT_SECRET} MAPBOX_ACCESS_TOKEN: ${MAPBOX_ACCESS_TOKEN} + METABASE_API_KEY: ${METABASE_API_KEY} + METABASE_URL_EXT: ${METABASE_URL_EXT} MINIO_PORT: ${MINIO_PORT} ORDNANCE_SURVEY_API_KEY: ${ORDNANCE_SURVEY_API_KEY} PORT: ${API_PORT} diff --git a/infrastructure/application/Pulumi.production.yaml b/infrastructure/application/Pulumi.production.yaml index 5010c70926..00fa794df5 100644 --- a/infrastructure/application/Pulumi.production.yaml +++ b/infrastructure/application/Pulumi.production.yaml @@ -148,3 +148,5 @@ config: aws:region: eu-west-2 cloudflare:apiToken: secure: AAABAMbIBX9N21ModHyDYCQCGZXkRUP62NIUMhsxkK+/YUPRQr6PEqgZX9LRP2UuVwvVHd2RxvKGT8lpq8oAgyeDYP1eZSUl + application:metabase-api-key: + secure: AAABAAdkLyFGZJ50HENSAkvYZ9Y3Qj2oKysmBKNWbqgcCLYz+dDvgEBPTbzG6lNgOqUyySJDmquMilyiUtTLau7rpF4txEM/O4DHmAinVg== diff --git a/infrastructure/application/Pulumi.staging.yaml b/infrastructure/application/Pulumi.staging.yaml index 6e5a233278..be082e9ca8 100644 --- a/infrastructure/application/Pulumi.staging.yaml +++ b/infrastructure/application/Pulumi.staging.yaml @@ -66,3 +66,5 @@ config: certificates:cloudflare-zone-id: dc27ac531ff8862559ed9ab5016c4953 cloudflare:apiToken: secure: AAABABWhDm+7RstbxLXd1D8CcxkylHS6UKMqk4kOaY7Y0E7FJS4bZfvyGs0nks80hl3vjENH4eDuFbUgA82/sA4SmDlfpNXr + application:metabase-api-key: + secure: AAABAFf+hW09AWupsY6adrPAJHCkrTMeRX7/gaUHLYXi3QS77MVelPp9K0L4zyUL8u7zDanjuE9G/bfIlcVXLwiLLKAJkt5to9knKJqTXg== diff --git a/infrastructure/application/index.ts b/infrastructure/application/index.ts index 5e599d415d..19a55a679f 100644 --- a/infrastructure/application/index.ts +++ b/infrastructure/application/index.ts @@ -451,6 +451,14 @@ export = async () => { name: "MAPBOX_ACCESS_TOKEN", value: config.requireSecret("mapbox-access-token"), }, + { + name: "METABASE_API_KEY", + value: config.requireSecret("metabase-api-key"), + }, + { + name: "METABASE_URL_EXT", + value: `https://metabase.${DOMAIN}`, + }, generateCORSAllowList(CUSTOM_DOMAINS, DOMAIN), ...generateTeamSecrets(config, env), ], From 2c668f18785636b9d5e7b8b1b9b41fc4ca135fc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Fri, 22 Nov 2024 09:16:35 +0000 Subject: [PATCH 5/9] fix: Restore `EditorNavMenu` for platformAdmins (#3999) --- .../src/components/EditorNavMenu.test.tsx | 205 ++++++++++++++++++ .../src/components/EditorNavMenu.tsx | 18 +- .../src/pages/FlowEditor/lib/store/user.ts | 8 +- 3 files changed, 220 insertions(+), 11 deletions(-) create mode 100644 editor.planx.uk/src/components/EditorNavMenu.test.tsx diff --git a/editor.planx.uk/src/components/EditorNavMenu.test.tsx b/editor.planx.uk/src/components/EditorNavMenu.test.tsx new file mode 100644 index 0000000000..6b83c7355f --- /dev/null +++ b/editor.planx.uk/src/components/EditorNavMenu.test.tsx @@ -0,0 +1,205 @@ +import { within } from "@testing-library/react"; +import { useStore } from "pages/FlowEditor/lib/store"; +import React from "react"; +import * as ReactNavi from "react-navi"; +import { setup } from "testUtils"; +import { Mocked, vi } from "vitest"; + +import EditorNavMenu from "./EditorNavMenu"; + +vi.mock("react-navi", () => ({ + useCurrentRoute: vi.fn(), + useNavigation: () => ({ navigate: vi.fn() }), + // Mock completed loading process + useLoadingRoute: () => undefined, +})); + +const mockNavi = ReactNavi as Mocked; + +let mockTeamName: string | undefined = undefined; +let mockFlowName: string | undefined = undefined; +let mockAnalyticsLink: string | undefined = undefined; +const mockGetUserRoleForCurrentTeam = vi.fn(); + +vi.mock("pages/FlowEditor/lib/store", async () => ({ + useStore: vi.fn(() => [ + mockTeamName, + mockFlowName, + mockAnalyticsLink, + mockGetUserRoleForCurrentTeam(), + ]), +})); + +describe("globalLayoutRoutes", () => { + beforeEach(() => { + mockNavi.useCurrentRoute.mockReturnValue({ + url: { href: "/" }, + } as ReturnType); + }); + + it("does not display for teamEditors", () => { + mockGetUserRoleForCurrentTeam.mockReturnValue("teamEditor"); + + const { queryAllByRole } = setup(); + const menuItems = queryAllByRole("listitem"); + expect(menuItems).toHaveLength(0); + }); + + it("displays for platformAdmins", () => { + mockGetUserRoleForCurrentTeam.mockReturnValue("platformAdmin"); + + const { getAllByRole } = setup(); + const menuItems = getAllByRole("listitem"); + expect(menuItems).toHaveLength(3); + expect(within(menuItems[0]).getByText("Select a team")).toBeInTheDocument(); + }); +}); + +describe("teamLayoutRoutes", () => { + beforeEach(() => { + mockNavi.useCurrentRoute.mockReturnValue({ + url: { href: "/test-team" }, + } as ReturnType); + mockTeamName = "test-team"; + }); + + it("does not display for teamViewers", () => { + mockGetUserRoleForCurrentTeam.mockReturnValue("teamViewer"); + + const { queryAllByRole } = setup(); + const menuItems = queryAllByRole("listitem"); + expect(menuItems).toHaveLength(0); + }); + + it("displays for teamEditors", () => { + mockGetUserRoleForCurrentTeam.mockReturnValue("teamEditor"); + + const { getAllByRole } = setup(); + const menuItems = getAllByRole("listitem"); + expect(menuItems).toHaveLength(4); + expect(within(menuItems[0]).getByText("Services")).toBeInTheDocument(); + }); + + it("displays for platformAdmins", () => { + mockGetUserRoleForCurrentTeam.mockReturnValue("platformAdmin"); + + const { getAllByRole } = setup(); + const menuItems = getAllByRole("listitem"); + expect(menuItems).toHaveLength(4); + expect(within(menuItems[0]).getByText("Services")).toBeInTheDocument(); + }); +}); + +describe("flowLayoutRoutes", () => { + beforeEach(() => { + mockNavi.useCurrentRoute.mockReturnValue({ + url: { href: "/test-team/test-flow" }, + } as ReturnType); + mockTeamName = "test-team"; + mockFlowName = "test-flow"; + }); + + it("does not display for teamViewers", () => { + mockGetUserRoleForCurrentTeam.mockReturnValue("teamViewer"); + + const { queryAllByRole } = setup(); + const menuItems = queryAllByRole("listitem"); + expect(menuItems).toHaveLength(0); + }); + + it("displays for teamEditors", () => { + mockGetUserRoleForCurrentTeam.mockReturnValue("teamEditor"); + + const { getAllByRole, getByLabelText } = setup(); + const menuItems = getAllByRole("listitem"); + expect(menuItems).toHaveLength(5); + expect(getByLabelText("Submissions log")).toBeInTheDocument(); + }); + + it("displays for platformAdmins", () => { + mockGetUserRoleForCurrentTeam.mockReturnValue("platformAdmin"); + + const { getAllByRole, getByLabelText } = setup(); + const menuItems = getAllByRole("listitem"); + expect(menuItems).toHaveLength(5); + expect(getByLabelText("Submissions log")).toBeInTheDocument(); + }); +}); + +describe("flowAnalyticsRoute", () => { + beforeEach(() => { + mockNavi.useCurrentRoute.mockReturnValue({ + url: { href: "/test-team/test-flow" }, + } as ReturnType); + mockTeamName = "test-team"; + mockFlowName = "test-flow"; + mockGetUserRoleForCurrentTeam.mockReturnValue("teamEditor"); + }); + + it("is disabled without an analytics link", () => { + const { getByRole } = setup(); + expect(getByRole("button", { name: /Analytics/ })).toBeDisabled(); + }); + + it("is enabled with an analytics link", () => { + mockAnalyticsLink = "https://link-to-metabase"; + + const { getByRole } = setup(); + expect(getByRole("button", { name: /Analytics/ })).not.toBeDisabled(); + }); +}); + +describe("layout", () => { + it("displays in a full mode on global routes", () => { + mockNavi.useCurrentRoute.mockReturnValue({ + url: { href: "/" }, + } as ReturnType); + mockGetUserRoleForCurrentTeam.mockReturnValue("platformAdmin"); + + const { queryAllByRole, queryByLabelText } = setup(); + const menuItems = queryAllByRole("listitem"); + + // Tooltip not present + expect(queryByLabelText("Select a team")).not.toBeInTheDocument(); + + // Full text present + expect(within(menuItems[0]).getByText("Select a team")).toBeInTheDocument(); + }); + + it("displays in a full mode on team routes", () => { + mockNavi.useCurrentRoute.mockReturnValue({ + url: { href: "/test-team" }, + } as ReturnType); + mockGetUserRoleForCurrentTeam.mockReturnValue("platformAdmin"); + mockTeamName = "test-team"; + + const { queryAllByRole, queryByLabelText } = setup(); + const menuItems = queryAllByRole("listitem"); + + // Tooltip not present + expect(queryByLabelText("Services")).not.toBeInTheDocument(); + + // Full text present + expect(within(menuItems[0]).getByText("Services")).toBeInTheDocument(); + }); + + it("displays in a compact mode on flow routes", () => { + mockNavi.useCurrentRoute.mockReturnValue({ + url: { href: "/test-team/test-flow" }, + } as ReturnType); + mockGetUserRoleForCurrentTeam.mockReturnValue("platformAdmin"); + mockTeamName = "test-team"; + mockFlowName = "test-flow"; + + const { queryAllByRole, getByLabelText } = setup(); + const menuItems = queryAllByRole("listitem"); + + // Tooltip present + expect(getByLabelText("Submissions log")).toBeInTheDocument(); + + // Full text present + expect( + within(menuItems[0]).queryByText("Submissions log"), + ).not.toBeInTheDocument(); + }); +}); diff --git a/editor.planx.uk/src/components/EditorNavMenu.tsx b/editor.planx.uk/src/components/EditorNavMenu.tsx index 05ad74e1a5..ff7d56fb3b 100644 --- a/editor.planx.uk/src/components/EditorNavMenu.tsx +++ b/editor.planx.uk/src/components/EditorNavMenu.tsx @@ -95,13 +95,12 @@ function EditorNavMenu() { const { navigate } = useNavigation(); const { url } = useCurrentRoute(); const isRouteLoading = useLoadingRoute(); - const [teamSlug, flowSlug, flowAnalyticsLink, role] = - useStore((state) => [ - state.teamSlug, - state.flowSlug, - state.flowAnalyticsLink, - state.getUserRoleForCurrentTeam() - ]); + const [teamSlug, flowSlug, flowAnalyticsLink, role] = useStore((state) => [ + state.teamSlug, + state.flowSlug, + state.flowAnalyticsLink, + state.getUserRoleForCurrentTeam(), + ]); const isActive = (route: string) => url.href.endsWith(route); @@ -242,7 +241,9 @@ function EditorNavMenu() { const { routes, compact } = getRoutesForUrl(url.href); - const visibleRoutes = routes.filter(({ accessibleBy }) => role && accessibleBy.includes(role)); + const visibleRoutes = routes.filter( + ({ accessibleBy }) => role && accessibleBy.includes(role), + ); // Hide menu if the user does not have a selection of items if (visibleRoutes.length < 2) return null; @@ -256,6 +257,7 @@ function EditorNavMenu() { { const { user, teamSlug } = get(); - if (!user || !teamSlug) return; + if (!user) return; if (user.isPlatformAdmin) return "platformAdmin"; - const currentUserTeam = user.teams.find(({ team: { slug } }) => slug === teamSlug ); + const currentUserTeam = user.teams.find( + ({ team: { slug } }) => slug === teamSlug, + ); if (!currentUserTeam) return; return currentUserTeam.role; - } + }, }); const getLoggedInUser = async () => { From d325ed738a08125980309feae265c9d5f76f898c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Fri, 22 Nov 2024 12:34:09 +0000 Subject: [PATCH 6/9] test: Add retry to flaky react unit test (#4002) --- .../@planx/components/Send/Public.test.tsx | 45 ++++++++++--------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/editor.planx.uk/src/@planx/components/Send/Public.test.tsx b/editor.planx.uk/src/@planx/components/Send/Public.test.tsx index 7feb633b89..004b2c3c16 100644 --- a/editor.planx.uk/src/@planx/components/Send/Public.test.tsx +++ b/editor.planx.uk/src/@planx/components/Send/Public.test.tsx @@ -5,7 +5,7 @@ import { FullStore, useStore } from "pages/FlowEditor/lib/store"; import React from "react"; import { act } from "react-dom/test-utils"; import { setup } from "testUtils"; -import { vi } from "vitest"; +import { it,vi } from "vitest"; import { axe } from "vitest-axe"; import hasuraEventsResponseMock from "./mocks/hasuraEventsResponseMock"; @@ -179,29 +179,34 @@ describe("Uniform overrides for Buckinghamshire", () => { }); }); -it("generates a valid breadcrumb", async () => { - const handleSubmit = vi.fn(); +it( + "generates a valid breadcrumb", + async () => { + const handleSubmit = vi.fn(); - setup( - , - ); + setup( + , + ); - await waitFor(() => expect(mockAxios.post).toHaveBeenCalledTimes(1)); - expect(handleSubmit).toHaveBeenCalledTimes(1); + await waitFor(() => expect(mockAxios.post).toHaveBeenCalledTimes(1)); + expect(handleSubmit).toHaveBeenCalledTimes(1); - const breadcrumb = handleSubmit.mock.calls[0][0]; + const breadcrumb = handleSubmit.mock.calls[0][0]; - expect(breadcrumb.data).toEqual( - expect.objectContaining({ - bopsSendEventId: hasuraEventsResponseMock.bops.event_id, - uniformSendEventId: hasuraEventsResponseMock.uniform.event_id, - }), - ); -}); + expect(breadcrumb.data).toEqual( + expect.objectContaining({ + bopsSendEventId: hasuraEventsResponseMock.bops.event_id, + uniformSendEventId: hasuraEventsResponseMock.uniform.event_id, + }), + ); + // Flaky test in CI + }, + { retry: 1 }, +); it("should not have any accessibility violations", async () => { const { container } = setup( From a55127fc3ab843bc9990abe856cf3e95931fdc0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Fri, 22 Nov 2024 12:34:26 +0000 Subject: [PATCH 7/9] chore: Replace jest mock with vitest mock (#4003) --- .../PlanningConstraints/Public.test.tsx | 67 ++++++++++--------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.test.tsx b/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.test.tsx index 96a6c8f898..8646fbd0de 100644 --- a/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.test.tsx +++ b/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.test.tsx @@ -4,6 +4,7 @@ import React from "react"; import { act } from "react-dom/test-utils"; import { ErrorBoundary } from "react-error-boundary"; import swr from "swr"; +import useSWR from "swr"; import { setup } from "testUtils"; import { vi } from "vitest"; import { axe } from "vitest-axe"; @@ -21,15 +22,13 @@ const { setState } = useStore; beforeEach(() => vi.clearAllMocks()); -const swrMock = (swr as jest.Mock).mock; - vi.mock("swr", () => ({ default: vi.fn((url: () => string) => { const isGISRequest = url()?.startsWith( - `${import.meta.env.VITE_APP_API_URL}/gis` + `${import.meta.env.VITE_APP_API_URL}/gis`, ); const isRoadsRequest = url()?.startsWith( - `${import.meta.env.VITE_APP_API_URL}/roads` + `${import.meta.env.VITE_APP_API_URL}/roads`, ); if (isGISRequest) return { data: digitalLandResponseMock }; @@ -50,7 +49,7 @@ describe("error state", () => { disclaimer="This page does not include information about historic planning conditions that may apply to this property." handleSubmit={vi.fn()} /> - + , ); expect(getByTestId("error-summary-invalid-graph")).toBeInTheDocument(); @@ -66,7 +65,7 @@ describe("error state", () => { fn="property.constraints.planning" disclaimer="This page does not include information about historic planning conditions that may apply to this property." /> - + , ); const results = await axe(container); expect(results).toHaveNoViolations(); @@ -82,7 +81,7 @@ describe("following a FindProperty component", () => { teamIntegrations: { hasPlanningData: true, }, - }) + }), ); }); @@ -96,11 +95,11 @@ describe("following a FindProperty component", () => { fn="property.constraints.planning" disclaimer="This page does not include information about historic planning conditions that may apply to this property." handleSubmit={handleSubmit} - /> + />, ); expect( - getByRole("heading", { name: "Planning constraints" }) + getByRole("heading", { name: "Planning constraints" }), ).toBeInTheDocument(); await user.click(getByTestId("continue-button")); @@ -115,7 +114,7 @@ describe("following a FindProperty component", () => { description="Things that might affect your project" fn="property.constraints.planning" disclaimer="This page does not include information about historic planning conditions that may apply to this property." - /> + />, ); const results = await axe(container); expect(results).toHaveNoViolations(); @@ -129,14 +128,14 @@ describe("following a FindProperty component", () => { fn="property.constraints.planning" disclaimer="This page does not include information about historic planning conditions that may apply to this property." handleSubmit={vi.fn()} - /> + />, ); expect(swr).toHaveBeenCalled(); // Planning data is called first - const swrURL = swrMock.calls[0][0](); - const swrResponse = swrMock.results[0].value; + const swrURL = (vi.mocked(useSWR).mock.calls[0][0] as () => {})(); + const swrResponse = vi.mocked(useSWR).mock.results[0].value; expect(swrURL).toContain("/gis"); expect(swrResponse).toEqual({ data: digitalLandResponseMock }); @@ -150,14 +149,14 @@ describe("following a FindProperty component", () => { fn="property.constraints.planning" disclaimer="This page does not include information about historic planning conditions that may apply to this property." handleSubmit={vi.fn()} - /> + />, ); expect(swr).toHaveBeenCalled(); // Classified roads are called second - const swrURL = swrMock.calls[1][0](); - const swrResponse = swrMock.results[1].value; + const swrURL = (vi.mocked(useSWR).mock.calls[1][0] as () => {})(); + const swrResponse = vi.mocked(useSWR).mock.results[1].value; expect(swrURL).toContain("/roads"); expect(swrResponse).toEqual({ data: classifiedRoadsResponseMock }); @@ -171,7 +170,7 @@ describe("following a FindProperty component", () => { teamIntegrations: { hasPlanningData: true, }, - }) + }), ); setup( @@ -181,14 +180,16 @@ describe("following a FindProperty component", () => { fn="property.constraints.planning" disclaimer="This page does not include information about historic planning conditions that may apply to this property." handleSubmit={vi.fn()} - /> + />, ); expect(swr).toHaveBeenCalled(); // Planning constraints API still called - const planingConstraintsURL = swrMock.calls[0][0](); - const planingConstraintsResponse = swrMock.results[0].value; + const planingConstraintsURL = ( + vi.mocked(useSWR).mock.calls[0][0] as () => {} + )(); + const planingConstraintsResponse = vi.mocked(useSWR).mock.results[0].value; expect(planingConstraintsURL).toContain("/gis"); expect(planingConstraintsResponse).toEqual({ @@ -196,8 +197,8 @@ describe("following a FindProperty component", () => { }); // Classified roads API not called due to missing USRN - const swrURL = swrMock.calls[1][0](); - const swrResponse = swrMock.results[1].value; + const swrURL = (vi.mocked(useSWR).mock.calls[1][0] as () => {})(); + const swrResponse = vi.mocked(useSWR).mock.results[1].value; expect(swrURL).toBeNull(); expect(swrResponse).toEqual({ data: null }); @@ -211,12 +212,12 @@ describe("following a FindProperty component", () => { fn="property.constraints.planning" disclaimer="This page does not include information about historic planning conditions that may apply to this property." handleSubmit={vi.fn()} - /> + />, ); // Positive constraints visible by default expect( - getByRole("heading", { name: /These are the planning constraints/ }) + getByRole("heading", { name: /These are the planning constraints/ }), ).toBeVisible(); expect(getByRole("button", { name: /Parks and gardens/ })).toBeVisible(); @@ -227,7 +228,7 @@ describe("following a FindProperty component", () => { expect(showNegativeConstraintsButton).toBeVisible(); const negativeConstraintsContainer = getByTestId( - "negative-constraints-list" + "negative-constraints-list", ); expect(negativeConstraintsContainer).not.toBeVisible(); @@ -248,12 +249,12 @@ describe("following a FindProperty component", () => { description="Things that might affect your project" fn="property.constraints.planning" handleSubmit={vi.fn()} - /> + />, ); expect( queryByText( - "This page does not include information about historic planning conditions that may apply to this property." - ) + "This page does not include information about historic planning conditions that may apply to this property.", + ), ).toBeVisible(); }); }); @@ -268,7 +269,7 @@ describe("demo state", () => { hasPlanningData: false, }, teamSlug: "demo", - }) + }), ); }); it("should render an error when teamSlug is demo", async () => { @@ -282,21 +283,21 @@ describe("demo state", () => { disclaimer="This page does not include information about historic planning conditions that may apply to this property." handleSubmit={handleSubmit} /> - + , ); const errorMessage = queryByText( - "Planning Constraints are not enabled for demo users" + "Planning Constraints are not enabled for demo users", ); expect(errorMessage).toBeVisible(); // Check planning constraints has not rendered // reused positive constraints from basic layout test expect( - queryByRole("heading", { name: /These are the planning constraints/ }) + queryByRole("heading", { name: /These are the planning constraints/ }), ).not.toBeInTheDocument(); expect( - queryByRole("button", { name: /Parks and gardens/ }) + queryByRole("button", { name: /Parks and gardens/ }), ).not.toBeInTheDocument(); // Ensure a demo user can continue on in the application From 7094c527aa1a51a72ff07b256c64f131a922ddf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Mon, 25 Nov 2024 09:15:09 +0000 Subject: [PATCH 8/9] fix: External Portal node graph styling (#4004) --- .../components/Flow/components/Portal.tsx | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/editor.planx.uk/src/pages/FlowEditor/components/Flow/components/Portal.tsx b/editor.planx.uk/src/pages/FlowEditor/components/Flow/components/Portal.tsx index aee50252dd..02a92074fa 100644 --- a/editor.planx.uk/src/pages/FlowEditor/components/Flow/components/Portal.tsx +++ b/editor.planx.uk/src/pages/FlowEditor/components/Flow/components/Portal.tsx @@ -89,17 +89,17 @@ const ExternalPortal: React.FC = (props) => { return ( <> + ); }; @@ -141,19 +141,22 @@ const InternalPortal: React.FC = (props) => { return ( <>