Skip to content

refactor(DF-1004): stamp $$__formVersion at write time in modifyDraft #794

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mokhld merged 3 commits into from
Apr 28, 2026
Merged

refactor(DF-1004): stamp $$__formVersion at write time in modifyDraft #794

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
527404a
refactor(DF-1004): stamp $$__formVersion at write time in modifyDraft
mokhld Apr 24, 2026
0deb6ce
fix(DF-1004): use session + projection on modifyDraft read, add stamp…
mokhld Apr 24, 2026
f412b8a
test(DF-1004): assert version allocation + snapshot happen exactly on…
mokhld Apr 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion src/api/forms/repositories/form-definition-repository.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Boom from '@hapi/boom'
import { ObjectId } from 'mongodb'

import {
FORM_VERSION_METADATA_KEY,
buildSectionsResponse,
getComponent,
getCondition,
Expand Down Expand Up @@ -76,6 +77,34 @@ export async function update(id, formDefinition, session, schema) {
return updateResult.draft
}

/**
* Stamps a form version onto the stored definition for the given state.
* No-op if the state sub-document does not exist.
* @param {string} formId - the form id
* @param {FormStatus} state - the form state (Draft or Live)
* @param {FormVersionMetadata} versionMetadata
* @param {ClientSession} session - mongo transaction session
*/
export async function setFormVersion(formId, state, versionMetadata, session) {
const coll =
/** @satisfies {Collection<Partial<{draft: FormDefinition, live: FormDefinition}>>} */ (
db.collection(DEFINITION_COLLECTION_NAME)
)

await coll.updateOne(
{
_id: new ObjectId(formId),
[state]: { $exists: true }
},
{
$set: {
[`${state}.metadata.${FORM_VERSION_METADATA_KEY}`]: versionMetadata
}
},
{ session }
)
}

/**
* Copy the draft form to live in the Form Store
* @param {string} id - id
Expand Down Expand Up @@ -727,7 +756,7 @@ export async function updateOption(formId, optionName, optionValue, session) {
}

/**
* @import { FormDefinition, Page, ComponentDef, PatchPageFields, List, Engine, ConditionWrapperV2, SectionAssignmentItem } from '@defra/forms-model'
* @import { FormDefinition, FormVersionMetadata, Page, ComponentDef, PatchPageFields, List, Engine, ConditionWrapperV2, SectionAssignmentItem } from '@defra/forms-model'
* @import { ClientSession, Collection, FindOptions } from 'mongodb'
* @import { ObjectSchema } from 'joi'
* @import { UpdateCallback, RemovePagePredicate } from '~/src/api/forms/repositories/helpers.js'
Expand Down
122 changes: 122 additions & 0 deletions src/api/forms/repositories/form-definition-repository.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ import {
updatePage,
updatePageFields
} from '~/src/api/forms/repositories/form-definition-repository.js'
import * as formMetadataRepository from '~/src/api/forms/repositories/form-metadata-repository.js'
import * as formVersionsRepository from '~/src/api/forms/repositories/form-versions-repository.js'
import { FORM_VERSION_METADATA_KEY } from '~/src/api/forms/repositories/helpers.js'
import { empty, emptyV2 } from '~/src/api/forms/templates.js'
import { getAuthor } from '~/src/helpers/get-author.js'
import { db } from '~/src/mongo.js'
Expand Down Expand Up @@ -83,6 +86,12 @@ const condition2Id = '91c10139-a0dd-46a4-a2c5-4d7a02fdf923'
const section1Id = 'f07566be-dd04-49df-890f-b226b92f3907'
const section2Id = 'cb185708-203d-4560-9929-ecc27750244a'

// modifyDraft/insertDraft now call into form-metadata-repository and
// form-versions-repository for version allocation + snapshotting. Mock them so
// their Mongo ops don't collide with the definition-repository assertions.
jest.mock('~/src/api/forms/repositories/form-metadata-repository.js')
jest.mock('~/src/api/forms/repositories/form-versions-repository.js')

jest.mock('~/src/mongo.js', () => {
let isPrepared = false
const collection =
Expand Down Expand Up @@ -943,6 +952,58 @@ describe('form-definition-repository', () => {
throw new Error('Unexpected empty draft on $setOnInsert')
}
})

it('should allocate a version exactly once, stamp the inserted draft, and snapshot to form-versions exactly once', async () => {
const definitionV1 = { ...draft, conditions: [] }
const createdAt = new Date('2026-04-24T10:00:00Z')
mockCollection.findOneAndUpdate.mockResolvedValue({ definitionV1 })
jest
.mocked(formMetadataRepository.getAndIncrementVersionNumber)
.mockResolvedValue(7)
jest.spyOn(global, 'Date').mockImplementation(() => createdAt)

await insert(formId, definitionV1, mockSession, formDefinitionSchema)

expect(
formMetadataRepository.getAndIncrementVersionNumber
).toHaveBeenCalledTimes(1)
expect(
formMetadataRepository.getAndIncrementVersionNumber
).toHaveBeenCalledWith(formId, mockSession)
expect(formMetadataRepository.addVersionMetadata).toHaveBeenCalledTimes(1)
expect(formMetadataRepository.addVersionMetadata).toHaveBeenCalledWith(
formId,
{ versionNumber: 7, createdAt },
mockSession
)

const [, update] = mockCollection.findOneAndUpdate.mock.calls[0]
/** @type {UpdateFilter<{ draft: FormDefinition }>} */
const updateFilter = update
/** @type {any} */
const stampedDraft = updateFilter.$setOnInsert?.draft
expect(stampedDraft?.metadata?.[FORM_VERSION_METADATA_KEY]).toEqual({
versionNumber: 7,
createdAt
})

expect(formVersionsRepository.createVersion).toHaveBeenCalledTimes(1)
expect(formVersionsRepository.createVersion).toHaveBeenCalledWith(
expect.objectContaining({
formId,
versionNumber: 7,
createdAt,
formDefinition: expect.objectContaining({
metadata: expect.objectContaining({
[FORM_VERSION_METADATA_KEY]: { versionNumber: 7, createdAt }
})
})
}),
mockSession
)

jest.restoreAllMocks()
})
})

