From ea9f33e82f4be7a4bd110bad59ecc79f050e0951 Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Mon, 9 Dec 2024 17:25:05 +0100 Subject: [PATCH] feat: editors can select which planning constraints to check (#3968) --- .../modules/flows/validate/service/index.ts | 12 +- .../validate/service/planningConstraints.ts | 38 ++ .../modules/flows/validate/validate.test.ts | 185 ++++++++ .../modules/gis/service/digitalLand.ts | 34 +- .../components/PlanningConstraints/Editor.tsx | 184 ++++---- .../PlanningConstraints/Public.test.tsx | 81 ++++ .../components/PlanningConstraints/Public.tsx | 56 +-- .../components/PlanningConstraints/model.ts | 4 +- ...automations.planningConstraintNots.test.ts | 188 -------- .../automations.planningConstraints.test.ts | 411 ++++++++++++++++++ .../src/pages/FlowEditor/lib/store/preview.ts | 108 +++-- 11 files changed, 967 insertions(+), 334 deletions(-) create mode 100644 api.planx.uk/modules/flows/validate/service/planningConstraints.ts delete mode 100644 editor.planx.uk/src/pages/FlowEditor/lib/__tests__/automations.planningConstraintNots.test.ts create mode 100644 editor.planx.uk/src/pages/FlowEditor/lib/__tests__/automations.planningConstraints.test.ts diff --git a/api.planx.uk/modules/flows/validate/service/index.ts b/api.planx.uk/modules/flows/validate/service/index.ts index 85710b307c..7b77f595e7 100644 --- a/api.planx.uk/modules/flows/validate/service/index.ts +++ b/api.planx.uk/modules/flows/validate/service/index.ts @@ -8,8 +8,9 @@ import * as jsondiffpatch from "jsondiffpatch"; import { dataMerged, getMostRecentPublishedFlow } from "../../../../helpers.js"; import { validateFileTypes } from "./fileTypes.js"; import { validateInviteToPay } from "./inviteToPay.js"; -import { validateSections } from "./sections.js"; +import { validatePlanningConstraints } from "./planningConstraints.js"; import { validateProjectTypes } from "./projectTypes.js"; +import { validateSections } from "./sections.js"; type AlteredNode = { id: string; @@ -54,7 +55,14 @@ const validateAndDiffFlow = async ( const inviteToPay = validateInviteToPay(flattenedFlow); const fileTypes = validateFileTypes(flattenedFlow); const projectTypes = validateProjectTypes(flattenedFlow); - validationChecks.push(sections, inviteToPay, fileTypes, projectTypes); + const planningConstraints = validatePlanningConstraints(flattenedFlow); + validationChecks.push( + sections, + inviteToPay, + fileTypes, + projectTypes, + planningConstraints, + ); // Arrange list of validation checks in order of status: Fail, Warn, Pass, Not applicable const failingChecks = validationChecks.filter((v) => v.status == "Fail"); diff --git a/api.planx.uk/modules/flows/validate/service/planningConstraints.ts b/api.planx.uk/modules/flows/validate/service/planningConstraints.ts new file mode 100644 index 0000000000..e86f073349 --- /dev/null +++ b/api.planx.uk/modules/flows/validate/service/planningConstraints.ts @@ -0,0 +1,38 @@ +import { + ComponentType, + type FlowGraph, +} from "@opensystemslab/planx-core/types"; +import { numberOfComponentType } from "../helpers.js"; +import type { FlowValidationResponse } from "./index.js"; + +const validatePlanningConstraints = ( + flowGraph: FlowGraph, +): FlowValidationResponse => { + const numberOfPlanningConstraintNodes = numberOfComponentType( + flowGraph, + ComponentType.PlanningConstraints, + ); + if (numberOfPlanningConstraintNodes > 1) { + return { + title: "Planning Constraints", + status: "Fail", + message: + "When using Planning Constraints, your flow must have exactly ONE Planning Constraints component", + }; + } else if (numberOfPlanningConstraintNodes === 1) { + // In future, add extra `hasPlanningData` validation step here to ensure integration is available for this team + return { + title: "Planning Constraints", + status: "Pass", + message: "Your flow has valid Planning Constraints", + }; + } else { + return { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }; + } +}; + +export { validatePlanningConstraints }; diff --git a/api.planx.uk/modules/flows/validate/validate.test.ts b/api.planx.uk/modules/flows/validate/validate.test.ts index 46aa4a0833..9850036cd1 100644 --- a/api.planx.uk/modules/flows/validate/validate.test.ts +++ b/api.planx.uk/modules/flows/validate/validate.test.ts @@ -131,6 +131,11 @@ describe("sections validation on diff", () => { message: 'Your flow is not using Checklists which set "proposal.projectType"', }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, ]); }); }); @@ -190,6 +195,11 @@ describe("sections validation on diff", () => { message: 'Your flow is not using Checklists which set "proposal.projectType"', }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, ]); }); }); @@ -240,6 +250,11 @@ describe("invite to pay validation on diff", () => { status: "Not applicable", message: "Your flow is not using FileUpload or UploadAndLabel", }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, ]); }); }); @@ -300,6 +315,11 @@ describe("invite to pay validation on diff", () => { status: "Not applicable", message: "Your flow is not using FileUpload or UploadAndLabel", }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, ]); }); }); @@ -356,6 +376,11 @@ describe("invite to pay validation on diff", () => { status: "Not applicable", message: "Your flow is not using FileUpload or UploadAndLabel", }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, ]); }); }); @@ -414,6 +439,11 @@ describe("invite to pay validation on diff", () => { status: "Not applicable", message: "Your flow is not using FileUpload or UploadAndLabel", }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, ]); }); }); @@ -468,6 +498,11 @@ describe("invite to pay validation on diff", () => { message: 'Your flow is not using Checklists which set "proposal.projectType"', }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, ]); }); }); @@ -555,6 +590,11 @@ describe("ODP Schema file type validation on diff", () => { message: 'Your flow is not using Checklists which set "proposal.projectType"', }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, ]); }); }); @@ -633,6 +673,151 @@ describe("ODP Schema file type validation on diff", () => { message: 'Your flow is not using Checklists which set "proposal.projectType"', }, + { + title: "Planning Constraints", + status: "Not applicable", + message: "Your flow is not using Planning Constraints", + }, + ]); + }); + }); +}); + +describe("planning constraints validation on diff", () => { + it("passes if there is exactly one planning constraints component", async () => { + const alteredFlow = { + ...mockFlowData, + PlanningConstraints: { + type: 11, + data: { + title: "Check all constraints", + fn: "property.constraints.planning", + }, + }, + }; + + queryMock.mockQuery({ + name: "GetFlowData", + matchOnVariables: false, + data: { + flow: { + data: alteredFlow, + slug: "altered-flow-name", + team_id: 1, + team: { + slug: "testing", + }, + publishedFlows: [{ data: alteredFlow }], + }, + }, + }); + + await supertest(app) + .post("/flows/1/diff") + .set(auth) + .expect(200) + .then((res) => { + expect(res.body.message).toEqual("Changes queued to publish"); + expect(res.body.validationChecks).toEqual([ + { + title: "Sections", + status: "Pass", + message: "Your flow has valid Sections", + }, + { + title: "Planning Constraints", + status: "Pass", + message: "Your flow has valid Planning Constraints", + }, + { + title: "Invite to Pay", + status: "Not applicable", + message: "Your flow is not using Invite to Pay", + }, + { + title: "File types", + status: "Not applicable", + message: "Your flow is not using FileUpload or UploadAndLabel", + }, + { + title: "Project types", + status: "Not applicable", + message: + 'Your flow is not using Checklists which set "proposal.projectType"', + }, + ]); + }); + }); + + it("warns if there is more than one planning constraints component", async () => { + const alteredFlow = { + ...mockFlowData, + PlanningConstraints: { + type: 11, + data: { + title: "Check all constraints", + fn: "property.constraints.planning", + }, + }, + PlanningConstraintsTwo: { + type: 11, + data: { + title: "Check all constraints (dupe)", + fn: "property.constraints.planning", + }, + }, + }; + + queryMock.mockQuery({ + name: "GetFlowData", + matchOnVariables: false, + data: { + flow: { + data: alteredFlow, + slug: "altered-flow-name", + team_id: 1, + team: { + slug: "testing", + }, + publishedFlows: [{ data: alteredFlow }], + }, + }, + }); + + await supertest(app) + .post("/flows/1/diff") + .set(auth) + .expect(200) + .then((res) => { + expect(res.body.message).toEqual("Changes queued to publish"); + expect(res.body.validationChecks).toEqual([ + { + title: "Planning Constraints", + status: "Fail", + message: + "When using Planning Constraints, your flow must have exactly ONE Planning Constraints component", + }, + { + title: "Sections", + status: "Pass", + message: "Your flow has valid Sections", + }, + { + title: "Invite to Pay", + status: "Not applicable", + message: "Your flow is not using Invite to Pay", + }, + { + title: "File types", + status: "Not applicable", + message: "Your flow is not using FileUpload or UploadAndLabel", + }, + { + title: "Project types", + status: "Not applicable", + message: + 'Your flow is not using Checklists which set "proposal.projectType"', + }, ]); }); }); diff --git a/api.planx.uk/modules/gis/service/digitalLand.ts b/api.planx.uk/modules/gis/service/digitalLand.ts index d96115c4c4..f1a715d143 100644 --- a/api.planx.uk/modules/gis/service/digitalLand.ts +++ b/api.planx.uk/modules/gis/service/digitalLand.ts @@ -71,15 +71,30 @@ async function go( geom: string, extras: Record, ): Promise { - // generate list of digital land datasets we should query based on 'active' planx schema variables + // generate list of digital land datasets we should query and their associated passport values const activeDatasets: string[] = []; - Object.keys(baseSchema).forEach((key) => { - if (baseSchema[key]["active"]) { - baseSchema[key]["digital-land-datasets"]?.forEach((dataset: string) => { - activeDatasets.push(dataset); - }); - } - }); + let activeDataValues: string[] = []; + if (extras?.vals) { + // if the editor selected constraints, prioritise their selection + activeDataValues = extras.vals.split(","); + Object.keys(baseSchema).forEach((key) => { + if (activeDataValues.includes(key)) { + baseSchema[key]["digital-land-datasets"]?.forEach((dataset: string) => { + activeDatasets.push(dataset); + }); + } + }); + } else { + // else default to the internally maintained list of all "active" datasets + Object.keys(baseSchema).forEach((key) => { + if (baseSchema[key]["active"]) { + activeDataValues.push(key); + baseSchema[key]["digital-land-datasets"]?.forEach((dataset: string) => { + activeDatasets.push(dataset); + }); + } + }); + } // set up request query params per https://www.planning.data.gov.uk/docs const options = { @@ -159,7 +174,8 @@ async function go( // TODO followup with digital land about how to return 'nots' via API (currently assumes any "active" metadata was successfully queried) const nots = Object.keys(baseSchema).filter( (key) => - baseSchema[key]["active"] && !Object.keys(formattedResult).includes(key), + activeDataValues.includes(key) && + !Object.keys(formattedResult).includes(key), ); nots.forEach((not) => { formattedResult[not] = { diff --git a/editor.planx.uk/src/@planx/components/PlanningConstraints/Editor.tsx b/editor.planx.uk/src/@planx/components/PlanningConstraints/Editor.tsx index 0e084932ed..9770ab1bdc 100644 --- a/editor.planx.uk/src/@planx/components/PlanningConstraints/Editor.tsx +++ b/editor.planx.uk/src/@planx/components/PlanningConstraints/Editor.tsx @@ -1,5 +1,4 @@ import Box from "@mui/material/Box"; -import Checkbox from "@mui/material/Checkbox"; import Table from "@mui/material/Table"; import TableBody from "@mui/material/TableBody"; import TableCell from "@mui/material/TableCell"; @@ -11,14 +10,16 @@ import { ComponentType as TYPES } from "@opensystemslab/planx-core/types"; import { EditorProps } from "@planx/components/shared/types"; import { useFormik } from "formik"; import React from "react"; -import { FONT_WEIGHT_BOLD } from "theme"; import InputGroup from "ui/editor/InputGroup"; 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 Checkbox from "ui/shared/Checkbox/Checkbox"; +import ErrorWrapper from "ui/shared/ErrorWrapper"; import Input from "ui/shared/Input/Input"; import InputRow from "ui/shared/InputRow"; +import { array, object, string } from "yup"; import { ICONS } from "../shared/icons"; import { availableDatasets, parseContent, PlanningConstraints } from "./model"; @@ -36,8 +37,37 @@ function PlanningConstraintsComponent(props: Props) { data: newValues, }); }, + validationSchema: object({ + dataValues: array(string()).min(1, "Select at least one constraint"), + }), }); + const changeSelectAll = + (vals: string[] | undefined) => + (_event?: React.MouseEvent | undefined) => { + let newCheckedVals: string[]; + if (vals?.length !== availableDatasets.length) { + newCheckedVals = availableDatasets.map((d) => d.val); + } else { + newCheckedVals = []; + } + formik.setFieldValue("dataValues", newCheckedVals); + }; + + const changeDataset = + (val: string) => + (_event?: React.MouseEvent | undefined) => { + let newCheckedVals; + if (formik.values.dataValues?.includes(val)) { + newCheckedVals = formik.values.dataValues.filter((dv) => dv !== val); + } else { + newCheckedVals = formik.values.dataValues + ? [...formik.values.dataValues, val] + : [val]; + } + formik.setFieldValue("dataValues", newCheckedVals); + }; + return (