From a53711e475c2aac7d10b9ecc10f0a79c727f62ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Tue, 17 Sep 2024 10:37:00 +0100 Subject: [PATCH 1/4] fix: Move 'invalid graph' warning to `MapFieldInput` --- .../@planx/components/List/Public/index.tsx | 44 ++----------------- .../Schema/InputFields/MapFieldInput.tsx | 14 ++++++ .../src/components/ErrorFallback.tsx | 2 +- 3 files changed, 19 insertions(+), 41 deletions(-) diff --git a/editor.planx.uk/src/@planx/components/List/Public/index.tsx b/editor.planx.uk/src/@planx/components/List/Public/index.tsx index aef0b30a2c..6b7f1f6888 100644 --- a/editor.planx.uk/src/@planx/components/List/Public/index.tsx +++ b/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(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 ( - - - - - Invalid graph - - - Edit this flow so that "List" is positioned after "FindProperty"; an - address is required for schemas that include a "map" field. - - - - ); - } const listContent = ( diff --git a/editor.planx.uk/src/@planx/components/shared/Schema/InputFields/MapFieldInput.tsx b/editor.planx.uk/src/@planx/components/shared/Schema/InputFields/MapFieldInput.tsx index 765ef75489..7b55fa14b2 100644 --- a/editor.planx.uk/src/@planx/components/shared/Schema/InputFields/MapFieldInput.tsx +++ b/editor.planx.uk/src/@planx/components/shared/Schema/InputFields/MapFieldInput.tsx @@ -1,4 +1,5 @@ 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 { Feature } from "geojson"; @@ -11,6 +12,19 @@ import { getFieldProps, Props } from "."; import { FieldInputDescription } from "./shared"; export const MapFieldInput: React.FC> = (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 Error( + 'Edit this flow so that this component is positioned after "FindProperty"; an address is required for schemas that include a "map" field.', + { cause: "Invalid graph" }, + ); + } + const { formik, data: { title, description, mapOptions }, diff --git a/editor.planx.uk/src/components/ErrorFallback.tsx b/editor.planx.uk/src/components/ErrorFallback.tsx index 68a85ff170..a2e2e1773d 100644 --- a/editor.planx.uk/src/components/ErrorFallback.tsx +++ b/editor.planx.uk/src/components/ErrorFallback.tsx @@ -12,7 +12,7 @@ function ErrorFallback(props: { error: Error }) { - Something went wrong + {(props.error.cause as string) || "Something went wrong"} {props.error?.message && ( From b39f8014d5d0a3bd264d30d6022937077b85e441 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Tue, 17 Sep 2024 17:15:01 +0100 Subject: [PATCH 2/4] feat: Catch graph error in boundary --- .../components/MapAndLabel/Public/index.tsx | 22 +-------- .../components/PlanningConstraints/Public.tsx | 30 ++----------- .../components/PropertyInformation/Public.tsx | 23 +++------- .../Schema/InputFields/MapFieldInput.tsx | 9 ++-- .../{ => Error}/ErrorFallback.stories.tsx | 0 .../components/{ => Error}/ErrorFallback.tsx | 17 ++++--- .../src/components/Error/GraphError.tsx | 45 +++++++++++++++++++ .../Settings/Submissions/EventsLog.tsx | 2 +- .../FlowEditor/components/forms/FormModal.tsx | 2 +- .../src/pages/Preview/Questions.tsx | 2 +- .../src/pages/layout/FlowEditorLayout.tsx | 2 +- .../src/pages/layout/PublicLayout.tsx | 2 +- 12 files changed, 73 insertions(+), 83 deletions(-) rename editor.planx.uk/src/components/{ => Error}/ErrorFallback.stories.tsx (100%) rename editor.planx.uk/src/components/{ => Error}/ErrorFallback.tsx (67%) create mode 100644 editor.planx.uk/src/components/Error/GraphError.tsx diff --git a/editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.tsx b/editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.tsx index 4b260271a7..f55921d07f 100644 --- a/editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.tsx +++ b/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,24 +305,6 @@ export const Presentational: React.FC = (props) => ( ); -const GraphError = (props: Props) => ( - - - - - Invalid graph - - - Edit this flow so that "MapAndLabel" is positioned after "FindProperty"; - an initial address is required to correctly display the map. - - - -); - function MapAndLabelComponent(props: Props) { const teamSettings = useStore.getState().teamSettings; const passport = useStore((state) => state.computePassport()); @@ -330,7 +312,7 @@ function MapAndLabelComponent(props: Props) { (passport?.data?._address as SiteAddress) || {}; if (!latitude || !longitude) { - return ; + throw new GraphError("nodeMustFollowFindProperty"); } return ( diff --git a/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.tsx b/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.tsx index f93b0a1edc..513961d632 100644 --- a/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.tsx +++ b/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 ; - const isLoading = isValidating || isValidatingRoads; if (isLoading) return ( @@ -396,28 +397,3 @@ const ConstraintsFetchError = (props: ConstraintsFetchErrorProps) => ( ); - -interface ConstraintsGraphErrorProps { - title: string; - description: string; - handleSubmit?: HandleSubmit; -} - -const ConstraintsGraphError = (props: ConstraintsGraphErrorProps) => ( - - - - - Invalid graph - - - Edit this flow so that "Planning constraints" is positioned after "Find - property"; an address or site boundary drawing is required to fetch - data. - - - -); diff --git a/editor.planx.uk/src/@planx/components/PropertyInformation/Public.tsx b/editor.planx.uk/src/@planx/components/PropertyInformation/Public.tsx index 5502fc42aa..6b6bfc17d9 100644 --- a/editor.planx.uk/src/@planx/components/PropertyInformation/Public.tsx +++ b/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) { client: publicClient, }); - return passport.data?._address ? ( + if (!passport.data?._address) + throw new GraphError("nodeMustFollowFindProperty"); + + return ( ) { }); }} /> - ) : ( - - - - Invalid graph - - - Edit this flow so that "Property information" is positioned after - "Find property"; an address is required to render. - - - ); } diff --git a/editor.planx.uk/src/@planx/components/shared/Schema/InputFields/MapFieldInput.tsx b/editor.planx.uk/src/@planx/components/shared/Schema/InputFields/MapFieldInput.tsx index 7b55fa14b2..7ddc054bc9 100644 --- a/editor.planx.uk/src/@planx/components/shared/Schema/InputFields/MapFieldInput.tsx +++ b/editor.planx.uk/src/@planx/components/shared/Schema/InputFields/MapFieldInput.tsx @@ -2,6 +2,7 @@ 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"; @@ -18,12 +19,8 @@ export const MapFieldInput: React.FC> = (props) => { (state.computePassport()?.data?.["_address"] as SiteAddress) || {}, ); - if (!longitude || !latitude) { - throw Error( - 'Edit this flow so that this component is positioned after "FindProperty"; an address is required for schemas that include a "map" field.', - { cause: "Invalid graph" }, - ); - } + if (!longitude || !latitude) + throw new GraphError("mapInputFieldMustFollowFindProperty"); const { formik, diff --git a/editor.planx.uk/src/components/ErrorFallback.stories.tsx b/editor.planx.uk/src/components/Error/ErrorFallback.stories.tsx similarity index 100% rename from editor.planx.uk/src/components/ErrorFallback.stories.tsx rename to editor.planx.uk/src/components/Error/ErrorFallback.stories.tsx diff --git a/editor.planx.uk/src/components/ErrorFallback.tsx b/editor.planx.uk/src/components/Error/ErrorFallback.tsx similarity index 67% rename from editor.planx.uk/src/components/ErrorFallback.tsx rename to editor.planx.uk/src/components/Error/ErrorFallback.tsx index a2e2e1773d..71aef0f689 100644 --- a/editor.planx.uk/src/components/ErrorFallback.tsx +++ b/editor.planx.uk/src/components/Error/ErrorFallback.tsx @@ -3,21 +3,24 @@ import Card from "@planx/components/shared/Preview/Card"; import { ErrorSummaryContainer } from "@planx/components/shared/Preview/ErrorSummaryContainer"; import React from "react"; -import { logger } from "../airbrake"; +import { logger } from "../../airbrake"; +import { GraphErrorComponent, isGraphError } from "./GraphError"; -function ErrorFallback(props: { error: Error }) { - logger.notify(props.error); +const ErrorFallback: React.FC<{ error: Error }> = ({ error }) => { + if (isGraphError(error)) return ; + + logger.notify(error); return ( - {(props.error.cause as string) || "Something went wrong"} + Something went wrong - {props.error?.message && ( + {error.message && ( 
-              {props.error.message}
+              {error.message}
)} @@ -27,6 +30,6 @@ function ErrorFallback(props: { error: Error }) { ); -} +}; export default ErrorFallback; diff --git a/editor.planx.uk/src/components/Error/GraphError.tsx b/editor.planx.uk/src/components/Error/GraphError.tsx new file mode 100644 index 0000000000..91afdb93c4 --- /dev/null +++ b/editor.planx.uk/src/components/Error/GraphError.tsx @@ -0,0 +1,45 @@ +import Typography from "@mui/material/Typography"; +import Card from "@planx/components/shared/Preview/Card"; +import CardHeader from "@planx/components/shared/Preview/CardHeader"; +import { ErrorSummaryContainer } from "@planx/components/shared/Preview/ErrorSummaryContainer"; +import React from "react"; + +type GraphErrorType = + | "nodeMustFollowFindProperty" + | "mapInputFieldMustFollowFindProperty"; + +const GRAPH_ERROR_MESSAGES: Record = { + nodeMustFollowFindProperty: + 'Edit this flow so that this node is positioned after "Find property"; an address or site boundary drawing is required to fetch data', + mapInputFieldMustFollowFindProperty: + 'Edit this flow so that this component is positioned after "FindProperty"; an address is required for schemas that include a "map" field.', +}; + +export class GraphError extends Error { + constructor(public type: GraphErrorType) { + super(); + this.type = type; + } +} + +export const isGraphError = (error: unknown): error is GraphError => + error instanceof GraphError; + +export const GraphErrorComponent: React.FC<{ error: GraphError }> = ({ + error, +}) => ( + + + + + Invalid graph + + + {GRAPH_ERROR_MESSAGES[error.type]} + + + +); diff --git a/editor.planx.uk/src/pages/FlowEditor/components/Settings/Submissions/EventsLog.tsx b/editor.planx.uk/src/pages/FlowEditor/components/Settings/Submissions/EventsLog.tsx index 4de7b76aac..29d1994471 100644 --- a/editor.planx.uk/src/pages/FlowEditor/components/Settings/Submissions/EventsLog.tsx +++ b/editor.planx.uk/src/pages/FlowEditor/components/Settings/Submissions/EventsLog.tsx @@ -15,7 +15,7 @@ import TableHead from "@mui/material/TableHead"; import TableRow from "@mui/material/TableRow"; import Typography from "@mui/material/Typography"; import DelayedLoadingIndicator from "components/DelayedLoadingIndicator"; -import ErrorFallback from "components/ErrorFallback"; +import ErrorFallback from "components/Error/ErrorFallback"; import { format } from "date-fns"; import React, { useState } from "react"; import ErrorSummary from "ui/shared/ErrorSummary"; diff --git a/editor.planx.uk/src/pages/FlowEditor/components/forms/FormModal.tsx b/editor.planx.uk/src/pages/FlowEditor/components/forms/FormModal.tsx index 9e400b8267..a937b8d259 100644 --- a/editor.planx.uk/src/pages/FlowEditor/components/forms/FormModal.tsx +++ b/editor.planx.uk/src/pages/FlowEditor/components/forms/FormModal.tsx @@ -9,7 +9,7 @@ import IconButton from "@mui/material/IconButton"; import { styled } from "@mui/material/styles"; import { ComponentType as TYPES } from "@opensystemslab/planx-core/types"; import { parseFormValues } from "@planx/components/shared"; -import ErrorFallback from "components/ErrorFallback"; +import ErrorFallback from "components/Error/ErrorFallback"; import { hasFeatureFlag } from "lib/featureFlags"; import React from "react"; import { ErrorBoundary } from "react-error-boundary"; diff --git a/editor.planx.uk/src/pages/Preview/Questions.tsx b/editor.planx.uk/src/pages/Preview/Questions.tsx index f287939af9..1819922734 100644 --- a/editor.planx.uk/src/pages/Preview/Questions.tsx +++ b/editor.planx.uk/src/pages/Preview/Questions.tsx @@ -12,7 +12,7 @@ import React, { useCallback, useEffect, useMemo, useState } from "react"; import { ErrorBoundary } from "react-error-boundary"; import { ApplicationPath, Session } from "types"; -import ErrorFallback from "../../components/ErrorFallback"; +import ErrorFallback from "../../components/Error/ErrorFallback"; import { useStore } from "../FlowEditor/lib/store"; import Node, { HandleSubmit } from "./Node"; diff --git a/editor.planx.uk/src/pages/layout/FlowEditorLayout.tsx b/editor.planx.uk/src/pages/layout/FlowEditorLayout.tsx index 327d8da975..d1d9e6eada 100644 --- a/editor.planx.uk/src/pages/layout/FlowEditorLayout.tsx +++ b/editor.planx.uk/src/pages/layout/FlowEditorLayout.tsx @@ -1,4 +1,4 @@ -import ErrorFallback from "components/ErrorFallback"; +import ErrorFallback from "components/Error/ErrorFallback"; import FlowEditor from "pages/FlowEditor"; import React, { PropsWithChildren } from "react"; import { ErrorBoundary } from "react-error-boundary"; diff --git a/editor.planx.uk/src/pages/layout/PublicLayout.tsx b/editor.planx.uk/src/pages/layout/PublicLayout.tsx index 23df5c9232..f7b21a23b1 100644 --- a/editor.planx.uk/src/pages/layout/PublicLayout.tsx +++ b/editor.planx.uk/src/pages/layout/PublicLayout.tsx @@ -6,7 +6,7 @@ import { ThemeProvider, } from "@mui/material/styles"; import Typography from "@mui/material/Typography"; -import ErrorFallback from "components/ErrorFallback"; +import ErrorFallback from "components/Error/ErrorFallback"; import Feedback from "components/Feedback"; import { useStore } from "pages/FlowEditor/lib/store"; import React, { PropsWithChildren } from "react"; From 944c297d19803af09756b23112c7501134aece87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Tue, 17 Sep 2024 20:51:21 +0100 Subject: [PATCH 3/4] test: Fix failing tests which by adding error boundaries --- .../PlanningConstraints/Public.test.tsx | 74 ++++++++++--------- .../PropertyInformation/Public.test.tsx | 12 ++- .../src/components/Error/GraphError.tsx | 4 +- 3 files changed, 50 insertions(+), 40 deletions(-) diff --git a/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.test.tsx b/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.test.tsx index 2eefae94fe..256dff7063 100644 --- a/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.test.tsx +++ b/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,7 +10,7 @@ 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/`, ); @@ -17,47 +18,54 @@ vi.mock("swr", () => ({ `${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( - , - ); - - expect(screen.getByText("Planning constraints")).toBeInTheDocument(); + const { getByRole, getByTestId } = setup( + + + , + , + ); - // 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( + + + , + , + ); + const results = await axe(container); + expect(results).toHaveNoViolations(); + }); }); -it("should not have any accessibility violations", async () => { - const { container } = setup( - , - ); - 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() ?? diff --git a/editor.planx.uk/src/@planx/components/PropertyInformation/Public.test.tsx b/editor.planx.uk/src/@planx/components/PropertyInformation/Public.test.tsx index 318aaf37f2..2e35f47189 100644 --- a/editor.planx.uk/src/@planx/components/PropertyInformation/Public.test.tsx +++ b/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( - + + + , ); diff --git a/editor.planx.uk/src/components/Error/GraphError.tsx b/editor.planx.uk/src/components/Error/GraphError.tsx index 91afdb93c4..2b86e7aaef 100644 --- a/editor.planx.uk/src/components/Error/GraphError.tsx +++ b/editor.planx.uk/src/components/Error/GraphError.tsx @@ -1,6 +1,5 @@ import Typography from "@mui/material/Typography"; import Card from "@planx/components/shared/Preview/Card"; -import CardHeader from "@planx/components/shared/Preview/CardHeader"; import { ErrorSummaryContainer } from "@planx/components/shared/Preview/ErrorSummaryContainer"; import React from "react"; @@ -29,12 +28,11 @@ export const GraphErrorComponent: React.FC<{ error: GraphError }> = ({ error, }) => ( - - + Invalid graph From 76ca89b12c065c44c784af08a4796d3e20bc7efe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Tue, 17 Sep 2024 21:10:31 +0100 Subject: [PATCH 4/4] test: Add coverage for GraphError --- .../src/components/Error/ErrorFallback.tsx | 2 +- .../src/components/Error/GraphError.test.tsx | 81 +++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 editor.planx.uk/src/components/Error/GraphError.test.tsx diff --git a/editor.planx.uk/src/components/Error/ErrorFallback.tsx b/editor.planx.uk/src/components/Error/ErrorFallback.tsx index 71aef0f689..ad9e822d91 100644 --- a/editor.planx.uk/src/components/Error/ErrorFallback.tsx +++ b/editor.planx.uk/src/components/Error/ErrorFallback.tsx @@ -1,9 +1,9 @@ import Typography from "@mui/material/Typography"; import Card from "@planx/components/shared/Preview/Card"; import { ErrorSummaryContainer } from "@planx/components/shared/Preview/ErrorSummaryContainer"; +import { logger } from "airbrake"; import React from "react"; -import { logger } from "../../airbrake"; import { GraphErrorComponent, isGraphError } from "./GraphError"; const ErrorFallback: React.FC<{ error: Error }> = ({ error }) => { diff --git a/editor.planx.uk/src/components/Error/GraphError.test.tsx b/editor.planx.uk/src/components/Error/GraphError.test.tsx new file mode 100644 index 0000000000..ec79de8dfc --- /dev/null +++ b/editor.planx.uk/src/components/Error/GraphError.test.tsx @@ -0,0 +1,81 @@ +import { logger } from "airbrake"; +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"; + +import { GraphError } from "./GraphError"; + +vi.mock("airbrake", () => ({ + logger: { + notify: vi.fn(), + }, +})); + +const ThrowError: React.FC = () => { + throw new Error("Something broke"); +}; + +const ThrowGraphError: React.FC = () => { + throw new GraphError("nodeMustFollowFindProperty"); +}; + +it("does not render if a child does not throw an error", () => { + const { queryByRole } = setup( + +

No error

+ , + ); + expect( + queryByRole("heading", { name: /Invalid graph/ }), + ).not.toBeInTheDocument(); +}); + +it("does not render if a child throws a non-Graph error", () => { + const { queryByRole, getByText } = setup( + + + , + ); + // ErrorFallback displays... + expect(getByText(/Something went wrong/)).toBeInTheDocument(); + // ...but does not show a GraphError + expect( + queryByRole("heading", { name: /Invalid graph/ }), + ).not.toBeInTheDocument(); +}); + +it("renders if a child throws an error", () => { + const { queryByText, getByRole } = setup( + + + , + ); + + expect(queryByText(/Something went wrong/)).not.toBeInTheDocument(); + expect(getByRole("heading", { name: /Invalid graph/ })).toBeInTheDocument(); +}); + +it("does not call Airbrake", () => { + const loggerSpy = vi.spyOn(logger, "notify"); + + setup( + + + , + ); + + expect(loggerSpy).not.toHaveBeenCalled(); +}); + +it("should not have accessability violations", async () => { + const { container } = setup( + + + , + ); + const results = await axe(container); + expect(results).toHaveNoViolations(); +});