From caf3fb9d5b2ed398b372042ab5b5216842a862bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Wed, 25 Oct 2023 14:01:26 +0100 Subject: [PATCH] fix: Failing regression E2E tests (#2337) * chore: Misc tidy ups * chore: Route tidyups * fix: Remove useSWRMutation due to incompatability with react-navi * fix: Typo on Uniform error handling * chore: Bump Playwright * test(wip): Reorder e2e tests --- api.planx.uk/send/uniform.ts | 5 +- e2e/tests/ui-driven/package.json | 2 +- e2e/tests/ui-driven/playwright.config.ts | 2 +- e2e/tests/ui-driven/pnpm-lock.yaml | 27 +++-- e2e/tests/ui-driven/src/create-flow.spec.ts | 112 +++++++++--------- editor.planx.uk/package.json | 2 +- editor.planx.uk/pnpm-lock.yaml | 13 +- .../components/Pay/Public/InviteToPayForm.tsx | 36 +++--- .../components/DelayedLoadingIndicator.tsx | 2 +- editor.planx.uk/src/pages/Pay/MakePayment.tsx | 1 - editor.planx.uk/src/routes/pay.tsx | 56 ++++----- editor.planx.uk/src/routes/team.tsx | 2 +- 12 files changed, 138 insertions(+), 122 deletions(-) diff --git a/api.planx.uk/send/uniform.ts b/api.planx.uk/send/uniform.ts index 3fad3a611f..4ec34f1b0c 100644 --- a/api.planx.uk/send/uniform.ts +++ b/api.planx.uk/send/uniform.ts @@ -225,7 +225,10 @@ async function authenticate({ throw Error("Failed to authenticate to Uniform - no access token returned"); } - if (!response.data["organisation-name"] || response.data["organisation-id"]) { + if ( + !response.data["organisation-name"] || + !response.data["organisation-id"] + ) { throw Error( "Failed to authenticate to Uniform - no organisation details returned", ); diff --git a/e2e/tests/ui-driven/package.json b/e2e/tests/ui-driven/package.json index 0ec4cc3f3d..483598cc75 100644 --- a/e2e/tests/ui-driven/package.json +++ b/e2e/tests/ui-driven/package.json @@ -20,7 +20,7 @@ "uuid": "^9.0.0" }, "devDependencies": { - "@playwright/test": "^1.35.1", + "@playwright/test": "^1.39.0", "@types/node": "18.16.1", "eslint-plugin-playwright": "^0.12.0" } diff --git a/e2e/tests/ui-driven/playwright.config.ts b/e2e/tests/ui-driven/playwright.config.ts index 1e9b9cee58..e76e9771e1 100644 --- a/e2e/tests/ui-driven/playwright.config.ts +++ b/e2e/tests/ui-driven/playwright.config.ts @@ -46,7 +46,7 @@ const config: PlaywrightTestConfig = { webServer: { command: `pnpm ui`, port: 3000, - reuseExistingServer: false, + reuseExistingServer: !process.env.CI, }, /* Configure projects for major browsers */ projects: [ diff --git a/e2e/tests/ui-driven/pnpm-lock.yaml b/e2e/tests/ui-driven/pnpm-lock.yaml index 92aac11b90..894fd3d4b7 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.35.1 - version: 1.37.1 + specifier: ^1.39.0 + version: 1.39.0 '@types/node': specifier: 18.16.1 version: 18.16.1 @@ -515,15 +515,12 @@ packages: '@nodelib/fs.scandir': 2.1.5 fastq: 1.15.0 - /@playwright/test@1.37.1: - resolution: {integrity: sha512-bq9zTli3vWJo8S3LwB91U0qDNQDpEXnw7knhxLM0nwDvexQAwx9tO8iKDZSqqneVq+URd/WIoz+BALMqUTgdSg==} + /@playwright/test@1.39.0: + resolution: {integrity: sha512-3u1iFqgzl7zr004bGPYiN/5EZpRUSFddQBra8Rqll5N0/vfpqlP9I9EXqAoGacuAbX6c9Ulg/Cjqglp5VkK6UQ==} engines: {node: '>=16'} hasBin: true dependencies: - '@types/node': 18.16.1 - playwright-core: 1.37.1 - optionalDependencies: - fsevents: 2.3.2 + playwright: 1.39.0 dev: true /@popperjs/core@2.11.8: @@ -1935,10 +1932,20 @@ packages: engines: {node: '>=8'} dev: false - /playwright-core@1.37.1: - resolution: {integrity: sha512-17EuQxlSIYCmEMwzMqusJ2ztDgJePjrbttaefgdsiqeLWidjYz9BxXaTaZWxH1J95SHGk6tjE+dwgWILJoUZfA==} + /playwright-core@1.39.0: + resolution: {integrity: sha512-+k4pdZgs1qiM+OUkSjx96YiKsXsmb59evFoqv8SKO067qBA+Z2s/dCzJij/ZhdQcs2zlTAgRKfeiiLm8PQ2qvw==} + engines: {node: '>=16'} + hasBin: true + dev: true + + /playwright@1.39.0: + resolution: {integrity: sha512-naE5QT11uC/Oiq0BwZ50gDmy8c8WLPRTEWuSSFVG2egBka/1qMoSqYQcROMT9zLwJ86oPofcTH2jBY/5wWOgIw==} engines: {node: '>=16'} hasBin: true + dependencies: + playwright-core: 1.39.0 + optionalDependencies: + fsevents: 2.3.2 dev: true /prelude-ls@1.2.1: diff --git a/e2e/tests/ui-driven/src/create-flow.spec.ts b/e2e/tests/ui-driven/src/create-flow.spec.ts index 51773d11c1..4c522859c6 100644 --- a/e2e/tests/ui-driven/src/create-flow.spec.ts +++ b/e2e/tests/ui-driven/src/create-flow.spec.ts @@ -35,62 +35,6 @@ test.describe("Navigation", () => { await tearDownTestContext(context); }); - test("Create a flow", async ({ browser }) => { - const page = await getTeamPage({ - browser, - userId: context.user!.id!, - teamName: context.team.name, - }); - - page.on("dialog", (dialog) => dialog.accept(serviceProps.name)); - await page.locator("button", { hasText: "Add a new service" }).click(); - - // update context to allow flow to be torn down - context.flow = { ...serviceProps }; - - await page.locator("li.hanger > a").click(); - await page.getByRole("dialog").waitFor(); - - const questionText = "Is this a test?"; - await page.getByPlaceholder("Text").fill(questionText); - - await page.locator("button").filter({ hasText: "add new" }).click(); - await page.getByPlaceholder("Option").fill("Yes"); - - await page.locator("button").filter({ hasText: "add new" }).click(); - await page.getByPlaceholder("Option").nth(1).fill("No"); - - await page.locator("button").filter({ hasText: "Create question" }).click(); - await expect( - page.locator("a").filter({ hasText: questionText }), - ).toBeVisible(); - - // Add a notice to the "Yes" path - const yesBranch = page.locator("#flow .card .options .option").nth(0); - await yesBranch.locator(".hanger > a").click(); - await page.getByRole("dialog").waitFor(); - - await page.locator("select").selectOption({ label: "Notice" }); - const yesBranchNoticeText = "Yes! this is a test"; - await page.getByPlaceholder("Notice").fill(yesBranchNoticeText); - await page.locator("button").filter({ hasText: "Create notice" }).click(); - - // Add a notice to the "No" path - const noBranch = page.locator("#flow .card .options .option").nth(1); - await noBranch.locator(".hanger > a").click(); - await page.getByRole("dialog").waitFor(); - - await page.locator("select").selectOption({ label: "Notice" }); - const noBranchNoticeText = "Sorry, this is a test"; - await page.getByPlaceholder("Notice").fill(noBranchNoticeText); - await page.locator("button").filter({ hasText: "Create notice" }).click(); - - const nodes = page.locator(".card"); - await expect(nodes.getByText(questionText)).toBeVisible(); - await expect(nodes.getByText(yesBranchNoticeText)).toBeVisible(); - await expect(nodes.getByText(noBranchNoticeText)).toBeVisible(); - }); - test("user data persists on page refresh @regression", async ({ browser, }) => { @@ -151,6 +95,62 @@ test.describe("Navigation", () => { await expect(teamSlugInHeader).toBeHidden(); }); + test("Create a flow", async ({ browser }) => { + const page = await getTeamPage({ + browser, + userId: context.user!.id!, + teamName: context.team.name, + }); + + page.on("dialog", (dialog) => dialog.accept(serviceProps.name)); + await page.locator("button", { hasText: "Add a new service" }).click(); + + // update context to allow flow to be torn down + context.flow = { ...serviceProps }; + + await page.locator("li.hanger > a").click(); + await page.getByRole("dialog").waitFor(); + + const questionText = "Is this a test?"; + await page.getByPlaceholder("Text").fill(questionText); + + await page.locator("button").filter({ hasText: "add new" }).click(); + await page.getByPlaceholder("Option").fill("Yes"); + + await page.locator("button").filter({ hasText: "add new" }).click(); + await page.getByPlaceholder("Option").nth(1).fill("No"); + + await page.locator("button").filter({ hasText: "Create question" }).click(); + await expect( + page.locator("a").filter({ hasText: questionText }), + ).toBeVisible(); + + // Add a notice to the "Yes" path + const yesBranch = page.locator("#flow .card .options .option").nth(0); + await yesBranch.locator(".hanger > a").click(); + await page.getByRole("dialog").waitFor(); + + await page.locator("select").selectOption({ label: "Notice" }); + const yesBranchNoticeText = "Yes! this is a test"; + await page.getByPlaceholder("Notice").fill(yesBranchNoticeText); + await page.locator("button").filter({ hasText: "Create notice" }).click(); + + // Add a notice to the "No" path + const noBranch = page.locator("#flow .card .options .option").nth(1); + await noBranch.locator(".hanger > a").click(); + await page.getByRole("dialog").waitFor(); + + await page.locator("select").selectOption({ label: "Notice" }); + const noBranchNoticeText = "Sorry, this is a test"; + await page.getByPlaceholder("Notice").fill(noBranchNoticeText); + await page.locator("button").filter({ hasText: "Create notice" }).click(); + + const nodes = page.locator(".card"); + await expect(nodes.getByText(questionText)).toBeVisible(); + await expect(nodes.getByText(yesBranchNoticeText)).toBeVisible(); + await expect(nodes.getByText(noBranchNoticeText)).toBeVisible(); + }); + test("Preview a created flow", async ({ browser }: { browser: Browser }) => { const page = await createAuthenticatedSession({ browser, diff --git a/editor.planx.uk/package.json b/editor.planx.uk/package.json index 4624a292a3..b64e5dcedc 100644 --- a/editor.planx.uk/package.json +++ b/editor.planx.uk/package.json @@ -84,7 +84,7 @@ "scroll-into-view-if-needed": "^2.2.31", "sharedb": "^3.3.1", "striptags": "^3.2.0", - "swr": "^2.2.0", + "swr": "^2.2.4", "tippy.js": "^6.3.7", "uuid": "^9.0.0", "wkt": "^0.1.1", diff --git a/editor.planx.uk/pnpm-lock.yaml b/editor.planx.uk/pnpm-lock.yaml index 9db4d8a538..f603686b45 100644 --- a/editor.planx.uk/pnpm-lock.yaml +++ b/editor.planx.uk/pnpm-lock.yaml @@ -256,8 +256,8 @@ dependencies: specifier: ^3.2.0 version: 3.2.0 swr: - specifier: ^2.2.0 - version: 2.2.0(react@18.2.0) + specifier: ^2.2.4 + version: 2.2.4(react@18.2.0) tippy.js: specifier: ^6.3.7 version: 6.3.7 @@ -9892,6 +9892,10 @@ packages: string-width: 5.1.2 dev: true + /client-only@0.0.1: + resolution: {integrity: sha512-IV3Ou0jSMzZrd3pZ48nLkT9DA7Ag1pnPzaiQhpW7c3RbcqqzvzzVu+L8gfqMp/8IM2MQtSiqaCxrrcfu8I8rMA==} + dev: false + /cliui@7.0.4: resolution: {integrity: sha512-OcRE68cOsVMXp1Yvonl/fzkQOyjLSu/8bhPDfQt0e0/Eb283TKP20Fs2MqoPsr9SwA595rRCA+QMzYc9nBP+JQ==} dependencies: @@ -19364,11 +19368,12 @@ packages: webpack: 5.88.1(@swc/core@1.3.71)(esbuild@0.14.54) dev: true - /swr@2.2.0(react@18.2.0): - resolution: {integrity: sha512-AjqHOv2lAhkuUdIiBu9xbuettzAzWXmCEcLONNKJRba87WAefz8Ca9d6ds/SzrPc235n1IxWYdhJ2zF3MNUaoQ==} + /swr@2.2.4(react@18.2.0): + resolution: {integrity: sha512-njiZ/4RiIhoOlAaLYDqwz5qH/KZXVilRLvomrx83HjzCWTfa+InyfAjv05PSFxnmLzZkNO9ZfvgoqzAaEI4sGQ==} peerDependencies: react: ^16.11.0 || ^17.0.0 || ^18.0.0 dependencies: + client-only: 0.0.1 react: 18.2.0 use-sync-external-store: 1.2.0(react@18.2.0) dev: false diff --git a/editor.planx.uk/src/@planx/components/Pay/Public/InviteToPayForm.tsx b/editor.planx.uk/src/@planx/components/Pay/Public/InviteToPayForm.tsx index 860074210a..91026798dd 100644 --- a/editor.planx.uk/src/@planx/components/Pay/Public/InviteToPayForm.tsx +++ b/editor.planx.uk/src/@planx/components/Pay/Public/InviteToPayForm.tsx @@ -13,10 +13,9 @@ import { WarningContainer } from "@planx/components/shared/Preview/WarningContai import DelayedLoadingIndicator from "components/DelayedLoadingIndicator"; import { useFormik } from "formik"; import { useStore } from "pages/FlowEditor/lib/store"; -import React, { useEffect } from "react"; +import React, { useEffect, useState } from "react"; import { useCurrentRoute, useNavigation } from "react-navi"; import { isPreviewOnlyDomain } from "routes/utils"; -import useSWRMutation from "swr/mutation"; import { ApplicationPath } from "types"; import ErrorWrapper from "ui/ErrorWrapper"; import Input from "ui/Input"; @@ -88,6 +87,8 @@ const InviteToPayForm: React.FC = ({ paymentStatus, }) => { const [sessionId, path] = useStore((state) => [state.sessionId, state.path]); + const [isLoading, setIsLoading] = useState(false); + const [error, setError] = useState(false); const isSaveReturn = path === ApplicationPath.SaveAndReturn; const navigation = useNavigation(); const { @@ -99,38 +100,39 @@ const InviteToPayForm: React.FC = ({ window.scrollTo(0, 0); }, []); - const postRequest = async ( - url: string, - { arg }: { arg: CreatePaymentRequest }, - ) => { + const postRequest = async (createPaymentRequest: CreatePaymentRequest) => { + setIsLoading(true); + const url = `${process.env.REACT_APP_API_URL}/invite-to-pay/${sessionId}`; const response = await fetch(url, { method: "POST", - body: JSON.stringify(arg), + body: JSON.stringify(createPaymentRequest), headers: { "Content-Type": "application/json", }, }); + setIsLoading(false); if (!response.ok) { + setError(true); throw new Error( - `Error generating payment request for session ${sessionId}: ${error}`, + `Error generating payment request for session ${sessionId}: ${response.body}`, ); } return response.json(); }; - const { trigger, isMutating, error } = useSWRMutation( - `${process.env.REACT_APP_API_URL}/invite-to-pay/${sessionId}`, - postRequest, - ); - const onSubmit = async (values: FormValues) => { const createPaymentRequest: CreatePaymentRequest = { ...values, sessionPreviewKeys: SESSION_PREVIEW_KEYS, }; - const paymentRequest: PaymentRequest = await trigger(createPaymentRequest); - if (!error && paymentRequest.id) - redirectToConfirmationPage(paymentRequest.id); + try { + const paymentRequest: PaymentRequest = await postRequest( + createPaymentRequest, + ); + if (paymentRequest.id) redirectToConfirmationPage(paymentRequest.id); + } catch (error) { + console.error(error); + } }; const redirectToConfirmationPage = (paymentRequestId: string) => { @@ -153,7 +155,7 @@ const InviteToPayForm: React.FC = ({ validationSchema, }); - return isMutating ? ( + return isLoading ? ( ) : ( diff --git a/editor.planx.uk/src/components/DelayedLoadingIndicator.tsx b/editor.planx.uk/src/components/DelayedLoadingIndicator.tsx index 347e5ebe9a..d09f68b9f3 100644 --- a/editor.planx.uk/src/components/DelayedLoadingIndicator.tsx +++ b/editor.planx.uk/src/components/DelayedLoadingIndicator.tsx @@ -33,7 +33,7 @@ const DelayedLoadingIndicator: React.FC<{ > - {text ?? "Loading…"} + {text ?? "Loading..."} ) : null; diff --git a/editor.planx.uk/src/pages/Pay/MakePayment.tsx b/editor.planx.uk/src/pages/Pay/MakePayment.tsx index aa190944ad..f577ee1e8a 100644 --- a/editor.planx.uk/src/pages/Pay/MakePayment.tsx +++ b/editor.planx.uk/src/pages/Pay/MakePayment.tsx @@ -305,7 +305,6 @@ function computePaymentState(govUkPayment: GovUKPayment | null): PaymentState { ) { return PaymentState.Pending; } - // PaymentStatus.cancelled, PaymentStatus.error, PaymentStatus.failed, return PaymentState.Failed; } diff --git a/editor.planx.uk/src/routes/pay.tsx b/editor.planx.uk/src/routes/pay.tsx index ca0da6e521..65bb57bfab 100644 --- a/editor.planx.uk/src/routes/pay.tsx +++ b/editor.planx.uk/src/routes/pay.tsx @@ -4,6 +4,7 @@ import { publicClient } from "lib/graphql"; import { getRetentionPeriod } from "lib/pay"; import { compose, + map, mount, NaviRequest, redirect, @@ -37,40 +38,39 @@ const payRoutes = compose( }), mount({ - "/": route(async (req) => { + "/": map(async (req) => { const paymentRequest = await getPaymentRequest(req); - if (!paymentRequest) { - return { - title: makeTitle("Sorry, we can’t find that payment link"), - view: ( - - Please check you have the right link. If it still doesn’t work, it - may mean the payment link has expired or payment has already been - made. -
-
- Please contact the person who invited you to pay. - - ), - }; - } - return { + if (!paymentRequest) return redirect("./not-found"); + + return route({ title: makeTitle("Make a payment"), view: , - }; + }); + }), + "/not-found": route({ + title: makeTitle("Sorry, we can’t find that payment link"), + view: ( + + Please check you have the right link. If it still doesn’t work, it may + mean the payment link has expired or payment has already been made. +
+
+ Please contact the person who invited you to pay. + + ), }), - "/invite": route(async (req) => { + "/invite": map(async (req) => { const paymentRequest = await getPaymentRequest(req); - if (!paymentRequest) { - return { - title: makeTitle("Failed to generate payment request"), - view: , - }; - } - return { + if (!paymentRequest) return redirect("./failed"); + + return route({ title: makeTitle("Invite to pay"), view: , - }; + }); + }), + "/invite/failed": route({ + title: makeTitle("Failed to generate payment request"), + view: , }), "/pages/:page": redirect( (req) => `../../../preview/pages/${req.params.page}`, @@ -84,7 +84,7 @@ const payRoutes = compose( const getPaymentRequest = async ( req: NaviRequest, ): Promise => { - const paymentRequestId = req.params["paymentRequestId"]; + const paymentRequestId = req.query["paymentRequestId"]; if (paymentRequestId) { const paymentRequest = await fetchPaymentRequest(paymentRequestId); return paymentRequest; diff --git a/editor.planx.uk/src/routes/team.tsx b/editor.planx.uk/src/routes/team.tsx index 12e3f355b2..5279c3afdd 100644 --- a/editor.planx.uk/src/routes/team.tsx +++ b/editor.planx.uk/src/routes/team.tsx @@ -14,7 +14,7 @@ let cached: { flowSlug?: string; teamSlug?: string } = { }; const routes = compose( - withView(teamView), + withView(async (req) => await teamView(req)), mount({ "/": route(() => ({