feat: write a `prototypeApplication` schema with definitions based on primary application type #236

jessicamcinchak · 2024-08-29T20:50:55Z

I know this is a VERY BIG PR - but it wasn't a particularly easy one to break up, so thanks in advance for patience reviewing !

Here's a summary of key changes:

The prototypeApplication schema now has "real" type definitions which are feature-parity with the original application schema - but now enhanced with rules-based variations based on the primary applicationType
- I've added comments below to highlight specific examples that demonstrate the new benefits of this, and places where there's still a few open questions
- In annotating these types, I've dropped a lot of @id notations as we used to use per Idox feedback about this and opted for just @descriptions, so please take a look at the generated JSON too and see how this compares
All application examples now have an equivalent prototypeApplication example for side-by-side comparisons of what's changing (13 example payloads in total => 26 new files here)

What I haven't done here:

Regenerated any example application data (it'd be great to do this in the near future in order to pick up & check against latest service changes, but I don't want to muddle the side-by-side comparisons for initial feedback)
Perfected the rules per primary application type - this is especially lacking across data.proposal for this MVP, definitely one to revisit separately soon ! Also, it'll never be perfect
Defined all application types - I've gone for feature-parity with types covered by the original application schema here, which is already "householder" centric in-line with upcoming integration expectations

jessicamcinchak · 2024-09-17T19:28:26Z

types/schemas/prototypeApplication/data/Applicant.ts

+
+export interface OwnersNoticeDate extends BaseOwners {
+  noticeDate: Date;
+}


Ownership is a nice migration story:

Our original application types used code comments to note which shape of owners we expected for various application types, ultimately justifying a type that was too-generic and made all of these options possible: https://github.com/theopensystemslab/digital-planning-data-schemas/blob/main/types/schemas/application/data/Applicant.ts#L88-L101

Now the LDCApplicant definition and so on specify only the ownership variation it actually expects!

jessicamcinchak · 2024-09-17T19:31:35Z

types/schemas/prototypeApplication/data/ApplicationData.ts

  pp: PPApplicationData;
+  listed: NonFeeCarryingApplicationData;
+  landDrainageConsent: FeeCarryingApplicationData;


Again, nice migration win in here of now explicitly defining which application types are "fee carrying" and which are not. There's a big semantic difference between fee: 0 & fee: "notApplicable" that is now better communicated, no longer confusable based on a union type !

Yeah this is nice! I had imagined these maps would always be application type specific, but this is a great solution.

jessicamcinchak · 2024-09-17T19:35:35Z

types/schemas/prototypeApplication/enums/PlanningDesignation.ts

@@ -174,15 +174,15 @@ type BasePlanningDesignation =
 */
 type NonIntersectingPlanningDesignation = {
  intersects: false;
-} & BasePlanningDesignation;
+} & {value: BasePlanningDesignation};


PlanningDesignations are actually an interesting case where I'm not totally convinced we want to migrate to a new enum structure? Will be curious to hear BOPS feedback on this.

Designations & constraints aren't the same typical value/description object enums like ProjectTypes (which make sense to simplify to string enums!) because other properties are also/still present (eg intersects).

Yeah this is good to flag that they aren't true "enums" as per the rest.

There might be a middle ground where they still end up in a fairly formal anyOf but keep a single enum-like key (as opposed to having to have a perfectly matching value and description).

Agreed that reaching out for feedback here seems best!

jessicamcinchak · 2024-09-17T19:39:46Z

types/shared/Responses.ts

