From 95bd622159573402e907950ff309c3c76aa14e47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Tue, 23 Apr 2024 09:22:07 +0100 Subject: [PATCH] fix: Break link between `isInviteToPay` Pay component prop and `paidViaInviteToPay` metadata value (#3038) --- api.planx.uk/modules/pay/middleware.ts | 9 ++-- api.planx.uk/package.json | 2 +- api.planx.uk/pnpm-lock.yaml | 8 ++-- api.planx.uk/tests/mocks/inviteToPayMocks.ts | 2 +- e2e/tests/api-driven/package.json | 2 +- e2e/tests/api-driven/pnpm-lock.yaml | 8 ++-- .../src/invite-to-pay/mocks/flow.json | 6 +-- e2e/tests/ui-driven/package.json | 2 +- e2e/tests/ui-driven/pnpm-lock.yaml | 8 ++-- .../src/mocks/flows/invite-to-pay-flow.ts | 6 +-- .../ui-driven/src/mocks/flows/pay-flow.json | 15 +++++-- editor.planx.uk/package.json | 2 +- editor.planx.uk/pnpm-lock.yaml | 10 ++--- .../src/@planx/components/Pay/Editor.test.tsx | 32 ++------------ .../src/@planx/components/Pay/Editor.tsx | 24 +++++----- .../src/@planx/components/Pay/Public/Pay.tsx | 8 ++-- .../src/@planx/components/Pay/model.test.ts | 21 +++++++-- .../src/@planx/components/Pay/model.ts | 44 +++++++++++++------ 18 files changed, 112 insertions(+), 97 deletions(-) diff --git a/api.planx.uk/modules/pay/middleware.ts b/api.planx.uk/modules/pay/middleware.ts index 3f687ba710..b036f0c09c 100644 --- a/api.planx.uk/modules/pay/middleware.ts +++ b/api.planx.uk/modules/pay/middleware.ts @@ -137,10 +137,11 @@ export async function buildPaymentPayload( } // Convert metadata to format required by GovPay - const metadata = formatGovPayMetadata( - res.locals.govPayMetadata, - res.locals.passport, - ); + const metadata = formatGovPayMetadata({ + metadata: res.locals.govPayMetadata, + userPassport: res.locals.passport, + paidViaInviteToPay: true, + }); const createPaymentBody: GovPayCreatePayment = { amount: parseInt(req.params.paymentAmount), diff --git a/api.planx.uk/package.json b/api.planx.uk/package.json index 820a7136e3..a8a33d00e3 100644 --- a/api.planx.uk/package.json +++ b/api.planx.uk/package.json @@ -4,7 +4,7 @@ "private": true, "dependencies": { "@airbrake/node": "^2.1.8", - "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#52f76e7", + "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#4e67b2e", "@types/isomorphic-fetch": "^0.0.36", "adm-zip": "^0.5.10", "aws-sdk": "^2.1467.0", diff --git a/api.planx.uk/pnpm-lock.yaml b/api.planx.uk/pnpm-lock.yaml index 1ca0966e55..951a25adb8 100644 --- a/api.planx.uk/pnpm-lock.yaml +++ b/api.planx.uk/pnpm-lock.yaml @@ -12,8 +12,8 @@ dependencies: specifier: ^2.1.8 version: 2.1.8 '@opensystemslab/planx-core': - specifier: git+https://github.com/theopensystemslab/planx-core#52f76e7 - version: github.com/theopensystemslab/planx-core/52f76e7 + specifier: git+https://github.com/theopensystemslab/planx-core#4e67b2e + version: github.com/theopensystemslab/planx-core/4e67b2e '@types/isomorphic-fetch': specifier: ^0.0.36 version: 0.0.36 @@ -8411,8 +8411,8 @@ packages: resolution: {integrity: sha512-iC+8Io04lddc+mVqQ9AZ7OQ2MrUKGN+oIQyq1vemgt46jwCwLfhq7/pwnBnNXXXZb8VTVLKwp9EDkx+ryxIWmg==} dev: false - github.com/theopensystemslab/planx-core/52f76e7: - resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/52f76e7} + github.com/theopensystemslab/planx-core/4e67b2e: + resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/4e67b2e} name: '@opensystemslab/planx-core' version: 1.0.0 prepare: true diff --git a/api.planx.uk/tests/mocks/inviteToPayMocks.ts b/api.planx.uk/tests/mocks/inviteToPayMocks.ts index 705876577e..ada75b529c 100644 --- a/api.planx.uk/tests/mocks/inviteToPayMocks.ts +++ b/api.planx.uk/tests/mocks/inviteToPayMocks.ts @@ -129,7 +129,7 @@ export const createPaymentRequestQueryMock = { sessionPreviewData: sessionPreviewData, govPayMetadata: [ { key: "source", value: "PlanX" }, - { key: "isInviteToPay", value: true }, + { key: "paidViaInviteToPay", value: true }, { key: "flow", value: validSession.flow.slug }, ], }, diff --git a/e2e/tests/api-driven/package.json b/e2e/tests/api-driven/package.json index 928afa4805..c4ed6b9091 100644 --- a/e2e/tests/api-driven/package.json +++ b/e2e/tests/api-driven/package.json @@ -6,7 +6,7 @@ }, "dependencies": { "@cucumber/cucumber": "^9.3.0", - "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#52f76e7", + "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#4e67b2e", "axios": "^1.6.8", "dotenv": "^16.3.1", "dotenv-expand": "^10.0.0", diff --git a/e2e/tests/api-driven/pnpm-lock.yaml b/e2e/tests/api-driven/pnpm-lock.yaml index d4153f3fe8..1735868ba8 100644 --- a/e2e/tests/api-driven/pnpm-lock.yaml +++ b/e2e/tests/api-driven/pnpm-lock.yaml @@ -9,8 +9,8 @@ dependencies: specifier: ^9.3.0 version: 9.3.0 '@opensystemslab/planx-core': - specifier: git+https://github.com/theopensystemslab/planx-core#52f76e7 - version: github.com/theopensystemslab/planx-core/52f76e7 + specifier: git+https://github.com/theopensystemslab/planx-core#4e67b2e + version: github.com/theopensystemslab/planx-core/4e67b2e axios: specifier: ^1.6.8 version: 1.6.8 @@ -2943,8 +2943,8 @@ packages: resolution: {integrity: sha512-iC+8Io04lddc+mVqQ9AZ7OQ2MrUKGN+oIQyq1vemgt46jwCwLfhq7/pwnBnNXXXZb8VTVLKwp9EDkx+ryxIWmg==} dev: false - github.com/theopensystemslab/planx-core/52f76e7: - resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/52f76e7} + github.com/theopensystemslab/planx-core/4e67b2e: + resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/4e67b2e} name: '@opensystemslab/planx-core' version: 1.0.0 prepare: true diff --git a/e2e/tests/api-driven/src/invite-to-pay/mocks/flow.json b/e2e/tests/api-driven/src/invite-to-pay/mocks/flow.json index f793d95c29..6ad9eb5a31 100644 --- a/e2e/tests/api-driven/src/invite-to-pay/mocks/flow.json +++ b/e2e/tests/api-driven/src/invite-to-pay/mocks/flow.json @@ -72800,9 +72800,9 @@ "secondaryPageTitle": "Invite someone else to pay for this application", "instructionsDescription": "

You can pay for your application by using GOV.UK Pay.

Your application will be sent after you have paid the fee. Wait until you see an application sent message before closing your browser.

", "govPayMetadata": [ - { "value": "flow-name", "key": "flow" }, - { "value": "PlanX", "key": "source" }, - { "value": true, "key": "isInviteToPay" } + { "key": "flow", "value": "flow-name" }, + { "key": "source", "value": "PlanX" }, + { "key": "paidViaInviteToPay", "value": "@paidViaInviteToPay" } ] }, "type": 400 diff --git a/e2e/tests/ui-driven/package.json b/e2e/tests/ui-driven/package.json index b8a870b394..54457d14f4 100644 --- a/e2e/tests/ui-driven/package.json +++ b/e2e/tests/ui-driven/package.json @@ -8,7 +8,7 @@ "postinstall": "./install-dependencies.sh" }, "dependencies": { - "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#52f76e7", + "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#4e67b2e", "axios": "^1.6.8", "dotenv": "^16.3.1", "eslint": "^8.56.0", diff --git a/e2e/tests/ui-driven/pnpm-lock.yaml b/e2e/tests/ui-driven/pnpm-lock.yaml index c030f168a3..43fb906b54 100644 --- a/e2e/tests/ui-driven/pnpm-lock.yaml +++ b/e2e/tests/ui-driven/pnpm-lock.yaml @@ -6,8 +6,8 @@ settings: dependencies: '@opensystemslab/planx-core': - specifier: git+https://github.com/theopensystemslab/planx-core#52f76e7 - version: github.com/theopensystemslab/planx-core/52f76e7 + specifier: git+https://github.com/theopensystemslab/planx-core#4e67b2e + version: github.com/theopensystemslab/planx-core/4e67b2e axios: specifier: ^1.6.8 version: 1.6.8 @@ -2609,8 +2609,8 @@ packages: resolution: {integrity: sha512-iC+8Io04lddc+mVqQ9AZ7OQ2MrUKGN+oIQyq1vemgt46jwCwLfhq7/pwnBnNXXXZb8VTVLKwp9EDkx+ryxIWmg==} dev: false - github.com/theopensystemslab/planx-core/52f76e7: - resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/52f76e7} + github.com/theopensystemslab/planx-core/4e67b2e: + resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/4e67b2e} name: '@opensystemslab/planx-core' version: 1.0.0 prepare: true diff --git a/e2e/tests/ui-driven/src/mocks/flows/invite-to-pay-flow.ts b/e2e/tests/ui-driven/src/mocks/flows/invite-to-pay-flow.ts index d30b2956d7..86579512ed 100644 --- a/e2e/tests/ui-driven/src/mocks/flows/invite-to-pay-flow.ts +++ b/e2e/tests/ui-driven/src/mocks/flows/invite-to-pay-flow.ts @@ -85,9 +85,9 @@ const flow: FlowGraph = { yourDetailsTitle: "Your details", yourDetailsLabel: "Your name or organisation name", govPayMetadata: [ - { value: "flow-name", key: "flow" }, - { value: "PlanX", key: "source" }, - { value: true, key: "isInviteToPay" }, + { key: "flow", value: "flow-name" }, + { key: "source", value: "PlanX" }, + { key: "paidViaInviteToPay", value: "@paidViaInviteToPay" }, ], }, type: ComponentType.Pay, diff --git a/e2e/tests/ui-driven/src/mocks/flows/pay-flow.json b/e2e/tests/ui-driven/src/mocks/flows/pay-flow.json index b20d9f528b..b0a670079e 100644 --- a/e2e/tests/ui-driven/src/mocks/flows/pay-flow.json +++ b/e2e/tests/ui-driven/src/mocks/flows/pay-flow.json @@ -22,9 +22,18 @@ "title": "Pay for your application", "description": "

The planning fee covers the cost of processing your application. Find out more about how planning fees are calculated here.

", "govPayMetadata": [ - { "value": "flow-name", "key": "flow" }, - { "value": "PlanX", "key": "source" }, - { "value": true, "key": "isInviteToPay" } + { + "key": "flow", + "value": "flow-name" + }, + { + "key": "source", + "value": "PlanX" + }, + { + "key": "paidViaInviteToPay", + "value": "@paidViaInviteToPay" + } ] }, "type": 400 diff --git a/editor.planx.uk/package.json b/editor.planx.uk/package.json index 86f212585e..10f52299c2 100644 --- a/editor.planx.uk/package.json +++ b/editor.planx.uk/package.json @@ -12,7 +12,7 @@ "@mui/material": "^5.15.2", "@mui/utils": "^5.15.2", "@opensystemslab/map": "^0.8.1", - "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#52f76e7", + "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#4e67b2e", "@tiptap/core": "^2.0.3", "@tiptap/extension-bold": "^2.0.3", "@tiptap/extension-bubble-menu": "^2.1.13", diff --git a/editor.planx.uk/pnpm-lock.yaml b/editor.planx.uk/pnpm-lock.yaml index cf98170858..49d780b6ff 100644 --- a/editor.planx.uk/pnpm-lock.yaml +++ b/editor.planx.uk/pnpm-lock.yaml @@ -38,8 +38,8 @@ dependencies: specifier: ^0.8.1 version: 0.8.1 '@opensystemslab/planx-core': - specifier: git+https://github.com/theopensystemslab/planx-core#52f76e7 - version: github.com/theopensystemslab/planx-core/52f76e7(@types/react@18.2.45) + specifier: git+https://github.com/theopensystemslab/planx-core#4e67b2e + version: github.com/theopensystemslab/planx-core/4e67b2e(@types/react@18.2.45) '@tiptap/core': specifier: ^2.0.3 version: 2.0.3(@tiptap/pm@2.0.3) @@ -21139,9 +21139,9 @@ packages: use-sync-external-store: 1.2.0(react@18.2.0) dev: false - github.com/theopensystemslab/planx-core/52f76e7(@types/react@18.2.45): - resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/52f76e7} - id: github.com/theopensystemslab/planx-core/52f76e7 + github.com/theopensystemslab/planx-core/4e67b2e(@types/react@18.2.45): + resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/4e67b2e} + id: github.com/theopensystemslab/planx-core/4e67b2e name: '@opensystemslab/planx-core' version: 1.0.0 prepare: true diff --git a/editor.planx.uk/src/@planx/components/Pay/Editor.test.tsx b/editor.planx.uk/src/@planx/components/Pay/Editor.test.tsx index ab5d70006c..3357aee964 100644 --- a/editor.planx.uk/src/@planx/components/Pay/Editor.test.tsx +++ b/editor.planx.uk/src/@planx/components/Pay/Editor.test.tsx @@ -67,7 +67,7 @@ describe("Pay component - Editor Modal", () => { ); expect(getByDisplayValue("flow")).toBeInTheDocument(); expect(getByDisplayValue("source")).toBeInTheDocument(); - expect(getByDisplayValue("isInviteToPay")).toBeInTheDocument(); + expect(getByDisplayValue("paidViaInviteToPay")).toBeInTheDocument(); }); it("does not allow default sections to be deleted", () => { @@ -85,32 +85,6 @@ describe("Pay component - Editor Modal", () => { expect(deleteIcons[2]).toBeDisabled(); }); - it("updates the 'isInviteToPay' metadata value inline with the 'isInviteToPay' form value", async () => { - const node = { - data: { - allowInviteToPay: false, - }, - }; - - const { user, getAllByLabelText, getByText } = setup( - - - , - ); - - const keyInputs = getAllByLabelText("Key"); - const valueInputs = getAllByLabelText("Value"); - - expect(keyInputs[2]).toHaveDisplayValue("isInviteToPay"); - expect(valueInputs[2]).toHaveDisplayValue("false"); - - await user.click( - getByText("Allow applicants to invite someone else to pay"), - ); - - expect(valueInputs[2]).toHaveValue("true"); - }); - it("pre-populates existing values", () => { const node = { data: { @@ -177,7 +151,7 @@ describe("Pay component - Editor Modal", () => { govPayMetadata: [ { key: "flow", value: "flowName" }, { key: "source", value: "PlanX" }, - { key: "isInviteToPay", value: "true" }, + { key: "paidViaInviteToPay", value: "@paidViaInviteToPay" }, { key: "deleteMe", value: "abc123" }, ], }, @@ -208,7 +182,7 @@ describe("Pay component - Editor Modal", () => { expect(handleSubmit.mock.lastCall[0].data.govPayMetadata).toEqual([ { key: "flow", value: "flowName" }, { key: "source", value: "PlanX" }, - { key: "isInviteToPay", value: "true" }, + { key: "paidViaInviteToPay", value: "@paidViaInviteToPay" }, ]); }); diff --git a/editor.planx.uk/src/@planx/components/Pay/Editor.tsx b/editor.planx.uk/src/@planx/components/Pay/Editor.tsx index 3b357ef2de..528756c6d2 100644 --- a/editor.planx.uk/src/@planx/components/Pay/Editor.tsx +++ b/editor.planx.uk/src/@planx/components/Pay/Editor.tsx @@ -2,7 +2,10 @@ import DataObjectIcon from "@mui/icons-material/DataObject"; import Box from "@mui/material/Box"; import Link from "@mui/material/Link"; import Typography from "@mui/material/Typography"; -import { GovPayMetadata, ComponentType as TYPES } from "@opensystemslab/planx-core/types"; +import { + ComponentType as TYPES, + GovPayMetadata, +} from "@opensystemslab/planx-core/types"; import { Pay, REQUIRED_GOVPAY_METADATA, @@ -173,8 +176,8 @@ const Component: React.FC = (props: Props) => { value: "PlanX", }, { - key: "isInviteToPay", - value: props.node?.data?.allowInviteToPay ?? true, + key: "paidViaInviteToPay", + value: "@paidViaInviteToPay", }, ], ...parseMoreInformation(props.node?.data), @@ -287,11 +290,14 @@ const Component: React.FC = (props: Props) => { {" "} for more details. - Any values beginning with @ will be dynamically read from data values set throughout the flow. + + Any values beginning with @ will be dynamically read from data + values set throughout the flow. + = (props: Props) => { selected={values.allowInviteToPay} onClick={() => { setFieldValue("allowInviteToPay", !values.allowInviteToPay); - // Update GovUKMetadata - const inviteToPayIndex = values.govPayMetadata?.findIndex( - ({ key }) => key === "isInviteToPay", - ); - setFieldValue( - `govPayMetadata[${inviteToPayIndex}].value`, - !values.allowInviteToPay, - ); }} style={{ width: "100%" }} > diff --git a/editor.planx.uk/src/@planx/components/Pay/Public/Pay.tsx b/editor.planx.uk/src/@planx/components/Pay/Public/Pay.tsx index c144d71c34..4cd9951ecc 100644 --- a/editor.planx.uk/src/@planx/components/Pay/Public/Pay.tsx +++ b/editor.planx.uk/src/@planx/components/Pay/Public/Pay.tsx @@ -72,7 +72,7 @@ function Component(props: Props) { const defaultMetadata = [ { key: "source", value: "PlanX" }, { key: "flow", value: flowSlug }, - { key: "isInviteToPay", value: false }, + { key: "paidViaInviteToPay", value: "@paidViaInviteToPay" }, ]; const metadata = props.govPayMetadata?.length ? props.govPayMetadata @@ -273,9 +273,9 @@ function Component(props: Props) { return ( <> {state.status === "init" || - state.status === "retry" || - state.status === "unsupported_team" || - state.status === "undefined_fee" ? ( + state.status === "retry" || + state.status === "unsupported_team" || + state.status === "undefined_fee" ? ( { const defaults = [ { key: "flow", value: "flowName" }, { key: "source", value: "PlanX" }, - { key: "isInviteToPay", value: "true" }, + { key: "paidViaInviteToPay", value: "@paidViaInviteToPay" }, ]; test("it requires all default values", async () => { const errors = await validate([]); expect(errors).toHaveLength(1); expect(errors[0]).toMatch( - /Keys flow, source and isInviteToPay must be present/ + /Keys flow, source and paidViaInviteToPay must be present/, ); }); @@ -57,7 +57,7 @@ describe("GovPayMetadata Schema", () => { expect(errors[0]).toMatch(/Value is a required field/); }); - test("value cannot be greater than 100 characters", async () => { + test("static values cannot be greater than 100 characters", async () => { const input = [ ...defaults, { @@ -71,6 +71,21 @@ describe("GovPayMetadata Schema", () => { expect(errors[0]).toMatch(/Value length cannot exceed 100 characters/); }); + test("dynamic passport value can be greater than 100 characters", async () => { + const input = [ + ...defaults, + { + key: "myPassportVariable", + value: + "@thisIsAVeryLongPassportVariable.itsUnlikelyToHappenButWeDontNeedToRestrict.theDynamicValueThisReadsWillBeTruncatedAtTimeOfSubmission", + }, + ]; + const result = await validate(input); + + // No errors, input returned from validation + expect(result).toEqual(input); + }); + test("keys must be unique", async () => { const input = [ ...defaults, diff --git a/editor.planx.uk/src/@planx/components/Pay/model.ts b/editor.planx.uk/src/@planx/components/Pay/model.ts index e042589cab..be2fbbea0f 100644 --- a/editor.planx.uk/src/@planx/components/Pay/model.ts +++ b/editor.planx.uk/src/@planx/components/Pay/model.ts @@ -1,14 +1,14 @@ -import { useStore } from "pages/FlowEditor/lib/store"; -import { ApplicationPath, Passport } from "types"; -import { array, boolean, object, string } from "yup"; - -import type { MoreInformation } from "../shared"; +import { formatGovPayMetadata } from "@opensystemslab/planx-core"; import { GovPayMetadata, GovUKCreatePaymentPayload, Passport as IPassport, } from "@opensystemslab/planx-core/types"; -import { formatGovPayMetadata } from "@opensystemslab/planx-core"; +import { useStore } from "pages/FlowEditor/lib/store"; +import { ApplicationPath, Passport } from "types"; +import { array, boolean, object, string } from "yup"; + +import type { MoreInformation } from "../shared"; export interface Pay extends MoreInformation { title: string; @@ -34,7 +34,7 @@ export const toDecimal = (pence: number) => pence / 100; export const formattedPriceWithCurrencySymbol = ( amount: number, - currency = "GBP" + currency = "GBP", ) => new Intl.NumberFormat("en-GB", { style: "currency", @@ -45,13 +45,17 @@ export const createPayload = ( fee: number, reference: string, metadata: GovPayMetadata[], - passport: Passport + passport: Passport, ): GovUKCreatePaymentPayload => ({ amount: toPence(fee), reference, description: "New application", return_url: getReturnURL(reference), - metadata: formatGovPayMetadata(metadata, passport as IPassport), + metadata: formatGovPayMetadata({ + metadata, + userPassport: passport as IPassport, + paidViaInviteToPay: false, + }), }); /** @@ -70,7 +74,11 @@ const getReturnURL = (sessionId: string): string => { export const GOV_UK_PAY_URL = `${process.env.REACT_APP_API_URL}/pay`; -export const REQUIRED_GOVPAY_METADATA = ["flow", "source", "isInviteToPay"]; +export const REQUIRED_GOVPAY_METADATA = [ + "flow", + "source", + "paidViaInviteToPay", +]; // Validation must match requirements set out here - // https://docs.payments.service.gov.uk/reporting/#add-more-information-to-a-payment-39-custom-metadata-39-or-39-reporting-columns-39 @@ -81,8 +89,18 @@ export const govPayMetadataSchema = array( .max(30, "Key length cannot exceed 30 characters"), value: string() .required("Value is a required field") - .max(100, "Value length cannot exceed 100 characters"), - }) + .test({ + name: "max-length", + message: "Value length cannot exceed 100 characters", + test: (value) => { + if (!value) return true; + // No limit to dynamic passport variable length, this is checked and truncated at runtime + if (value.startsWith("@")) return true; + // Static strings must be 100 characters or less + return value.length <= 100; + }, + }), + }), ) .max(10, "A maximum of 10 fields can be set as metadata") .test({ @@ -109,7 +127,7 @@ export const govPayMetadataSchema = array( const keys = metadata.map((item) => item.key); const allRequiredKeysPresent = REQUIRED_GOVPAY_METADATA.every( - (requiredKey) => keys.includes(requiredKey) + (requiredKey) => keys.includes(requiredKey), ); return allRequiredKeysPresent; },