describe('update', () => {
Expand Down Expand Up @@ -980,6 +1041,67 @@ describe('form-definition-repository', () => {
}
)
})

it('should allocate a version exactly once, stamp the updated draft, and snapshot to form-versions exactly once', async () => {
const newDefinition = emptyV2()
const createdAt = new Date('2026-04-24T10:00:00Z')
jest
.mocked(formMetadataRepository.getAndIncrementVersionNumber)
.mockResolvedValue(11)
jest.spyOn(global, 'Date').mockImplementation(() => createdAt)

await helper(
async () => {
await update(
formId,
newDefinition,
mockSession,
formDefinitionV2Schema
)
},
(persistedDraft) => {
expect(
formMetadataRepository.getAndIncrementVersionNumber
).toHaveBeenCalledTimes(1)
expect(
formMetadataRepository.getAndIncrementVersionNumber
).toHaveBeenCalledWith(formId, mockSession)
expect(
formMetadataRepository.addVersionMetadata
).toHaveBeenCalledTimes(1)
expect(
formMetadataRepository.addVersionMetadata
).toHaveBeenCalledWith(
formId,
{ versionNumber: 11, createdAt },
mockSession
)
expect(persistedDraft.metadata?.[FORM_VERSION_METADATA_KEY]).toEqual({
versionNumber: 11,
createdAt
})
expect(formVersionsRepository.createVersion).toHaveBeenCalledTimes(1)
expect(formVersionsRepository.createVersion).toHaveBeenCalledWith(
expect.objectContaining({
formId,
versionNumber: 11,
createdAt,
formDefinition: expect.objectContaining({
metadata: expect.objectContaining({
[FORM_VERSION_METADATA_KEY]: {
versionNumber: 11,
createdAt
}
})
})
}),
mockSession
)
}
)

jest.restoreAllMocks()
})
})

