Add data.application.type for PreApplication schema #281
Conversation
This mirrors the Application schema and reduces the need for branching when submitting to an endpoint that handles both pre-application and application requests.
| build |
There was a problem hiding this comment.
The tsconfig.json file specifies build for the outDir option. The only thing in there is tsconfig.tsbuildinfo but just to be safe I've added it to the list of ignored files.
| @@ -33,14 +33,14 @@ const walkDirectory = async (dir: string) => { | |||
| // Write file to mirrored directory, outside the /data folder where the TS examples are stored | |||
| const jsonExampleFilePath = path.join( | |||
| dir.replace('/data', ''), | |||
| `${path.basename(file, '.ts')}.json` | |||
| `${path.basename(file, '.ts')}.json`, | |||
There was a problem hiding this comment.
Added by the linter
| ); | ||
| await convertTypeScriptObjectToJSON(filePath, jsonExampleFilePath); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| (async () => { | ||
| void (async () => { |
There was a problem hiding this comment.
The linter was complaining about this so i added void to keep it quiet
| @@ -1,5 +1,8 @@ | |||
| #!/bin/bash | |||
|
|
|||
| set -e | |||
| set -o pipefail | |||
|
|
|||
There was a problem hiding this comment.
This so the script fails if a subprocess fails - the pre-application schema was failing but it continued on and there was just a single 'Error' line so it was hard to spot in the output.
| @@ -53,46 +60,49 @@ const schemas = [ | |||
| describe.each(schemas)('$name', ({schema, examples}) => { | |||
| const validator = new Validator(); | |||
|
|
|||
| const _applicationTypeProperty = | |||
| '$data.application.type.description' || '$applicationType'; | |||
|
|
|||
There was a problem hiding this comment.
Removed as we don't need this now it's in the same place
| expect(result.errors).not.toHaveLength(0); | ||
| }); | ||
| }); | ||
| describe.each<Application | PreApplication>(examples)( |
There was a problem hiding this comment.
Jest couldn't infer the type so we have to be explicit
| @@ -4,10 +4,14 @@ | |||
| "rootDir": ".", | |||
| "outDir": "build", | |||
| "skipLibCheck": true, | |||
| "esModuleInterop": true, | |||
| "resolveJsonModule": true | |||
There was a problem hiding this comment.
This is so we can use import with the JSON files in the tests
| @@ -1,4 +1,4 @@ | |||
| import {Entity} from '../../../shared/Constraints'; | |||
| import {Entity} from '../../../shared/Entity'; | |||
There was a problem hiding this comment.
Entity was being defined twice so I moved it to the shared directory
| preApp: 'Pre-application advice', | ||
| 'preApp.householder': 'Pre-application advice - Householder', | ||
| 'preApp.minor': 'Pre-application advice - Minor', | ||
| 'preApp.major': 'Pre-application advice - Major', |
There was a problem hiding this comment.
We're just using the one type for now (preApp) but it's possible there may be others later so this is more an indication of intent
This mirrors the Application schema and reduces the need for branching when submitting to an endpoint that handles both pre-application and application requests.
In BOPS we're planning to create pre-applications as a type of planning application as the workflow closely mirrors that of an application and by having
applicationTypeat the root it makes it difficult to handle cleanly. I've leftapplicationTypein the root so that the change wouldn't break anyone depending on that.