Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wip: find+replace DrawBoundary dataFieldBoundary prop to fn and remove dataFieldArea prop #4085

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api.planx.uk/modules/admin/session/html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const getHTMLExport: HTMLExportHandler = async (req, res, next) => {

const responses = await $api.export.csvData(req.params.sessionId);
const boundingBox = session.data.passport.data[
"property.boundary.site.buffered"
"proposal.site.buffered"
] as unknown as GeoJSON.Feature;
const userAction = session.data.passport.data?.[
"drawBoundary.action"
Expand Down Expand Up @@ -75,7 +75,7 @@ export const getRedactedHTMLExport: HTMLExportHandler = async (
req.params.sessionId,
);
const boundingBox = session.data.passport.data[
"property.boundary.site.buffered"
"proposal.site.buffered"
] as unknown as GeoJSON.Feature;
const userAction = session.data.passport.data?.[
"drawBoundary.action"
Expand Down
5 changes: 1 addition & 4 deletions api.planx.uk/modules/flows/downloadSchema/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ export const getFlowSchema = async (flowId: string) => {
type: nodeData?.type?.toString() || "_root",
text: nodeData.data?.text,
planx_variable:
nodeData.data?.fn ||
nodeData.data?.val ||
nodeData.data?.output ||
nodeData.data?.dataFieldBoundary,
nodeData.data?.fn || nodeData.data?.val || nodeData.data?.output,
}),
);

Expand Down
4 changes: 2 additions & 2 deletions api.planx.uk/modules/send/utils/exportZip.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ describe("buildSubmissionExportZip", () => {
});

test("geojson and location plan is excluded when not present", async () => {
// set-up mock session passport excluding "property.boundary.site"
// set-up mock session passport excluding "proposal.site"
const lowcalSessionWithoutBoundary: Partial<LowCalSession> = {
...mockLowcalSession,
id: "1234",
Expand All @@ -146,7 +146,7 @@ describe("buildSubmissionExportZip", () => {
passport: {
data: {
...mockLowcalSession.data!.passport.data,
"property.boundary.site": undefined,
"proposal.site": undefined,
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions api.planx.uk/modules/send/utils/exportZip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ export async function buildSubmissionExportZip({
}
}

const boundingBox = passport.data["property.boundary.site.buffered"];
const boundingBox = passport.data["proposal.site.buffered"];
const userAction = passport.data?.["drawBoundary.action"];

// generate and add an HTML overview document for the submission to zip
Expand All @@ -173,7 +173,7 @@ export async function buildSubmissionExportZip({

// add an optional GeoJSON file to zip
const geojson: GeoJSON.Feature | undefined =
passport?.data?.["property.boundary.site"];
passport?.data?.["proposal.site"];
if (geojson) {
if (userAction) {
geojson["properties"] ??= {};
Expand Down
2 changes: 1 addition & 1 deletion api.planx.uk/tests/mocks/saveAndReturnMocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const mockLowcalSession: LowCalSession = {
},
"drawBoundary.action": "Amended the title boundary",
"proposal.projectType": ["new.office"],
"property.boundary.site": {
"proposal.site": {
type: "Feature",
geometry: {
type: "Polygon",
Expand Down
3 changes: 1 addition & 2 deletions e2e/tests/api-driven/src/invite-to-pay/mocks/flow.json
Original file line number Diff line number Diff line change
Expand Up @@ -157048,9 +157048,8 @@
"policyRef": "<p><a target=\"_blank\" rel=\"noopener noreferrer nofollow\" href=\"https://www.legislation.gov.uk/uksi/2015/595/article/7\">The Town and Country Planning (Development Management Procedure) (England) Order 2015</a>,</p><p><a target=\"_blank\" rel=\"noopener noreferrer nofollow\" href=\"https://www.gov.uk/government/collections/planning-practice-guidance\">Planning Practice Guidance (PPG)</a></p>",
"description": "<p>The red line shown below should include:</p><ul><li><p>the outline of your property boundary</p></li><li><p>any works outside the property boundary</p></li><li><p>areas that will be closed off or you&apos;ll need access to during the works</p></li></ul><p>If the red line already includes all these, select continue. If not, select More information for guidance on how to amend or redraw the outline.</p>",
"howMeasured": "<p>We have pre-populated the map with a red outline that includes the entire property using information from the Land Registry.</p><p>In some cases, this outline might not include all the works or the areas that will be closed off. This might be because you&apos;re proposing works to a public highway (such as a dropped kerb), doing works that involve multiple properties, or works to a building that is part of a larger estate.</p><p>In these cases, you should amend the red outline by dragging the edges, or erase it by clicking the 🗑-icon on the map and draw a new outline.</p><p></p><h1>How to draw and amend the outline</h1><ol><li><p>Move the cursor to the corner you want to start with and click or tap once.</p></li><li><p>Move the cursor to the next corner and click or tap.</p></li><li><p>Repeat until you have the shape you need.</p></li><li><p>Click or tap the last corner again to stop drawing.</p></li><li><p>To amend the outline, click or tap on a line and drag it into a new position.</p></li></ol><img src=\"https://api.editor.planx.uk/file/public/dni98ojg/Draw_Outline_2.gif\">",
"dataFieldArea": "property.boundary.area",
"hideFileUpload": false,
"dataFieldBoundary": "property.boundary.site",
"fn": "proposal.site",
"titleForUploading": "Upload a location plan",
"descriptionForUploading": "<p>Your location plan must:</p><ul><li><p>be based on an accurate, recognisable map</p></li><li><p>be drawn to a scale, labelled, and/or marked with a scale bar</p></li><li><p>show the site outline in red</p></li><li><p>include a<strong> </strong>north point</p></li></ul>"
},
Expand Down
4 changes: 2 additions & 2 deletions e2e/tests/api-driven/src/invite-to-pay/mocks/session.json
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
"uniform.consentRegime": ["Certificate of Lawfulness"],
"application.about.form": ["Proposed building works"],
"property.EPCKnown.form": ["No"],
"property.boundary.site": {
"proposal.site": {
"type": "Feature",
"geometry": {
"type": "Polygon",
Expand Down Expand Up @@ -814,7 +814,7 @@
"auto": false,
"data": {
"proposal.siteArea": 21.6,
"property.boundary.site": {
"proposal.site": {
"type": "Feature",
"geometry": {
"type": "Polygon",
Expand Down
20 changes: 4 additions & 16 deletions editor.planx.uk/src/@planx/components/DrawBoundary/Editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,25 +53,13 @@ function DrawBoundaryComponent(props: Props) {
onChange={formik.handleChange}
/>
</InputRow>
<InputGroup label="Boundary data field">
<InputGroup label="Data field">
<InputRow>
<Input
name="dataFieldBoundary"
placeholder=""
name="fn"
format="data"
value={formik.values.dataFieldBoundary}
onChange={formik.handleChange}
/>
</InputRow>
</InputGroup>
<InputGroup label="Area data field (square metres)">
<InputRow>
<Input
name="dataFieldArea"
placeholder="property.boundary.area"
format="data"
value={formik.values.dataFieldArea}
onChange={formik.handleChange}
value={formik.values.fn}
disabled
Copy link
Member Author

@jessicamcinchak jessicamcinchak Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is a good summary / convergence point of main significant changes throughout:

  • Only support a single data field prop (rather than two data fields)
  • Change the underlying prop name for the data field so it's picked up in graph styling, search facets by default
  • Disable the data field input similar to PlanningConstraints & Pay as it's actually very important for integrations that it's called exactly this
  • Change the actual data field value prefix from property. to proposal. to better align to schema

Before:
Screenshot from 2024-12-17 17-59-28
Screenshot from 2024-12-17 17-59-04

After:
Screenshot from 2024-12-17 17-58-28
Screenshot from 2024-12-17 17-58-02

/>
</InputRow>
</InputGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ test("recovers previously submitted files when clicking the back button", async

const { user } = setup(
<DrawBoundary
dataFieldBoundary="property.boundary.site"
dataFieldArea="property.area.site"
fn="proposal.site"
description=""
descriptionForUploading=""
title="Draw a boundary"
Expand All @@ -65,7 +64,7 @@ test("recovers previously submitted files when clicking the back button", async
test("recovers previously submitted drawing when clicking the back button", async () => {
const handleSubmit = vi.fn();
const previouslySubmittedData = {
"property.boundary.site": {
"proposal.site": {
type: "Feature",
properties: {},
geometry: {
Expand All @@ -85,8 +84,7 @@ test("recovers previously submitted drawing when clicking the back button", asyn

const { user } = setup(
<DrawBoundary
dataFieldBoundary="property.boundary.site"
dataFieldArea="property.area.site"
fn="proposal.site"
description=""
descriptionForUploading=""
title="Draw a boundary"
Expand All @@ -108,8 +106,7 @@ test("recovers previously submitted drawing when clicking the back button", asyn
it("should not have any accessibility violations", async () => {
const { container } = setup(
<DrawBoundary
dataFieldBoundary="property.boundary.site"
dataFieldArea="property.area.site"
fn="proposal.site"
description="description1"
descriptionForUploading="description1"
title="Draw a boundary"
Expand All @@ -125,8 +122,7 @@ test("shows the file upload option by default and requires user data to continue

const { user } = setup(
<DrawBoundary
dataFieldBoundary="property.boundary.site"
dataFieldArea="property.area.site"
fn="proposal.site"
description=""
descriptionForUploading=""
title="Draw a boundary"
Expand Down Expand Up @@ -161,8 +157,7 @@ test("hides the upload option and allows user to continue without drawing if edi

const { user } = setup(
<DrawBoundary
dataFieldBoundary="property.boundary.site"
dataFieldArea="property.area.site"
fn="proposal.site"
description=""
descriptionForUploading=""
title="Draw a boundary"
Expand Down Expand Up @@ -197,8 +192,7 @@ test("captures output data in the correct format when uploading a file", async (

const { user } = setup(
<DrawBoundary
dataFieldBoundary="property.boundary.site"
dataFieldArea="property.area.site"
fn="proposal.site"
description=""
descriptionForUploading=""
title="Draw a boundary"
Expand Down Expand Up @@ -334,8 +328,7 @@ test("appends to existing '_requestedFiles' value", async () => {
titleForUploading: "Upload a location plan",
descriptionForUploading: "",
hideFileUpload: false,
dataFieldBoundary: "property.boundary.site",
dataFieldArea: "property.boundary.area",
fn: "proposal.site",
},
},
};
Expand All @@ -344,8 +337,7 @@ test("appends to existing '_requestedFiles' value", async () => {

const { user } = setup(
<DrawBoundary
dataFieldBoundary="property.boundary.site"
dataFieldArea="property.area.site"
fn="proposal.site"
description=""
descriptionForUploading=""
title="Draw a boundary"
Expand Down Expand Up @@ -407,7 +399,7 @@ test("submits data based on the page you continue onwards from", async () => {

// Previously submitted data is a good proxy for having previously fetched a title boundary and arriving to Draw with geojson in passport !
const previouslySubmittedData = {
"property.boundary.site": {
"proposal.site": {
type: "Feature",
properties: {},
geometry: {
Expand All @@ -427,8 +419,7 @@ test("submits data based on the page you continue onwards from", async () => {

const { user } = setup(
<DrawBoundary
dataFieldBoundary="property.boundary.site"
dataFieldArea="property.area.site"
fn="proposal.site"
description=""
descriptionForUploading=""
title="Draw a boundary"
Expand Down Expand Up @@ -458,8 +449,8 @@ test("submits data based on the page you continue onwards from", async () => {
// Confirm that file is NOT saved to passport, but geojson is
const submitted = handleSubmit.mock.calls[0][0];
expect(submitted.data).not.toHaveProperty(PASSPORT_UPLOAD_KEY);
expect(submitted.data["property.boundary.site"]).toEqual(
previouslySubmittedData["property.boundary.site"],
expect(submitted.data["proposal.site"]).toEqual(
previouslySubmittedData["proposal.site"],
);

// DrawBoundary action captured correctly based on page
Expand Down
22 changes: 11 additions & 11 deletions editor.planx.uk/src/@planx/components/DrawBoundary/Public/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ export default function Component(props: Props) {
const passport = useStore((state) => state.computePassport());

const previousBoundary =
props.previouslySubmittedData?.data?.[props.dataFieldBoundary] ||
passport.data?.["property.boundary.title"];
props.previouslySubmittedData?.data?.[props.fn] ||
passport.data?.["property.boundary"];
const previousArea =
props.previouslySubmittedData?.data?.[props.dataFieldArea] ||
passport.data?.["property.boundary.title.area"];
props.previouslySubmittedData?.data?.[props.fn] ||
passport.data?.["proposal.site.area"];
const [boundary, setBoundary] = useState<Boundary>(previousBoundary);
const [area, setArea] = useState<number | undefined>(previousArea);
const [mapValidationError, setMapValidationError] = useState<string>();
Expand Down Expand Up @@ -150,24 +150,24 @@ export default function Component(props: Props) {
props.handleSubmit?.({ data: { ...newPassportData } });
}

if (boundary && props.dataFieldBoundary) {
newPassportData[props.dataFieldBoundary] = boundary;
newPassportData[`${props.dataFieldBoundary}.buffered`] = buffer(
if (boundary && props.fn) {
newPassportData[props.fn] = boundary;
newPassportData[`${props.fn}.buffered`] = buffer(
boundary,
bufferInMeters,
{ units: "meters" },
);

if (area && props.dataFieldArea) {
newPassportData[props.dataFieldArea] = area;
newPassportData[`${props.dataFieldArea}.hectares`] =
if (area && props.fn) {
newPassportData[`${props.fn}.area`] = area;
newPassportData[`${props.fn}.hectares`] =
squareMetresToHectares(area);
}

// Track the type of map interaction
if (
boundary?.geometry ===
passport.data?.["property.boundary.title"]?.geometry
passport.data?.["property.boundary"]?.geometry
) {
newPassportData[PASSPORT_COMPONENT_ACTION_KEY] =
DrawBoundaryUserAction.Accept;
Expand Down
10 changes: 3 additions & 7 deletions editor.planx.uk/src/@planx/components/DrawBoundary/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ export interface DrawBoundary extends BaseNodeData {
titleForUploading: string;
descriptionForUploading: string;
hideFileUpload?: boolean;
dataFieldBoundary: string;
dataFieldArea: string;
fn: string;
}

export const parseDrawBoundary = (
Expand All @@ -29,9 +28,7 @@ export const parseDrawBoundary = (
data?.descriptionForUploading ||
defaultContent?.["descriptionForUploading"],
hideFileUpload: data?.hideFileUpload || defaultContent?.["hideFileUpload"],
dataFieldBoundary:
data?.dataFieldBoundary || defaultContent?.["dataFieldBoundary"],
dataFieldArea: data?.dataFieldArea || defaultContent?.["dataFieldArea"],
fn: data?.fn || defaultContent?.["fn"],
info: data?.info || defaultContent?.["info"],
policyRef: data?.policyRef || defaultContent?.["policyRef"],
howMeasured: data?.howMeasured || defaultContent?.["howMeasured"],
Expand All @@ -54,8 +51,7 @@ const defaultContent: DrawBoundary = {
howMeasured:
'<p>We have pre-populated the map with a red outline that includes the entire property using information from Land Registry.</p><p>In some cases, this outline might not include all the works or the areas that will be closed off. This might be because you&apos;re proposing works to a public highway (such as a dropped kerb), doing works that involve multiple properties, or works to a building that is part of a larger estate.</p><p>In these cases, you should amend the red outline by dragging the edges, or erase it by clicking the :wastebasket:-icon on the map and draw a new outline.</p><p></p><h1>How to draw and amend the outline</h1><ol><li><p>Move the cursor to the corner you want to start with and click or tap once.</p></li><li><p>Move the cursor to the next corner and click or tap.</p></li><li><p>Repeat until you have the shape you need.</p></li><li><p>Click or tap the last corner again to stop drawing.</p></li><li><p>To amend the outline, click or tap on a line and drag it into a new position.</p></li></ol><img src="https://api.editor.planx.uk/file/public/dni98ojg/Draw_Outline_2.gif">',
hideFileUpload: false,
dataFieldBoundary: "property.boundary.site",
dataFieldArea: "property.boundary.area",
fn: "proposal.site",
titleForUploading: "Upload a location plan",
descriptionForUploading:
"<p>Your location plan must:</p><ul><li><p>be based on an accurate, recognisable map</p></li><li><p>be drawn to a scale, labelled, and/or marked with a scale bar</p></li><li><p>show the site outline in red</p></li><li><p>include a<strong> </strong>north point</p></li></ul>",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ const osAddressProps = {
"property.type": ["residential.HMO.parent"],
"property.localAuthorityDistrict": ["Southwark"],
"property.region": ["London"],
"property.boundary.title.area": 1232.22,
"property.boundary.title.area.hectares": 0.123222,
"property.boundary.title": {
"property.boundary.area": 1232.22,
"property.boundary.area.hectares": 0.123222,
"property.boundary": {
geometry: {
type: "MultiPolygon",
coordinates: [
Expand Down Expand Up @@ -85,9 +85,9 @@ const proposedAddressProps = {
},
"property.localAuthorityDistrict": ["Southwark"],
"property.region": ["London"],
"property.boundary.title.area": 1232.22,
"property.boundary.title.area.hectares": 0.123222,
"property.boundary.title": {
"property.boundary.area": 1232.22,
"property.boundary.area.hectares": 0.123222,
"property.boundary": {
geometry: {
type: "MultiPolygon",
coordinates: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,9 @@ function Component(props: Props) {
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"] =
newPassportData["property.boundary"] = titleBoundary;
newPassportData["property.boundary.area"] = areaSquareMetres;
newPassportData["property.boundary.area.hectares"] =
squareMetresToHectares(areaSquareMetres);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ const Root = () => {
basemap={basemap}
ariaLabelOlFixedOverlay={`An interactive map for plotting and describing individual ${schemaName.toLocaleLowerCase()}`}
geojsonData={
passport && JSON.stringify(passport["property.boundary.site"])
passport && JSON.stringify(passport["proposal.site"])
}
geojsonBuffer={30}
drawMode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ function Component(props: Props) {
state.cachedBreadcrumbs,
state.teamSlug,
state.teamIntegrations?.hasPlanningData,
state.computePassport().data?.["property.boundary.site"],
state.computePassport().data?.["proposal.site"],
state.computePassport().data?.["_overrides"],
(state.computePassport().data?.["_address"] as SiteAddress) || {},
]);
Expand Down
Loading
Loading