From ff6cc5441e9dcb9e5d38631392323ce63d45e59c Mon Sep 17 00:00:00 2001 From: Rory Doak <138574807+RODO94@users.noreply.github.com> Date: Wed, 18 Dec 2024 13:00:54 +0000 Subject: [PATCH 1/6] feat: E2E coverage for Draw Boundary (#4035) --- .../src/create-flow-with-geospatial.spec.ts | 70 ++++++++++++++-- .../ui-driven/src/helpers/geospatialChecks.ts | 81 ++++++++++++++++++- .../ui-driven/src/mocks/geospatialMocks.ts | 35 +++++++- .../ui-driven/src/mocks/osMapsMockData.ts | 22 +++++ .../ui-driven/src/mocks/osMapsResponse.ts | 27 +++++++ 5 files changed, 221 insertions(+), 14 deletions(-) create mode 100644 e2e/tests/ui-driven/src/mocks/osMapsMockData.ts create mode 100644 e2e/tests/ui-driven/src/mocks/osMapsResponse.ts diff --git a/e2e/tests/ui-driven/src/create-flow-with-geospatial.spec.ts b/e2e/tests/ui-driven/src/create-flow-with-geospatial.spec.ts index 8bc6fe181b..02ecfbc29e 100644 --- a/e2e/tests/ui-driven/src/create-flow-with-geospatial.spec.ts +++ b/e2e/tests/ui-driven/src/create-flow-with-geospatial.spec.ts @@ -19,11 +19,24 @@ import { } from "./helpers/navigateAndPublish"; import { TestContext } from "./helpers/types"; import { serviceProps } from "./helpers/serviceData"; -import { checkGeoJsonContent } from "./helpers/geospatialChecks"; import { - mockMapGeoJson, + alterDrawGeoJson, + checkGeoJsonContent, + checkUploadFileAltRoute, + getMapProperties, + resetMapBoundary, + waitForMapComponent, +} from "./helpers/geospatialChecks"; +import { + GeoJsonChangeHandler, + mockChangedMapGeoJson, mockPropertyTypeOptions, + mockTitleBoundaryGeoJson, } from "./mocks/geospatialMocks"; +import { + setupOSMapsStyles, + setupOSMapsVectorTiles, +} from "./mocks/osMapsResponse"; test.describe("Flow creation, publish and preview", () => { let context: TestContext = { @@ -72,8 +85,7 @@ test.describe("Flow creation, publish and preview", () => { await editor.createInternalPortal(); await editor.populateInternalPortal(); await page.getByRole("link", { name: "start" }).click(); // return to main flow - await editor.createUploadAndLabel(); - // TODO: editor.createPropertyInfo() + // await editor.createUploadAndLabel(); await editor.createDrawBoundary(); await editor.createPlanningConstraints(); // await editor.createFileUpload(); @@ -81,7 +93,6 @@ test.describe("Flow creation, publish and preview", () => { await expect(editor.nodeList).toContainText([ "Find property", "an internal portalEdit Portal", - "Upload and label", "Confirm your location plan", "Planning constraints", // "File upload", @@ -119,6 +130,9 @@ test.describe("Flow creation, publish and preview", () => { `/${context.team.slug}/${serviceProps.slug}/published?analytics=false`, ); + setupOSMapsStyles(page); + setupOSMapsVectorTiles(page); + await expect( page.locator("h1", { hasText: "Find the property" }), ).toBeVisible(); @@ -130,7 +144,7 @@ test.describe("Flow creation, publish and preview", () => { ).toBeVisible(); // Check map component has geoJson content - await checkGeoJsonContent(page, mockMapGeoJson); + await checkGeoJsonContent(page, "geojsondata", mockTitleBoundaryGeoJson); // Check property info is being shown await expect(page.getByText("Test Street, Testville")).toBeVisible(); @@ -169,7 +183,49 @@ test.describe("Flow creation, publish and preview", () => { ).toBeVisible(); await clickContinue({ page }); + const drawBoundaryTitle = page.getByRole("heading", { + name: "Confirm your location plan", + }); + await expect(drawBoundaryTitle).toBeVisible(); + + await checkGeoJsonContent( + page, + "drawgeojsondata", + mockTitleBoundaryGeoJson, + ); + + const area = "The property boundary you have drawn is 490.37"; + + await expect(page.getByText(area)).toBeVisible(); + + // navigate to upload file page and back + await checkUploadFileAltRoute(page); + + await expect( + drawBoundaryTitle, + "We have navigated back to the map component", + ).toBeVisible(); + + // ensure map has loaded correctly + await waitForMapComponent(page); + + await resetMapBoundary(page); + + await alterDrawGeoJson(page); + + // extract new GeoJSON data + const newGeoJson = await getMapProperties(page, "drawgeojsondata"); + const parsedJson: GeoJsonChangeHandler = JSON.parse(newGeoJson!); + + // check it matches our static mock + await checkGeoJsonContent(page, "drawgeojsondata", mockChangedMapGeoJson); + + await expect( + page.getByText(`${parsedJson.properties!["area.squareMetres"]}`), + "The correct value for area comes from the map properties ", + ).toBeVisible(); + // TODO: answer uploadAndLabel - // TODO: answerPropertyInfo, answerDrawBoundary, answerPlanningConstraints + // TODO: answerPropertyInfo, answerPlanningConstraints }); }); diff --git a/e2e/tests/ui-driven/src/helpers/geospatialChecks.ts b/e2e/tests/ui-driven/src/helpers/geospatialChecks.ts index 7b55e01997..ea13894975 100644 --- a/e2e/tests/ui-driven/src/helpers/geospatialChecks.ts +++ b/e2e/tests/ui-driven/src/helpers/geospatialChecks.ts @@ -1,12 +1,87 @@ import { expect, Page } from "@playwright/test"; import { Feature } from "geojson"; -export const checkGeoJsonContent = async (page: Page, geoJson: Feature) => { +export const waitForMapComponent = async (page: Page) => { + await page.waitForFunction(() => { + const map = document.getElementById("draw-boundary-map"); + return map; + }); +}; + +export const getMapProperties = async ( + page: Page, + attribute: "geojsondata" | "drawgeojsondata", +) => { + const mapComponent = await page.waitForSelector("my-map"); + return await mapComponent.getAttribute(attribute); +}; + +export const alterDrawGeoJson = async (page: Page) => { + const map = page.getByTestId("map-test-id"); + + await map.click({ button: "left", position: { x: 100, y: 200 } }); + await map.click({ button: "left", position: { x: 150, y: 250 } }); + await map.click({ button: "left", position: { x: 200, y: 250 } }); + await map.click({ button: "left", position: { x: 100, y: 200 } }); +}; + +export const resetMapBoundary = async (page: Page) => { + const resetButton = page.getByLabel("Reset map view"); + await resetButton.click(); + + const resetGeoJson = await getMapProperties(page, "drawgeojsondata"); + + expect(resetGeoJson, "drawGeoJsonData should be reset").toEqual(null); +}; + +export const checkGeoJsonContent = async ( + page: Page, + attribute: "geojsondata" | "drawgeojsondata", + geoJson: Feature, +) => { // Wait for the map component to be present const mapComponent = await page.waitForSelector("my-map"); + await page.waitForFunction(() => customElements.get("my-map")); + await expect( + page.getByTestId("map-test-id"), + "Check we can see the map", + ).toBeVisible(); + // Get the geojsonData attribute - const geojsonData = await mapComponent.getAttribute("geojsondata"); + const geojsonData = await mapComponent.getAttribute(attribute); + + expect( + JSON.parse(geojsonData!), + "map attribute matches expected mock attribute", + ).toEqual(geoJson); +}; + +export const checkUploadFileAltRoute = async (page: Page) => { + const uploadButton = page.getByTestId("upload-file-button"); + + await expect( + uploadButton, + "We can see a button to upload a file instead", + ).toBeVisible(); + + await uploadButton.click(); + + await expect( + page.getByRole("heading", { name: "Upload a location plan" }), + "Should be in a page for uploading a file", + ).toBeVisible(); + + await expect( + page.getByRole("button", { name: "Drop file here or choose" }), + "A button for uploading files is visible", + ).toBeVisible(); + + await page.getByTestId("continue-button").click(); + + await page.getByTestId("error-message-upload-location-plan").isVisible(); + + const useMapButton = page.getByTestId("use-map-button"); - expect(JSON.parse(geojsonData!)).toEqual(geoJson); + await useMapButton.click(); }; diff --git a/e2e/tests/ui-driven/src/mocks/geospatialMocks.ts b/e2e/tests/ui-driven/src/mocks/geospatialMocks.ts index fb26308b1d..e923545f46 100644 --- a/e2e/tests/ui-driven/src/mocks/geospatialMocks.ts +++ b/e2e/tests/ui-driven/src/mocks/geospatialMocks.ts @@ -1,13 +1,21 @@ import { OptionWithDataValues } from "../helpers/types"; +import { Feature, Polygon } from "geojson"; + +type ChangeHandlerProperties = { + label: string; + "area.squareMetres": number; + "area.hectares": number; +}; + +export type GeoJsonChangeHandler = Feature; export const mockPropertyTypeOptions: OptionWithDataValues[] = [ { optionText: "Residential", dataValue: "residential" }, { optionText: "Commercial", dataValue: "commercial" }, ]; -import { Feature } from "geojson"; - -export const mockMapGeoJson: Feature = { +export const mockTitleBoundaryGeoJson: Feature = { + type: "Feature", geometry: { type: "MultiPolygon", coordinates: [ @@ -23,7 +31,6 @@ export const mockMapGeoJson: Feature = { ], ], }, - type: "Feature", properties: { "entry-date": "2024-05-06", "start-date": "2010-05-12", @@ -37,3 +44,23 @@ export const mockMapGeoJson: Feature = { "organisation-entity": "13", }, }; + +export const mockChangedMapGeoJson: GeoJsonChangeHandler = { + type: "Feature", + geometry: { + type: "Polygon", + coordinates: [ + [ + [-0.6341888375038146, 51.60562241658701], + [-0.6341217822784424, 51.605580770520504], + [-0.63405472705307, 51.605580770520504], + [-0.6341888375038146, 51.60562241658701], + ], + ], + }, + properties: { + label: "1", + "area.squareMetres": 10.72, + "area.hectares": 0.001072, + }, +}; diff --git a/e2e/tests/ui-driven/src/mocks/osMapsMockData.ts b/e2e/tests/ui-driven/src/mocks/osMapsMockData.ts new file mode 100644 index 0000000000..b99f14f4d3 --- /dev/null +++ b/e2e/tests/ui-driven/src/mocks/osMapsMockData.ts @@ -0,0 +1,22 @@ +export const osMapsStylesResponse = { + version: 8, + sprite: + "https://api.os.uk/maps/vector/v1/vts/resources/sprites/sprite?key=YOUR_KEY&srs=3857", + glyphs: + "https://api.os.uk/maps/vector/v1/vts/resources/fonts/{fontstack}/{range}.pbf?key=YOUR_KEY&srs=3857", + sources: { + esri: { + type: "vector", + url: "https://api.os.uk/maps/vector/v1/vts?key=YOUR_KEY&srs=3857", + }, + }, + layers: [ + { + id: "background", + type: "background", + paint: { + "background-color": "#0437F2", + }, + }, + ], +}; diff --git a/e2e/tests/ui-driven/src/mocks/osMapsResponse.ts b/e2e/tests/ui-driven/src/mocks/osMapsResponse.ts new file mode 100644 index 0000000000..ccd6f6de6f --- /dev/null +++ b/e2e/tests/ui-driven/src/mocks/osMapsResponse.ts @@ -0,0 +1,27 @@ +import { Page } from "@playwright/test"; +import { osMapsStylesResponse } from "./osMapsMockData"; + +export async function setupOSMapsStyles(page: Page) { + const ordnanceSurveyMapsStyles = new RegExp( + /\/proxy\/ordnance-survey\/maps\/vector\/v1\/vts\/resources\/styles.*/, + ); + await page.route(ordnanceSurveyMapsStyles, async (route) => { + await route.fulfill({ + status: 200, + body: JSON.stringify(osMapsStylesResponse), + }); + }); +} + +export async function setupOSMapsVectorTiles(page: Page) { + const ordnanceSurveyVectorTiles = new RegExp( + /\/proxy\/ordnance-survey\/maps\/vector\/v1\/vts\/tile/, + ); + + await page.route(ordnanceSurveyVectorTiles, async (route) => { + await route.fulfill({ + status: 200, + body: Buffer.from([]), + }); + }); +} From 2a7be85ef8ecc5f691663d306f12654a3d3c6e22 Mon Sep 17 00:00:00 2001 From: Jo Humphrey <31373245+jamdelion@users.noreply.github.com> Date: Wed, 18 Dec 2024 15:23:42 +0000 Subject: [PATCH 2/6] feat: exclusive 'Or' option in checklists (#4056) --- .../components/Checklist/Editor/Editor.tsx | 35 ++++-- .../components/Checklist/Editor/Options.tsx | 72 +++++++++++-- .../components/Checklist/Public/Public.tsx | 97 +++++++++++++---- .../components/Checklist/Public/helpers.ts | 38 ++++++- .../Public/tests/Public.exclusiveOr.test.tsx | 101 ++++++++++++++++++ .../Checklist/Public/tests/Public.test.tsx | 24 ++--- .../Checklist/Public/tests/mockOptions.ts | 6 ++ .../Checklist/Public/tests/testUtils.ts | 20 ++++ .../src/@planx/components/Checklist/model.ts | 8 +- .../src/@planx/components/shared/index.ts | 1 + editor.planx.uk/src/@planx/graph/index.ts | 18 ++-- editor.planx.uk/src/lib/featureFlags.ts | 2 +- .../src/ui/editor/ListManager/ListManager.tsx | 16 +-- 13 files changed, 352 insertions(+), 86 deletions(-) create mode 100644 editor.planx.uk/src/@planx/components/Checklist/Public/tests/Public.exclusiveOr.test.tsx create mode 100644 editor.planx.uk/src/@planx/components/Checklist/Public/tests/testUtils.ts diff --git a/editor.planx.uk/src/@planx/components/Checklist/Editor/Editor.tsx b/editor.planx.uk/src/@planx/components/Checklist/Editor/Editor.tsx index 2fcc03c3de..3ba495ce53 100644 --- a/editor.planx.uk/src/@planx/components/Checklist/Editor/Editor.tsx +++ b/editor.planx.uk/src/@planx/components/Checklist/Editor/Editor.tsx @@ -7,18 +7,19 @@ import { ModalFooter } from "ui/editor/ModalFooter"; import ModalSection from "ui/editor/ModalSection"; import ModalSectionContent from "ui/editor/ModalSectionContent"; import RichTextInput from "ui/editor/RichTextInput/RichTextInput"; +import ErrorWrapper from "ui/shared/ErrorWrapper"; import Input from "ui/shared/Input/Input"; import InputRow from "ui/shared/InputRow"; import { Switch } from "ui/shared/Switch"; -import { parseBaseNodeData } from "../../shared"; +import { Option, parseBaseNodeData } from "../../shared"; import { ICONS } from "../../shared/icons"; import type { Checklist } from "../model"; import { toggleExpandableChecklist } from "../model"; import { ChecklistProps } from "../types"; import { Options } from "./Options"; -export const ChecklistComponent: React.FC = (props) => { +export const ChecklistEditor: React.FC = (props) => { const type = TYPES.Checklist; const formik = useFormik({ @@ -39,7 +40,7 @@ export const ChecklistComponent: React.FC = (props) => { : groupedOptions?.flatMap((group) => group.children); const filteredOptions = (sourceOptions || []).filter( - (option) => option.data.text, + (option) => option.data.text ); const processedOptions = filteredOptions.map((option) => ({ @@ -66,14 +67,22 @@ export const ChecklistComponent: React.FC = (props) => { }), }, }, - processedOptions, + processedOptions ); } else { alert(JSON.stringify({ type, ...values, options }, null, 2)); } }, - validate: ({ options, groupedOptions, ...values }) => { + validate: ({ options, groupedOptions, allRequired, ...values }) => { const errors: FormikErrors = {}; + + const exclusiveOptions: Option[] | undefined = options?.filter( + (option) => option.data.exclusive + ); + if (allRequired && exclusiveOptions && exclusiveOptions.length > 0) { + errors.allRequired = + 'Cannot configure exclusive "or" option alongside "all required" setting'; + } // Account for flat or expandable Checklist options options = options || groupedOptions?.map((group) => group.children)?.flat(); @@ -81,6 +90,10 @@ export const ChecklistComponent: React.FC = (props) => { errors.fn = "At least one option must set a data value when the checklist has a data field"; } + if (exclusiveOptions && exclusiveOptions.length > 1) { + errors.options = + "There should be a maximum of one exclusive option configured"; + } return errors; }, }); @@ -160,19 +173,20 @@ export const ChecklistComponent: React.FC = (props) => { onChange={() => formik.setFieldValue( "allRequired", - !formik.values.allRequired, + !formik.values.allRequired ) } label="All required" /> + formik.setFieldValue( "neverAutoAnswer", - !formik.values.neverAutoAnswer, + !formik.values.neverAutoAnswer ) } label="Always put to user (forgo automation)" @@ -180,8 +194,9 @@ export const ChecklistComponent: React.FC = (props) => { - - + + + @@ -189,4 +204,4 @@ export const ChecklistComponent: React.FC = (props) => { ); }; -export default ChecklistComponent; +export default ChecklistEditor; diff --git a/editor.planx.uk/src/@planx/components/Checklist/Editor/Options.tsx b/editor.planx.uk/src/@planx/components/Checklist/Editor/Options.tsx index 388c021dac..c5fa9ee02b 100644 --- a/editor.planx.uk/src/@planx/components/Checklist/Editor/Options.tsx +++ b/editor.planx.uk/src/@planx/components/Checklist/Editor/Options.tsx @@ -2,6 +2,9 @@ import Delete from "@mui/icons-material/Delete"; import Box from "@mui/material/Box"; import Button from "@mui/material/Button"; import IconButton from "@mui/material/IconButton"; +import { BaseOptionsEditor } from "@planx/components/shared/BaseOptionsEditor"; +import { hasFeatureFlag } from "lib/featureFlags"; +import { partition } from "lodash"; import adjust from "ramda/src/adjust"; import compose from "ramda/src/compose"; import remove from "ramda/src/remove"; @@ -9,6 +12,7 @@ import React from "react"; import { FormikHookReturn } from "types"; import ListManager from "ui/editor/ListManager/ListManager"; import ModalSectionContent from "ui/editor/ModalSectionContent"; +import ErrorWrapper from "ui/shared/ErrorWrapper"; import Input from "ui/shared/Input/Input"; import InputRow from "ui/shared/InputRow"; @@ -17,6 +21,14 @@ import type { Group } from "../model"; import ChecklistOptionsEditor from "./OptionsEditor"; export const Options: React.FC<{ formik: FormikHookReturn }> = ({ formik }) => { + const [exclusiveOptions, nonExclusiveOptions]: Option[][] = partition( + formik.values.options, + (option) => option.data.exclusive + ); + + const exclusiveOrOptionManagerShouldRender = + hasFeatureFlag("EXCLUSIVE_OR") && nonExclusiveOptions.length; + return ( {formik.values.groupedOptions ? ( @@ -42,7 +54,7 @@ export const Options: React.FC<{ formik: FormikHookReturn }> = ({ formik }) => { onClick={() => { formik.setFieldValue( `groupedOptions`, - remove(groupIndex, 1, formik.values.groupedOptions), + remove(groupIndex, 1, formik.values.groupedOptions) ); }} size="large" @@ -57,7 +69,7 @@ export const Options: React.FC<{ formik: FormikHookReturn }> = ({ formik }) => { onChange={(newOptions) => { formik.setFieldValue( `groupedOptions[${groupIndex}].children`, - newOptions, + newOptions ); }} newValue={() => @@ -76,7 +88,7 @@ export const Options: React.FC<{ formik: FormikHookReturn }> = ({ formik }) => { showValueField: !!formik.values.fn, onMoveToGroup: ( movedItemIndex: number, - moveToGroupIndex: number, + moveToGroupIndex: number ) => { const item = groupedOption.children[movedItemIndex]; formik.setFieldValue( @@ -87,27 +99,27 @@ export const Options: React.FC<{ formik: FormikHookReturn }> = ({ formik }) => { (option: Group