From 8b284edc875d07e5b05ab82a60de8acf97d74554 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Thu, 5 Dec 2024 14:15:18 +0000 Subject: [PATCH 01/25] feat(fee-breakdown): Initial setup and logic with `useFeeBreakdown()` hook (#4010) --- .../@planx/components/Pay/Editor/Editor.tsx | 6 +- .../@planx/components/Pay/Public/Confirm.tsx | 2 +- .../components/Pay/Public/FeeBreakdown.tsx | 111 -------- .../{ => FeeBreakdown}/FeeBreakdown.test.tsx | 0 .../Pay/Public/FeeBreakdown/FeeBreakdown.tsx | 153 +++++++++++ .../Pay/Public/FeeBreakdown/types.ts | 21 ++ .../FeeBreakdown/useFeeBreakdown.test.ts | 256 ++++++++++++++++++ .../Public/FeeBreakdown/useFeeBreakdown.tsx | 32 +++ .../Pay/Public/FeeBreakdown/utils.test.ts | 91 +++++++ .../Pay/Public/FeeBreakdown/utils.ts | 83 ++++++ .../@planx/components/Pay/Public/Pay.test.tsx | 4 - 11 files changed, 640 insertions(+), 119 deletions(-) delete mode 100644 editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown.tsx rename editor.planx.uk/src/@planx/components/Pay/Public/{ => FeeBreakdown}/FeeBreakdown.test.tsx (100%) create mode 100644 editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/FeeBreakdown.tsx create mode 100644 editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/types.ts create mode 100644 editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/useFeeBreakdown.test.ts create mode 100644 editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/useFeeBreakdown.tsx create mode 100644 editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/utils.test.ts create mode 100644 editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/utils.ts diff --git a/editor.planx.uk/src/@planx/components/Pay/Editor/Editor.tsx b/editor.planx.uk/src/@planx/components/Pay/Editor/Editor.tsx index 450c13a4d5..82f05da942 100644 --- a/editor.planx.uk/src/@planx/components/Pay/Editor/Editor.tsx +++ b/editor.planx.uk/src/@planx/components/Pay/Editor/Editor.tsx @@ -102,9 +102,9 @@ const Component: React.FC = (props: Props) => { - - - + + + ({ - [`& .${tableCellClasses.root}`]: { - paddingLeft: 0, - paddingRight: 0, - }, -})); - -const BoldTableRow = styled(TableRow)(() => ({ - [`& .${tableCellClasses.root}`]: { - fontWeight: FONT_WEIGHT_SEMI_BOLD, - }, -})); - -const VAT_RATE = 20; - -const DESCRIPTION = - "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat."; - -const Header = () => ( - - - Description - Amount - - -); - -const ApplicationFee = () => ( - - Application fee - {formattedPriceWithCurrencySymbol(100)} - -); - -const Exemptions = () => ( - - Exemptions - {formattedPriceWithCurrencySymbol(-20)} - -); - -const Reductions = () => ( - - Reductions - {formattedPriceWithCurrencySymbol(-30)} - -); - -const ServiceCharge = () => ( - - Service charge - {formattedPriceWithCurrencySymbol(30)} - -); - -const VAT = () => ( - - {`VAT (${VAT_RATE}%)`} - - - -); - -const Total = () => ( - - Total - {formattedPriceWithCurrencySymbol(80)} - -); - -export const FeeBreakdown: React.FC = () => { - if (!hasFeatureFlag("FEE_BREAKDOWN")) return null; - - return ( - - - Fee breakdown - - - {DESCRIPTION} - - - -
- - - - - - - - - - - - ); -}; diff --git a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown.test.tsx b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/FeeBreakdown.test.tsx similarity index 100% rename from editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown.test.tsx rename to editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/FeeBreakdown.test.tsx diff --git a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/FeeBreakdown.tsx b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/FeeBreakdown.tsx new file mode 100644 index 0000000000..5ef73fc59f --- /dev/null +++ b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/FeeBreakdown.tsx @@ -0,0 +1,153 @@ +import Box from "@mui/material/Box"; +import { styled } from "@mui/material/styles"; +import Table from "@mui/material/Table"; +import TableBody from "@mui/material/TableBody"; +import TableCell, { tableCellClasses } from "@mui/material/TableCell"; +import TableContainer from "@mui/material/TableContainer"; +import TableHead from "@mui/material/TableHead"; +import TableRow from "@mui/material/TableRow"; +import Typography from "@mui/material/Typography"; +import React from "react"; +import { FONT_WEIGHT_SEMI_BOLD } from "theme"; + +import { formattedPriceWithCurrencySymbol } from "../../model"; +import { useFeeBreakdown } from "./useFeeBreakdown"; + +const StyledTable = styled(Table)(() => ({ + [`& .${tableCellClasses.root}`]: { + paddingLeft: 0, + paddingRight: 0, + }, +})); + +const BoldTableRow = styled(TableRow)(() => ({ + [`& .${tableCellClasses.root}`]: { + fontWeight: FONT_WEIGHT_SEMI_BOLD, + }, +})); + +const VAT_RATE = 0.2; + +const DESCRIPTION = + "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat."; + +const Header = () => ( + + + Description + Amount + + +); + +const ApplicationFee: React.FC<{ amount: number }> = ({ amount }) => ( + + Application fee + + {formattedPriceWithCurrencySymbol(amount)} + + +); + +const Reductions: React.FC<{ amount?: number, reductions: string[] }> = ({ amount, reductions }) => { + if (!amount) return null; + + return ( + <> + + Reductions + + {formattedPriceWithCurrencySymbol(-amount)} + + + { + reductions.map((reduction) => ( + + + {reduction} + + + )) + } + + ); +}; + +// TODO: This won't show as if a fee is 0, we hide the whole Pay component from the user +const Exemptions: React.FC<{ amount: number, exemptions: string[] }> = ({ amount, exemptions }) => { + if (!exemptions.length) return null; + + return ( + <> + + Exemptions + + {formattedPriceWithCurrencySymbol(-amount)} + + + { + exemptions.map((exemption) => ( + + + {exemption} + + + )) + } + + ); +}; + +const VAT: React.FC<{ amount?: number }> = ({ amount }) => { + if (!amount) return null; + + return ( + + {`Includes VAT (${ + VAT_RATE * 100 + }%)`} + + {formattedPriceWithCurrencySymbol(amount)} + + + ); +}; + +const Total: React.FC<{ amount: number }> = ({ amount }) => ( + + Total + + {formattedPriceWithCurrencySymbol(amount)} + + +); + +export const FeeBreakdown: React.FC = () => { + const breakdown = useFeeBreakdown(); + if (!breakdown) return null; + + const { amount, reductions, exemptions } = breakdown; + + return ( + + + Fee breakdown + + + {DESCRIPTION} + + + +
+ + + + + + + + + + + ); +}; diff --git a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/types.ts b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/types.ts new file mode 100644 index 0000000000..176cf87724 --- /dev/null +++ b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/types.ts @@ -0,0 +1,21 @@ +export interface FeeBreakdown { + amount: { + applicationFee: number; + total: number; + reduction: number; + vat: number | undefined; + }; + reductions: string[]; + exemptions: string[]; +} + +export interface PassportFeeFields { + "application.fee.calculated": number; + "application.fee.payable": number; + "application.fee.payable.vat": number; + "application.fee.reduction.alternative": boolean; + "application.fee.reduction.parishCouncil": boolean; + "application.fee.reduction.sports": boolean; + "application.fee.exemption.disability": boolean; + "application.fee.exemption.resubmission": boolean; +}; \ No newline at end of file diff --git a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/useFeeBreakdown.test.ts b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/useFeeBreakdown.test.ts new file mode 100644 index 0000000000..5312cb12c2 --- /dev/null +++ b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/useFeeBreakdown.test.ts @@ -0,0 +1,256 @@ +import { logger } from "airbrake"; +import { useStore } from "pages/FlowEditor/lib/store"; +import { vi } from "vitest"; + +import { useFeeBreakdown } from "./useFeeBreakdown"; + +vi.mock("pages/FlowEditor/lib/store", () => ({ + useStore: vi.fn(), +})); + +vi.mock("airbrake", () => ({ + logger: { + notify: vi.fn(), + }, +})); + +describe("useFeeBreakdown() hook", () => { + describe("valid data", () => { + it("returns a fee breakdown for number inputs", () => { + const mockPassportData = { + "application.fee.calculated": 1000, + "application.fee.payable": 800, + "application.fee.payable.vat": 160, + "some.other.fields": ["abc", "xyz"], + }; + + vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); + + const result = useFeeBreakdown(); + + expect(result).toEqual({ + amount: { + applicationFee: 1000, + total: 800, + reduction: 200, + vat: 160, + }, + exemptions: [], + reductions: [], + }); + }); + + it("returns a fee breakdown for number tuple inputs", () => { + const mockPassportData = { + "application.fee.calculated": [1000], + "application.fee.payable": [800], + "application.fee.payable.vat": [160], + "some.other.fields": ["abc", "xyz"], + }; + + vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); + + const result = useFeeBreakdown(); + + expect(result).toEqual({ + amount: { + applicationFee: 1000, + total: 800, + reduction: 200, + vat: 160, + }, + exemptions: [], + reductions: [], + }); + }); + + it("parses 'true' reduction values to a list of keys", () => { + const mockPassportData = { + "application.fee.calculated": 1000, + "application.fee.payable": 800, + "application.fee.payable.vat": 160, + "application.fee.reduction.alternative": ["true"], + "application.fee.reduction.parishCouncil": ["true"], + "some.other.fields": ["abc", "xyz"], + }; + + vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); + + const result = useFeeBreakdown(); + + expect(result?.reductions).toHaveLength(2); + expect(result?.reductions).toEqual( + expect.arrayContaining(["alternative", "parishCouncil"]) + ); + }); + + it("does not parse 'false' reduction values to a list of keys", () => { + const mockPassportData = { + "application.fee.calculated": 1000, + "application.fee.payable": 800, + "application.fee.payable.vat": 160, + "application.fee.reduction.alternative": ["false"], + "application.fee.reduction.parishCouncil": ["false"], + "some.other.fields": ["abc", "xyz"], + }; + + vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); + + const result = useFeeBreakdown(); + + expect(result?.reductions).toHaveLength(0); + }); + + it("does not parse non-schema reduction values", () => { + const mockPassportData = { + "application.fee.calculated": 1000, + "application.fee.payable": 800, + "application.fee.payable.vat": 160, + "application.fee.reduction.alternative": ["true"], + "application.fee.reduction.parishCouncil": ["false"], + "application.fee.reduction.someReason": ["true"], + "application.fee.reduction.someOtherReason": ["false"], + "some.other.fields": ["abc", "xyz"], + }; + + vi.mocked(useStore).mockReturnValue([ + mockPassportData, + "test-session", + ]); + + const result = useFeeBreakdown(); + + expect(result?.reductions).toEqual(expect.not.arrayContaining(["someReason"])) + expect(result?.reductions).toEqual(expect.not.arrayContaining(["someOtherReason"])) + }); + + it("parses 'true' exemption values to a list of keys", () => { + const mockPassportData = { + "application.fee.calculated": 1000, + "application.fee.payable": 800, + "application.fee.payable.vat": 160, + "application.fee.exemption.disability": ["true"], + "application.fee.exemption.resubmission": ["true"], + "some.other.fields": ["abc", "xyz"], + }; + + vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); + + const result = useFeeBreakdown(); + + expect(result?.exemptions).toHaveLength(2); + expect(result?.exemptions).toEqual( + expect.arrayContaining(["disability", "resubmission"]) + ); + }); + + it("does not parse 'false' exemption values to a list of keys", () => { + const mockPassportData = { + "application.fee.calculated": 1000, + "application.fee.payable": 800, + "application.fee.payable.vat": 160, + "application.fee.exemption.disability": ["false"], + "application.fee.exemption.resubmission": ["false"], + "some.other.fields": ["abc", "xyz"], + }; + + vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); + + const result = useFeeBreakdown(); + + expect(result?.exemptions).toHaveLength(0); + }); + + it("does not parse non-schema exemption values", () => { + const mockPassportData = { + "application.fee.calculated": 1000, + "application.fee.payable": 800, + "application.fee.payable.vat": 160, + "application.fee.exemption.disability": ["false"], + "application.fee.exemption.resubmission": ["false"], + "application.fee.exemption.someReason": ["true"], + "application.fee.exemption.someOtherReason": ["false"], + "some.other.fields": ["abc", "xyz"], + }; + + vi.mocked(useStore).mockReturnValue([ + mockPassportData, + "test-session", + ]); + + const result = useFeeBreakdown(); + + expect(result?.exemptions).toEqual( + expect.not.arrayContaining(["someReason"]) + ); + expect(result?.exemptions).toEqual( + expect.not.arrayContaining(["someOtherReason"]) + ); + }); + }); + + describe("invalid inputs", () => { + it("returns undefined for missing data", () => { + const mockPassportData = { + "some.other.fields": ["abc", "xyz"], + }; + + vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); + + const result = useFeeBreakdown(); + + expect(result).toBeUndefined(); + }); + + it("returns undefined for partial data", () => { + const mockPassportData = { + "application.fee.calculated": [1000], + "application.fee.payable.vat": [160], + "some.other.fields": ["abc", "xyz"], + }; + + vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); + + const result = useFeeBreakdown(); + + expect(result).toBeUndefined(); + }); + + it("returns undefined for incorrect data", () => { + const mockPassportData = { + "application.fee.calculated": "some string", + "application.fee.payable": [800, 700], + "application.fee.payable.vat": false, + "some.other.fields": ["abc", "xyz"], + }; + + vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); + + const result = useFeeBreakdown(); + + expect(result).toBeUndefined(); + }); + + it("calls Airbrake if invalid inputs are provided", () => { + const mockPassportData = { + "some.other.fields": ["abc", "xyz"], + }; + const mockSessionId = "test-session"; + + vi.mocked(useStore).mockReturnValue([mockPassportData, mockSessionId]); + const loggerSpy = vi.spyOn(logger, "notify"); + + const result = useFeeBreakdown(); + + expect(result).toBeUndefined(); + + expect(loggerSpy).toHaveBeenCalledWith( + expect.stringContaining(mockSessionId), + ); + + expect(loggerSpy).toHaveBeenCalledWith( + expect.stringContaining("ZodError"), + ); + }); + }); +}); diff --git a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/useFeeBreakdown.tsx b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/useFeeBreakdown.tsx new file mode 100644 index 0000000000..b1e6b10c1d --- /dev/null +++ b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/useFeeBreakdown.tsx @@ -0,0 +1,32 @@ +import { logger } from "airbrake"; +import { useStore } from "pages/FlowEditor/lib/store"; + +import { FeeBreakdown } from "./types"; +import { createPassportSchema } from "./utils"; + +/** + * Parses the users's Passport for data variables associated with their fee + * Currently relies on static `application.fee.x` variables + * + * If fee variables not found, or not successfully parsed, we do not show the user a fee breakdown + * Instead an internal error will be raised allowing us to correct the flow content + */ +export const useFeeBreakdown = (): FeeBreakdown | undefined => { + const [passportData, sessionId] = useStore((state) => [ + state.computePassport().data, + state.sessionId, + ]); + if (!passportData) return + + const schema = createPassportSchema(); + const result = schema.safeParse(passportData); + + if (!result.success) { + logger.notify( + `Failed to parse fee breakdown data from passport for session ${sessionId}. Error: ${result.error}`, + ); + return; + } + + return result.data; +}; diff --git a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/utils.test.ts b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/utils.test.ts new file mode 100644 index 0000000000..935c8cb066 --- /dev/null +++ b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/utils.test.ts @@ -0,0 +1,91 @@ +import { PassportFeeFields } from "./types"; +import { calculateReduction, toFeeBreakdown, toNumber } from "./utils"; + +describe("toNumber() helper function", () => { + it("outputs a number when passed a number", () => { + const input = 12; + const output = toNumber(input); + + expect(output).toEqual(input); + }); + + it("outputs a number when passed a number tuple", () => { + const input: [number] = [12]; + const output = toNumber(input); + + expect(output).toEqual(12); + }); +}); + +describe("calculateReduction() helper function", () => { + it("correctly outputs the reduction when a calculated value is provided", () => { + const input: PassportFeeFields = { + "application.fee.calculated": 100, + "application.fee.payable": 50, + "application.fee.payable.vat": 0, + "application.fee.reduction.alternative": false, + "application.fee.reduction.parishCouncil": false, + "application.fee.reduction.sports": false, + "application.fee.exemption.disability": false, + "application.fee.exemption.resubmission": false, + }; + const reduction = calculateReduction(input); + + expect(reduction).toEqual(50); + }); + + it("defaults to 0 when calculated is 0", () => { + const input: PassportFeeFields = { + "application.fee.calculated": 0, + "application.fee.payable": 100, + "application.fee.payable.vat": 0, + "application.fee.reduction.alternative": false, + "application.fee.reduction.parishCouncil": false, + "application.fee.reduction.sports": false, + "application.fee.exemption.disability": false, + "application.fee.exemption.resubmission": false, + }; + const reduction = calculateReduction(input); + + expect(reduction).toEqual(0); + }); +}); + +describe("toFeeBreakdown() helper function", () => { + it("correctly maps fields", () => { + const input: PassportFeeFields = { + "application.fee.calculated": 100, + "application.fee.payable": 50, + "application.fee.payable.vat": 10, + "application.fee.reduction.alternative": false, + "application.fee.reduction.parishCouncil": false, + "application.fee.reduction.sports": false, + "application.fee.exemption.disability": false, + "application.fee.exemption.resubmission": false, + }; + + const { amount } = toFeeBreakdown(input); + + expect(amount.applicationFee).toEqual(input["application.fee.calculated"]); + expect(amount.total).toEqual(input["application.fee.payable"]); + expect(amount.vat).toEqual(input["application.fee.payable.vat"]); + expect(amount.reduction).toEqual(50); + }); + + it("sets applicationFee to payable amount if no calculated value is provided", () => { + const input: PassportFeeFields = { + "application.fee.calculated": 0, + "application.fee.payable.vat": 10, + "application.fee.payable": 50, + "application.fee.reduction.alternative": false, + "application.fee.reduction.parishCouncil": false, + "application.fee.reduction.sports": false, + "application.fee.exemption.disability": false, + "application.fee.exemption.resubmission": false, + }; + + const { amount } = toFeeBreakdown(input); + + expect(amount.applicationFee).toEqual(input["application.fee.payable"]); + }); +}); diff --git a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/utils.ts b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/utils.ts new file mode 100644 index 0000000000..759c5438db --- /dev/null +++ b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/utils.ts @@ -0,0 +1,83 @@ +import { z } from "zod"; + +import { FeeBreakdown, PassportFeeFields } from "./types"; + +export const toNumber = (input: number | [number]) => + Array.isArray(input) ? input[0] : input; + +/** + * Convert a Passport value to an actual boolean + */ +const toBoolean = (val: ["true" | "false"]) => val[0] === "true"; + + +/** + * Iterate over exemptions or reductions to find matches, returning the granular keys + */ + const getGranularKeys = ( + data: PassportFeeFields, + prefix: "application.fee.reduction" | "application.fee.exemption" + ) => { + const keys = Object.keys(data) as (keyof PassportFeeFields)[]; + const intersectingKeys = keys.filter( + (key) => key.startsWith(prefix) && Boolean(data[key]) + ); + const granularKeys = intersectingKeys.map((key) => + key.replace(prefix + ".", "") + ); + + return granularKeys; + }; + +/** + * A "reduction" is the sum of the difference between calculated and payable + */ +export const calculateReduction = (data: PassportFeeFields) => + data["application.fee.calculated"] + ? data["application.fee.calculated"] - data["application.fee.payable"] + : 0; + +/** + * Transform Passport data to a FeeBreakdown + */ +export const toFeeBreakdown = (data: PassportFeeFields): FeeBreakdown => ({ + amount: { + applicationFee: + data["application.fee.calculated"] || + data["application.fee.payable"], + total: data["application.fee.payable"], + vat: data["application.fee.payable.vat"], + reduction: calculateReduction(data), + }, + reductions: getGranularKeys(data, "application.fee.reduction"), + exemptions: getGranularKeys(data, "application.fee.exemption"), +}); + +export const createPassportSchema = () => { + const questionSchema = z.number().nonnegative(); + const setValueSchema = z.tuple([z.coerce.number().nonnegative()]); + const feeSchema = z + .union([questionSchema, setValueSchema]) + .transform(toNumber); + + /** Describes how boolean values are set via PlanX components */ + const booleanSchema = z + .tuple([z.enum(["true", "false"])]) + .default(["false"]) + .transform(toBoolean) + + const schema = z + .object({ + "application.fee.calculated": feeSchema.optional().default(0), + "application.fee.payable": feeSchema, + "application.fee.payable.vat": feeSchema.optional().default(0), + "application.fee.reduction.alternative": booleanSchema, + "application.fee.reduction.parishCouncil": booleanSchema, + "application.fee.reduction.sports": booleanSchema, + "application.fee.exemption.disability": booleanSchema, + "application.fee.exemption.resubmission": booleanSchema, + }) + .transform(toFeeBreakdown); + + return schema; +}; diff --git a/editor.planx.uk/src/@planx/components/Pay/Public/Pay.test.tsx b/editor.planx.uk/src/@planx/components/Pay/Public/Pay.test.tsx index 71e43ed2d5..0391100764 100644 --- a/editor.planx.uk/src/@planx/components/Pay/Public/Pay.test.tsx +++ b/editor.planx.uk/src/@planx/components/Pay/Public/Pay.test.tsx @@ -1,7 +1,6 @@ import { PaymentStatus } from "@opensystemslab/planx-core/types"; import { ComponentType as TYPES } from "@opensystemslab/planx-core/types"; import { screen } from "@testing-library/react"; -import { hasFeatureFlag } from "lib/featureFlags"; import { FullStore, Store, useStore } from "pages/FlowEditor/lib/store"; import React from "react"; import { act } from "react-dom/test-utils"; @@ -9,7 +8,6 @@ import * as ReactNavi from "react-navi"; import { setup } from "testUtils"; import { ApplicationPath, Breadcrumbs } from "types"; import { vi } from "vitest"; -import { Mock } from "vitest"; import { axe } from "vitest-axe"; import Confirm, { Props } from "./Confirm"; @@ -27,8 +25,6 @@ vi.mock("lib/featureFlags", () => ({ hasFeatureFlag: vi.fn(), })); -const mockHasFeatureFlag = hasFeatureFlag as Mock; - const resumeButtonText = "Resume an application you have already started"; const saveButtonText = "Save and return to this application later"; From 05a7c62c9b06cda6df7ba369914d8167a9e40598 Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Thu, 5 Dec 2024 17:53:10 +0100 Subject: [PATCH 02/25] fix: account for expandable checklists in editor modal passport value validation (#4045) --- .../src/@planx/components/Checklist/Editor/Editor.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/editor.planx.uk/src/@planx/components/Checklist/Editor/Editor.tsx b/editor.planx.uk/src/@planx/components/Checklist/Editor/Editor.tsx index 49c592a064..2fcc03c3de 100644 --- a/editor.planx.uk/src/@planx/components/Checklist/Editor/Editor.tsx +++ b/editor.planx.uk/src/@planx/components/Checklist/Editor/Editor.tsx @@ -72,8 +72,11 @@ export const ChecklistComponent: React.FC = (props) => { alert(JSON.stringify({ type, ...values, options }, null, 2)); } }, - validate: ({ options, ...values }) => { + validate: ({ options, groupedOptions, ...values }) => { const errors: FormikErrors = {}; + // Account for flat or expandable Checklist options + options = + options || groupedOptions?.map((group) => group.children)?.flat(); if (values.fn && !options?.some((option) => option.data.val)) { errors.fn = "At least one option must set a data value when the checklist has a data field"; From d8968881d366a53cd13a8d3c4a009e421aae73af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Thu, 5 Dec 2024 17:05:15 +0000 Subject: [PATCH 03/25] chore(fee-breakdown): Import `getFeeBreakdown()` from `planx-core` (#4044) --- api.planx.uk/package.json | 2 +- api.planx.uk/pnpm-lock.yaml | 34 +-- e2e/tests/api-driven/package.json | 2 +- e2e/tests/api-driven/pnpm-lock.yaml | 34 +-- e2e/tests/ui-driven/package.json | 2 +- e2e/tests/ui-driven/pnpm-lock.yaml | 34 +-- editor.planx.uk/package.json | 2 +- editor.planx.uk/pnpm-lock.yaml | 44 +-- .../Pay/Public/FeeBreakdown/FeeBreakdown.tsx | 6 +- .../Pay/Public/FeeBreakdown/types.ts | 21 -- .../FeeBreakdown/useFeeBreakdown.test.ts | 256 ------------------ .../Public/FeeBreakdown/useFeeBreakdown.tsx | 17 +- .../Pay/Public/FeeBreakdown/utils.test.ts | 91 ------- .../Pay/Public/FeeBreakdown/utils.ts | 83 ------ 14 files changed, 89 insertions(+), 539 deletions(-) delete mode 100644 editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/types.ts delete mode 100644 editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/useFeeBreakdown.test.ts delete mode 100644 editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/utils.test.ts delete mode 100644 editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/utils.ts diff --git a/api.planx.uk/package.json b/api.planx.uk/package.json index 3f17c1f479..27b1560166 100644 --- a/api.planx.uk/package.json +++ b/api.planx.uk/package.json @@ -13,7 +13,7 @@ "@airbrake/node": "^2.1.8", "@aws-sdk/client-s3": "^3.696.0", "@aws-sdk/s3-request-presigner": "^3.701.0", - "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#ccf9ac3", + "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#29b4851", "@types/isomorphic-fetch": "^0.0.36", "adm-zip": "^0.5.10", "axios": "^1.7.4", diff --git a/api.planx.uk/pnpm-lock.yaml b/api.planx.uk/pnpm-lock.yaml index e1beae8474..9279cf8f9e 100644 --- a/api.planx.uk/pnpm-lock.yaml +++ b/api.planx.uk/pnpm-lock.yaml @@ -21,8 +21,8 @@ dependencies: specifier: ^3.701.0 version: 3.701.0 '@opensystemslab/planx-core': - specifier: git+https://github.com/theopensystemslab/planx-core#ccf9ac3 - version: github.com/theopensystemslab/planx-core/ccf9ac3 + specifier: git+https://github.com/theopensystemslab/planx-core#29b4851 + version: github.com/theopensystemslab/planx-core/29b4851 '@types/isomorphic-fetch': specifier: ^0.0.36 version: 0.0.36 @@ -3959,8 +3959,8 @@ packages: dependencies: esutils: 2.0.3 - /docx@9.0.3: - resolution: {integrity: sha512-Ao/8v1BIRL0+uhzoqk6uvAPIjxNR5+uqQMphVWe8egxGdohkd/PgDLn1/+/c/te1DlnX1Chz9lIxuNi0Hs+Dbg==} + /docx@9.1.0: + resolution: {integrity: sha512-XOtseSTRrkKN/sV5jNBqyLazyhNpWfaUhpuKc22cs+5DavNjRQvchnohb0g0S+x/96/D06U/i0/U/Gc4E5kwuQ==} engines: {node: '>=10'} dependencies: '@types/node': 22.9.0 @@ -5225,7 +5225,7 @@ packages: js-yaml: 4.1.0 lodash: 4.17.21 minimist: 1.2.8 - prettier: 3.3.3 + prettier: 3.4.2 tinyglobby: 0.2.10 dev: false @@ -5481,8 +5481,8 @@ packages: semver: 7.6.3 dev: true - /marked@14.1.4: - resolution: {integrity: sha512-vkVZ8ONmUdPnjCKc5uTRvmkRbx4EAi2OkTOXmfTDhZz3OFqMNBM1oTTWwTr4HY4uAEojhzPf+Fy8F1DWa3Sndg==} + /marked@15.0.3: + resolution: {integrity: sha512-Ai0cepvl2NHnTcO9jYDtcOEtVBNVYR31XnEA3BndO7f5As1wzpcOceSUM8FDkNLJNIODcLpDTWay/qQhqbuMvg==} engines: {node: '>= 18'} hasBin: true dev: false @@ -6059,8 +6059,8 @@ packages: hasBin: true dev: true - /prettier@3.3.3: - resolution: {integrity: sha512-i2tDNA0O5IrMO757lfrdQZCc2jPNDVntV0m/+4whiDfWaTKfMNgR7Qz0NAeGz/nRqF4m5/6CLzbP4/liHt12Ew==} + /prettier@3.4.2: + resolution: {integrity: sha512-e9MewbtFo+Fevyuxn/4rrcDAaq0IYxPGLvObpQjiZBMAzB9IGmzlnG9RZy3FFas+eBMu2vA0CszMeduow5dIuQ==} engines: {node: '>=14'} hasBin: true dev: false @@ -6891,8 +6891,8 @@ packages: engines: {node: '>=16'} dev: false - /type-fest@4.27.0: - resolution: {integrity: sha512-3IMSWgP7C5KSQqmo1wjhKrwsvXAtF33jO3QY+Uy++ia7hqvgSK6iXbbg5PbDBc1P2ZbNEDgejOrN4YooXvhwCw==} + /type-fest@4.30.0: + resolution: {integrity: sha512-G6zXWS1dLj6eagy6sVhOMQiLtJdxQBHIA9Z6HFUNLOlr6MFOgzV8wvmidtPONfPtEUv0uZsy77XJNzTAfwPDaA==} engines: {node: '>=16'} dev: false @@ -7306,8 +7306,8 @@ packages: resolution: {integrity: sha512-XBx9AXhXktjUqnepgTiE5flcKIYWi/rme0Eaj+5Y0lftuGBq+jyRu/md4WnuxqgP1ubdpNCsYEYPxrzVHD8d6g==} dev: false - github.com/theopensystemslab/planx-core/ccf9ac3: - resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/ccf9ac3} + github.com/theopensystemslab/planx-core/29b4851: + resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/29b4851} name: '@opensystemslab/planx-core' version: 1.0.0 prepare: true @@ -7322,18 +7322,18 @@ packages: ajv-formats: 2.1.1(ajv@8.17.1) cheerio: 1.0.0 copyfiles: 2.4.1 - docx: 9.0.3 + docx: 9.1.0 eslint: 8.57.1 fast-xml-parser: 4.5.0 graphql: 16.9.0 graphql-request: 6.1.0(graphql@16.9.0) json-schema-to-typescript: 15.0.3 lodash: 4.17.21 - marked: 14.1.4 - prettier: 3.3.3 + marked: 15.0.3 + prettier: 3.4.2 react: 18.3.1 react-dom: 18.3.1(react@18.3.1) - type-fest: 4.27.0 + type-fest: 4.30.0 uuid: 11.0.3 zod: 3.23.8 transitivePeerDependencies: diff --git a/e2e/tests/api-driven/package.json b/e2e/tests/api-driven/package.json index 074c8c301b..606a64005e 100644 --- a/e2e/tests/api-driven/package.json +++ b/e2e/tests/api-driven/package.json @@ -7,7 +7,7 @@ "packageManager": "pnpm@8.6.6", "dependencies": { "@cucumber/cucumber": "^9.3.0", - "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#ccf9ac3", + "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#29b4851", "axios": "^1.7.4", "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 26645a568a..e9f30826e1 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#ccf9ac3 - version: github.com/theopensystemslab/planx-core/ccf9ac3 + specifier: git+https://github.com/theopensystemslab/planx-core#29b4851 + version: github.com/theopensystemslab/planx-core/29b4851 axios: specifier: ^1.7.4 version: 1.7.4 @@ -1209,8 +1209,8 @@ packages: esutils: 2.0.3 dev: false - /docx@9.0.3: - resolution: {integrity: sha512-Ao/8v1BIRL0+uhzoqk6uvAPIjxNR5+uqQMphVWe8egxGdohkd/PgDLn1/+/c/te1DlnX1Chz9lIxuNi0Hs+Dbg==} + /docx@9.1.0: + resolution: {integrity: sha512-XOtseSTRrkKN/sV5jNBqyLazyhNpWfaUhpuKc22cs+5DavNjRQvchnohb0g0S+x/96/D06U/i0/U/Gc4E5kwuQ==} engines: {node: '>=10'} dependencies: '@types/node': 22.9.0 @@ -1785,7 +1785,7 @@ packages: js-yaml: 4.1.0 lodash: 4.17.21 minimist: 1.2.8 - prettier: 3.3.3 + prettier: 3.4.2 tinyglobby: 0.2.10 dev: false @@ -1955,8 +1955,8 @@ packages: resolution: {integrity: sha512-s8UhlNe7vPKomQhC1qFelMokr/Sc3AgNbso3n74mVPA5LTZwkB9NlXf4XPamLxJE8h0gh73rM94xvwRT2CVInw==} dev: true - /marked@14.1.4: - resolution: {integrity: sha512-vkVZ8ONmUdPnjCKc5uTRvmkRbx4EAi2OkTOXmfTDhZz3OFqMNBM1oTTWwTr4HY4uAEojhzPf+Fy8F1DWa3Sndg==} + /marked@15.0.3: + resolution: {integrity: sha512-Ai0cepvl2NHnTcO9jYDtcOEtVBNVYR31XnEA3BndO7f5As1wzpcOceSUM8FDkNLJNIODcLpDTWay/qQhqbuMvg==} engines: {node: '>= 18'} hasBin: true dev: false @@ -2191,8 +2191,8 @@ packages: engines: {node: '>= 0.8.0'} dev: false - /prettier@3.3.3: - resolution: {integrity: sha512-i2tDNA0O5IrMO757lfrdQZCc2jPNDVntV0m/+4whiDfWaTKfMNgR7Qz0NAeGz/nRqF4m5/6CLzbP4/liHt12Ew==} + /prettier@3.4.2: + resolution: {integrity: sha512-e9MewbtFo+Fevyuxn/4rrcDAaq0IYxPGLvObpQjiZBMAzB9IGmzlnG9RZy3FFas+eBMu2vA0CszMeduow5dIuQ==} engines: {node: '>=14'} hasBin: true dev: false @@ -2607,8 +2607,8 @@ packages: engines: {node: '>=10'} dev: false - /type-fest@4.27.0: - resolution: {integrity: sha512-3IMSWgP7C5KSQqmo1wjhKrwsvXAtF33jO3QY+Uy++ia7hqvgSK6iXbbg5PbDBc1P2ZbNEDgejOrN4YooXvhwCw==} + /type-fest@4.30.0: + resolution: {integrity: sha512-G6zXWS1dLj6eagy6sVhOMQiLtJdxQBHIA9Z6HFUNLOlr6MFOgzV8wvmidtPONfPtEUv0uZsy77XJNzTAfwPDaA==} engines: {node: '>=16'} dev: false @@ -2810,8 +2810,8 @@ packages: resolution: {integrity: sha512-XBx9AXhXktjUqnepgTiE5flcKIYWi/rme0Eaj+5Y0lftuGBq+jyRu/md4WnuxqgP1ubdpNCsYEYPxrzVHD8d6g==} dev: false - github.com/theopensystemslab/planx-core/ccf9ac3: - resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/ccf9ac3} + github.com/theopensystemslab/planx-core/29b4851: + resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/29b4851} name: '@opensystemslab/planx-core' version: 1.0.0 prepare: true @@ -2826,18 +2826,18 @@ packages: ajv-formats: 2.1.1(ajv@8.17.1) cheerio: 1.0.0 copyfiles: 2.4.1 - docx: 9.0.3 + docx: 9.1.0 eslint: 8.57.1 fast-xml-parser: 4.5.0 graphql: 16.9.0 graphql-request: 6.1.0(graphql@16.9.0) json-schema-to-typescript: 15.0.3 lodash: 4.17.21 - marked: 14.1.4 - prettier: 3.3.3 + marked: 15.0.3 + prettier: 3.4.2 react: 18.3.1 react-dom: 18.3.1(react@18.3.1) - type-fest: 4.27.0 + type-fest: 4.30.0 uuid: 11.0.3 zod: 3.23.8 transitivePeerDependencies: diff --git a/e2e/tests/ui-driven/package.json b/e2e/tests/ui-driven/package.json index b5aaa364e3..514b990987 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#ccf9ac3", + "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#29b4851", "axios": "^1.7.4", "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 75407766b7..4552b235fd 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#ccf9ac3 - version: github.com/theopensystemslab/planx-core/ccf9ac3 + specifier: git+https://github.com/theopensystemslab/planx-core#29b4851 + version: github.com/theopensystemslab/planx-core/29b4851 axios: specifier: ^1.7.4 version: 1.7.4 @@ -1090,8 +1090,8 @@ packages: dependencies: esutils: 2.0.3 - /docx@9.0.3: - resolution: {integrity: sha512-Ao/8v1BIRL0+uhzoqk6uvAPIjxNR5+uqQMphVWe8egxGdohkd/PgDLn1/+/c/te1DlnX1Chz9lIxuNi0Hs+Dbg==} + /docx@9.1.0: + resolution: {integrity: sha512-XOtseSTRrkKN/sV5jNBqyLazyhNpWfaUhpuKc22cs+5DavNjRQvchnohb0g0S+x/96/D06U/i0/U/Gc4E5kwuQ==} engines: {node: '>=10'} dependencies: '@types/node': 22.9.0 @@ -1690,7 +1690,7 @@ packages: js-yaml: 4.1.0 lodash: 4.17.21 minimist: 1.2.8 - prettier: 3.3.3 + prettier: 3.4.2 tinyglobby: 0.2.10 dev: false @@ -1814,8 +1814,8 @@ packages: js-tokens: 4.0.0 dev: false - /marked@14.1.4: - resolution: {integrity: sha512-vkVZ8ONmUdPnjCKc5uTRvmkRbx4EAi2OkTOXmfTDhZz3OFqMNBM1oTTWwTr4HY4uAEojhzPf+Fy8F1DWa3Sndg==} + /marked@15.0.3: + resolution: {integrity: sha512-Ai0cepvl2NHnTcO9jYDtcOEtVBNVYR31XnEA3BndO7f5As1wzpcOceSUM8FDkNLJNIODcLpDTWay/qQhqbuMvg==} engines: {node: '>= 18'} hasBin: true dev: false @@ -2072,8 +2072,8 @@ packages: resolution: {integrity: sha512-vkcDPrRZo1QZLbn5RLGPpg/WmIQ65qoWWhcGKf/b5eplkkarX0m9z8ppCat4mlOqUsWpyNuYgO3VRyrYHSzX5g==} engines: {node: '>= 0.8.0'} - /prettier@3.3.3: - resolution: {integrity: sha512-i2tDNA0O5IrMO757lfrdQZCc2jPNDVntV0m/+4whiDfWaTKfMNgR7Qz0NAeGz/nRqF4m5/6CLzbP4/liHt12Ew==} + /prettier@3.4.2: + resolution: {integrity: sha512-e9MewbtFo+Fevyuxn/4rrcDAaq0IYxPGLvObpQjiZBMAzB9IGmzlnG9RZy3FFas+eBMu2vA0CszMeduow5dIuQ==} engines: {node: '>=14'} hasBin: true dev: false @@ -2431,8 +2431,8 @@ packages: engines: {node: '>=12.20'} dev: false - /type-fest@4.27.0: - resolution: {integrity: sha512-3IMSWgP7C5KSQqmo1wjhKrwsvXAtF33jO3QY+Uy++ia7hqvgSK6iXbbg5PbDBc1P2ZbNEDgejOrN4YooXvhwCw==} + /type-fest@4.30.0: + resolution: {integrity: sha512-G6zXWS1dLj6eagy6sVhOMQiLtJdxQBHIA9Z6HFUNLOlr6MFOgzV8wvmidtPONfPtEUv0uZsy77XJNzTAfwPDaA==} engines: {node: '>=16'} dev: false @@ -2599,8 +2599,8 @@ packages: resolution: {integrity: sha512-XBx9AXhXktjUqnepgTiE5flcKIYWi/rme0Eaj+5Y0lftuGBq+jyRu/md4WnuxqgP1ubdpNCsYEYPxrzVHD8d6g==} dev: false - github.com/theopensystemslab/planx-core/ccf9ac3: - resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/ccf9ac3} + github.com/theopensystemslab/planx-core/29b4851: + resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/29b4851} name: '@opensystemslab/planx-core' version: 1.0.0 prepare: true @@ -2615,18 +2615,18 @@ packages: ajv-formats: 2.1.1(ajv@8.17.1) cheerio: 1.0.0 copyfiles: 2.4.1 - docx: 9.0.3 + docx: 9.1.0 eslint: 8.57.1 fast-xml-parser: 4.5.0 graphql: 16.9.0 graphql-request: 6.1.0(graphql@16.9.0) json-schema-to-typescript: 15.0.3 lodash: 4.17.21 - marked: 14.1.4 - prettier: 3.3.3 + marked: 15.0.3 + prettier: 3.4.2 react: 18.3.1 react-dom: 18.3.1(react@18.3.1) - type-fest: 4.27.0 + type-fest: 4.30.0 uuid: 11.0.3 zod: 3.23.8 transitivePeerDependencies: diff --git a/editor.planx.uk/package.json b/editor.planx.uk/package.json index 637e6d9f84..d81643d9a0 100644 --- a/editor.planx.uk/package.json +++ b/editor.planx.uk/package.json @@ -15,7 +15,7 @@ "@mui/material": "^5.15.10", "@mui/utils": "^5.15.11", "@opensystemslab/map": "1.0.0-alpha.4", - "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#ccf9ac3", + "@opensystemslab/planx-core": "git+https://github.com/theopensystemslab/planx-core#29b4851", "@tiptap/core": "^2.4.0", "@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 c985baee32..c683e55797 100644 --- a/editor.planx.uk/pnpm-lock.yaml +++ b/editor.planx.uk/pnpm-lock.yaml @@ -48,8 +48,8 @@ dependencies: specifier: 1.0.0-alpha.4 version: 1.0.0-alpha.4 '@opensystemslab/planx-core': - specifier: git+https://github.com/theopensystemslab/planx-core#ccf9ac3 - version: github.com/theopensystemslab/planx-core/ccf9ac3(@types/react@18.2.45) + specifier: git+https://github.com/theopensystemslab/planx-core#29b4851 + version: github.com/theopensystemslab/planx-core/29b4851(@types/react@18.2.45) '@tiptap/core': specifier: ^2.4.0 version: 2.4.0(@tiptap/pm@2.0.3) @@ -3291,7 +3291,7 @@ packages: '@babel/runtime': 7.26.0 '@floating-ui/react-dom': 2.1.2(react-dom@18.2.0)(react@18.2.0) '@mui/types': 7.2.19(@types/react@18.2.45) - '@mui/utils': 5.15.11(@types/react@18.2.45)(react@18.2.0) + '@mui/utils': 5.16.6(@types/react@18.2.45)(react@18.2.0) '@popperjs/core': 2.11.8 '@types/react': 18.2.45 clsx: 2.1.1 @@ -3314,7 +3314,7 @@ packages: '@babel/runtime': 7.26.0 '@floating-ui/react-dom': 2.1.2(react-dom@18.3.1)(react@18.3.1) '@mui/types': 7.2.19(@types/react@18.2.45) - '@mui/utils': 5.15.11(@types/react@18.2.45)(react@18.3.1) + '@mui/utils': 5.16.6(@types/react@18.2.45)(react@18.3.1) '@popperjs/core': 2.11.8 '@types/react': 18.2.45 clsx: 2.1.1 @@ -4279,7 +4279,7 @@ packages: '@storybook/csf': 0.1.11 '@storybook/global': 5.0.0 '@storybook/icons': 1.2.12(react-dom@18.2.0)(react@18.2.0) - '@types/lodash': 4.14.202 + '@types/lodash': 4.17.13 color-convert: 2.0.1 dequal: 2.0.3 lodash: 4.17.21 @@ -5490,7 +5490,6 @@ packages: /@types/lodash@4.17.13: resolution: {integrity: sha512-lfx+dftrEZcdBPczf9d0Qv0x+j/rfNCMuC6OcfXmO8gkfeNAY88PgKUbvG56whcN23gc27yenwF6oJZXGFpYxg==} - dev: false /@types/markdown-it@14.1.2: resolution: {integrity: sha512-promo4eFwuiW+TfGxhi+0x3czqTYJkG8qB17ZUJiVF10Xm7NLVRSLUsfRTU/6h1e24VvRnXCx+hG7li58lkzog==} @@ -7634,8 +7633,8 @@ packages: dependencies: esutils: 2.0.3 - /docx@9.0.3: - resolution: {integrity: sha512-Ao/8v1BIRL0+uhzoqk6uvAPIjxNR5+uqQMphVWe8egxGdohkd/PgDLn1/+/c/te1DlnX1Chz9lIxuNi0Hs+Dbg==} + /docx@9.1.0: + resolution: {integrity: sha512-XOtseSTRrkKN/sV5jNBqyLazyhNpWfaUhpuKc22cs+5DavNjRQvchnohb0g0S+x/96/D06U/i0/U/Gc4E5kwuQ==} engines: {node: '>=10'} dependencies: '@types/node': 22.9.0 @@ -10023,7 +10022,7 @@ packages: js-yaml: 4.1.0 lodash: 4.17.21 minimist: 1.2.8 - prettier: 3.3.3 + prettier: 3.4.2 tinyglobby: 0.2.10 dev: false @@ -10450,8 +10449,8 @@ packages: react: 18.2.0 dev: true - /marked@14.1.4: - resolution: {integrity: sha512-vkVZ8ONmUdPnjCKc5uTRvmkRbx4EAi2OkTOXmfTDhZz3OFqMNBM1oTTWwTr4HY4uAEojhzPf+Fy8F1DWa3Sndg==} + /marked@15.0.3: + resolution: {integrity: sha512-Ai0cepvl2NHnTcO9jYDtcOEtVBNVYR31XnEA3BndO7f5As1wzpcOceSUM8FDkNLJNIODcLpDTWay/qQhqbuMvg==} engines: {node: '>= 18'} hasBin: true dev: false @@ -11438,8 +11437,8 @@ packages: hasBin: true dev: true - /prettier@3.3.3: - resolution: {integrity: sha512-i2tDNA0O5IrMO757lfrdQZCc2jPNDVntV0m/+4whiDfWaTKfMNgR7Qz0NAeGz/nRqF4m5/6CLzbP4/liHt12Ew==} + /prettier@3.4.2: + resolution: {integrity: sha512-e9MewbtFo+Fevyuxn/4rrcDAaq0IYxPGLvObpQjiZBMAzB9IGmzlnG9RZy3FFas+eBMu2vA0CszMeduow5dIuQ==} engines: {node: '>=14'} hasBin: true dev: false @@ -13523,6 +13522,11 @@ packages: engines: {node: '>=16'} dev: false + /type-fest@4.30.0: + resolution: {integrity: sha512-G6zXWS1dLj6eagy6sVhOMQiLtJdxQBHIA9Z6HFUNLOlr6MFOgzV8wvmidtPONfPtEUv0uZsy77XJNzTAfwPDaA==} + engines: {node: '>=16'} + dev: false + /type-is@1.6.18: resolution: {integrity: sha512-TkRKr9sUTxEH8MdfuCSP7VizJyzRNMjj2J2do2Jr3Kym598JVdEksuzPQCnlFPW4ky9Q+iA+ma9BGm06XQBy8g==} engines: {node: '>= 0.6'} @@ -14462,9 +14466,9 @@ packages: use-sync-external-store: 1.2.0(react@18.2.0) dev: false - github.com/theopensystemslab/planx-core/ccf9ac3(@types/react@18.2.45): - resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/ccf9ac3} - id: github.com/theopensystemslab/planx-core/ccf9ac3 + github.com/theopensystemslab/planx-core/29b4851(@types/react@18.2.45): + resolution: {tarball: https://codeload.github.com/theopensystemslab/planx-core/tar.gz/29b4851} + id: github.com/theopensystemslab/planx-core/29b4851 name: '@opensystemslab/planx-core' version: 1.0.0 prepare: true @@ -14479,18 +14483,18 @@ packages: ajv-formats: 2.1.1(ajv@8.17.1) cheerio: 1.0.0 copyfiles: 2.4.1 - docx: 9.0.3 + docx: 9.1.0 eslint: 8.57.1 fast-xml-parser: 4.5.0 graphql: 16.9.0 graphql-request: 6.1.0(graphql@16.9.0) json-schema-to-typescript: 15.0.3 lodash: 4.17.21 - marked: 14.1.4 - prettier: 3.3.3 + marked: 15.0.3 + prettier: 3.4.2 react: 18.3.1 react-dom: 18.3.1(react@18.3.1) - type-fest: 4.27.0 + type-fest: 4.30.0 uuid: 11.0.3 zod: 3.23.8 transitivePeerDependencies: diff --git a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/FeeBreakdown.tsx b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/FeeBreakdown.tsx index 5ef73fc59f..ae545d4098 100644 --- a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/FeeBreakdown.tsx +++ b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/FeeBreakdown.tsx @@ -140,10 +140,10 @@ export const FeeBreakdown: React.FC = () => {
- + - - + + diff --git a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/types.ts b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/types.ts deleted file mode 100644 index 176cf87724..0000000000 --- a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/types.ts +++ /dev/null @@ -1,21 +0,0 @@ -export interface FeeBreakdown { - amount: { - applicationFee: number; - total: number; - reduction: number; - vat: number | undefined; - }; - reductions: string[]; - exemptions: string[]; -} - -export interface PassportFeeFields { - "application.fee.calculated": number; - "application.fee.payable": number; - "application.fee.payable.vat": number; - "application.fee.reduction.alternative": boolean; - "application.fee.reduction.parishCouncil": boolean; - "application.fee.reduction.sports": boolean; - "application.fee.exemption.disability": boolean; - "application.fee.exemption.resubmission": boolean; -}; \ No newline at end of file diff --git a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/useFeeBreakdown.test.ts b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/useFeeBreakdown.test.ts deleted file mode 100644 index 5312cb12c2..0000000000 --- a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/useFeeBreakdown.test.ts +++ /dev/null @@ -1,256 +0,0 @@ -import { logger } from "airbrake"; -import { useStore } from "pages/FlowEditor/lib/store"; -import { vi } from "vitest"; - -import { useFeeBreakdown } from "./useFeeBreakdown"; - -vi.mock("pages/FlowEditor/lib/store", () => ({ - useStore: vi.fn(), -})); - -vi.mock("airbrake", () => ({ - logger: { - notify: vi.fn(), - }, -})); - -describe("useFeeBreakdown() hook", () => { - describe("valid data", () => { - it("returns a fee breakdown for number inputs", () => { - const mockPassportData = { - "application.fee.calculated": 1000, - "application.fee.payable": 800, - "application.fee.payable.vat": 160, - "some.other.fields": ["abc", "xyz"], - }; - - vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); - - const result = useFeeBreakdown(); - - expect(result).toEqual({ - amount: { - applicationFee: 1000, - total: 800, - reduction: 200, - vat: 160, - }, - exemptions: [], - reductions: [], - }); - }); - - it("returns a fee breakdown for number tuple inputs", () => { - const mockPassportData = { - "application.fee.calculated": [1000], - "application.fee.payable": [800], - "application.fee.payable.vat": [160], - "some.other.fields": ["abc", "xyz"], - }; - - vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); - - const result = useFeeBreakdown(); - - expect(result).toEqual({ - amount: { - applicationFee: 1000, - total: 800, - reduction: 200, - vat: 160, - }, - exemptions: [], - reductions: [], - }); - }); - - it("parses 'true' reduction values to a list of keys", () => { - const mockPassportData = { - "application.fee.calculated": 1000, - "application.fee.payable": 800, - "application.fee.payable.vat": 160, - "application.fee.reduction.alternative": ["true"], - "application.fee.reduction.parishCouncil": ["true"], - "some.other.fields": ["abc", "xyz"], - }; - - vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); - - const result = useFeeBreakdown(); - - expect(result?.reductions).toHaveLength(2); - expect(result?.reductions).toEqual( - expect.arrayContaining(["alternative", "parishCouncil"]) - ); - }); - - it("does not parse 'false' reduction values to a list of keys", () => { - const mockPassportData = { - "application.fee.calculated": 1000, - "application.fee.payable": 800, - "application.fee.payable.vat": 160, - "application.fee.reduction.alternative": ["false"], - "application.fee.reduction.parishCouncil": ["false"], - "some.other.fields": ["abc", "xyz"], - }; - - vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); - - const result = useFeeBreakdown(); - - expect(result?.reductions).toHaveLength(0); - }); - - it("does not parse non-schema reduction values", () => { - const mockPassportData = { - "application.fee.calculated": 1000, - "application.fee.payable": 800, - "application.fee.payable.vat": 160, - "application.fee.reduction.alternative": ["true"], - "application.fee.reduction.parishCouncil": ["false"], - "application.fee.reduction.someReason": ["true"], - "application.fee.reduction.someOtherReason": ["false"], - "some.other.fields": ["abc", "xyz"], - }; - - vi.mocked(useStore).mockReturnValue([ - mockPassportData, - "test-session", - ]); - - const result = useFeeBreakdown(); - - expect(result?.reductions).toEqual(expect.not.arrayContaining(["someReason"])) - expect(result?.reductions).toEqual(expect.not.arrayContaining(["someOtherReason"])) - }); - - it("parses 'true' exemption values to a list of keys", () => { - const mockPassportData = { - "application.fee.calculated": 1000, - "application.fee.payable": 800, - "application.fee.payable.vat": 160, - "application.fee.exemption.disability": ["true"], - "application.fee.exemption.resubmission": ["true"], - "some.other.fields": ["abc", "xyz"], - }; - - vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); - - const result = useFeeBreakdown(); - - expect(result?.exemptions).toHaveLength(2); - expect(result?.exemptions).toEqual( - expect.arrayContaining(["disability", "resubmission"]) - ); - }); - - it("does not parse 'false' exemption values to a list of keys", () => { - const mockPassportData = { - "application.fee.calculated": 1000, - "application.fee.payable": 800, - "application.fee.payable.vat": 160, - "application.fee.exemption.disability": ["false"], - "application.fee.exemption.resubmission": ["false"], - "some.other.fields": ["abc", "xyz"], - }; - - vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); - - const result = useFeeBreakdown(); - - expect(result?.exemptions).toHaveLength(0); - }); - - it("does not parse non-schema exemption values", () => { - const mockPassportData = { - "application.fee.calculated": 1000, - "application.fee.payable": 800, - "application.fee.payable.vat": 160, - "application.fee.exemption.disability": ["false"], - "application.fee.exemption.resubmission": ["false"], - "application.fee.exemption.someReason": ["true"], - "application.fee.exemption.someOtherReason": ["false"], - "some.other.fields": ["abc", "xyz"], - }; - - vi.mocked(useStore).mockReturnValue([ - mockPassportData, - "test-session", - ]); - - const result = useFeeBreakdown(); - - expect(result?.exemptions).toEqual( - expect.not.arrayContaining(["someReason"]) - ); - expect(result?.exemptions).toEqual( - expect.not.arrayContaining(["someOtherReason"]) - ); - }); - }); - - describe("invalid inputs", () => { - it("returns undefined for missing data", () => { - const mockPassportData = { - "some.other.fields": ["abc", "xyz"], - }; - - vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); - - const result = useFeeBreakdown(); - - expect(result).toBeUndefined(); - }); - - it("returns undefined for partial data", () => { - const mockPassportData = { - "application.fee.calculated": [1000], - "application.fee.payable.vat": [160], - "some.other.fields": ["abc", "xyz"], - }; - - vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); - - const result = useFeeBreakdown(); - - expect(result).toBeUndefined(); - }); - - it("returns undefined for incorrect data", () => { - const mockPassportData = { - "application.fee.calculated": "some string", - "application.fee.payable": [800, 700], - "application.fee.payable.vat": false, - "some.other.fields": ["abc", "xyz"], - }; - - vi.mocked(useStore).mockReturnValue([mockPassportData, "test-session"]); - - const result = useFeeBreakdown(); - - expect(result).toBeUndefined(); - }); - - it("calls Airbrake if invalid inputs are provided", () => { - const mockPassportData = { - "some.other.fields": ["abc", "xyz"], - }; - const mockSessionId = "test-session"; - - vi.mocked(useStore).mockReturnValue([mockPassportData, mockSessionId]); - const loggerSpy = vi.spyOn(logger, "notify"); - - const result = useFeeBreakdown(); - - expect(result).toBeUndefined(); - - expect(loggerSpy).toHaveBeenCalledWith( - expect.stringContaining(mockSessionId), - ); - - expect(loggerSpy).toHaveBeenCalledWith( - expect.stringContaining("ZodError"), - ); - }); - }); -}); diff --git a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/useFeeBreakdown.tsx b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/useFeeBreakdown.tsx index b1e6b10c1d..bad94a386e 100644 --- a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/useFeeBreakdown.tsx +++ b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/useFeeBreakdown.tsx @@ -1,9 +1,8 @@ +import { getFeeBreakdown } from "@opensystemslab/planx-core"; +import { FeeBreakdown } from "@opensystemslab/planx-core/types"; import { logger } from "airbrake"; import { useStore } from "pages/FlowEditor/lib/store"; -import { FeeBreakdown } from "./types"; -import { createPassportSchema } from "./utils"; - /** * Parses the users's Passport for data variables associated with their fee * Currently relies on static `application.fee.x` variables @@ -18,15 +17,13 @@ export const useFeeBreakdown = (): FeeBreakdown | undefined => { ]); if (!passportData) return - const schema = createPassportSchema(); - const result = schema.safeParse(passportData); - - if (!result.success) { + try { + const feeBreakdown = getFeeBreakdown(passportData); + return feeBreakdown; + } catch (error) { logger.notify( - `Failed to parse fee breakdown data from passport for session ${sessionId}. Error: ${result.error}`, + `Failed to parse fee breakdown data from passport for session ${sessionId}. Error: ${error}`, ); return; } - - return result.data; }; diff --git a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/utils.test.ts b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/utils.test.ts deleted file mode 100644 index 935c8cb066..0000000000 --- a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/utils.test.ts +++ /dev/null @@ -1,91 +0,0 @@ -import { PassportFeeFields } from "./types"; -import { calculateReduction, toFeeBreakdown, toNumber } from "./utils"; - -describe("toNumber() helper function", () => { - it("outputs a number when passed a number", () => { - const input = 12; - const output = toNumber(input); - - expect(output).toEqual(input); - }); - - it("outputs a number when passed a number tuple", () => { - const input: [number] = [12]; - const output = toNumber(input); - - expect(output).toEqual(12); - }); -}); - -describe("calculateReduction() helper function", () => { - it("correctly outputs the reduction when a calculated value is provided", () => { - const input: PassportFeeFields = { - "application.fee.calculated": 100, - "application.fee.payable": 50, - "application.fee.payable.vat": 0, - "application.fee.reduction.alternative": false, - "application.fee.reduction.parishCouncil": false, - "application.fee.reduction.sports": false, - "application.fee.exemption.disability": false, - "application.fee.exemption.resubmission": false, - }; - const reduction = calculateReduction(input); - - expect(reduction).toEqual(50); - }); - - it("defaults to 0 when calculated is 0", () => { - const input: PassportFeeFields = { - "application.fee.calculated": 0, - "application.fee.payable": 100, - "application.fee.payable.vat": 0, - "application.fee.reduction.alternative": false, - "application.fee.reduction.parishCouncil": false, - "application.fee.reduction.sports": false, - "application.fee.exemption.disability": false, - "application.fee.exemption.resubmission": false, - }; - const reduction = calculateReduction(input); - - expect(reduction).toEqual(0); - }); -}); - -describe("toFeeBreakdown() helper function", () => { - it("correctly maps fields", () => { - const input: PassportFeeFields = { - "application.fee.calculated": 100, - "application.fee.payable": 50, - "application.fee.payable.vat": 10, - "application.fee.reduction.alternative": false, - "application.fee.reduction.parishCouncil": false, - "application.fee.reduction.sports": false, - "application.fee.exemption.disability": false, - "application.fee.exemption.resubmission": false, - }; - - const { amount } = toFeeBreakdown(input); - - expect(amount.applicationFee).toEqual(input["application.fee.calculated"]); - expect(amount.total).toEqual(input["application.fee.payable"]); - expect(amount.vat).toEqual(input["application.fee.payable.vat"]); - expect(amount.reduction).toEqual(50); - }); - - it("sets applicationFee to payable amount if no calculated value is provided", () => { - const input: PassportFeeFields = { - "application.fee.calculated": 0, - "application.fee.payable.vat": 10, - "application.fee.payable": 50, - "application.fee.reduction.alternative": false, - "application.fee.reduction.parishCouncil": false, - "application.fee.reduction.sports": false, - "application.fee.exemption.disability": false, - "application.fee.exemption.resubmission": false, - }; - - const { amount } = toFeeBreakdown(input); - - expect(amount.applicationFee).toEqual(input["application.fee.payable"]); - }); -}); diff --git a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/utils.ts b/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/utils.ts deleted file mode 100644 index 759c5438db..0000000000 --- a/editor.planx.uk/src/@planx/components/Pay/Public/FeeBreakdown/utils.ts +++ /dev/null @@ -1,83 +0,0 @@ -import { z } from "zod"; - -import { FeeBreakdown, PassportFeeFields } from "./types"; - -export const toNumber = (input: number | [number]) => - Array.isArray(input) ? input[0] : input; - -/** - * Convert a Passport value to an actual boolean - */ -const toBoolean = (val: ["true" | "false"]) => val[0] === "true"; - - -/** - * Iterate over exemptions or reductions to find matches, returning the granular keys - */ - const getGranularKeys = ( - data: PassportFeeFields, - prefix: "application.fee.reduction" | "application.fee.exemption" - ) => { - const keys = Object.keys(data) as (keyof PassportFeeFields)[]; - const intersectingKeys = keys.filter( - (key) => key.startsWith(prefix) && Boolean(data[key]) - ); - const granularKeys = intersectingKeys.map((key) => - key.replace(prefix + ".", "") - ); - - return granularKeys; - }; - -/** - * A "reduction" is the sum of the difference between calculated and payable - */ -export const calculateReduction = (data: PassportFeeFields) => - data["application.fee.calculated"] - ? data["application.fee.calculated"] - data["application.fee.payable"] - : 0; - -/** - * Transform Passport data to a FeeBreakdown - */ -export const toFeeBreakdown = (data: PassportFeeFields): FeeBreakdown => ({ - amount: { - applicationFee: - data["application.fee.calculated"] || - data["application.fee.payable"], - total: data["application.fee.payable"], - vat: data["application.fee.payable.vat"], - reduction: calculateReduction(data), - }, - reductions: getGranularKeys(data, "application.fee.reduction"), - exemptions: getGranularKeys(data, "application.fee.exemption"), -}); - -export const createPassportSchema = () => { - const questionSchema = z.number().nonnegative(); - const setValueSchema = z.tuple([z.coerce.number().nonnegative()]); - const feeSchema = z - .union([questionSchema, setValueSchema]) - .transform(toNumber); - - /** Describes how boolean values are set via PlanX components */ - const booleanSchema = z - .tuple([z.enum(["true", "false"])]) - .default(["false"]) - .transform(toBoolean) - - const schema = z - .object({ - "application.fee.calculated": feeSchema.optional().default(0), - "application.fee.payable": feeSchema, - "application.fee.payable.vat": feeSchema.optional().default(0), - "application.fee.reduction.alternative": booleanSchema, - "application.fee.reduction.parishCouncil": booleanSchema, - "application.fee.reduction.sports": booleanSchema, - "application.fee.exemption.disability": booleanSchema, - "application.fee.exemption.resubmission": booleanSchema, - }) - .transform(toFeeBreakdown); - - return schema; -}; From d9b70e0017c406eef614e79b07b7ce5ff5a92e65 Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Fri, 6 Dec 2024 11:03:30 +0100 Subject: [PATCH 04/25] fix: ensure empty JSON files aren't written to S3 (#4048) --- api.planx.uk/modules/file/service/uploadFile.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api.planx.uk/modules/file/service/uploadFile.ts b/api.planx.uk/modules/file/service/uploadFile.ts index ead3cee9af..a6ee74227b 100644 --- a/api.planx.uk/modules/file/service/uploadFile.ts +++ b/api.planx.uk/modules/file/service/uploadFile.ts @@ -79,9 +79,9 @@ export function generateFileParams( ACL: "public-read", Bucket: process.env.AWS_S3_BUCKET, Key: key, - Body: file.buffer, + Body: file.buffer || JSON.stringify(file), ContentDisposition: `inline;filename="${filename}"`, - ContentType: file.mimetype, + ContentType: file.mimetype || "application/json", }; return { From 4fe75035b71932cb04a4dee05299a03026af1bda Mon Sep 17 00:00:00 2001 From: Dan G Date: Fri, 6 Dec 2024 15:36:45 +0000 Subject: [PATCH 05/25] extend Hasura service boot grace period from 0 to 180s (#4051) --- infrastructure/application/services/hasura.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/infrastructure/application/services/hasura.ts b/infrastructure/application/services/hasura.ts index d6f9696753..c27698d872 100644 --- a/infrastructure/application/services/hasura.ts +++ b/infrastructure/application/services/hasura.ts @@ -110,6 +110,8 @@ export const createHasuraService = async ({ }, }, desiredCount: 1, + // experiment with non-zero grace period to see if it resolves scale up failure + healthCheckGracePeriodSeconds: 180, }); new cloudflare.Record("hasura", { From b4f6930267ad7789c182a7d7769c7960aa22b10d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Fri, 6 Dec 2024 15:57:41 +0000 Subject: [PATCH 06/25] fix: CVE-2024-52798 (#4050) --- api.planx.uk/package.json | 2 +- api.planx.uk/pnpm-lock.yaml | 32 ++++++++++++++++---------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/api.planx.uk/package.json b/api.planx.uk/package.json index 27b1560166..fe0456e5ff 100644 --- a/api.planx.uk/package.json +++ b/api.planx.uk/package.json @@ -25,7 +25,7 @@ "csv-stringify": "^6.5.0", "date-fns": "^3.3.1", "dompurify": "^3.1.6", - "express": "^4.21.1", + "express": "^4.21.2", "express-async-errors": "^3.1.1", "express-jwt": "^8.4.1", "express-pino-logger": "^7.0.0", diff --git a/api.planx.uk/pnpm-lock.yaml b/api.planx.uk/pnpm-lock.yaml index 9279cf8f9e..88eaf57dac 100644 --- a/api.planx.uk/pnpm-lock.yaml +++ b/api.planx.uk/pnpm-lock.yaml @@ -57,11 +57,11 @@ dependencies: specifier: ^3.1.6 version: 3.1.6 express: - specifier: ^4.21.1 - version: 4.21.1 + specifier: ^4.21.2 + version: 4.21.2 express-async-errors: specifier: ^3.1.1 - version: 3.1.1(express@4.21.1) + version: 3.1.1(express@4.21.2) express-jwt: specifier: ^8.4.1 version: 8.4.1 @@ -70,7 +70,7 @@ dependencies: version: 7.0.0 express-rate-limit: specifier: ^7.1.5 - version: 7.1.5(express@4.21.1) + version: 7.1.5(express@4.21.2) form-data: specifier: ^4.0.0 version: 4.0.0 @@ -142,7 +142,7 @@ dependencies: version: 6.2.8(openapi-types@12.1.3) swagger-ui-express: specifier: ^5.0.0 - version: 5.0.0(express@4.21.1) + version: 5.0.0(express@4.21.2) type-fest: specifier: ^4.18.1 version: 4.18.1 @@ -4335,12 +4335,12 @@ packages: strip-final-newline: 3.0.0 dev: true - /express-async-errors@3.1.1(express@4.21.1): + /express-async-errors@3.1.1(express@4.21.2): resolution: {integrity: sha512-h6aK1da4tpqWSbyCa3FxB/V6Ehd4EEB15zyQq9qe75OZBp0krinNKuH4rAY+S/U/2I36vdLAUFSjQJ+TFmODng==} peerDependencies: express: ^4.16.2 dependencies: - express: 4.21.1 + express: 4.21.2 dev: false /express-jwt@8.4.1: @@ -4359,21 +4359,21 @@ packages: pino-http: 6.6.0 dev: false - /express-rate-limit@7.1.5(express@4.21.1): + /express-rate-limit@7.1.5(express@4.21.2): resolution: {integrity: sha512-/iVogxu7ueadrepw1bS0X0kaRC/U0afwiYRSLg68Ts+p4Dc85Q5QKsOnPS/QUjPMHvOJQtBDrZgvkOzf8ejUYw==} engines: {node: '>= 16'} peerDependencies: express: 4 || 5 || ^5.0.0-beta.1 dependencies: - express: 4.21.1 + express: 4.21.2 dev: false /express-unless@2.1.3: resolution: {integrity: sha512-wj4tLMyCVYuIIKHGt0FhCtIViBcwzWejX0EjNxveAa6dG+0XBCQhMbx+PnkLkFCxLC69qoFrxds4pIyL88inaQ==} dev: false - /express@4.21.1: - resolution: {integrity: sha512-YSFlK1Ee0/GC8QaO91tHcDxJiE/X4FbpAyQWkxAvG6AXCuR65YzK8ua6D9hvi/TzUfZMpc+BwuM1IPw8fmQBiQ==} + /express@4.21.2: + resolution: {integrity: sha512-28HqgMZAmih1Czt9ny7qr6ek2qddF4FclbMzwhCREB6OFfH+rXAnuNCwo1/wFvrtbgsQDb4kSbX9de9lFbrXnA==} engines: {node: '>= 0.10.0'} dependencies: accepts: 1.3.8 @@ -4395,7 +4395,7 @@ packages: methods: 1.1.2 on-finished: 2.4.1 parseurl: 1.3.3 - path-to-regexp: 0.1.10 + path-to-regexp: 0.1.12 proxy-addr: 2.0.7 qs: 6.13.0 range-parser: 1.2.1 @@ -5928,8 +5928,8 @@ packages: minipass: 7.1.2 dev: true - /path-to-regexp@0.1.10: - resolution: {integrity: sha512-7lf7qcQidTku0Gu3YDPc8DJ1q7OOucfa/BSsIwjuh56VU7katFvuM8hULfkwB3Fns/rsVF7PwPKVw1sl5KQS9w==} + /path-to-regexp@0.1.12: + resolution: {integrity: sha512-RA1GjUVMnvYFxuqovrEqZoxxW5NUZqbwKtYz/Tt7nXerk0LbLblQmrsgdeOxV5SFHf0UDggjS/bSeOZwt1pmEQ==} dev: false /path-type@4.0.0: @@ -6728,13 +6728,13 @@ packages: '@scarf/scarf': 1.4.0 dev: false - /swagger-ui-express@5.0.0(express@4.21.1): + /swagger-ui-express@5.0.0(express@4.21.2): resolution: {integrity: sha512-tsU9tODVvhyfkNSvf03E6FAk+z+5cU3lXAzMy6Pv4av2Gt2xA0++fogwC4qo19XuFf6hdxevPuVCSKFuMHJhFA==} engines: {node: '>= v0.10.32'} peerDependencies: express: '>=4.0.0 || >=5.0.0-beta' dependencies: - express: 4.21.1 + express: 4.21.2 swagger-ui-dist: 5.18.2 dev: false From b398905492b59cba022561b0a2c25be2daa6e4fd Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Fri, 6 Dec 2024 17:02:32 +0100 Subject: [PATCH 07/25] fix: add missing BOPS app type suffixes to Slack notification pilot criteria (#4052) --- .../modules/webhooks/service/sendNotification/index.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/api.planx.uk/modules/webhooks/service/sendNotification/index.ts b/api.planx.uk/modules/webhooks/service/sendNotification/index.ts index c6ff6409b5..462fff7e25 100644 --- a/api.planx.uk/modules/webhooks/service/sendNotification/index.ts +++ b/api.planx.uk/modules/webhooks/service/sendNotification/index.ts @@ -36,7 +36,11 @@ export const sendSlackNotification = async ( const pilotServices = [ "uniform", "happ", + "hapr", + "hret", "ldc", + "minor", + "major", "apply for planning permission", "apply for a lawful development certificate", ]; From cb83df3027dcc60689d7a81123fd97c6b718b617 Mon Sep 17 00:00:00 2001 From: augustlindemer <118665588+augustlindemer@users.noreply.github.com> Date: Fri, 6 Dec 2024 17:17:54 +0000 Subject: [PATCH 08/25] fix: correct editor link validation (#4053) --- editor.planx.uk/src/ui/editor/RichTextInput/RichTextInput.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/editor.planx.uk/src/ui/editor/RichTextInput/RichTextInput.tsx b/editor.planx.uk/src/ui/editor/RichTextInput/RichTextInput.tsx index 5b679ea204..a04721cafd 100644 --- a/editor.planx.uk/src/ui/editor/RichTextInput/RichTextInput.tsx +++ b/editor.planx.uk/src/ui/editor/RichTextInput/RichTextInput.tsx @@ -200,8 +200,8 @@ export const emptyContent = "

