From c7b9fc6356c23bb2f87e0104e4560f6aecae2743 Mon Sep 17 00:00:00 2001 From: Mohammed Khalid Date: Tue, 24 Mar 2026 15:33:13 +0000 Subject: [PATCH 1/2] feat(engine): read form version from definition metadata Use $$__formVersion from form definition metadata as the source of truth for version metadata in form context and adapter v1 payloads, with tests covering fallback precedence. --- src/server/constants.js | 1 + .../plugins/engine/beta/form-context.test.ts | 76 +++++++++++++++++-- .../plugins/engine/beta/form-context.ts | 13 ++-- src/server/plugins/engine/helpers.ts | 17 +++++ .../outputFormatters/adapter/v1.test.ts | 54 +++++++++++++ .../engine/outputFormatters/adapter/v1.ts | 12 +-- 6 files changed, 154 insertions(+), 19 deletions(-) diff --git a/src/server/constants.js b/src/server/constants.js index b9e68652b..bdc1a941c 100644 --- a/src/server/constants.js +++ b/src/server/constants.js @@ -1,3 +1,4 @@ +export const FORM_VERSION_METADATA_KEY = '$$__formVersion' export const PREVIEW_PATH_PREFIX = '/preview' export const FORM_PREFIX = '' export const EXTERNAL_STATE_PAYLOAD = 'EXTERNAL_STATE_PAYLOAD' diff --git a/src/server/plugins/engine/beta/form-context.test.ts b/src/server/plugins/engine/beta/form-context.test.ts index a3152b503..1f23c1736 100644 --- a/src/server/plugins/engine/beta/form-context.test.ts +++ b/src/server/plugins/engine/beta/form-context.test.ts @@ -44,7 +44,7 @@ jest.mock('../pageControllers/index.ts', () => { }) jest.mock('../helpers.ts', () => ({ - __esModule: true, + ...jest.requireActual('../helpers.ts'), getCacheService: (...args: unknown[]) => mockGetCacheService(...args), checkEmailAddressForLiveFormSubmission: (...args: unknown[]) => mockCheckEmailAddressForLiveFormSubmission(...args) @@ -134,10 +134,17 @@ describe('getFormModel helper', () => { class CustomController extends PageController {} const controllers = { CustomController } const metadata = { - id: 'form-meta-123', - versions: [{ versionNumber: 17 }] + id: 'form-meta-123' + } + const definition = { + pages: [{ path: '/start' }], + metadata: { + $$__formVersion: { + versionNumber: 17, + createdAt: new Date('2024-10-15T10:00:00Z') + } + } } - const definition = { pages: [{ path: '/start' }] } let formsService: FormsService let services: Services let formModelInstance: { id: string } @@ -176,7 +183,7 @@ describe('getFormModel helper', () => { definition, { basePath: slug, - versionNumber: metadata.versions[0].versionNumber, + versionNumber: 17, ordnanceSurveyApiKey: undefined, formId: metadata.id }, @@ -195,6 +202,29 @@ describe('getFormModel helper', () => { ) }) + test('prefers $$__formVersion from definition metadata over metadata.versions', async () => { + const definitionWithFormVersion = { + ...definition, + metadata: { + $$__formVersion: { versionNumber: 42 } + } + } + formsService.getFormDefinition = jest + .fn() + .mockResolvedValue(definitionWithFormVersion) + + await getFormModel(slug, state, { services, controllers }) + + expect(FormModel).toHaveBeenCalledWith( + definitionWithFormVersion, + expect.objectContaining({ + versionNumber: 42 + }), + services, + controllers + ) + }) + test('throws when no form definition is available', async () => { jest.mocked(formsService.getFormDefinition).mockResolvedValue(undefined) @@ -210,11 +240,18 @@ describe('getFormModel helper', () => { describe('resolveFormModel helper', () => { const slug = 'tb-origin' - const definition = { pages: [] } + const definition = { + pages: [], + metadata: { + $$__formVersion: { + versionNumber: 9, + createdAt: new Date('2024-10-15T10:00:00Z') + } + } + } const metadata = { id: 'metadata-123', live: { updatedAt: new Date('2024-10-15T10:00:00Z') }, - versions: [{ versionNumber: 9 }], notificationEmail: 'enrique.chase@defra.gov.uk' } let server: Request['server'] @@ -274,7 +311,7 @@ describe('resolveFormModel helper', () => { definition, expect.objectContaining({ basePath: 'forms/preview/live/tb-origin', - versionNumber: metadata.versions[0].versionNumber, + versionNumber: 9, ordnanceSurveyApiKey: 'os-api-key', formId: metadata.id }), @@ -283,6 +320,29 @@ describe('resolveFormModel helper', () => { ) }) + test('prefers $$__formVersion from definition metadata over metadata.versions', async () => { + const definitionWithFormVersion = { + ...definition, + metadata: { + $$__formVersion: { versionNumber: 55 } + } + } + mockFormsService.getFormDefinition = jest + .fn() + .mockResolvedValue(definitionWithFormVersion) + + await resolveFormModel(server, slug, FormStatus.Live) + + expect(FormModel).toHaveBeenCalledWith( + definitionWithFormVersion, + expect.objectContaining({ + versionNumber: 55 + }), + mockServices, + undefined + ) + }) + test('throws when requested form state does not exist on metadata', async () => { mockFormsService.getFormMetadata.mockResolvedValue({ id: 'metadata-123', diff --git a/src/server/plugins/engine/beta/form-context.ts b/src/server/plugins/engine/beta/form-context.ts index 304a1de5f..fa5d8836f 100644 --- a/src/server/plugins/engine/beta/form-context.ts +++ b/src/server/plugins/engine/beta/form-context.ts @@ -5,7 +5,8 @@ import { isEqual } from 'date-fns' import { PREVIEW_PATH_PREFIX } from '~/src/server/constants.js' import { checkEmailAddressForLiveFormSubmission, - getCacheService + getCacheService, + getFormVersion } from '~/src/server/plugins/engine/helpers.js' import { FormModel } from '~/src/server/plugins/engine/models/index.js' import { type PageController } from '~/src/server/plugins/engine/pageControllers/PageController.js' @@ -27,7 +28,6 @@ export interface FormModelOptions { services?: Services controllers?: Record basePath?: string - versionNumber?: number ordnanceSurveyApiKey?: string formId?: string routePrefix?: string @@ -53,8 +53,6 @@ export async function getFormModel( const formState = resolveState(state) const metadata = await formsService.getFormMetadata(slug) - const versionNumber = - options.versionNumber ?? metadata.versions?.[0]?.versionNumber const definition = await formsService.getFormDefinition( metadata.id, @@ -67,6 +65,8 @@ export async function getFormModel( ) } + const versionNumber = getFormVersion(definition)?.versionNumber + return new FormModel( definition, { @@ -182,14 +182,15 @@ export async function resolveFormModel( const routePrefix = options.routePrefix ?? server.realm.modifiers.route.prefix + const versionNumber = getFormVersion(definition)?.versionNumber + const model = new FormModel( definition, { basePath: options.basePath ?? buildBasePath(routePrefix, slug, formState, isPreview), - versionNumber: - options.versionNumber ?? metadata.versions?.[0]?.versionNumber, + versionNumber, ordnanceSurveyApiKey: options.ordnanceSurveyApiKey, formId: options.formId ?? metadata.id }, diff --git a/src/server/plugins/engine/helpers.ts b/src/server/plugins/engine/helpers.ts index e67a9d833..2935fcd60 100644 --- a/src/server/plugins/engine/helpers.ts +++ b/src/server/plugins/engine/helpers.ts @@ -16,6 +16,7 @@ import { type Schema, type ValidationErrorItem } from 'joi' import { Liquid } from 'liquidjs' import { createLogger } from '~/src/server/common/helpers/logging/logger.js' +import { FORM_VERSION_METADATA_KEY } from '~/src/server/constants.js' import { getAnswer, type Field @@ -416,6 +417,22 @@ export function handleLegacyRedirect(h: ResponseToolkit, targetUrl: string) { * If the page doesn't have a title, set it from the title of the first form component * @param def - the form definition */ +export interface FormVersionMetadata { + versionNumber: number + createdAt: Date +} + +/** + * Extracts form version metadata from a form definition + */ +export function getFormVersion( + definition: Pick +): FormVersionMetadata | undefined { + return definition.metadata?.[FORM_VERSION_METADATA_KEY] as + | FormVersionMetadata + | undefined +} + export function setPageTitles(def: FormDefinition) { def.pages.forEach((page) => { if (!page.title) { diff --git a/src/server/plugins/engine/outputFormatters/adapter/v1.test.ts b/src/server/plugins/engine/outputFormatters/adapter/v1.test.ts index 03a9cb04e..11f0be182 100644 --- a/src/server/plugins/engine/outputFormatters/adapter/v1.test.ts +++ b/src/server/plugins/engine/outputFormatters/adapter/v1.test.ts @@ -764,6 +764,60 @@ describe('Adapter v1 formatter', () => { }) describe('version metadata handling', () => { + it('should prefer $$__formVersion from definition metadata over formMetadata.versions', () => { + const definitionWithFormVersion = { + ...definition, + metadata: { + $$__formVersion: { + versionNumber: 42, + createdAt: new Date('2024-06-01T00:00:00.000Z') + } + } + } + + const modelWithFormVersion = new FormModel(definitionWithFormVersion, { + basePath: 'test' + }) + + const contextWithFormVersion = modelWithFormVersion.getFormContext( + request, + state + ) + + const formMetadata: Partial = { + id: 'form-123', + slug: 'test-form', + title: 'Test Form', + notificationEmail: 'test@example.com', + versions: [ + { + versionNumber: 1, + createdAt: new Date('2024-01-01T00:00:00.000Z') + } + ] + } + + const formStatus = { + isPreview: false, + state: FormStatus.Live + } + + const body = format( + contextWithFormVersion, + items, + modelWithFormVersion, + submitResponse, + formStatus, + formMetadata as FormMetadata + ) + const parsedBody = JSON.parse(body) as FormAdapterSubmissionMessagePayload + + expect(parsedBody.meta.versionMetadata).toEqual({ + versionNumber: 42, + createdAt: '2024-06-01T00:00:00.000Z' + }) + }) + it('should include versionMetadata when context has submittedVersionNumber and formMetadata has versions', () => { const formMetadata: Partial = { id: 'form-123', diff --git a/src/server/plugins/engine/outputFormatters/adapter/v1.ts b/src/server/plugins/engine/outputFormatters/adapter/v1.ts index 7eb377189..f5728c320 100644 --- a/src/server/plugins/engine/outputFormatters/adapter/v1.ts +++ b/src/server/plugins/engine/outputFormatters/adapter/v1.ts @@ -3,7 +3,10 @@ import { type SubmitResponsePayload } from '@defra/forms-model' -import { type checkFormStatus } from '~/src/server/plugins/engine/helpers.js' +import { + getFormVersion, + type checkFormStatus +} from '~/src/server/plugins/engine/helpers.js' import { type FormModel } from '~/src/server/plugins/engine/models/FormModel.js' import { type DetailItem } from '~/src/server/plugins/engine/models/types.js' import { categoriseData } from '~/src/server/plugins/engine/outputFormatters/machine/v2.js' @@ -28,10 +31,9 @@ export function format( const { main: v2Main, ...v2Data } = categoriseData(items) - const versionMetadata = getVersionMetadata( - context.submittedVersionNumber, - formMetadata - ) + const versionMetadata = + getFormVersion(model.def) ?? + getVersionMetadata(context.submittedVersionNumber, formMetadata) const meta: FormAdapterSubmissionMessageMeta = { schemaVersion: FormAdapterSubmissionSchemaVersion.V1, From 3c8465fc019512953c14ab60c23b7a4e13e53aaf Mon Sep 17 00:00:00 2001 From: Mohammed Khalid Date: Tue, 24 Mar 2026 16:01:52 +0000 Subject: [PATCH 2/2] refactor: remove redundant $$__formVersion preference tests --- .../plugins/engine/beta/form-context.test.ts | 46 ------------------- 1 file changed, 46 deletions(-) diff --git a/src/server/plugins/engine/beta/form-context.test.ts b/src/server/plugins/engine/beta/form-context.test.ts index 1f23c1736..bda790707 100644 --- a/src/server/plugins/engine/beta/form-context.test.ts +++ b/src/server/plugins/engine/beta/form-context.test.ts @@ -202,29 +202,6 @@ describe('getFormModel helper', () => { ) }) - test('prefers $$__formVersion from definition metadata over metadata.versions', async () => { - const definitionWithFormVersion = { - ...definition, - metadata: { - $$__formVersion: { versionNumber: 42 } - } - } - formsService.getFormDefinition = jest - .fn() - .mockResolvedValue(definitionWithFormVersion) - - await getFormModel(slug, state, { services, controllers }) - - expect(FormModel).toHaveBeenCalledWith( - definitionWithFormVersion, - expect.objectContaining({ - versionNumber: 42 - }), - services, - controllers - ) - }) - test('throws when no form definition is available', async () => { jest.mocked(formsService.getFormDefinition).mockResolvedValue(undefined) @@ -320,29 +297,6 @@ describe('resolveFormModel helper', () => { ) }) - test('prefers $$__formVersion from definition metadata over metadata.versions', async () => { - const definitionWithFormVersion = { - ...definition, - metadata: { - $$__formVersion: { versionNumber: 55 } - } - } - mockFormsService.getFormDefinition = jest - .fn() - .mockResolvedValue(definitionWithFormVersion) - - await resolveFormModel(server, slug, FormStatus.Live) - - expect(FormModel).toHaveBeenCalledWith( - definitionWithFormVersion, - expect.objectContaining({ - versionNumber: 55 - }), - mockServices, - undefined - ) - }) - test('throws when requested form state does not exist on metadata', async () => { mockFormsService.getFormMetadata.mockResolvedValue({ id: 'metadata-123',