From 48a4be0438a7de5dd0f8d525ab7a51c35ee2b971 Mon Sep 17 00:00:00 2001
From: Jessica McInchak <jessica.mcinchak@gmail.com>
Date: Mon, 13 May 2024 16:23:10 +0200
Subject: [PATCH] always enable continue button on findproperty map page

---
 .../components/FindProperty/Public/Map.tsx    | 133 ++++++++++-------
 .../FindProperty/Public/Public.test.tsx       |  15 +-
 .../components/FindProperty/Public/index.tsx  | 136 +++++++++---------
 3 files changed, 161 insertions(+), 123 deletions(-)

diff --git a/editor.planx.uk/src/@planx/components/FindProperty/Public/Map.tsx b/editor.planx.uk/src/@planx/components/FindProperty/Public/Map.tsx
index 90978ee9d6..1ad63c2512 100644
--- a/editor.planx.uk/src/@planx/components/FindProperty/Public/Map.tsx
+++ b/editor.planx.uk/src/@planx/components/FindProperty/Public/Map.tsx
@@ -11,10 +11,10 @@ import {
   MapContainer,
   MapFooter,
 } from "@planx/components/shared/Preview/MapContainer";
-import { GeoJSONObject } from "@turf/helpers";
 import { useStore } from "pages/FlowEditor/lib/store";
-import React, { useEffect, useState } from "react";
+import React, { useEffect, useRef, useState } from "react";
 import InputLabel from "ui/public/InputLabel";
+import ErrorWrapper from "ui/shared/ErrorWrapper";
 import Input from "ui/shared/Input";
 
 import { DEFAULT_NEW_ADDRESS_LABEL, SiteAddress } from "../model";
