Add digital land requirements doc #2886
Conversation
Scoping document for implementing a third submission source in BOPS, aligned to the MHCLG/Digital Land planning application data specification (v0.1.57). Covers standard overview, current BOPS state, gap analysis, MVP proposal, and open questions/risks.
Documents critical issues in the DL spec's generated JSON schemas: permissive additionalProperties, missing version field, empty required arrays, untyped references, boolean-as-string inconsistencies, and missing format constraints. Includes recommendations for the DL team and notes that BOPS will need application-level validation on top.
| **`additionalProperties: true` at root level — the schema barely validates** | ||
|
|
||
| The root schema allows any extra keys. A typo like `"sit-details"` instead of `"site-details"` passes validation silently — you get an apparently valid submission with missing data. This defeats the core purpose of schema validation. Should be `false`. | ||
|
|
There was a problem hiding this comment.
There are perfectly good reasons to allow additional keys to be added. I'm not convinced that this is a good recommendation.
(As far as I know the ODP schema allows additional keys and we do rely on this behaviour, for example.)
There was a problem hiding this comment.
The example that Claude gives here is misleading - a simple site-details to sit-details type would be picked up because site-details is a required parameter.
| **`reference` claims UUID but doesn't enforce it, and the example contradicts the spec** | ||
|
|
||
| The markdown spec says `reference` MUST be a UUID. The JSON schema only says `"type": "string"` with no `format: "uuid"` or pattern. The sole example payload uses `"HH/2025/001"` — not a UUID. The spec, the schema, and the example are three-way inconsistent. |
There was a problem hiding this comment.
Nitpick, but this is not three-way inconsistent. It's two-way inconsistent. The example given conforms with the JSON schema (because it's a string). The markdown spec seems to contradict the JSON schema, that's true.
(My guess is that whoever wrote it didn't literally mean UUID in the technical sense but a general concept of a unique identifier, but that could benefit from clarification.)
| **Multi-schema validation is entirely consumer-side** | ||
|
|
||
| The spec says: "The responsibility for handling multi-type applications is placed on the consuming application." Every vendor must independently implement the same orchestration logic. This should be a provided utility or composite schema. | ||
|
|
There was a problem hiding this comment.
Is this our problem?
| **Cross-reference integrity is unenforceable** | ||
|
|
||
| Supporting documents reference `application.documents` by `reference`. The spec says "must match", but JSON Schema cannot validate cross-references between parts of a document. This needs application-level validation, but the spec doesn't make that distinction clear. | ||
|
|
There was a problem hiding this comment.
This doesn't seem like something a schema could solve.
| ### 6.4 Recommendations for Digital Land Team | ||
|
|
||
| | Issue | Recommendation | | ||
| |---|---| | ||
| | `additionalProperties: true` | Set to `false` to catch typos and malformed payloads | | ||
| | No version field | Add `spec-version` to the `application` object | | ||
| | Untyped `reference` | Add `"format": "uuid"` or a regex pattern; fix the example | | ||
| | Empty `required: []` | Define meaningful required fields for `site-location`, `phone-number`, `agent-details` | | ||
| | Empty enum `[]` | Fix schema generator to omit the field or populate valid subtypes | | ||
| | Boolean-as-string | Use a consistent pattern across all modules | | ||
| | No date/email formats | Add `"format": "date"`, `"format": "email"`, postcode pattern, WKT pattern | | ||
| | Free-text addresses | Add structured address component alongside or replacing `address-text` | | ||
| | Redundant `modules` array | Drop it or enforce consistency with top-level keys | | ||
| | No metadata envelope | Add `submission-metadata`: `source`, `submitted-at`, `spec-version`, `correlation-id` | | ||
| | Consumer-side multi-validation | Provide a composite meta-schema or reference validation library | | ||
| | Missing test payloads | Generate valid examples for all 25 types | |
There was a problem hiding this comment.
This just reiterates the previous section.
|
|
||
| | # | Decision | Options | | ||
| |---|---|---| | ||
| | D1 | **Schema version pinning** | DL has no version in payload. Do we: (a) pin to a vendored version, (b) accept a version query param, (c) use `application-types` to infer? | |
There was a problem hiding this comment.
Isn't it the case that we already only support one ODP version at a time in practice?
It might not be ideal, but I don't know if DL makes this much worse.
(Perhaps something we could solve for both at once, though.)
There was a problem hiding this comment.
Technically we do support multiple ODP versions
| | File | Change | | ||
| |---|---| | ||
| | `engines/bops_submissions/app/jobs/bops_submissions/submission_processor_job.rb` | Add `digital-land` dispatch branch | | ||
| | `engines/bops_api/app/controllers/concerns/bops_api/schema_validation.rb` | Add DL schema detection logic | |
There was a problem hiding this comment.
Note that we want to revisit the schema validation approach anyway.
| - **Audit trail**: Already handled — raw payload stored in `Submission#request_body`, headers in `request_headers` | ||
| - **Retries**: `SubmissionProcessorJob` already retries on failure via Sidekiq; `PlanningApplicationDependencyJob` retries 5x | ||
| - **Observability**: AppSignal integration already in place for error reporting | ||
| - **Rate limiting**: Consider adding per-API-user rate limits (not in MVP) |
There was a problem hiding this comment.
This is already on the backlog. 👍🏻
| Use `application.reference` (UUID) for deduplication: | ||
| - Before creating submission, check if a `Submission` with matching `application_reference` already exists for the local authority | ||
| - If found and `completed`: return existing UUID with 200 (idempotent replay) | ||
| - If found and `started/submitted`: return existing UUID with 202 (still processing) | ||
| - If not found: create new submission, return 201 |
There was a problem hiding this comment.
Alternatively: maybe it's an error to resubmit with an existing reference?
There was a problem hiding this comment.
I think there's a danger of overloading this reference field - we should have a UUID that's generated by the submitter and also a reference number that's generated by the consumer (in our case BOPS) which probably doesn't appear here because it's only generated once the application is processed.
3 participants
Description of change
A brief description of the change, with enough context for the reviewer to be able to understand why we're making this change.
Story Link
https://link-to-issue
Screenshots
If you have made changes to the frontend please add screenshots
Decisions [OPTIONAL]
If you had to make any decisions between different ways of doing things, what where they and why?
Known issues [OPTIONAL]
Things you know need further follow on work but aren't in scope of this issue
Further testing or sign off required [OPTIONAL]
e.g.