fix: Move "invalid graph" warning to MapFieldInput (#3689)

editor.planx.uk/src/@planx/components/List/Public/index.tsx

@@ -8,11 +8,8 @@ import TableBody from "@mui/material/TableBody";
 import TableCell from "@mui/material/TableCell";
 import TableRow from "@mui/material/TableRow";
 import Typography from "@mui/material/Typography";
-import { SiteAddress } from "@planx/components/FindProperty/model";
-import { ErrorSummaryContainer } from "@planx/components/shared/Preview/ErrorSummaryContainer";
 import { SchemaFields } from "@planx/components/shared/Schema/SchemaFields";
 import { PublicProps } from "@planx/components/ui";
-import { useStore } from "pages/FlowEditor/lib/store";
 import React, { useEffect, useRef } from "react";
 import { FONT_WEIGHT_SEMI_BOLD } from "theme";
 import FullWidthWrapper from "ui/public/FullWidthWrapper";
@@ -56,14 +53,8 @@ const InactiveListCardLayout = styled(Box)(({ theme }) => ({
 const ActiveListCard: React.FC<{
   index: number;
 }> = ({ index: i }) => {
-  const {
-    schema,
-    saveItem,
-    cancelEditItem,
-    errors,
-    formik,
-    activeIndex,
-  } = useListContext();
+  const { schema, saveItem, cancelEditItem, errors, formik, activeIndex } =
+    useListContext();
 
   const ref = useRef<HTMLDivElement>(null);
   useEffect(() => {
@@ -119,8 +110,7 @@ const ActiveListCard: React.FC<{
 const InactiveListCard: React.FC<{
   index: number;
 }> = ({ index: i }) => {
-  const { schema, formik, removeItem, editItem } =
-    useListContext();
+  const { schema, formik, removeItem, editItem } = useListContext();
 
   const mapPreview = schema.fields.find((field) => field.type === "map");
 
@@ -189,8 +179,7 @@ const Root = () => {
     listProps,
   } = useListContext();
 
-  const { title, description, info, policyRef, howMeasured, handleSubmit } =
-    listProps;
+  const { title, description, info, policyRef, howMeasured } = listProps;
 
   const rootError: string =
     (errors.min && `You must provide at least ${schema.min} response(s)`) ||
@@ -201,32 +190,7 @@ const Root = () => {
   const shouldShowAddAnotherButton =
     schema.max !== 1 || formik.values.schemaData.length < 1;
 
-  // If the selected schema has a "map" field, ensure there's a FindProperty component preceding it (eg address data in state to position map view)
   const hasMapField = schema.fields.some((field) => field.type === "map");
-  const { longitude, latitude } = useStore(
-    (state) =>
-      (state.computePassport()?.data?.["_address"] as SiteAddress) || {},
-  );
-
-  if (hasMapField && (!longitude || !latitude)) {
-    return (
-      <Card handleSubmit={handleSubmit} isValid>
-        <CardHeader title={title} description={description} />
-        <ErrorSummaryContainer
-          role="status"
-          data-testid="error-summary-invalid-graph"
-        >
-          <Typography variant="h4" component="h2" gutterBottom>
-            Invalid graph
-          </Typography>
-          <Typography variant="body2">
-            Edit this flow so that "List" is positioned after "FindProperty"; an
-            address is required for schemas that include a "map" field.
-          </Typography>
-        </ErrorSummaryContainer>
-      </Card>
-    );
-  }
 
   const listContent = (
     <ErrorWrapper error={rootError}>

editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.tsx

@@ -8,8 +8,8 @@ import Tab, { tabClasses, TabProps } from "@mui/material/Tab";
 import Tabs from "@mui/material/Tabs";
 import Typography from "@mui/material/Typography";
 import { SiteAddress } from "@planx/components/FindProperty/model";
-import { ErrorSummaryContainer } from "@planx/components/shared/Preview/ErrorSummaryContainer";
 import { SchemaFields } from "@planx/components/shared/Schema/SchemaFields";
+import { GraphError } from "components/Error/GraphError";
 import { GeoJsonObject } from "geojson";
 import sortBy from "lodash/sortBy";
 import { useStore } from "pages/FlowEditor/lib/store";
@@ -305,32 +305,14 @@ export const Presentational: React.FC<PresentationalProps> = (props) => (
   </MapAndLabelProvider>
 );
 
-const GraphError = (props: Props) => (
-  <Card handleSubmit={props.handleSubmit} isValid>
-    <CardHeader title={props.title} description={props.description} />
-    <ErrorSummaryContainer
-      role="status"
-      data-testid="error-summary-invalid-graph"
-    >
-      <Typography variant="h4" component="h2" gutterBottom>
-        Invalid graph
-      </Typography>
-      <Typography variant="body2">
-        Edit this flow so that "MapAndLabel" is positioned after "FindProperty";
-        an initial address is required to correctly display the map.
-      </Typography>
-    </ErrorSummaryContainer>
-  </Card>
-);
-
 function MapAndLabelComponent(props: Props) {
   const teamSettings = useStore.getState().teamSettings;
   const passport = useStore((state) => state.computePassport());
   const { latitude, longitude } =
     (passport?.data?._address as SiteAddress) || {};
 
   if (!latitude || !longitude) {
-    return <GraphError {...props} />;
+    throw new GraphError("nodeMustFollowFindProperty");
   }
 
   return (

editor.planx.uk/src/@planx/components/PlanningConstraints/Public.test.tsx

@@ -1,5 +1,6 @@
-import { screen } from "@testing-library/react";
+import ErrorFallback from "components/Error/ErrorFallback";
 import React from "react";
+import { ErrorBoundary } from "react-error-boundary";
 import { setup } from "testUtils";
 import { vi } from "vitest";
 import { axe } from "vitest-axe";
@@ -9,55 +10,62 @@ import digitalLandResponseMock from "./mocks/digitalLandResponseMock";
 import PlanningConstraints from "./Public";
 
 vi.mock("swr", () => ({
-  default: vi.fn((url: any) => {
+  default: vi.fn((url: () => string) => {
     const isGISRequest = url()?.startsWith(
       `${import.meta.env.VITE_APP_API_URL}/gis/`,
     );
     const isRoadsRequest = url()?.startsWith(
       `${import.meta.env.VITE_APP_API_URL}/roads/`,
     );
 
-    if (isGISRequest) return { data: digitalLandResponseMock } as any;
-    if (isRoadsRequest) return { data: classifiedRoadsResponseMock } as any;
+    if (isGISRequest) return { data: digitalLandResponseMock };
+    if (isRoadsRequest) return { data: classifiedRoadsResponseMock };
 
     return { data: null };
   }),
 }));
 
-it("renders correctly", async () => {
-  const handleSubmit = vi.fn();
+describe("error state", () => {
+  it("renders an error if no addres is present in the passport", async () => {
+    const handleSubmit = vi.fn();
 
-  const { user } = setup(
-    <PlanningConstraints
-      title="Planning constraints"
-      description="Things that might affect your project"
-      fn="property.constraints.planning"
-      disclaimer="This page does not include information about historic planning conditions that may apply to this property."
-      handleSubmit={handleSubmit}
-    />,
-  );
-
-  expect(screen.getByText("Planning constraints")).toBeInTheDocument();
+    const { getByRole, getByTestId } = setup(
+      <ErrorBoundary FallbackComponent={ErrorFallback}>
+        <PlanningConstraints
+          title="Planning constraints"
+          description="Things that might affect your project"
+          fn="property.constraints.planning"
+          disclaimer="This page does not include information about historic planning conditions that may apply to this property."
+          handleSubmit={handleSubmit}
+        />
+        ,
+      </ErrorBoundary>,
+    );
 
-  // TODO mock passport _address so that SWR request is actually triggered to return mock response
-  expect(screen.getByTestId("error-summary-invalid-graph")).toBeInTheDocument();
+    expect(getByTestId("error-summary-invalid-graph")).toBeInTheDocument();
+    expect(getByRole("heading", { name: "Invalid graph" })).toBeInTheDocument();
+  });
 
-  await user.click(screen.getByTestId("continue-button"));
-  expect(handleSubmit).toHaveBeenCalledTimes(1);
+  it("should not have any accessibility violations", async () => {
+    const { container } = setup(
+      <ErrorBoundary FallbackComponent={ErrorFallback}>
+        <PlanningConstraints
+          title="Planning constraints"
+          description="Things that might affect your project"
+          fn="property.constraints.planning"
+          disclaimer="This page does not include information about historic planning conditions that may apply to this property."
+        />
+        ,
+      </ErrorBoundary>,
+    );
+    const results = await axe(container);
+    expect(results).toHaveNoViolations();
+  });
 });
 
-it("should not have any accessibility violations", async () => {
-  const { container } = setup(
-    <PlanningConstraints
-      title="Planning constraints"
-      description="Things that might affect your project"
-      fn="property.constraints.planning"
-      disclaimer="This page does not include information about historic planning conditions that may apply to this property."
-    />,
-  );
-  const results = await axe(container);
-  expect(results).toHaveNoViolations();
-});
+it.todo("renders correctly");
+
+it.todo("should not have any accessibility violations");
 
 it.todo("fetches classified roads only when we have a siteBoundary"); // using expect(spy).toHaveBeenCalled() ??
 

editor.planx.uk/src/@planx/components/PlanningConstraints/Public.tsx

@@ -9,6 +9,7 @@ import Card from "@planx/components/shared/Preview/Card";
 import CardHeader from "@planx/components/shared/Preview/CardHeader";
 import type { PublicProps } from "@planx/components/ui";
 import DelayedLoadingIndicator from "components/DelayedLoadingIndicator";
+import { GraphError } from "components/Error/GraphError";
 import capitalize from "lodash/capitalize";
 import { useStore } from "pages/FlowEditor/lib/store";
 import { HandleSubmit } from "pages/Preview/Node";
@@ -63,6 +64,8 @@ function Component(props: Props) {
 
   // PlanningConstraints must come after at least a FindProperty in the graph
   const showGraphError = !x || !y || !longitude || !latitude;
+  if (showGraphError)
+    throw new GraphError("mapInputFieldMustFollowFindProperty");
 
   // Even though this component will fetch fresh GIS data when coming "back",
   //   still prepopulate any previously marked inaccurateConstraints
@@ -145,8 +148,6 @@ function Component(props: Props) {
     ...roads?.metadata,
   };
 
-  if (showGraphError) return <ConstraintsGraphError {...props} />;
-
   const isLoading = isValidating || isValidatingRoads;
   if (isLoading)
     return (
@@ -396,28 +397,3 @@ const ConstraintsFetchError = (props: ConstraintsFetchErrorProps) => (
     </ErrorSummaryContainer>
   </Card>
 );
-
-interface ConstraintsGraphErrorProps {
-  title: string;
-  description: string;
-  handleSubmit?: HandleSubmit;
-}
-
-const ConstraintsGraphError = (props: ConstraintsGraphErrorProps) => (
-  <Card handleSubmit={props.handleSubmit} isValid>
-    <CardHeader title={props.title} description={props.description} />
-    <ErrorSummaryContainer
-      role="status"
-      data-testid="error-summary-invalid-graph"
-    >
-      <Typography variant="h4" component="h2" gutterBottom>
-        Invalid graph
-      </Typography>
-      <Typography variant="body2">
-        Edit this flow so that "Planning constraints" is positioned after "Find
-        property"; an address or site boundary drawing is required to fetch
-        data.
-      </Typography>
-    </ErrorSummaryContainer>
-  </Card>
-);

editor.planx.uk/src/@planx/components/PropertyInformation/Public.test.tsx

@@ -1,6 +1,8 @@
 import { MockedProvider } from "@apollo/client/testing";
 import { screen } from "@testing-library/react";
+import ErrorFallback from "components/Error/ErrorFallback";
 import React from "react";
+import { ErrorBoundary } from "react-error-boundary";
 import { setup } from "testUtils";
 import { vi } from "vitest";
 
@@ -18,10 +20,12 @@ const defaultPresentationalProps: PresentationalProps = {
 test("renders a warning for editors if address data is not in state", async () => {
   setup(
     <MockedProvider>
-      <PropertyInformation
-        title="About the property"
-        description="This is the information we currently have about the property"
-      />
+      <ErrorBoundary FallbackComponent={ErrorFallback}>
+        <PropertyInformation
+          title="About the property"
+          description="This is the information we currently have about the property"
+        />
+      </ErrorBoundary>
     </MockedProvider>,
   );
 

editor.planx.uk/src/@planx/components/PropertyInformation/Public.tsx

@@ -1,12 +1,12 @@
 import { useQuery } from "@apollo/client";
 import Box from "@mui/material/Box";
 import Link from "@mui/material/Link";
-import Typography from "@mui/material/Typography";
 import { visuallyHidden } from "@mui/utils";
 import Card from "@planx/components/shared/Preview/Card";
 import CardHeader from "@planx/components/shared/Preview/CardHeader";
 import { SummaryListTable } from "@planx/components/shared/Preview/SummaryList";
 import type { PublicProps } from "@planx/components/ui";
+import { GraphError } from "components/Error/GraphError";
 import { Feature } from "geojson";
 import { publicClient } from "lib/graphql";
 import find from "lodash/find";
@@ -17,7 +17,6 @@ import React from "react";
 
 import type { SiteAddress } from "../FindProperty/model";
 import { FETCH_BLPU_CODES } from "../FindProperty/Public";
-import { ErrorSummaryContainer } from "../shared/Preview/ErrorSummaryContainer";
 import { MapContainer } from "../shared/Preview/MapContainer";
 import type { PropertyInformation } from "./model";
 
@@ -32,7 +31,10 @@ function Component(props: PublicProps<PropertyInformation>) {
     client: publicClient,
   });
 
-  return passport.data?._address ? (
+  if (!passport.data?._address)
+    throw new GraphError("nodeMustFollowFindProperty");
+
+  return (
     <Presentational
       title={props.title}
       description={props.description}
@@ -60,21 +62,6 @@ function Component(props: PublicProps<PropertyInformation>) {
         });
       }}
     />
-  ) : (
-    <Card>
-      <ErrorSummaryContainer
-        role="status"
-        data-testid="error-summary-invalid-graph"
-      >
-        <Typography variant="h4" component="h2" gutterBottom>
-          Invalid graph
-        </Typography>
-        <Typography variant="body2">
-          Edit this flow so that "Property information" is positioned after
-          "Find property"; an address is required to render.
-        </Typography>
-      </ErrorSummaryContainer>
-    </Card>
   );
 }
 

editor.planx.uk/src/@planx/components/shared/Schema/InputFields/MapFieldInput.tsx

@@ -1,6 +1,8 @@
 import Box from "@mui/material/Box";
+import { SiteAddress } from "@opensystemslab/planx-core/types";
 import { MapContainer } from "@planx/components/shared/Preview/MapContainer";
 import type { MapField } from "@planx/components/shared/Schema/model";
+import { GraphError } from "components/Error/GraphError";
 import { Feature } from "geojson";
 import { useStore } from "pages/FlowEditor/lib/store";
 import React, { useEffect, useState } from "react";
@@ -11,6 +13,15 @@ import { getFieldProps, Props } from ".";
 import { FieldInputDescription } from "./shared";
 
 export const MapFieldInput: React.FC<Props<MapField>> = (props) => {
+  // Ensure there's a FindProperty component preceding this field (eg address data in state to position map view)
+  const { longitude, latitude } = useStore(
+    (state) =>
+      (state.computePassport()?.data?.["_address"] as SiteAddress) || {},
+  );
+
+  if (!longitude || !latitude)
+    throw new GraphError("mapInputFieldMustFollowFindProperty");
+
   const {
     formik,
     data: { title, description, mapOptions },