@@ -1,4 +1,4 @@
-import {URL} from '../../shared/utils';
+import {URL} from './utils';

 /**
 * @id #Responses


Note that Responses are now shared between the application & prototypeApplication, but nearly all else is re-defined fresh (eg even Metadata requires updated FileType enum structure).

The shared folder is generally still sparsely populated, this will make more sense to refactor once prototypeApplication replaces application in my opinion ! It's felt much easier to just keep them totally seperate while doing this comparison/migration exercise rather than try to refactor/share more in same go.

Makes sense!

jessicamcinchak · 2024-09-17T21:24:29Z

schemas/prototypeApplication.json

+    "Integer": {
+      "format": "integer",
+      "type": "number"
+    },


This is a first attempt at adding a specific "Integer" type via utils defintions - this works well as an initial placeholder, but isn't technically meaningful yet !

I'm aiming for "type": "integer" as supported by JSON Schema, but with a way of generating from ts-json-schema-generator.

Okay further investigation here:

our lib ts-json-schema-generator does NOT support integer types (yet?): https://github.com/vega/ts-json-schema-generator?tab=readme-ov-file#current-state

alternatively typescript-json-schema does support with an annotation like this: https://github.com/YousefED/typescript-json-schema?tab=readme-ov-file#annotations (this is a good example of our goal state)

Is it insane to think about switching TS generation libs? This thread has a lot of comparison notes, opinions: vega/ts-json-schema-generator#101 (one for a followup discussion/PR for sure!)

Ah that's a frustrating limitation to hit.

It looks like ajv-formats support int32 and floats, so it might just be that this is the recommended library to use (or one with similar support).

Not sure how well this would work cross-language or in other contexts - it's definitely far from ideal, and we should strive to have the generated .json file be the source of truth here.

Another option (I've not tried yet) might be multipleOf: 1 (docs).

Awkward workaround, but maybe one of these two options might get us to v1.

jessicamcinchak · 2024-09-18T10:43:54Z

tests/usage.test.ts

+  //   name: 'PrototypeApplication',
+  //   schema: prototypeApplicationSchema,
+  //   examples: getJSONExamples<PrototypeApplication>('prototypeApplication'),
+  // },


This didn't work so smoothly & throws a number of type errors !

One to sort out in a follow-up PR I think, and for here I'm just relying on TS definitions & pnpm build-json-examples as the "validation" check (eg building the JSON example fails if type errors).

Happy for this to be a follow up!

jessicamcinchak · 2024-09-18T10:47:27Z

types/schemas/prototypeApplication/enums/FileType.ts

 /**
 * @id #FileType
 * @description Types of planning documents and drawings
 */
-export type FileType =
+export type PrototypeFileType =


A few temporary "renames" required else we'll throw an error that more than one type with same name can't be exported ! Again, these will get simplified once prototypeApplication replaces application

jessicamcinchak · 2024-09-18T11:27:35Z

examples/application/data/landDrainageConsent.ts

@@ -712,6 +712,6 @@ export const landDrainageConsent: Application = {
        payable: [],
      },
    },
-    schema: `https://theopensystemslab.github.io/digital-planning-data-schemas/${version}/schema.json`,
+    schema: `https://theopensystemslab.github.io/digital-planning-data-schemas/${version}/schemas/application.json`,


When we re-organised the file structure, we missed updating existing example payload URLs - so those are handled here! Sorry it makes for another 13 x 2 files changed though 🙈

Good catch thanks!

jessicamcinchak · 2024-09-18T11:32:23Z

Extend ODP Schema repository to support multiple schemas (Application, PreApp, Assessment, etc) and improve rules-based definitions (release v1.0.0🚀)

DafyddLlyr

Huge effort! 🙌

A few very minor comments - pretty much all typos and annotation suggestions.

All looks good to me, happy to help and fix forward some of the outstanding issues around tests and types.

DafyddLlyr · 2024-09-19T09:02:23Z

examples/application/data/landDrainageConsent.ts

@@ -712,6 +712,6 @@ export const landDrainageConsent: Application = {
        payable: [],
      },
    },
-    schema: `https://theopensystemslab.github.io/digital-planning-data-schemas/${version}/schema.json`,
+    schema: `https://theopensystemslab.github.io/digital-planning-data-schemas/${version}/schemas/application.json`,


Good catch thanks!

DafyddLlyr · 2024-09-19T16:11:45Z

types/schemas/prototypeApplication/enums/PlanningDesignation.ts