describe('createLiveFromDraft', () => {
Expand Down
95 changes: 83 additions & 12 deletions src/api/forms/repositories/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,15 @@ import {
import Boom from '@hapi/boom'
import { ObjectId } from 'mongodb'

import * as formMetadataRepository from '~/src/api/forms/repositories/form-metadata-repository.js'
import * as formVersionsRepository from '~/src/api/forms/repositories/form-versions-repository.js'
import { validate } from '~/src/api/forms/service/helpers/definition.js'
import { repositionPaymentAndSummary } from '~/src/api/forms/service/migration-helpers.js'
import { createLogger } from '~/src/helpers/logging/logger.js'
import { DEFINITION_COLLECTION_NAME, db } from '~/src/mongo.js'

export const FORM_VERSION_METADATA_KEY = '$$__formVersion'

const logger = createLogger()

/**
Expand Down Expand Up @@ -275,7 +279,8 @@ export function uniquePathGate(
}

/**
* Inserts a draft form definition
* Inserts a draft form definition, stamping the initial version and
* snapshotting to form-versions in the same transaction.
* @param {string} formId - the form id
* @param {FormDefinition} definition - the form definitiom
* @param {ClientSession} session - the mongo transaction session
Expand All @@ -287,12 +292,13 @@ export async function insertDraft(
session,
schema = formDefinitionV2Schema
) {
// Validate form definition
const draft = validate(definition, schema)
const validated = validate(definition, schema)

const versionMetadata = await allocateDraftVersion(formId, session)
const draft = stampFormVersion(validated, versionMetadata)

const id = { _id: new ObjectId(formId) }

// Persist the new draft
const coll = /** @satisfies {Collection<{draft: FormDefinition}>} */ (
db.collection(DEFINITION_COLLECTION_NAME)
)
Expand All @@ -311,11 +317,65 @@ export async function insertDraft(
throw Boom.notFound(`Unexpected empty result from 'findOneAndUpdate'`)
}

await formVersionsRepository.createVersion(
{
formId,
versionNumber: versionMetadata.versionNumber,
formDefinition: draft,
createdAt: versionMetadata.createdAt
},
session
)
Comment on lines +320 to +328

return insertResult
}

/**
* Updates a draft form definition
* Allocates the next version number for a form.
* @param {string} formId - the form id
* @param {ClientSession} session - the mongo transaction session
* @returns {Promise<FormVersionMetadata>}
*/
export async function allocateDraftVersion(formId, session) {
const versionNumber =
await formMetadataRepository.getAndIncrementVersionNumber(formId, session)
const createdAt = new Date()
const versionMetadata = /** @type {FormVersionMetadata} */ ({
versionNumber,
createdAt
})

await formMetadataRepository.addVersionMetadata(
formId,
versionMetadata,
session
)

return versionMetadata
}

/**
* Stamps a form version onto a definition's metadata.
* @param {FormDefinition} definition
* @param {{ versionNumber: number, createdAt: Date }} versionMetadata
* @returns {FormDefinition}
*/
export function stampFormVersion(definition, versionMetadata) {
return {
...definition,
metadata: {
...definition.metadata,
[FORM_VERSION_METADATA_KEY]: {
versionNumber: versionMetadata.versionNumber,
createdAt: versionMetadata.createdAt
}
}
}
}

/**
* Updates a draft form definition, stamping a new version onto it and
* snapshotting to form-versions in the same transaction.
* @param {string} formId - the form id
* @param {UpdateCallback} updateCallback - the update callback
* @param {ClientSession} session - the mongo transaction session
Expand All @@ -332,7 +392,10 @@ export async function modifyDraft(
)

const id = { _id: new ObjectId(formId) }
const document = await coll.findOne(id)
const document = await coll.findOne(id, {
session,
projection: { draft: 1 }
})

if (!document) {
throw Boom.notFound(`Document not found '${formId}'`)
Expand All @@ -342,15 +405,13 @@ export async function modifyDraft(
throw Boom.notFound(`Draft not found in document '${formId}'`)
}

// Apply the update
const updated = updateCallback(document.draft)

const repositioned = repositionPaymentAndSummary(updated)
const validated = validate(repositioned, schema)

// Validate form definition
const draft = validate(repositioned, schema)
const versionMetadata = await allocateDraftVersion(formId, session)
const draft = stampFormVersion(validated, versionMetadata)
Comment on lines 409 to +413
Copy link
Copy Markdown
Contributor

@alexluckett alexluckett Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually @mokhld this is a good spot


// Persist the updated draft
const coll2 = /** @satisfies {Collection<{draft: FormDefinition}>} */ (
db.collection(DEFINITION_COLLECTION_NAME)
)
Expand All @@ -368,6 +429,16 @@ export async function modifyDraft(
throw Boom.notFound(`Unexpected empty result from 'findOneAndUpdate'`)
}

await formVersionsRepository.createVersion(
{
formId,
versionNumber: versionMetadata.versionNumber,
formDefinition: draft,
createdAt: versionMetadata.createdAt
},
session
)
Comment on lines +432 to +440

return updateResult
}

Expand Down Expand Up @@ -898,7 +969,7 @@ export function modifyUpdateOption(definition, optionName, optionValue) {
*/

/**
* @import { FormDefinition, FormOptions, Page, ComponentDef, List, PatchPageFields, Engine, ConditionWrapperV2, PageSummary, PageSummaryWithConfirmationEmail, SectionAssignmentItem } from '@defra/forms-model'
* @import { FormDefinition, FormOptions, FormVersionMetadata, Page, ComponentDef, List, PatchPageFields, Engine, ConditionWrapperV2, PageSummary, PageSummaryWithConfirmationEmail, SectionAssignmentItem } from '@defra/forms-model'
* @import { ClientSession, Collection } from 'mongodb'
* @import { ObjectSchema } from 'joi'
*/
Loading
Loading