feat: importable getFormContext#269
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extracts form model resolution and context retrieval logic into a new reusable module (form-context.ts), making key functions like getFormContext, getFormModel, and related utilities importable for use outside the route handlers.
Key Changes
- Created new
form-context.tsmodule with form model resolution, caching, and context retrieval logic - Refactored
routes/index.tsto use the newresolveFormModelfunction, reducing duplication - Exported form context utilities from the main engine plugin index for public API use
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
src/server/plugins/engine/form-context.ts |
New module containing form model resolution, caching logic, and helper functions for form context and answer mapping |
src/server/plugins/engine/form-context.test.ts |
Comprehensive test suite for the new form-context module covering all exported functions |
src/server/plugins/engine/routes/index.ts |
Refactored to use resolveFormModel function, removing ~70 lines of duplicated code |
src/server/plugins/engine/index.ts |
Added exports for getFormContext, getFormModel, getFirstJourneyPage, mapFormContextToAnswers, and resolveFormModel |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
alexluckett
left a comment
alexluckett
left a comment
There was a problem hiding this comment.
Nice work, @davidkelley! Few questions.
| const summaryRequest: SummaryRequest = { | ||
| app: {}, | ||
| method: 'get', | ||
| params: { | ||
| path: 'summary', | ||
| slug: journey, | ||
| ...(isPreviewState(state, options) && { | ||
| state: resolveState(state) | ||
| }) | ||
| }, | ||
| path: `/${formModel.basePath}/summary`, | ||
| query: {}, | ||
| url: new URL( | ||
| `/${formModel.basePath}/summary`, | ||
| 'https://form-context.local' | ||
| ), | ||
| server, | ||
| yar | ||
| } |
There was a problem hiding this comment.
Just trying to think into our chat a few weeks ago, hoping you can refresh my memory...
Are we mocking out this request to speed up delivery of the feature and unblock everyone, with a view that this will be uplifted later on to do it the 'right' way and avoid the request being passed too further down into the hierarchy?
Did you/your team look into how feasible that would be? If so, did you have an idea of effort/time required?
There was a problem hiding this comment.
Hey @alexluckett , unfortunately we ran out of time to do so really.. we're a bit behind on the migration
| journey: string, | ||
| state: JourneyState = FormStatus.Live, | ||
| options: FormContextOptions = {} | ||
| ): Promise<FormContext> { | ||
| const formModel = await resolveFormModel(server, journey, state, options) |
There was a problem hiding this comment.
Could we keep to consistent slug terminology with the rest of the codebase please? journey isn't a term we use.
|
|
||
| const formState = { | ||
| ...cachedState, | ||
| $$__referenceNumber: cachedState.$$__referenceNumber ?? 'TODO' |
There was a problem hiding this comment.
Can we remove 'TODO and just leave it as undefined? Presumably engine generates one if it's empty anyway?
|
Refactored the form context helper so it now shares the same model-loading path as the core routes:
resolveFormModelbuilds/reuses cached models with correct base paths (including preview/prefix handling), email checks, and version metadata. The externalgetFormContextnow pulls cached state, keeps reference numbers, and returns contexts via the shared model. Exposed the helper exports through the plugin entrypoint for consumers. Tests were updated to use typed mocks and tidy import ordering. Core runtime behavior stays the same; we just removed duplication and made the helper safe to consume outside the journey.