chore: add support for multiple schemas within export/digitalPlanning

[jessicamcinchak]

Picks up outstanding tasks outlined here #625

See implementation in theopensystemslab/planx-new#4206

We now have a preApplication schema in addition to an application schema and need to be able to correctly map and validate any PlanX passport against the correct one to generate the application.json files included in submission payloads.

The logic is:

• If application.type = "preApp", then make and validate a preApplication payload
• If application.type is any defined statutory application type, then make and validate an application payload
• If application.type is unrecognised or undefined, then make an application payload with skipValidation = true

[jessicamcinchak]

In an ideal world, I think we'd have two separate classes that each return a precise type rather than a single class that returns ApplicationPayload | PreApplicationPayload - but this approach seemed like the path of least resistance to share as many of the existing passport helper methods as possible without a massive refactor or duplication right now (eg they're private methods on the class and not easy to export/import!)

As we move forward with the "prototypeApplication" and a top-level discriminated union, we know that bigger refactor is coming soon enough anyways - this is very much an interim working solution.

[DafyddLlyr]

Agreed on general approach here for sure.

If we hit any type issues with this in planx-new in the short term, we could maybe consider passing an additional argument into the class -

type DigitalPlanningExportParams = {
  client: Client;
  sessionId: string;
  skipValidation?: boolean;
  type: "preApp" | ApplictionTypeUnion; // Create type discriminator here
};

export async function generateDigitalPlanningPayload({
  client,
  sessionId,
  skipValidation,
  type,
}: DigitalPlanningExportParams): Promise
 //  Modify return type based on this
  type extends "preApp'"? PreApplicationPayload : ApplicationPayload
> {
  ...
}

Returning a union though is exactly what I would have done here - I don't think we use the return type much in PlanX at this stage.

[jessicamcinchak]

Appreciate you coming back to this one ! There's an explicit reason I haven't gone with an additional argument from the planx-new side which is the (admittedly awkward) reality that discretionary/undefined-application-type payloads are also getting routed through this function to create a generic "application.json" for submission payloads (send to email & S3).

Therefore the discriminated type exposed to planx-new wouldn't be so tidy, but have to be something like this which felt pretty meaningless!

"preApp" | ApplicationTypeUnion | string | undefined

[jessicamcinchak]

These helper functions (which we call in planx-new for things like publish validation checks) are going to continue only reading against the application schema - but I think this is generally fine & expected behavior! Enums referenced throughout preApplication are shared anyways currently.

[jessicamcinchak]

Similar to comments above, I'm introducing a new mapPassportToPreApplicationPayload() within the existing class here to share/re-use as many of the existing mappings as possible.

this.payload and validatePayload() are the only other pieces that need to be application-type aware.

[jessicamcinchak]

These commands aren't the most eloquent and will be worth refactoring to crawl the directory or similar if we find ourselves with a third schema, but they work for now (and no known plans for a third schema on our side anytime soon)!

[jamdelion]

Clear to follow (once I'd unfurled model.ts anyway, before that I was a bit confused! 😅 ) and all looks sensible to me! Definitely pro sharing as many methods as possible, and happy with the slight duplication throughout while we just have two schemas - still easy to read.

[jessicamcinchak]

I'm generating two types.d.ts files now - one per schema - which makes sense for function return types, but does duplicate what the "source types" (eg digital-planning-data-schemas repo) know to be "shared" types. Now they're essentially just duplicated again

[DafyddLlyr]

Agreed on general approach here for sure.

If we hit any type issues with this in planx-new in the short term, we could maybe consider passing an additional argument into the class -

type DigitalPlanningExportParams = {
  client: Client;
  sessionId: string;
  skipValidation?: boolean;
  type: "preApp" | ApplictionTypeUnion; // Create type discriminator here
};

export async function generateDigitalPlanningPayload({
  client,
  sessionId,
  skipValidation,
  type,
}: DigitalPlanningExportParams): Promise
 //  Modify return type based on this
  type extends "preApp'"? PreApplicationPayload : ApplicationPayload
> {
  ...
}

Returning a union though is exactly what I would have done here - I don't think we use the return type much in PlanX at this stage.

[DafyddLlyr]

One thought on the whole multiple classes thing I've just hit seeing some of this code.

We can inherit based on classes so maybe there's a decent way forward here without a huge rewrite that has the majority of the functionality in a single abstract / base class, and then have per-app type classes that just handle the differences.

This might actually make things a whole lot simpler - highlighting the differences and maintaining the joint functionality.

Very happy to have a pass at this so we can compare and make a decision - worth exploring before going for a third schema 👍

[jessicamcinchak]

Super nice idea 🔖