"; const linkSelectionError = (selectionHtml: string): string | null => { if (selectionHtml.startsWith("

") && selectionHtml.endsWith("

")) { const text = selectionHtml.slice(3, -4); - const lowercaseText = text.toLowerCase(); - if (lowercaseText.includes("click") || lowercaseText.includes("here")) { + const lowercaseText = text.toLowerCase().trim().replace(/[.,]/g, ""); + if (lowercaseText === "click here" || lowercaseText === "clicking here") { return "Links must be set over text that accurately describes what the link is for. Avoid generic language such as 'click here'."; } if (text[0] && text[0] !== text[0].toUpperCase() && text.length < 8) { From f0234a861d1654da79897937802698f170c6548f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Mon, 9 Dec 2024 09:04:50 +0000 Subject: [PATCH 09/25] fix: Convert Digital Planning payload to JSON file before upload (#4054) --- .../modules/file/service/uploadFile.ts | 4 ++-- api.planx.uk/modules/file/service/utils.ts | 21 +++++++++++++++++++ api.planx.uk/modules/send/s3/index.ts | 21 ++++++++++--------- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/api.planx.uk/modules/file/service/uploadFile.ts b/api.planx.uk/modules/file/service/uploadFile.ts index a6ee74227b..ead3cee9af 100644 --- a/api.planx.uk/modules/file/service/uploadFile.ts +++ b/api.planx.uk/modules/file/service/uploadFile.ts @@ -79,9 +79,9 @@ export function generateFileParams( ACL: "public-read", Bucket: process.env.AWS_S3_BUCKET, Key: key, - Body: file.buffer || JSON.stringify(file), + Body: file.buffer, ContentDisposition: `inline;filename="${filename}"`, - ContentType: file.mimetype || "application/json", + ContentType: file.mimetype, }; return { diff --git a/api.planx.uk/modules/file/service/utils.ts b/api.planx.uk/modules/file/service/utils.ts index 0f95e170e5..85e58cd110 100644 --- a/api.planx.uk/modules/file/service/utils.ts +++ b/api.planx.uk/modules/file/service/utils.ts @@ -1,5 +1,6 @@ import { S3 } from "@aws-sdk/client-s3"; import { isLiveEnv } from "../../../helpers.js"; +import { Readable } from "stream"; export function s3Factory() { return new S3({ @@ -40,3 +41,23 @@ export function getS3KeyFromURL(fileURL: string): string { const key = [folder, file].map(decodeURIComponent).join("/"); return key; } + +export const convertObjectToMulterJSONFile = ( + data: Record, + fileName: string, +): Express.Multer.File => { + const buffer = Buffer.from(JSON.stringify(data)); + + return { + buffer: buffer, + originalname: fileName, + mimetype: "application/json", + size: buffer.length, + fieldname: "file", + encoding: "7bit", + stream: Readable.from(buffer), + destination: "", + filename: "", + path: "", + }; +}; diff --git a/api.planx.uk/modules/send/s3/index.ts b/api.planx.uk/modules/send/s3/index.ts index 6f7186ae8b..2a414d4e94 100644 --- a/api.planx.uk/modules/send/s3/index.ts +++ b/api.planx.uk/modules/send/s3/index.ts @@ -8,6 +8,7 @@ import { uploadPrivateFile } from "../../file/service/uploadFile.js"; import { markSessionAsSubmitted } from "../../saveAndReturn/service/utils.js"; import { isApplicationTypeSupported } from "../utils/helpers.js"; import type { SendIntegrationController } from "../types.js"; +import { convertObjectToMulterJSONFile } from "../../file/service/utils.js"; interface CreateS3Application { insertS3Application: { @@ -44,16 +45,16 @@ const sendToS3: SendIntegrationController = async (_req, res, next) => { const flowName = session?.flow?.name; // Generate the ODP Schema JSON, skipping validation if not a supported application type - const doValidation = isApplicationTypeSupported(passport); - const exportData = doValidation - ? await $api.export.digitalPlanningDataPayload(sessionId) - : await $api.export.digitalPlanningDataPayload(sessionId, true); + const skipValidation = !isApplicationTypeSupported(passport); + const exportData = await $api.export.digitalPlanningDataPayload( + sessionId, + skipValidation, + ); // Create and upload the data as an S3 file - const { fileUrl } = await uploadPrivateFile( - exportData, - `${sessionId}.json`, - ); + const filename = `${sessionId}.json`; + const file = convertObjectToMulterJSONFile(exportData, filename); + const { fileUrl } = await uploadPrivateFile(file, filename); // Send a notification with the file URL to the Power Automate webhook const webhookRequest: AxiosRequestConfig = { @@ -69,7 +70,7 @@ const sendToS3: SendIntegrationController = async (_req, res, next) => { service: flowName, environment: env, file: fileUrl, - payload: doValidation ? "Validated ODP Schema" : "Discretionary", + payload: skipValidation ? "Discretionary" : "Validated ODP Schema", }, }; const webhookResponse = await axios(webhookRequest) @@ -125,7 +126,7 @@ const sendToS3: SendIntegrationController = async (_req, res, next) => { res.status(200).send({ message: `Successfully uploaded submission to S3: ${fileUrl}`, - payload: doValidation ? "Validated ODP Schema" : "Discretionary", + payload: skipValidation ? "Discretionary" : "Validated ODP Schema", webhookResponse: webhookResponse.axiosResponse.status, auditEntryId: webhookResponse.id, }); From 6390a70adefcbd64d516f525fb6ef4f6a4dd6f13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Mon, 9 Dec 2024 14:32:29 +0000 Subject: [PATCH 10/25] test(api): Coverage for send/s3 module (#4055) --- api.planx.uk/modules/send/s3/index.test.ts | 352 ++++++++++++++++----- api.planx.uk/modules/send/s3/index.ts | 96 +++--- api.planx.uk/vitest.config.ts | 8 +- 3 files changed, 331 insertions(+), 125 deletions(-) diff --git a/api.planx.uk/modules/send/s3/index.test.ts b/api.planx.uk/modules/send/s3/index.test.ts index 5570fb7205..4215e17a84 100644 --- a/api.planx.uk/modules/send/s3/index.test.ts +++ b/api.planx.uk/modules/send/s3/index.test.ts @@ -1,58 +1,47 @@ import supertest from "supertest"; -import type planxCore from "@opensystemslab/planx-core"; import app from "../../../server.js"; import { expectedPlanningPermissionPayload } from "../../../tests/mocks/digitalPlanningDataMocks.js"; import { queryMock } from "../../../tests/graphqlQueryMock.js"; +import { $api } from "../../../client/index.js"; +import { mockLowcalSession } from "../../../tests/mocks/saveAndReturnMocks.js"; +import axios, { type AxiosRequestConfig } from "axios"; +import { markSessionAsSubmitted } from "../../saveAndReturn/service/utils.js"; + +const sessionId = "3188f052-a032-4755-be63-72b0ba497eb6"; +const mockPowerAutomateWebhookURL = "http://www.example.com"; +const mockPowerAutomateAPIKey = "my-power-automate-api-key"; vi.mock("../../saveAndReturn/service/utils", () => ({ markSessionAsSubmitted: vi.fn(), })); -vi.mock("@opensystemslab/planx-core", async (importOriginal) => { - const actualCore = await importOriginal(); - const actualCoreDomainClient = actualCore.CoreDomainClient; - - return { - CoreDomainClient: class extends actualCoreDomainClient { - constructor() { - super(); - this.export.digitalPlanningDataPayload = async () => - vi.fn().mockResolvedValue({ - exportData: expectedPlanningPermissionPayload, - }); - } - }, - }; -}); +vi.mock("../../file/service/uploadFile.js", () => ({ + uploadPrivateFile: vi + .fn() + .mockResolvedValue({ fileUrl: "https://my-file-url.com" }), +})); + +vi.mock("../../client/index.js"); + +vi.mock("axios", () => ({ + default: vi.fn(), +})); +const mockedAxios = vi.mocked(axios, true); -describe(`uploading an application to S3`, () => { +describe("uploading an application to S3", () => { beforeEach(() => { - queryMock.mockQuery({ - name: "GetStagingIntegrations", - data: { - teams: [ - { - integrations: { - powerAutomateWebhookURL: "test.azure.com/whatever", - powerAutomateAPIKey: "secret", - }, - }, - ], - }, - variables: { - slug: "barnet", - }, + $api.team.getIntegrations = vi.fn().mockResolvedValue({ + powerAutomateWebhookURL: mockPowerAutomateWebhookURL, + powerAutomateAPIKey: mockPowerAutomateAPIKey, }); - queryMock.mockQuery({ - name: "GetStagingIntegrations", - data: { - teams: [], - }, - variables: { - slug: "unsupported-team", - }, - }); + $api.session.find = vi.fn().mockResolvedValue(mockLowcalSession); + + $api.export.digitalPlanningDataPayload = vi + .fn() + .mockResolvedValue(expectedPlanningPermissionPayload); + + mockedAxios.mockResolvedValue({ data: { success: true }, status: 200 }); queryMock.mockQuery({ name: "CreateS3Application", @@ -63,39 +52,260 @@ describe(`uploading an application to S3`, () => { }); }); - it("requires auth", async () => { - await supertest(app) - .post("/upload-submission/barnet") - .send({ payload: { sessionId: "123" } }) - .expect(401); - }); + describe("request validation", () => { + it("requires auth", async () => { + await supertest(app) + .post("/upload-submission/barnet") + .send({ payload: { sessionId: "123" } }) + .expect(401); + }); - it("throws an error if payload is missing", async () => { - await supertest(app) - .post("/upload-submission/barnet") - .set({ Authorization: process.env.HASURA_PLANX_API_KEY! }) - .send({ payload: null }) - .expect(400) - .then((res) => { - expect(res.body).toHaveProperty("issues"); - expect(res.body).toHaveProperty("name", "ZodError"); + it("throws an error if payload is missing", async () => { + await supertest(app) + .post("/upload-submission/barnet") + .set({ Authorization: process.env.HASURA_PLANX_API_KEY! }) + .send({ payload: null }) + .expect(400) + .then((res) => { + expect(res.body).toHaveProperty("issues"); + expect(res.body).toHaveProperty("name", "ZodError"); + }); + }); + + it("throws an error if powerAutomateWebhookURL is not set", async () => { + $api.team.getIntegrations = vi.fn().mockResolvedValueOnce({ + powerAutomateWebhookURL: null, + powerAutomateAPIKey: "some-key", }); - }); - it("throws an error if team is unsupported", async () => { - await supertest(app) - .post("/upload-submission/unsupported-team") - .set({ Authorization: process.env.HASURA_PLANX_API_KEY! }) - .send({ - payload: { sessionId: "3188f052-a032-4755-be63-72b0ba497eb6" }, - }) - .expect(500) - .then((res) => { - expect(res.body.error).toMatch( - /No team matching "unsupported-team" found/, - ); + await supertest(app) + .post("/upload-submission/barnet") + .set({ Authorization: process.env.HASURA_PLANX_API_KEY! }) + .send({ + payload: { sessionId }, + }) + .expect(400) + .then((res) => { + expect(res.body.error).toMatch( + /Upload to S3 is not enabled for this local authority/, + ); + }); + }); + + it("throws an error if powerAutomateAPIKey is not set", async () => { + $api.team.getIntegrations = vi.fn().mockResolvedValueOnce({ + powerAutomateWebhookURL: "https://www.example.com", + powerAutomateAPIKey: null, }); + + await supertest(app) + .post("/upload-submission/barnet") + .set({ Authorization: process.env.HASURA_PLANX_API_KEY! }) + .send({ + payload: { sessionId }, + }) + .expect(400) + .then((res) => { + expect(res.body.error).toMatch( + /Upload to S3 is not enabled for this local authority/, + ); + }); + }); + }); + + describe("payload validation", () => { + it("validates statutory payloads", async () => { + await supertest(app) + .post("/upload-submission/barnet") + .set({ Authorization: process.env.HASURA_PLANX_API_KEY! }) + .send({ + payload: { sessionId }, + }); + + // Verify mock session is a statutory application + expect( + mockLowcalSession.data.passport.data?.["application.type"]?.[0], + ).toBe("ldc.proposed"); + + expect($api.export.digitalPlanningDataPayload).toHaveBeenCalledTimes(1); + expect($api.export.digitalPlanningDataPayload).toHaveBeenCalledWith( + sessionId, + false, // Validation not skipped + ); + + // Validation status passed to webhook + expect(mockedAxios).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + payload: "Validated ODP Schema", + }), + }), + ); + }); + + it("does not validate discretionary payloads", async () => { + // Set up mock discretionary payload + const mockDiscretionarySession = structuredClone(mockLowcalSession); + mockDiscretionarySession.data.passport.data["application.type"][0] = + "reportAPlanningBreach"; + $api.session.find = vi + .fn() + .mockResolvedValueOnce(mockDiscretionarySession); + + await supertest(app) + .post("/upload-submission/barnet") + .set({ Authorization: process.env.HASURA_PLANX_API_KEY! }) + .send({ + payload: { sessionId }, + }); + + expect($api.export.digitalPlanningDataPayload).toHaveBeenCalledTimes(1); + expect($api.export.digitalPlanningDataPayload).toHaveBeenCalledWith( + sessionId, + true, // Validation skipped + ); + + // Validation status passed to webhook + expect(mockedAxios).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + payload: "Discretionary", + }), + }), + ); + }); }); - it.todo("succeeds"); // mock uploadPrivateFile ?? + describe("error handling", () => { + it("throws an error if the webhook request fails", async () => { + mockedAxios.mockRejectedValueOnce( + new Error( + "Something went wrong with the webhook request to Power Automate", + ), + ); + + await supertest(app) + .post("/upload-submission/barnet") + .set({ Authorization: process.env.HASURA_PLANX_API_KEY! }) + .send({ + payload: { sessionId }, + }) + .expect(500) + .then((res) => { + expect(res.body.error).toMatch( + /Failed to send submission notification/, + ); + }); + }); + + it("throws an error if an internal process fails", async () => { + $api.session.find = vi + .fn() + .mockRejectedValueOnce(new Error("Something went wrong!")); + + await supertest(app) + .post("/upload-submission/barnet") + .set({ Authorization: process.env.HASURA_PLANX_API_KEY! }) + .send({ + payload: { sessionId }, + }) + .expect(500) + .then((res) => { + expect(res.body.error).toMatch(/Failed to upload submission to S3/); + }); + }); + }); + + describe("success", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + const callAPI = async () => + await supertest(app) + .post("/upload-submission/barnet") + .set({ Authorization: process.env.HASURA_PLANX_API_KEY! }) + .send({ + payload: { sessionId }, + }); + + it("makes a request to the configured Power Automate webhook", async () => { + await callAPI(); + + expect(mockedAxios).toHaveBeenCalledOnce(); + const request = mockedAxios.mock.calls[0][0] as AxiosRequestConfig; + expect(request.url).toEqual(mockPowerAutomateWebhookURL); + }); + + it("sets Power Automate API key in request header", async () => { + await callAPI(); + + expect(mockedAxios).toHaveBeenCalledOnce(); + const request = mockedAxios.mock.calls[0][0] as AxiosRequestConfig; + expect(request.headers).toHaveProperty("apiKey", mockPowerAutomateAPIKey); + }); + + it("generates the expected body for the Power Automate webhook", async () => { + await callAPI(); + + expect(mockedAxios).toHaveBeenCalledOnce(); + const request = mockedAxios.mock.calls[0][0] as AxiosRequestConfig; + expect(request.data).toEqual({ + message: "New submission from PlanX", + environment: "staging", + file: "https://my-file-url.com", + payload: "Validated ODP Schema", + service: "Apply for a Lawful Development Certificate", + }); + }); + + it("passes along the correct application environment details", async () => { + vi.stubEnv("APP_ENVIRONMENT", "production"); + + await callAPI(); + + expect(mockedAxios).toHaveBeenCalledOnce(); + const request = mockedAxios.mock.calls[0][0] as AxiosRequestConfig; + expect(request.data.environment).toEqual("production"); + }); + + it("marks a session as submitted", async () => { + await callAPI(); + + expect(markSessionAsSubmitted).toHaveBeenCalledOnce(); + expect(markSessionAsSubmitted).toHaveBeenCalledWith(sessionId); + }); + + it("writes an audit record the the s3_applications table", async () => { + await callAPI(); + + const graphQLCalls = queryMock.getCalls(); + + expect(graphQLCalls).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: "CreateS3Application", + }), + ]), + ); + }); + + it("returns a success message upon completion", async () => { + const res = await callAPI(); + + expect($api.export.digitalPlanningDataPayload).toHaveBeenCalledWith( + sessionId, + false, + ); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ + message: + "Successfully uploaded submission to S3: https://my-file-url.com", + payload: "Validated ODP Schema", + webhookResponse: 200, + auditEntryId: 1, + }); + }); + }); }); diff --git a/api.planx.uk/modules/send/s3/index.ts b/api.planx.uk/modules/send/s3/index.ts index 2a414d4e94..eed4ea0b0a 100644 --- a/api.planx.uk/modules/send/s3/index.ts +++ b/api.planx.uk/modules/send/s3/index.ts @@ -73,62 +73,58 @@ const sendToS3: SendIntegrationController = async (_req, res, next) => { payload: skipValidation ? "Discretionary" : "Validated ODP Schema", }, }; - const webhookResponse = await axios(webhookRequest) - .then(async (res) => { - // Mark session as submitted so that reminder and expiry emails are not triggered - markSessionAsSubmitted(sessionId); - // Create an audit entry - const applicationId = await $api.client.request( - gql` - mutation CreateS3Application( - $session_id: String! - $team_slug: String! - $webhook_request: jsonb! - $webhook_response: jsonb = {} - ) { - insertS3Application: insert_s3_applications_one( - object: { - session_id: $session_id - team_slug: $team_slug - webhook_request: $webhook_request - webhook_response: $webhook_response - } - ) { - id - } - } - `, - { - session_id: sessionId, - team_slug: localAuthority, - webhook_request: webhookRequest, - webhook_response: { - status: res.status, - statusText: res.statusText, - headers: res.headers, - config: res.config, - data: res.data, - }, - }, - ); + const webhookResponse = await axios(webhookRequest).catch((error) => { + throw new Error( + `Failed to send submission notification to ${localAuthority}'s Power Automate Webhook (${sessionId}): ${error}`, + ); + }); - return { - id: applicationId.insertS3Application?.id, - axiosResponse: res, - }; - }) - .catch((error) => { - throw new Error( - `Failed to send submission notification to ${localAuthority}'s Power Automate Webhook (${sessionId}): ${error}`, - ); - }); + // Mark session as submitted so that reminder and expiry emails are not triggered + markSessionAsSubmitted(sessionId); + + // Create an audit entry + const { + insertS3Application: { id: auditEntryId }, + } = await $api.client.request( + gql` + mutation CreateS3Application( + $session_id: String! + $team_slug: String! + $webhook_request: jsonb! + $webhook_response: jsonb = {} + ) { + insertS3Application: insert_s3_applications_one( + object: { + session_id: $session_id + team_slug: $team_slug + webhook_request: $webhook_request + webhook_response: $webhook_response + } + ) { + id + } + } + `, + { + session_id: sessionId, + team_slug: localAuthority, + webhook_request: webhookRequest, + webhook_response: { + status: webhookResponse.status, + statusText: webhookResponse.statusText, + headers: webhookResponse.headers, + config: webhookResponse.config, + data: webhookResponse.data, + }, + }, + ); res.status(200).send({ message: `Successfully uploaded submission to S3: ${fileUrl}`, payload: skipValidation ? "Discretionary" : "Validated ODP Schema", - webhookResponse: webhookResponse.axiosResponse.status, - auditEntryId: webhookResponse.id, + webhookResponse: webhookResponse.status, + auditEntryId, }); } catch (error) { return next({ diff --git a/api.planx.uk/vitest.config.ts b/api.planx.uk/vitest.config.ts index 77fcbeff21..16b2a43efd 100644 --- a/api.planx.uk/vitest.config.ts +++ b/api.planx.uk/vitest.config.ts @@ -13,10 +13,10 @@ export default defineConfig({ // html reporter required to inspect coverage in Vitest UI dashboard reporter: ["lcov", "html", "text-summary"], thresholds: { - statements: 73.28, - branches: 55.27, - functions: 73.59, - lines: 73.42, + statements: 73.41, + branches: 55.76, + functions: 74.09, + lines: 73.54, autoUpdate: true, }, }, From ea9f33e82f4be7a4bd110bad59ecc79f050e0951 Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Mon, 9 Dec 2024 17:25:05 +0100 Subject: [PATCH 11/25] feat: editors can select which planning constraints to check (#3968) --- .../modules/flows/validate/service/index.ts | 12 +- .../validate/service/planningConstraints.ts | 38 ++ .../modules/flows/validate/validate.test.ts | 185 ++++++++ .../modules/gis/service/digitalLand.ts | 34 +- .../components/PlanningConstraints/Editor.tsx | 184 ++++---- .../PlanningConstraints/Public.test.tsx | 81 ++++ .../components/PlanningConstraints/Public.tsx | 56 +-- .../components/PlanningConstraints/model.ts | 4 +- ...automations.planningConstraintNots.test.ts | 188 -------- .../automations.planningConstraints.test.ts | 411 ++++++++++++++++++ .../src/pages/FlowEditor/lib/store/preview.ts | 108 +++-- 11 files changed, 967 insertions(+), 334 deletions(-) create mode 100644 api.planx.uk/modules/flows/validate/service/planningConstraints.ts delete mode 100644 editor.planx.uk/src/pages/FlowEditor/lib/__tests__/automations.planningConstraintNots.test.ts create mode 100644 editor.planx.uk/src/pages/FlowEditor/lib/__tests__/automations.planningConstraints.test.ts diff --git a/api.planx.uk/modules/flows/validate/service/index.ts b/api.planx.uk/modules/flows/validate/service/index.ts index 85710b307c..7b77f595e7 100644 --- a/api.planx.uk/modules/flows/validate/service/index.ts +++ b/api.planx.uk/modules/flows/validate/service/index.ts @@ -8,8 +8,9 @@ import * as jsondiffpatch from "jsondiffpatch"; import { dataMerged, getMostRecentPublishedFlow } from "../../../../helpers.js"; import { validateFileTypes } from "./fileTypes.js"; import { validateInviteToPay } from "./inviteToPay.js"; -import { validateSections } from "./sections.js"; +import { validatePlanningConstraints } from "./planningConstraints.js"; import { validateProjectTypes } from "./projectTypes.js"; +import { validateSections } from "./sections.js"; type AlteredNode = { id: string; @@ -54,7 +55,14 @@ const validateAndDiffFlow = async ( const inviteToPay = validateInviteToPay(flattenedFlow); const fileTypes = validateFileTypes(flattenedFlow); const projectTypes = validateProjectTypes(flattenedFlow); - validationChecks.push(sections, inviteToPay, fileTypes, projectTypes); + const planningConstraints = validatePlanningConstraints(flattenedFlow); + validationChecks.push( + sections, + inviteToPay, + fileTypes, + projectTypes, + planningConstraints, + ); // Arrange list of validation checks in order of status: Fail, Warn, Pass, Not applicable const failingChecks = validationChecks.filter((v) => v.status == "Fail"); diff --git a/api.planx.uk/modules/flows/validate/service/planningConstraints.ts b/api.planx.uk/modules/flows/validate/service/planningConstraints.ts new file mode 100644 index 0000000000..e86f073349 --- /dev/null +++ b/api.planx.uk/modules/flows/validate/service/planningConstraints.ts @@ -0,0 +1,38 @@ +import { + ComponentType, + type FlowGraph, +} from "@opensystemslab/planx-core/types"; +import { numberOfComponentType } from "../helpers.js"; +import type { FlowValidationResponse } from "./index.js"; + +const validatePlanningConstraints = ( + flowGraph: FlowGraph, +): FlowValidationResponse => { + const numberOfPlanningConstraintNodes = numberOfComponentType( + flowGraph, + ComponentType.PlanningConstraints, + ); + if (numberOfPlanningConstraintNodes > 1) { + return { + title: "Planning Constraints", + status: "Fail", + message: + "When using Planning Constraints, your flow must have exactly ONE Planning Constraints component", + }; + } else if (numberOfPlanningConstraintNodes === 1) { + // In future, add extra `hasPlanningData` validation step here to ensure integration is available for this team + return { + title: "Planning Constraints", + status: "Pass", + message: "Your flow has valid Planning Constraints", + }; + } else { + return { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }; + } +}; + +export { validatePlanningConstraints }; diff --git a/api.planx.uk/modules/flows/validate/validate.test.ts b/api.planx.uk/modules/flows/validate/validate.test.ts index 46aa4a0833..9850036cd1 100644 --- a/api.planx.uk/modules/flows/validate/validate.test.ts +++ b/api.planx.uk/modules/flows/validate/validate.test.ts @@ -131,6 +131,11 @@ describe("sections validation on diff", () => { message: 'Your flow is not using Checklists which set "proposal.projectType"', }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, ]); }); }); @@ -190,6 +195,11 @@ describe("sections validation on diff", () => { message: 'Your flow is not using Checklists which set "proposal.projectType"', }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, ]); }); }); @@ -240,6 +250,11 @@ describe("invite to pay validation on diff", () => { status: "Not applicable", message: "Your flow is not using FileUpload or UploadAndLabel", }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, ]); }); }); @@ -300,6 +315,11 @@ describe("invite to pay validation on diff", () => { status: "Not applicable", message: "Your flow is not using FileUpload or UploadAndLabel", }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, ]); }); }); @@ -356,6 +376,11 @@ describe("invite to pay validation on diff", () => { status: "Not applicable", message: "Your flow is not using FileUpload or UploadAndLabel", }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, ]); }); }); @@ -414,6 +439,11 @@ describe("invite to pay validation on diff", () => { status: "Not applicable", message: "Your flow is not using FileUpload or UploadAndLabel", }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, ]); }); }); @@ -468,6 +498,11 @@ describe("invite to pay validation on diff", () => { message: 'Your flow is not using Checklists which set "proposal.projectType"', }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, ]); }); }); @@ -555,6 +590,11 @@ describe("ODP Schema file type validation on diff", () => { message: 'Your flow is not using Checklists which set "proposal.projectType"', }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, ]); }); }); @@ -633,6 +673,151 @@ describe("ODP Schema file type validation on diff", () => { message: 'Your flow is not using Checklists which set "proposal.projectType"', }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, + ]); + }); + }); +}); + +describe("planning constraints validation on diff", () => { + it("passes if there is exactly one planning constraints component", async () => { + const alteredFlow = { + ...mockFlowData, + PlanningConstraints: { + type: 11, + data: { + title: "Check all constraints", + fn: "property.constraints.planning", + }, + }, + }; + + queryMock.mockQuery({ + name: "GetFlowData", + matchOnVariables: false, + data: { + flow: { + data: alteredFlow, + slug: "altered-flow-name", + team_id: 1, + team: { + slug: "testing", + }, + publishedFlows: [{ data: alteredFlow }], + }, + }, + }); + + await supertest(app) + .post("/flows/1/diff") + .set(auth) + .expect(200) + .then((res) => { + expect(res.body.message).toEqual("Changes queued to publish"); + expect(res.body.validationChecks).toEqual([ + { + title: "Sections", + status: "Pass", + message: "Your flow has valid Sections", + }, + { + title: "Planning Constraints", + status: "Pass", + message: "Your flow has valid Planning Constraints", + }, + { + title: "Invite to Pay", + status: "Not applicable", + message: "Your flow is not using Invite to Pay", + }, + { + title: "File types", + status: "Not applicable", + message: "Your flow is not using FileUpload or UploadAndLabel", + }, + { + title: "Project types", + status: "Not applicable", + message: + 'Your flow is not using Checklists which set "proposal.projectType"', + }, + ]); + }); + }); + + it("warns if there is more than one planning constraints component", async () => { + const alteredFlow = { + ...mockFlowData, + PlanningConstraints: { + type: 11, + data: { + title: "Check all constraints", + fn: "property.constraints.planning", + }, + }, + PlanningConstraintsTwo: { + type: 11, + data: { + title: "Check all constraints (dupe)", + fn: "property.constraints.planning", + }, + }, + }; + + queryMock.mockQuery({ + name: "GetFlowData", + matchOnVariables: false, + data: { + flow: { + data: alteredFlow, + slug: "altered-flow-name", + team_id: 1, + team: { + slug: "testing", + }, + publishedFlows: [{ data: alteredFlow }], + }, + }, + }); + + await supertest(app) + .post("/flows/1/diff") + .set(auth) + .expect(200) + .then((res) => { + expect(res.body.message).toEqual("Changes queued to publish"); + expect(res.body.validationChecks).toEqual([ + { + title: "Planning Constraints", + status: "Fail", + message: + "When using Planning Constraints, your flow must have exactly ONE Planning Constraints component", + }, + { + title: "Sections", + status: "Pass", + message: "Your flow has valid Sections", + }, + { + title: "Invite to Pay", + status: "Not applicable", + message: "Your flow is not using Invite to Pay", + }, + { + title: "File types", + status: "Not applicable", + message: "Your flow is not using FileUpload or UploadAndLabel", + }, + { + title: "Project types", + status: "Not applicable", + message: + 'Your flow is not using Checklists which set "proposal.projectType"', + }, ]); }); }); diff --git a/api.planx.uk/modules/gis/service/digitalLand.ts b/api.planx.uk/modules/gis/service/digitalLand.ts index d96115c4c4..f1a715d143 100644 --- a/api.planx.uk/modules/gis/service/digitalLand.ts +++ b/api.planx.uk/modules/gis/service/digitalLand.ts @@ -71,15 +71,30 @@ async function go( geom: string, extras: Record, ): Promise { - // generate list of digital land datasets we should query based on 'active' planx schema variables + // generate list of digital land datasets we should query and their associated passport values const activeDatasets: string[] = []; - Object.keys(baseSchema).forEach((key) => { - if (baseSchema[key]["active"]) { - baseSchema[key]["digital-land-datasets"]?.forEach((dataset: string) => { - activeDatasets.push(dataset); - }); - } - }); + let activeDataValues: string[] = []; + if (extras?.vals) { + // if the editor selected constraints, prioritise their selection + activeDataValues = extras.vals.split(","); + Object.keys(baseSchema).forEach((key) => { + if (activeDataValues.includes(key)) { + baseSchema[key]["digital-land-datasets"]?.forEach((dataset: string) => { + activeDatasets.push(dataset); + }); + } + }); + } else { + // else default to the internally maintained list of all "active" datasets + Object.keys(baseSchema).forEach((key) => { + if (baseSchema[key]["active"]) { + activeDataValues.push(key); + baseSchema[key]["digital-land-datasets"]?.forEach((dataset: string) => { + activeDatasets.push(dataset); + }); + } + }); + } // set up request query params per https://www.planning.data.gov.uk/docs const options = { @@ -159,7 +174,8 @@ async function go( // TODO followup with digital land about how to return 'nots' via API (currently assumes any "active" metadata was successfully queried) const nots = Object.keys(baseSchema).filter( (key) => - baseSchema[key]["active"] && !Object.keys(formattedResult).includes(key), + activeDataValues.includes(key) && + !Object.keys(formattedResult).includes(key), ); nots.forEach((not) => { formattedResult[not] = { diff --git a/editor.planx.uk/src/@planx/components/PlanningConstraints/Editor.tsx b/editor.planx.uk/src/@planx/components/PlanningConstraints/Editor.tsx index 0e084932ed..9770ab1bdc 100644 --- a/editor.planx.uk/src/@planx/components/PlanningConstraints/Editor.tsx +++ b/editor.planx.uk/src/@planx/components/PlanningConstraints/Editor.tsx @@ -1,5 +1,4 @@ import Box from "@mui/material/Box"; -import Checkbox from "@mui/material/Checkbox"; import Table from "@mui/material/Table"; import TableBody from "@mui/material/TableBody"; import TableCell from "@mui/material/TableCell"; @@ -11,14 +10,16 @@ import { ComponentType as TYPES } from "@opensystemslab/planx-core/types"; import { EditorProps } from "@planx/components/shared/types"; import { useFormik } from "formik"; import React from "react"; -import { FONT_WEIGHT_BOLD } from "theme"; import InputGroup from "ui/editor/InputGroup"; import { ModalFooter } from "ui/editor/ModalFooter"; import ModalSection from "ui/editor/ModalSection"; import ModalSectionContent from "ui/editor/ModalSectionContent"; import RichTextInput from "ui/editor/RichTextInput/RichTextInput"; +import Checkbox from "ui/shared/Checkbox/Checkbox"; +import ErrorWrapper from "ui/shared/ErrorWrapper"; import Input from "ui/shared/Input/Input"; import InputRow from "ui/shared/InputRow"; +import { array, object, string } from "yup"; import { ICONS } from "../shared/icons"; import { availableDatasets, parseContent, PlanningConstraints } from "./model"; @@ -36,8 +37,37 @@ function PlanningConstraintsComponent(props: Props) { data: newValues, }); }, + validationSchema: object({ + dataValues: array(string()).min(1, "Select at least one constraint"), + }), }); + const changeSelectAll = + (vals: string[] | undefined) => + (_event?: React.MouseEvent | undefined) => { + let newCheckedVals: string[]; + if (vals?.length !== availableDatasets.length) { + newCheckedVals = availableDatasets.map((d) => d.val); + } else { + newCheckedVals = []; + } + formik.setFieldValue("dataValues", newCheckedVals); + }; + + const changeDataset = + (val: string) => + (_event?: React.MouseEvent | undefined) => { + let newCheckedVals; + if (formik.values.dataValues?.includes(val)) { + newCheckedVals = formik.values.dataValues.filter((dv) => dv !== val); + } else { + newCheckedVals = formik.values.dataValues + ? [...formik.values.dataValues, val] + : [val]; + } + formik.setFieldValue("dataValues", newCheckedVals); + }; + return (