Skip to content

Commit

Permalink
feat: editors can select which planning constraints to check (#3968)
Browse files Browse the repository at this point in the history
  • Loading branch information
jessicamcinchak authored Dec 9, 2024
1 parent 6390a70 commit ea9f33e
Show file tree
Hide file tree
Showing 11 changed files with 967 additions and 334 deletions.
12 changes: 10 additions & 2 deletions api.planx.uk/modules/flows/validate/service/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
38 changes: 38 additions & 0 deletions api.planx.uk/modules/flows/validate/service/planningConstraints.ts
Original file line number Diff line number Diff line change
@@ -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 };
185 changes: 185 additions & 0 deletions api.planx.uk/modules/flows/validate/validate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
]);
});
});
Expand Down Expand Up @@ -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",
},
]);
});
});
Expand Down Expand Up @@ -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",
},
]);
});
});
Expand Down Expand Up @@ -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",
},
]);
});
});
Expand Down Expand Up @@ -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",
},
]);
});
});
Expand Down Expand Up @@ -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",
},
]);
});
});
Expand Down Expand Up @@ -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",
},
]);
});
});
Expand Down Expand Up @@ -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",
},
]);
});
});
Expand Down Expand Up @@ -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"',
},
]);
});
});
Expand Down
34 changes: 25 additions & 9 deletions api.planx.uk/modules/gis/service/digitalLand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,30 @@ async function go(
geom: string,
extras: Record<string, string>,
): Promise<GISResponse> {
// 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 = {
Expand Down Expand Up @@ -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] = {
Expand Down
Loading

0 comments on commit ea9f33e

Please sign in to comment.