From ea7046eebaec3d2f7fd7557c5587100f652734f7 Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Fri, 10 May 2024 17:08:11 +0200 Subject: [PATCH] always enable continue on file uploads --- .../DrawBoundary/Public/Public.test.tsx | 8 +- .../components/DrawBoundary/Public/index.tsx | 137 ++++++++++++------ .../components/FileUpload/Public.test.tsx | 18 ++- .../@planx/components/FileUpload/Public.tsx | 8 +- 4 files changed, 117 insertions(+), 54 deletions(-) diff --git a/editor.planx.uk/src/@planx/components/DrawBoundary/Public/Public.test.tsx b/editor.planx.uk/src/@planx/components/DrawBoundary/Public/Public.test.tsx index 7d39c9f59f..bce6b9989d 100644 --- a/editor.planx.uk/src/@planx/components/DrawBoundary/Public/Public.test.tsx +++ b/editor.planx.uk/src/@planx/components/DrawBoundary/Public/Public.test.tsx @@ -139,7 +139,13 @@ test("shows the file upload option by default and requires user data to continue // Navigate to upload a file screen await user.click(screen.getByTestId("upload-file-button")); expect(screen.getByText("Upload a file")).toBeInTheDocument(); - expect(screen.getByTestId("continue-button")).toBeDisabled(); + + // Continue is enabled by default, but requires data to proceed + expect(screen.getByTestId("continue-button")).toBeEnabled(); + await user.click(screen.getByTestId("continue-button")); + expect( + screen.getByTestId("error-message-upload-location-plan"), + ).toBeInTheDocument(); }); test("hides the upload option and allows user to continue without drawing if editor specifies", async () => { diff --git a/editor.planx.uk/src/@planx/components/DrawBoundary/Public/index.tsx b/editor.planx.uk/src/@planx/components/DrawBoundary/Public/index.tsx index 84ecd9d1e8..0ee776900f 100644 --- a/editor.planx.uk/src/@planx/components/DrawBoundary/Public/index.tsx +++ b/editor.planx.uk/src/@planx/components/DrawBoundary/Public/index.tsx @@ -14,11 +14,13 @@ import { PrivateFileUpload } from "@planx/components/shared/PrivateFileUpload/Pr import { squareMetresToHectares } from "@planx/components/shared/utils"; import type { PublicProps } from "@planx/components/ui"; import buffer from "@turf/buffer"; -import { type Feature, point } from "@turf/helpers"; +import { type Feature,point } from "@turf/helpers"; import { Store, useStore } from "pages/FlowEditor/lib/store"; import React, { useEffect, useRef, useState } from "react"; import { FONT_WEIGHT_SEMI_BOLD } from "theme"; import FullWidthWrapper from "ui/public/FullWidthWrapper"; +import ErrorWrapper from "ui/shared/ErrorWrapper"; +import { array } from "yup"; import { DrawBoundary, @@ -31,6 +33,21 @@ export type Props = PublicProps; export type Boundary = Feature | undefined; +const slotsSchema = array() + .required() + .test({ + name: "nonUploading", + message: "Upload a location plan.", + test: (slots?: Array) => { + return Boolean( + slots && + slots.length === 1 && + !slots.some((slot) => slot.status === "uploading") && + slots.every((slot) => slot.url && slot.status === "success"), + ); + }, + }); + export default function Component(props: Props) { const isMounted = useRef(false); const passport = useStore((state) => state.computePassport()); @@ -52,7 +69,9 @@ export default function Component(props: Props) { props.previouslySubmittedData?.data?.[PASSPORT_UPLOAD_KEY]; const startPage = previousFile ? "upload" : "draw"; const [page, setPage] = useState<"draw" | "upload">(startPage); + const [slots, setSlots] = useState(previousFile ?? []); + const [fileValidationError, setFileValidationError] = useState(); const addressPoint = passport?.data?._address?.longitude && @@ -94,44 +113,69 @@ export default function Component(props: Props) { }; }, [page, setArea, setBoundary, setSlots]); - return ( - { - const newPassportData: Store.userData["data"] = {}; + /** + * Declare a ref to hold a mutable copy the up-to-date validation error. + * The intention is to prevent frequent unnecessary update loops that clears the + * validation error state if it is already empty. + */ + const validationErrorRef = useRef(fileValidationError); + useEffect(() => { + validationErrorRef.current = fileValidationError; + }, [fileValidationError]); + + useEffect(() => { + if (validationErrorRef.current) { + setFileValidationError(undefined); + } + }, [slots]); - // Used the map - if (page === "draw" && boundary && props.dataFieldBoundary) { - newPassportData[props.dataFieldBoundary] = boundary; - newPassportData[`${props.dataFieldBoundary}.buffered`] = buffer( - boundary, - bufferInMeters, - { units: "meters" }, - ); + const validateAndSubmit = () => { + const newPassportData: Store.userData["data"] = {}; - if (area && props.dataFieldArea) { - newPassportData[props.dataFieldArea] = area; - newPassportData[`${props.dataFieldArea}.hectares`] = - squareMetresToHectares(area); - } + // Used the map + if (page === "draw") { + if (boundary && props.dataFieldBoundary) { + newPassportData[props.dataFieldBoundary] = boundary; + newPassportData[`${props.dataFieldBoundary}.buffered`] = buffer( + boundary, + bufferInMeters, + { units: "meters" }, + ); - // Track the type of map interaction - if ( - boundary?.geometry === - passport.data?.["property.boundary.title"]?.geometry - ) { - newPassportData[PASSPORT_COMPONENT_ACTION_KEY] = - DrawBoundaryUserAction.Accept; - } else if (boundary?.properties?.dataset === "title-boundary") { - newPassportData[PASSPORT_COMPONENT_ACTION_KEY] = - DrawBoundaryUserAction.Amend; - } else { - newPassportData[PASSPORT_COMPONENT_ACTION_KEY] = - DrawBoundaryUserAction.Draw; - } + if (area && props.dataFieldArea) { + newPassportData[props.dataFieldArea] = area; + newPassportData[`${props.dataFieldArea}.hectares`] = + squareMetresToHectares(area); + } + + // Track the type of map interaction + if ( + boundary?.geometry === + passport.data?.["property.boundary.title"]?.geometry + ) { + newPassportData[PASSPORT_COMPONENT_ACTION_KEY] = + DrawBoundaryUserAction.Accept; + } else if (boundary?.properties?.dataset === "title-boundary") { + newPassportData[PASSPORT_COMPONENT_ACTION_KEY] = + DrawBoundaryUserAction.Amend; + } else { + newPassportData[PASSPORT_COMPONENT_ACTION_KEY] = + DrawBoundaryUserAction.Draw; } - // Uploaded a file - if (page === "upload" && slots.length) { + props.handleSubmit?.({ data: { ...newPassportData } }); + } + + if (props.hideFileUpload && !boundary) { + props.handleSubmit?.({ data: { ...newPassportData } }); + } + } + + // Uploaded a file + if (page === "upload") { + slotsSchema + .validate(slots, { context: { slots } }) + .then(() => { newPassportData[PASSPORT_UPLOAD_KEY] = slots; newPassportData[PASSPORT_COMPONENT_ACTION_KEY] = DrawBoundaryUserAction.Upload; @@ -146,24 +190,27 @@ export default function Component(props: Props) { recommended, optional, }; - } - props.handleSubmit?.({ data: { ...newPassportData } }); - }} + props.handleSubmit?.({ data: { ...newPassportData } }); + }) + .catch((err) => setFileValidationError(err?.message)); + } + }; + + return ( + - {getBody(bufferInMeters)} + {getBody(bufferInMeters, fileValidationError)} ); - function getBody(bufferInMeters: number) { + function getBody(bufferInMeters: number, fileValidationError?: string) { if (page === "draw") { return ( <> @@ -257,7 +304,9 @@ export default function Component(props: Props) { howMeasured={props.howMeasured} definitionImg={props.definitionImg} /> - + + + { +test("renders correctly", async () => { const handleSubmit = jest.fn(); setup(); - expect(screen.getByRole("button", { name: "Continue" })).toBeDisabled(); + expect(screen.getByRole("button", { name: "Continue" })).toBeEnabled(); expect(handleSubmit).toHaveBeenCalledTimes(0); }); +test("shows error if user tries to continue before adding files", async () => { + const handleSubmit = jest.fn(); + + const { user } = setup( + , + ); + + await user.click(screen.getByTestId("continue-button")); + expect(screen.getByText("Upload at least one file.")).toBeInTheDocument(); + + // Blocked by validation error + expect(handleSubmit).toHaveBeenCalledTimes(0); +}); + test("recovers previously submitted files when clicking the back button", async () => { const handleSubmit = jest.fn(); const componentId = uniqueId(); diff --git a/editor.planx.uk/src/@planx/components/FileUpload/Public.tsx b/editor.planx.uk/src/@planx/components/FileUpload/Public.tsx index aa2008a1f0..fb5a867508 100644 --- a/editor.planx.uk/src/@planx/components/FileUpload/Public.tsx +++ b/editor.planx.uk/src/@planx/components/FileUpload/Public.tsx @@ -115,13 +115,7 @@ const FileUpload: React.FC = (props) => { }, [slots]); return ( - 0 && - slots.every((slot) => slot.url && slot.status === "success") - } - handleSubmit={handleSubmit} - > +