@@ -174,15 +174,15 @@ type BasePlanningDesignation =
 */
 type NonIntersectingPlanningDesignation = {
  intersects: false;
-} & BasePlanningDesignation;
+} & {value: BasePlanningDesignation};


Yeah this is good to flag that they aren't true "enums" as per the rest.

There might be a middle ground where they still end up in a fairly formal anyOf but keep a single enum-like key (as opposed to having to have a perfectly matching value and description).

Agreed that reaching out for feedback here seems best!

DafyddLlyr · 2024-09-19T16:12:05Z

types/shared/Responses.ts

@@ -1,4 +1,4 @@
-import {URL} from '../../shared/utils';
+import {URL} from './utils';

 /**
 * @id #Responses


Makes sense!

DafyddLlyr · 2024-09-19T16:25:47Z

schemas/prototypeApplication.json

+    "Integer": {
+      "format": "integer",
+      "type": "number"
+    },


Ah that's a frustrating limitation to hit.

It looks like ajv-formats support int32 and floats, so it might just be that this is the recommended library to use (or one with similar support).

Not sure how well this would work cross-language or in other contexts - it's definitely far from ideal, and we should strive to have the generated .json file be the source of truth here.

DafyddLlyr · 2024-09-19T16:26:28Z

tests/usage.test.ts

+  //   name: 'PrototypeApplication',
+  //   schema: prototypeApplicationSchema,
+  //   examples: getJSONExamples<PrototypeApplication>('prototypeApplication'),
+  // },


Happy for this to be a follow up!

DafyddLlyr · 2024-09-19T19:53:19Z

types/schemas/prototypeApplication/data/ApplicationData.ts

  pp: PPApplicationData;
+  listed: NonFeeCarryingApplicationData;
+  landDrainageConsent: FeeCarryingApplicationData;


Yeah this is nice! I had imagined these maps would always be application type specific, but this is a great solution.

DafyddLlyr · 2024-09-19T19:55:55Z

types/schemas/prototypeApplication/data/Property.ts

+ * @id #UKRegion
+ * @description The UK region that contains this address sourced from planning.data.gov.uk/dataset/region, where London is a proxy for the Greater London Authority (GLA) area
+ */
+export type UKRegion =


(Not new in this PR, but just noticed!) These are actually regions of England and not the UK - maybe a question for Planning Data would be if this is always going to be the case.

Aha this is a great shout - I think we should probably be using "England" throughout for clarity. Going to find + replace this one in a followup PR 👍

DafyddLlyr · 2024-09-19T19:58:18Z

types/schemas/prototypeApplication/data/Property.ts