@@ -23,10 +23,15 @@ interface PlotNewAddressProps {
   setAddress: React.Dispatch<React.SetStateAction<SiteAddress | undefined>>;
   setPage: React.Dispatch<React.SetStateAction<"os-address" | "new-address">>;
   initialProposedAddress?: SiteAddress;
-  boundary?: GeoJSONObject | undefined;
   id?: string;
   description?: string;
   descriptionLabel?: string;
+  mapValidationError?: string;
+  setMapValidationError: React.Dispatch<
+    React.SetStateAction<string | undefined>
+  >;
+  showSiteDescriptionError: boolean;
+  setShowSiteDescriptionError: React.Dispatch<React.SetStateAction<boolean>>;
 }
 
 type Coordinates = {
@@ -55,8 +60,6 @@ export default function PlotNewAddress(props: PlotNewAddressProps): FCReturn {
   const [siteDescription, setSiteDescription] = useState<string | null>(
     props.initialProposedAddress?.title ?? null,
   );
-  const [showSiteDescriptionError, setShowSiteDescriptionError] =
-    useState<boolean>(false);
 
   const [environment, boundaryBBox] = useStore((state) => [
     state.previewEnvironment,
@@ -85,6 +88,17 @@ export default function PlotNewAddress(props: PlotNewAddressProps): FCReturn {
     };
   }, [setCoordinates]);
 
+  const mapValidationErrorRef = useRef(props.mapValidationError);
+  useEffect(() => {
+    mapValidationErrorRef.current = props.mapValidationError;
+  }, [props.mapValidationError]);
+
+  useEffect(() => {
+    if (mapValidationErrorRef.current) {
+      props.setMapValidationError(undefined);
+    }
+  }, [coordinates]);
+
   useEffect(() => {
     // when we have all required address parts, call setAddress to enable the "Continue" button
     if (siteDescription && coordinates) {
@@ -99,7 +113,7 @@ export default function PlotNewAddress(props: PlotNewAddressProps): FCReturn {
   }, [coordinates, siteDescription]);
 
   const handleSiteDescriptionCheck = () => {
-    if (!siteDescription) setShowSiteDescriptionError(true);
+    if (!siteDescription) props.setShowSiteDescriptionError(true);
   };
 
   const handleSiteDescriptionInputChange = (
@@ -111,54 +125,65 @@ export default function PlotNewAddress(props: PlotNewAddressProps): FCReturn {
 
   return (
     <>
-      <MapContainer environment={environment} size="large">
-        <p style={visuallyHidden}>
-          An interactive map centred on the local authority district, showing
-          the Ordnance Survey basemap. Click to place a point representing your
-          proposed site location.
-        </p>
-        {/* @ts-ignore */}
-        <my-map
-          id="plot-new-address-map"
-          ariaLabelOlFixedOverlay="An interactive map for providing your site location"
-          data-testid="map-web-component"
-          zoom={14}
-          drawMode
-          drawType="Point"
-          geojsonData={JSON.stringify(props?.boundary)}
-          geojsonColor="#efefef"
-          geojsonBuffer={10}
-          resetControlImage="trash"
-          showScale
-          showNorthArrow
-          osProxyEndpoint={`${process.env.REACT_APP_API_URL}/proxy/ordnance-survey`}
-          clipGeojsonData={JSON.stringify(boundaryBBox)}
-          collapseAttributions={window.innerWidth < 500 ? true : undefined}
-        />
-        <MapFooter>
+      <ErrorWrapper error={props.mapValidationError} id="plot-new-address-map">
+        <MapContainer environment={environment} size="large">
+          <p style={visuallyHidden}>
+            An interactive map centred on the local authority district, showing
+            the Ordnance Survey basemap. Click to place a point representing
+            your proposed site location.
+          </p>
+          {/* @ts-ignore */}
+          <my-map
+            id="plot-new-address-map"
+            ariaLabelOlFixedOverlay="An interactive map for providing your site location"
+            data-testid="map-web-component"
+            zoom={14}
+            drawMode
+            drawType="Point"
+            drawGeojsonData={
+              coordinates &&
+              JSON.stringify({
+                type: "Feature",
+                geometry: {
+                  type: "Point",
+                  coordinates: [coordinates?.longitude, coordinates?.latitude],
+                },
+                properties: {},
+              })
+            }
+            drawGeojsonDataBuffer={20}
+            resetControlImage="trash"
+            showScale
+            showNorthArrow
+            osProxyEndpoint={`${process.env.REACT_APP_API_URL}/proxy/ordnance-survey`}
+            clipGeojsonData={JSON.stringify(boundaryBBox)}
+            collapseAttributions={window.innerWidth < 500 ? true : undefined}
+          />
+        </MapContainer>
+      </ErrorWrapper>
+      <MapFooter>
+        <Typography variant="body2">
+          The coordinate location of your address point is:{" "}
+          <strong>
+            {`${
+              (coordinates?.x && Math.round(coordinates.x)) || 0
+            } Easting (X), ${
+              (coordinates?.y && Math.round(coordinates.y)) || 0
+            } Northing (Y)`}
+          </strong>
+        </Typography>
+        <Link
+          component="button"
+          onClick={() => {
+            props.setPage("os-address");
+            props.setAddress(undefined);
+          }}
+        >
           <Typography variant="body2">
-            The coordinate location of your address point is:{" "}
-            <strong>
-              {`${
-                (coordinates?.x && Math.round(coordinates.x)) || 0
-              } Easting (X), ${
-                (coordinates?.y && Math.round(coordinates.y)) || 0
-              } Northing (Y)`}
-            </strong>
+            I want to select an existing address
           </Typography>
-          <Link
-            component="button"
-            onClick={() => {
-              props.setPage("os-address");
-              props.setAddress(undefined);
-            }}
-          >
-            <Typography variant="body2">
-              I want to select an existing address
-            </Typography>
-          </Link>
-        </MapFooter>
-      </MapContainer>
+        </Link>
+      </MapFooter>
       <DescriptionInput data-testid="new-address-input" mt={2}>
         <InputLabel
           label={props.descriptionLabel || DEFAULT_NEW_ADDRESS_LABEL}
@@ -171,7 +196,7 @@ export default function PlotNewAddress(props: PlotNewAddressProps): FCReturn {
             id={`${props.id}-siteDescription`}
             value={siteDescription || ""}
             errorMessage={
-              showSiteDescriptionError && !siteDescription
+              props.showSiteDescriptionError && !siteDescription
                 ? `Enter a site description such as "Land at..."`
                 : ""
             }
@@ -183,7 +208,7 @@ export default function PlotNewAddress(props: PlotNewAddressProps): FCReturn {
             inputProps={{
               "aria-describedby": [
                 props.description ? DESCRIPTION_TEXT : "",
-                showSiteDescriptionError && !siteDescription
+                props.showSiteDescriptionError && !siteDescription
                   ? `${ERROR_MESSAGE}-${props.id}`
                   : "",
               ]
diff --git a/editor.planx.uk/src/@planx/components/FindProperty/Public/Public.test.tsx b/editor.planx.uk/src/@planx/components/FindProperty/Public/Public.test.tsx
index 343e6d7990..7e5dee7686 100644
--- a/editor.planx.uk/src/@planx/components/FindProperty/Public/Public.test.tsx
+++ b/editor.planx.uk/src/@planx/components/FindProperty/Public/Public.test.tsx
@@ -198,8 +198,9 @@ describe("render states", () => {
     const descriptionInput = screen.getByTestId("new-address-input");
     expect(descriptionInput).toBeInTheDocument();
 
-    // expect continue to be disabled because an address has not been selected
-    expect(screen.getByTestId("continue-button")).toBeDisabled();
+    // Continue button is always enabled, but validation prevents submit until we have complete address details
+    expect(screen.getByTestId("continue-button")).toBeEnabled();
+    await user.click(screen.getByTestId("continue-button"));
     expect(handleSubmit).not.toHaveBeenCalled();
   });
 
@@ -416,9 +417,15 @@ describe("plotting a new address that does not have a uprn yet", () => {
       screen.getByText(`Enter a site description such as "Land at..."`),
     ).toBeInTheDocument();
 
-    // expect continue to be disabled because we have incomplete address details
-    expect(screen.getByTestId("continue-button")).toBeDisabled();
+    // Continue button is always enabled, but validation prevents submit until we have complete address details
+    expect(screen.getByTestId("continue-button")).toBeEnabled();
+    await user.click(screen.getByTestId("continue-button"));
     expect(handleSubmit).not.toHaveBeenCalled();
+
+    // continue should trigger map error wrapper too
+    expect(
+      screen.getByTestId("error-message-plot-new-address-map"),
+    ).toBeInTheDocument();
   });
 
   it("recovers previously submitted address when clicking the back button and lands on the map page", async () => {
diff --git a/editor.planx.uk/src/@planx/components/FindProperty/Public/index.tsx b/editor.planx.uk/src/@planx/components/FindProperty/Public/index.tsx
index 02307ff308..10bdab7330 100644
--- a/editor.planx.uk/src/@planx/components/FindProperty/Public/index.tsx
+++ b/editor.planx.uk/src/@planx/components/FindProperty/Public/index.tsx
@@ -7,9 +7,9 @@ import QuestionHeader from "@planx/components/shared/Preview/QuestionHeader";
 import { squareMetresToHectares } from "@planx/components/shared/utils";
 import { PublicProps } from "@planx/components/ui";
 import area from "@turf/area";
-import { Feature, GeoJSONObject } from "@turf/helpers";
+import { Feature } from "@turf/helpers";
 import DelayedLoadingIndicator from "components/DelayedLoadingIndicator";
-import { Store, useStore } from "pages/FlowEditor/lib/store";
+import { Store } from "pages/FlowEditor/lib/store";
 import React, { useEffect, useState } from "react";
 import useSWR from "swr";
 import ExternalPlanningSiteDialog, {
@@ -51,6 +51,10 @@ function Component(props: Props) {
       : "os-address";
   const [page, setPage] = useState<"os-address" | "new-address">(startPage);
 
+  const [mapValidationError, setMapValidationError] = useState<string>();
+  const [showSiteDescriptionError, setShowSiteDescriptionError] =
+    useState<boolean>(false);
+
   const [address, setAddress] = useState<SiteAddress | undefined>(
     previouslySubmittedData?._address,
   );
@@ -59,9 +63,6 @@ function Component(props: Props) {
   >();
   const [regions, setRegions] = useState<string[] | undefined>();
   const [titleBoundary, setTitleBoundary] = useState<Feature | undefined>();
-  const [boundary, setBoundary] = useState<GeoJSONObject | undefined>();
-
-  const teamSettings = useStore((state) => state.teamSettings);
 
   // Use the address point to fetch the Local Authority District(s) & region via Digital Land
   const options = new URLSearchParams({
@@ -89,21 +90,6 @@ function Component(props: Props) {
     },
   );
 
-  // if allowNewAddresses is on, fetch the boundary geojson for this team to position the map view or default to London
-  //   example value for team.settings.boundary is https://www.planning.data.gov.uk/entity/8600093.geojson
-  const { data: geojson } = useSWR(
-    () =>
-      props.allowNewAddresses && teamSettings?.boundary
-        ? teamSettings.boundary
-        : null,
-    fetcher,
-    {
-      shouldRetryOnError: true,
-      errorRetryInterval: 500,
-      errorRetryCount: 1,
-    },
-  );
-
   useEffect(() => {
     if (address && data?.features?.length > 0) {
       const lads: string[] = [];
@@ -124,11 +110,62 @@ function Component(props: Props) {
     }
   }, [data, address]);
 
-  useEffect(() => {
-    if (geojson) setBoundary(geojson);
-  }, [geojson]);
+  const validateAndSubmit = () => {
+    // TODO `if (isValidating)` on either page, wrap Continue button in error mesage?
+
+    if (page === "new-address") {
+      if (address?.x === undefined && address?.y === undefined)
+        setMapValidationError("Click or tap to place a point on the map");
+
+      if (address?.title === undefined) setShowSiteDescriptionError(true);
+    }
+
+    if (address) {
+      const newPassportData: Store.userData["data"] = {};
+      newPassportData["_address"] = address;
+      if (address?.planx_value) {
+        newPassportData["property.type"] = [address.planx_value];
+      }
+
+      if (localAuthorityDistricts) {
+        newPassportData["property.localAuthorityDistrict"] =
+          localAuthorityDistricts;
+      }
+      if (regions) {
+        newPassportData["property.region"] = regions;
+      }
+      if (titleBoundary) {
+        const areaSquareMetres =
+          Math.round(area(titleBoundary as Feature) * 100) / 100;
+        newPassportData["property.boundary.title"] = titleBoundary;
+        newPassportData["property.boundary.title.area"] = areaSquareMetres;
+        newPassportData["property.boundary.title.area.hectares"] =
+          squareMetresToHectares(areaSquareMetres);
+      }
+
+      newPassportData[PASSPORT_COMPONENT_ACTION_KEY] =
+        address?.source === "os"
+          ? FindPropertyUserAction.Existing
+          : FindPropertyUserAction.New;
+
+      props.handleSubmit?.({ data: { ...newPassportData } });
+    }
+  };
+
+  return (
+    <Card
+      handleSubmit={validateAndSubmit}
+      isValid={
+        page === "new-address" && !isValidating
+          ? true
+          : Boolean(address) && !isValidating
+      }
+    >
+      {getBody()}
+    </Card>
+  );
 
-  function getPageBody() {
+  function getBody() {
     if (props.allowNewAddresses && page === "new-address") {
       return (
         <>
@@ -143,11 +180,20 @@ function Component(props: Props) {
               previouslySubmittedData?._address?.source === "proposed" &&
               previouslySubmittedData?._address
             }
-            boundary={boundary}
             id={props.id}
             description={props.newAddressDescription || ""}
             descriptionLabel={props.newAddressDescriptionLabel || ""}
+            mapValidationError={mapValidationError}
+            setMapValidationError={setMapValidationError}
+            showSiteDescriptionError={showSiteDescriptionError}
+            setShowSiteDescriptionError={setShowSiteDescriptionError}
           />
+          {Boolean(address) && isValidating && (
+            <DelayedLoadingIndicator
+              msDelayBeforeVisible={50}
+              text="Fetching data..."
+            />
+          )}
         </>
       );
     } else {
@@ -200,44 +246,4 @@ function Component(props: Props) {
       );
     }
   }
-
-  return (
-    <Card
-      isValid={Boolean(address) && !isValidating}
-      handleSubmit={() => {
-        if (address) {
-          const newPassportData: Store.userData["data"] = {};
-          newPassportData["_address"] = address;
-          if (address?.planx_value) {
-            newPassportData["property.type"] = [address.planx_value];
-          }
-
-          if (localAuthorityDistricts) {
-            newPassportData["property.localAuthorityDistrict"] =
-              localAuthorityDistricts;
-          }
-          if (regions) {
-            newPassportData["property.region"] = regions;
-          }
-          if (titleBoundary) {
-            const areaSquareMetres =
-              Math.round(area(titleBoundary as Feature) * 100) / 100;
-            newPassportData["property.boundary.title"] = titleBoundary;
-            newPassportData["property.boundary.title.area"] = areaSquareMetres;
-            newPassportData["property.boundary.title.area.hectares"] =
-              squareMetresToHectares(areaSquareMetres);
-          }
-
-          newPassportData[PASSPORT_COMPONENT_ACTION_KEY] =
-            address?.source === "os"
-              ? FindPropertyUserAction.Existing
-              : FindPropertyUserAction.New;
-
-          props.handleSubmit?.({ data: { ...newPassportData } });
-        }
-      }}
-    >
-      {getPageBody()}
-    </Card>
-  );
 }