From a60ed07233aed5e6822f17d143a6dc8a34ce316e 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` --- editor.planx.uk/src/components/Error/ErrorFallback.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editor.planx.uk/src/components/Error/ErrorFallback.tsx b/editor.planx.uk/src/components/Error/ErrorFallback.tsx index ad9e822d91..6a0e0d799f 100644 --- a/editor.planx.uk/src/components/Error/ErrorFallback.tsx +++ b/editor.planx.uk/src/components/Error/ErrorFallback.tsx @@ -15,7 +15,7 @@ const ErrorFallback: React.FC<{ error: Error }> = ({ error }) => { - Something went wrong + {(props.error.cause as string) || "Something went wrong"} {error.message && ( From bc15d6eaef984f28c34ed9465c4ef3f63a14740f 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 --- editor.planx.uk/src/components/Error/ErrorFallback.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editor.planx.uk/src/components/Error/ErrorFallback.tsx b/editor.planx.uk/src/components/Error/ErrorFallback.tsx index 6a0e0d799f..ad9e822d91 100644 --- a/editor.planx.uk/src/components/Error/ErrorFallback.tsx +++ b/editor.planx.uk/src/components/Error/ErrorFallback.tsx @@ -15,7 +15,7 @@ const ErrorFallback: React.FC<{ error: Error }> = ({ error }) => { - {(props.error.cause as string) || "Something went wrong"} + Something went wrong {error.message && ( From 44e449aec27b292e1b1417b65dc7bd4287a02c10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Wed, 18 Sep 2024 09:11:28 +0100 Subject: [PATCH 3/4] test: Basic coverage for `PlanningConstraints` public interface --- .../PlanningConstraints/Public.test.tsx | 144 ++++++++++++++++-- .../components/PlanningConstraints/Public.tsx | 1 + .../PlanningConstraints/mocks/simpleFlow.ts | 95 ++++++++++++ .../shared/Preview/SimpleExpand.tsx | 2 +- 4 files changed, 230 insertions(+), 12 deletions(-) create mode 100644 editor.planx.uk/src/@planx/components/PlanningConstraints/mocks/simpleFlow.ts 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 256dff7063..b129676048 100644 --- a/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.test.tsx +++ b/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.test.tsx @@ -1,21 +1,27 @@ import ErrorFallback from "components/Error/ErrorFallback"; +import { useStore } from "pages/FlowEditor/lib/store"; import React from "react"; +import { act } from "react-dom/test-utils"; import { ErrorBoundary } from "react-error-boundary"; +import swr from "swr"; import { setup } from "testUtils"; import { vi } from "vitest"; import { axe } from "vitest-axe"; import classifiedRoadsResponseMock from "./mocks/classifiedRoadsResponseMock"; import digitalLandResponseMock from "./mocks/digitalLandResponseMock"; +import { simpleBreadcrumbs, simpleFlow } from "./mocks/simpleFlow"; import PlanningConstraints from "./Public"; +const swrMock = (swr as jest.Mock).mock; + vi.mock("swr", () => ({ default: vi.fn((url: () => string) => { const isGISRequest = url()?.startsWith( - `${import.meta.env.VITE_APP_API_URL}/gis/`, + `${import.meta.env.VITE_APP_API_URL}/gis`, ); const isRoadsRequest = url()?.startsWith( - `${import.meta.env.VITE_APP_API_URL}/roads/`, + `${import.meta.env.VITE_APP_API_URL}/roads`, ); if (isGISRequest) return { data: digitalLandResponseMock }; @@ -27,8 +33,6 @@ vi.mock("swr", () => ({ describe("error state", () => { it("renders an error if no addres is present in the passport", async () => { - const handleSubmit = vi.fn(); - const { getByRole, getByTestId } = setup( { 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} + handleSubmit={vi.fn()} /> - , , ); @@ -55,7 +58,6 @@ describe("error state", () => { 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); @@ -63,10 +65,130 @@ describe("error state", () => { }); }); -it.todo("renders correctly"); +describe("following a FindProperty component", () => { + beforeAll(() => { + act(() => + useStore.setState({ + breadcrumbs: simpleBreadcrumbs, + flow: simpleFlow, + teamIntegrations: { + hasPlanningData: true, + }, + }), + ); + }); + + it("renders correctly", async () => { + const handleSubmit = vi.fn(); + + const { user, getByRole, getByTestId } = setup( + , + ); + + expect( + getByRole("heading", { name: "Planning constraints" }), + ).toBeInTheDocument(); + + await user.click(getByTestId("continue-button")); + + expect(handleSubmit).toHaveBeenCalled(); + }); + + it("should not have any accessibility violations", async () => { + const { container } = setup( + , + ); + const results = await axe(container); + expect(results).toHaveNoViolations(); + }); + + it("fetches planning constraints when we have lng, lat or siteBoundary", async () => { + setup( + , + ); + + expect(swr).toHaveBeenCalled(); + + // Planning data is called first + const swrURL = swrMock.calls[0][0](); + const swrResponse = swrMock.results[0].value; + + expect(swrURL).toContain("/gis"); + expect(swrResponse).toEqual({ data: digitalLandResponseMock }); + }); + + it("fetches classified roads only when we have a siteBoundary", () => { + setup( + , + ); + + expect(swr).toHaveBeenCalled(); + + // Classified roads are called second + const swrURL = swrMock.calls[1][0](); + const swrResponse = swrMock.results[1].value; -it.todo("should not have any accessibility violations"); + expect(swrURL).toContain("/roads"); + expect(swrResponse).toEqual({ data: classifiedRoadsResponseMock }); + }); + + test("basic layout and interactions", async () => { + const { user, getByRole, queryByRole, getByTestId } = setup( + , + ); + + // Positive constraints visible by default + expect( + getByRole("heading", { name: /These are the planning constraints/ }), + ).toBeVisible(); + expect(getByRole("button", { name: /Parks and gardens/ })).toBeVisible(); + + // Negative constraints hidden by default + const showNegativeConstraintsButton = getByRole("button", { + name: /Constraints that don't apply/, + }); + expect(showNegativeConstraintsButton).toBeVisible(); -it.todo("fetches classified roads only when we have a siteBoundary"); // using expect(spy).toHaveBeenCalled() ?? + const negativeConstraintsContainer = getByTestId( + "negative-constraints-list", + ); + expect(negativeConstraintsContainer).not.toBeVisible(); + + expect(queryByRole("heading", { name: /Ecology/ })).not.toBeInTheDocument(); + + // Negative constraints viewable on toggle + await user.click(showNegativeConstraintsButton); -it.todo("fetches planning constraints when we have lng,lat or siteBoundary"); + expect(negativeConstraintsContainer).toBeVisible(); + expect(getByRole("heading", { name: /Ecology/ })).toBeVisible(); + }); +}); diff --git a/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.tsx b/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.tsx index 513961d632..2cfcf8fbbd 100644 --- a/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.tsx +++ b/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.tsx @@ -302,6 +302,7 @@ export function PlanningConstraintsContent( {negativeConstraints.length > 0 && ( This page does not include information about historic planning conditions that may apply to this property.

", + }, + }, +}; + +export const simpleBreadcrumbs: Store.Breadcrumbs = { + findProperty: { + auto: false, + data: { + _address: { + uprn: "100071417680", + usrn: "2702440", + blpu_code: "2", + latitude: 52.4804358, + longitude: -1.9034539, + organisation: null, + sao: "", + saoEnd: "", + pao: "COUNCIL HOUSE", + paoEnd: "", + street: "VICTORIA SQUARE", + town: "BIRMINGHAM", + postcode: "B1 1BB", + ward: "E05011151", + x: 406653.64, + y: 286948.41, + planx_description: "Local Government Service", + planx_value: "commercial.office.workspace.gov.local", + single_line_address: + "COUNCIL HOUSE, VICTORIA SQUARE, BIRMINGHAM, B1 1BB", + title: "COUNCIL HOUSE, VICTORIA SQUARE", + source: "os", + }, + "property.type": ["commercial.office.workspace.gov.local"], + "property.localAuthorityDistrict": ["Birmingham"], + "property.region": ["West Midlands"], + "property.boundary.title": { + geometry: { + type: "MultiPolygon", + coordinates: [ + [ + [ + [-1.903955, 52.480237], + [-1.903881, 52.480179], + [-1.903955, 52.480237], + ], + ], + ], + }, + type: "Feature", + properties: { + "entry-date": "2024-05-06", + "start-date": "2021-03-25", + "end-date": "", + entity: 12001049997, + name: "", + dataset: "title-boundary", + typology: "geography", + reference: "61385289", + prefix: "title-boundary", + "organisation-entity": "13", + }, + }, + "property.boundary.title.area": 8242.37, + "property.boundary.title.area.hectares": 0.8242370000000001, + "findProperty.action": "Selected an existing address", + }, + }, +}; diff --git a/editor.planx.uk/src/@planx/components/shared/Preview/SimpleExpand.tsx b/editor.planx.uk/src/@planx/components/shared/Preview/SimpleExpand.tsx index beb2475701..2bc82c9396 100644 --- a/editor.planx.uk/src/@planx/components/shared/Preview/SimpleExpand.tsx +++ b/editor.planx.uk/src/@planx/components/shared/Preview/SimpleExpand.tsx @@ -48,7 +48,7 @@ const SimpleExpand: React.FC> = ({ /> - + {children} From e0dd5f821625e4d15d4b8b12f199925ab069cf22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Wed, 18 Sep 2024 10:46:14 +0100 Subject: [PATCH 4/4] test: PR feedback - better USRN tests --- .../PlanningConstraints/Public.test.tsx | 52 +++++++++++++++++-- .../PlanningConstraints/mocks/simpleFlow.ts | 3 ++ 2 files changed, 50 insertions(+), 5 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 b129676048..86504baec4 100644 --- a/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.test.tsx +++ b/editor.planx.uk/src/@planx/components/PlanningConstraints/Public.test.tsx @@ -10,9 +10,13 @@ import { axe } from "vitest-axe"; import classifiedRoadsResponseMock from "./mocks/classifiedRoadsResponseMock"; import digitalLandResponseMock from "./mocks/digitalLandResponseMock"; -import { simpleBreadcrumbs, simpleFlow } from "./mocks/simpleFlow"; +import { breadcrumbsWithoutUSRN, simpleBreadcrumbs, simpleFlow } from "./mocks/simpleFlow"; import PlanningConstraints from "./Public"; +const { setState } = useStore; + +beforeEach(() => vi.clearAllMocks()); + const swrMock = (swr as jest.Mock).mock; vi.mock("swr", () => ({ @@ -66,9 +70,9 @@ describe("error state", () => { }); describe("following a FindProperty component", () => { - beforeAll(() => { + beforeEach(() => { act(() => - useStore.setState({ + setState({ breadcrumbs: simpleBreadcrumbs, flow: simpleFlow, teamIntegrations: { @@ -134,7 +138,7 @@ describe("following a FindProperty component", () => { expect(swrResponse).toEqual({ data: digitalLandResponseMock }); }); - it("fetches classified roads only when we have a siteBoundary", () => { + it("fetches classified roads when a USRN is provided", () => { setup( { expect(swrResponse).toEqual({ data: classifiedRoadsResponseMock }); }); + it("does not fetch classified roads when a USRN is not provided", async () => { + act(() => + setState({ + breadcrumbs: breadcrumbsWithoutUSRN, + flow: simpleFlow, + teamIntegrations: { + hasPlanningData: true, + }, + }) + ); + + setup( + , + ); + + expect(swr).toHaveBeenCalled(); + + // Planning constraints API still called + const planingConstraintsURL = swrMock.calls[0][0](); + const planingConstraintsResponse = swrMock.results[0].value; + + expect(planingConstraintsURL).toContain("/gis"); + expect(planingConstraintsResponse).toEqual({ data: digitalLandResponseMock }); + + // Classified roads API not called due to missing USRN + const swrURL = swrMock.calls[1][0](); + const swrResponse = swrMock.results[1].value; + + expect(swrURL).toBeNull(); + expect(swrResponse).toEqual({ data: null }); + }); + test("basic layout and interactions", async () => { const { user, getByRole, queryByRole, getByTestId } = setup( { expect(negativeConstraintsContainer).toBeVisible(); expect(getByRole("heading", { name: /Ecology/ })).toBeVisible(); }); -}); +}); \ No newline at end of file diff --git a/editor.planx.uk/src/@planx/components/PlanningConstraints/mocks/simpleFlow.ts b/editor.planx.uk/src/@planx/components/PlanningConstraints/mocks/simpleFlow.ts index ed015395e8..ae7a984bea 100644 --- a/editor.planx.uk/src/@planx/components/PlanningConstraints/mocks/simpleFlow.ts +++ b/editor.planx.uk/src/@planx/components/PlanningConstraints/mocks/simpleFlow.ts @@ -1,3 +1,4 @@ +import { cloneDeep, merge } from "lodash"; import { Store } from "pages/FlowEditor/lib/store"; export const simpleFlow: Store.Flow = { @@ -93,3 +94,5 @@ export const simpleBreadcrumbs: Store.Breadcrumbs = { }, }, }; + +export const breadcrumbsWithoutUSRN = merge(cloneDeep(simpleBreadcrumbs), { findProperty: { data: { _address: { usrn: null }}}}); \ No newline at end of file