+export interface LondonProperty extends UKProperty {
+  region: Extract<UKRegion, 'London'>;
+  titleNumber?: {
+    known: 'Yes' | 'No';


We may want a pass through all binary choices to make them booleans.

DafyddLlyr · 2024-09-19T20:00:18Z

types/schemas/prototypeApplication/data/Proposal.ts

+   */
+  newStoreys?: NewBuildingsOrStoreys;
+  /**
+   * @description Project cost


nit: Worth adding GBP?

types/schemas/prototypeApplication/data/Proposal.ts

Co-authored-by: Dafydd Llŷr Pearson <[email protected]>

jessicamcinchak added 3 commits August 29, 2024 22:47

rewrite data.applicant, remove data.user

345a2e6

setup examples data for prototype application

c16aa9f

share responses and metadata, define prototype applicant and property

69fa626

jessicamcinchak mentioned this pull request Sep 13, 2024

chore: extend SiteAddress type to account for ranges theopensystemslab/planx-new#3670

Merged

jessicamcinchak added 2 commits September 17, 2024 20:51

add back application data types

fee54da

add back application data types

df6f4a5

jessicamcinchak commented Sep 17, 2024

View reviewed changes

jessicamcinchak added 2 commits September 17, 2024 23:15

data.proposal plus all pp examples

5584845

fix pp examples

834f4a4

jessicamcinchak commented Sep 17, 2024

View reviewed changes

all remaining PA examples

5ccaad1

jessicamcinchak commented Sep 18, 2024

View reviewed changes

jessicamcinchak added 3 commits September 18, 2024 13:13

correct metadata file requirement enums

7bc00b4

correct schema URLs for all examples

c766ace

tidy unused imports

e694fed

jessicamcinchak changed the title ~~wip: prototype a new spec that supports definitions specific to primary application types~~ feat: write a prototypeApplication schema with definitions based on primary application type Sep 18, 2024

jessicamcinchak marked this pull request as ready for review September 18, 2024 11:26

jessicamcinchak commented Sep 18, 2024

View reviewed changes

jessicamcinchak requested a review from a team September 18, 2024 11:28

DafyddLlyr approved these changes Sep 19, 2024

View reviewed changes

jessicamcinchak mentioned this pull request Sep 19, 2024

feat: define optional overrides on PlanX metadata #237

Closed

jessicamcinchak and others added 2 commits September 19, 2024 22:25

pr feedback

b187eae

Co-authored-by: Dafydd Llŷr Pearson <[email protected]>

add GBP units

f88f0c1

jessicamcinchak merged commit 610f2f7 into main Sep 19, 2024
3 checks passed

jessicamcinchak deleted the jess/prototype-application-definitions branch September 19, 2024 20:36

jessicamcinchak mentioned this pull request Sep 19, 2024

Differentiate between "number" & "integer" types #239

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: write a `prototypeApplication` schema with definitions based on primary application type #236

feat: write a `prototypeApplication` schema with definitions based on primary application type #236

jessicamcinchak commented Aug 29, 2024 •

edited

Loading

jessicamcinchak Sep 17, 2024

jessicamcinchak Sep 17, 2024 •

edited

Loading

DafyddLlyr Sep 19, 2024

jessicamcinchak Sep 17, 2024

DafyddLlyr Sep 19, 2024

jessicamcinchak Sep 17, 2024 •

edited

Loading

DafyddLlyr Sep 19, 2024

jessicamcinchak Sep 17, 2024 •

edited

Loading

jessicamcinchak Sep 18, 2024 •

edited

Loading

DafyddLlyr Sep 19, 2024

DafyddLlyr Sep 19, 2024

jessicamcinchak Sep 18, 2024

DafyddLlyr Sep 19, 2024

jessicamcinchak Sep 18, 2024 •

edited

Loading

jessicamcinchak Sep 18, 2024

DafyddLlyr Sep 19, 2024

jessicamcinchak commented Sep 18, 2024

DafyddLlyr left a comment

DafyddLlyr Sep 19, 2024

DafyddLlyr Sep 19, 2024

DafyddLlyr Sep 19, 2024

DafyddLlyr Sep 19, 2024

DafyddLlyr Sep 19, 2024

DafyddLlyr Sep 19, 2024

DafyddLlyr Sep 19, 2024

jessicamcinchak Sep 19, 2024

DafyddLlyr Sep 19, 2024

DafyddLlyr Sep 19, 2024

feat: write a prototypeApplication schema with definitions based on primary application type #236

feat: write a prototypeApplication schema with definitions based on primary application type #236

Conversation

jessicamcinchak commented Aug 29, 2024 • edited Loading

Choose a reason for hiding this comment

jessicamcinchak Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jessicamcinchak Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jessicamcinchak Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

jessicamcinchak Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jessicamcinchak Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jessicamcinchak commented Sep 18, 2024

DafyddLlyr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feat: write a `prototypeApplication` schema with definitions based on primary application type #236

feat: write a `prototypeApplication` schema with definitions based on primary application type #236

jessicamcinchak commented Aug 29, 2024 •

edited

Loading

jessicamcinchak Sep 17, 2024 •

edited

Loading

jessicamcinchak Sep 17, 2024 •

edited

Loading

jessicamcinchak Sep 17, 2024 •

edited

Loading

jessicamcinchak Sep 18, 2024 •

edited

Loading

jessicamcinchak Sep 18, 2024 •

edited

Loading