From 5dcb666b4b0da7ba538e136960b41a21f5c443fb Mon Sep 17 00:00:00 2001 From: whitewater design Date: Mon, 22 Sep 2025 18:32:26 +0100 Subject: [PATCH 1/6] test: DF-380 - Add test coverage for DetailItem names --- .../engine/models/SummaryViewModel.test.ts | 254 ++++++++++-------- .../SummaryViewModel.test.ts.snap | 52 ++++ .../outputFormatters/machine/v2.test.ts | 41 +++ 3 files changed, 235 insertions(+), 112 deletions(-) create mode 100644 src/server/plugins/engine/models/__snapshots__/SummaryViewModel.test.ts.snap diff --git a/src/server/plugins/engine/models/SummaryViewModel.test.ts b/src/server/plugins/engine/models/SummaryViewModel.test.ts index 9c95c7165..fc4a432e0 100644 --- a/src/server/plugins/engine/models/SummaryViewModel.test.ts +++ b/src/server/plugins/engine/models/SummaryViewModel.test.ts @@ -65,7 +65,9 @@ describe('SummaryViewModel', () => { 'Pizzas', 'Pizza' ], - values: ['Collection', 'Not supplied'] + values: ['Collection', 'Not supplied'], + answers: ['Collection', ''], + names: ['orderType', 'pizza'] }, { description: '1 item', @@ -87,7 +89,9 @@ describe('SummaryViewModel', () => { 'Pizzas', 'Pizza' ], - values: ['Delivery', 'You added 1 Pizza'] + values: ['Delivery', 'You added 1 Pizza'], + answers: ['Delivery', 'You added 1 Pizza'], + names: ['orderType', 'pizza'] }, { description: '2 items', @@ -114,142 +118,168 @@ describe('SummaryViewModel', () => { 'Pizzas', 'Pizza' ], - values: ['Delivery', 'You added 2 Pizzas'] + values: ['Delivery', 'You added 2 Pizzas'], + answers: ['Delivery', 'You added 2 Pizzas'], + names: ['orderType', 'pizza'] } - ])('Check answers ($description)', ({ state, keys, values }) => { - beforeEach(() => { - context = model.getFormContext(request, state) - summaryViewModel = new SummaryViewModel(request, page, context) - }) + ])( + 'Check answers ($description)', + ({ state, keys, values, names, answers }) => { + beforeEach(() => { + context = model.getFormContext(request, state) + summaryViewModel = new SummaryViewModel(request, page, context) + }) - it('should add title for each section', () => { - const [checkAnswers1, checkAnswers2] = summaryViewModel.checkAnswers + it('should add title for each section', () => { + const [checkAnswers1, checkAnswers2] = summaryViewModel.checkAnswers - // 1st summary list has no title - expect(checkAnswers1).toHaveProperty('title', undefined) + // 1st summary list has no title + expect(checkAnswers1).toHaveProperty('title', undefined) - // 2nd summary list has section title - expect(checkAnswers2).toHaveProperty('title', { - text: 'Food' + // 2nd summary list has section title + expect(checkAnswers2).toHaveProperty('title', { + text: 'Food' + }) }) - }) - it('should add summary list for each section', () => { - expect(summaryViewModel.checkAnswers).toHaveLength(2) + it('should add summary list for each section', () => { + expect(summaryViewModel.checkAnswers).toHaveLength(2) - const [checkAnswers1, checkAnswers2] = summaryViewModel.checkAnswers + const [checkAnswers1, checkAnswers2] = summaryViewModel.checkAnswers - const { summaryList: summaryList1 } = checkAnswers1 - const { summaryList: summaryList2 } = checkAnswers2 + const { summaryList: summaryList1 } = checkAnswers1 + const { summaryList: summaryList2 } = checkAnswers2 - expect(summaryList1).toHaveProperty('rows', [ - { - key: { - text: keys[2] - }, - value: { - classes: 'app-prose-scope', - html: values[0] - }, - actions: { - items: [ - { - classes: 'govuk-link--no-visited-state', - href: `${basePath}/delivery-or-collection?returnUrl=${encodeURIComponent(`${basePath}/summary`)}`, - text: 'Change', - visuallyHiddenText: keys[0] - } - ] + expect(summaryList1).toHaveProperty('rows', [ + { + key: { + text: keys[2] + }, + value: { + classes: 'app-prose-scope', + html: values[0] + }, + actions: { + items: [ + { + classes: 'govuk-link--no-visited-state', + href: `${basePath}/delivery-or-collection?returnUrl=${encodeURIComponent(`${basePath}/summary`)}`, + text: 'Change', + visuallyHiddenText: keys[0] + } + ] + } } - } - ]) + ]) - expect(summaryList2).toHaveProperty('rows', [ - { - key: { - text: keys[1] - }, - value: { - classes: 'app-prose-scope', - html: values[1] - }, - actions: { - items: [ - { - classes: 'govuk-link--no-visited-state', - href: `${basePath}/pizza-order/summary?returnUrl=${encodeURIComponent(`${basePath}/summary`)}`, - text: 'Change', - visuallyHiddenText: 'Pizza' - } - ] + expect(summaryList2).toHaveProperty('rows', [ + { + key: { + text: keys[1] + }, + value: { + classes: 'app-prose-scope', + html: values[1] + }, + actions: { + items: [ + { + classes: 'govuk-link--no-visited-state', + href: `${basePath}/pizza-order/summary?returnUrl=${encodeURIComponent(`${basePath}/summary`)}`, + text: 'Change', + visuallyHiddenText: 'Pizza' + } + ] + } } - } - ]) - }) + ]) + }) - it('should add summary list for each section (preview URL direct access)', () => { - request.query.force = '' // Preview URL '?force' - context = model.getFormContext(request, state) - summaryViewModel = new SummaryViewModel(request, page, context) + it('should add summary list for each section (preview URL direct access)', () => { + request.query.force = '' // Preview URL '?force' + context = model.getFormContext(request, state) + summaryViewModel = new SummaryViewModel(request, page, context) - expect(summaryViewModel.checkAnswers).toHaveLength(2) + expect(summaryViewModel.checkAnswers).toHaveLength(2) - const [checkAnswers1, checkAnswers2] = summaryViewModel.checkAnswers + const [checkAnswers1, checkAnswers2] = summaryViewModel.checkAnswers - const { summaryList: summaryList1 } = checkAnswers1 - const { summaryList: summaryList2 } = checkAnswers2 + const { summaryList: summaryList1 } = checkAnswers1 + const { summaryList: summaryList2 } = checkAnswers2 - expect(summaryList1).toHaveProperty('rows', [ - { - key: { - text: keys[2] - }, - value: { - classes: 'app-prose-scope', - html: values[0] - }, - actions: { - items: [] + expect(summaryList1).toHaveProperty('rows', [ + { + key: { + text: keys[2] + }, + value: { + classes: 'app-prose-scope', + html: values[0] + }, + actions: { + items: [] + } } - } - ]) + ]) - expect(summaryList2).toHaveProperty('rows', [ - { - key: { - text: keys[1] - }, - value: { - classes: 'app-prose-scope', - html: values[1] - }, - actions: { - items: [] + expect(summaryList2).toHaveProperty('rows', [ + { + key: { + text: keys[1] + }, + value: { + classes: 'app-prose-scope', + html: values[1] + }, + actions: { + items: [] + } } - } - ]) - }) + ]) + }) - it('should use correct summary labels', () => { - request.query.force = '' // Preview URL '?force' - context = model.getFormContext(request, state) - summaryViewModel = new SummaryViewModel(request, page, context) + it('should use correct summary labels', () => { + request.query.force = '' // Preview URL '?force' + context = model.getFormContext(request, state) + summaryViewModel = new SummaryViewModel(request, page, context) - expect(summaryViewModel.details).toHaveLength(2) + expect(summaryViewModel.details).toHaveLength(2) - const [details1, details2] = summaryViewModel.details + const [details1, details2] = summaryViewModel.details - expect(details1.items[0]).toMatchObject({ - title: keys[2], - label: keys[0] - }) + expect(details1.items[0]).toMatchObject({ + name: names[0], + value: answers[0], + title: keys[2], + label: keys[0] + }) + + expect(details2.items[0]).toMatchObject({ + name: names[1], + value: answers[1], + title: keys[1], + label: keys[4] + }) - expect(details2.items[0]).toMatchObject({ - title: keys[1], - label: keys[4] + const snapshot = [ + { + name: names[0], + value: answers[0], + title: keys[2], + label: keys[0] + }, + { + name: names[1], + value: answers[1], + title: keys[1], + label: keys[4] + } + ] + + expect(snapshot).toMatchSnapshot() }) - }) - }) + } + ) }) describe('SummaryPageController', () => { diff --git a/src/server/plugins/engine/models/__snapshots__/SummaryViewModel.test.ts.snap b/src/server/plugins/engine/models/__snapshots__/SummaryViewModel.test.ts.snap new file mode 100644 index 000000000..fae5a769a --- /dev/null +++ b/src/server/plugins/engine/models/__snapshots__/SummaryViewModel.test.ts.snap @@ -0,0 +1,52 @@ +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing + +exports[`SummaryViewModel Check answers (0 items) should use correct summary labels 1`] = ` +[ + { + "label": "How would you like to receive your pizza?", + "name": "orderType", + "title": "How you would like to receive your pizza", + "value": "Collection", + }, + { + "label": "Pizza", + "name": "pizza", + "title": "Pizzas", + "value": "", + }, +] +`; + +exports[`SummaryViewModel Check answers (1 item) should use correct summary labels 1`] = ` +[ + { + "label": "How would you like to receive your pizza?", + "name": "orderType", + "title": "How you would like to receive your pizza", + "value": "Delivery", + }, + { + "label": "Pizza", + "name": "pizza", + "title": "Pizza added", + "value": "You added 1 Pizza", + }, +] +`; + +exports[`SummaryViewModel Check answers (2 items) should use correct summary labels 1`] = ` +[ + { + "label": "How would you like to receive your pizza?", + "name": "orderType", + "title": "How you would like to receive your pizza", + "value": "Delivery", + }, + { + "label": "Pizza", + "name": "pizza", + "title": "Pizzas added", + "value": "You added 2 Pizzas", + }, +] +`; diff --git a/src/server/plugins/engine/outputFormatters/machine/v2.test.ts b/src/server/plugins/engine/outputFormatters/machine/v2.test.ts index a29a99ff3..78703fc3d 100644 --- a/src/server/plugins/engine/outputFormatters/machine/v2.test.ts +++ b/src/server/plugins/engine/outputFormatters/machine/v2.test.ts @@ -9,6 +9,10 @@ import { type DetailItemRepeat } from '~/src/server/plugins/engine/models/types.js' import { format } from '~/src/server/plugins/engine/outputFormatters/machine/v2.js' +import { + SummaryPageController, + getFormSubmissionData +} from '~/src/server/plugins/engine/pageControllers/SummaryPageController.js' import { buildFormContextRequest } from '~/src/server/plugins/engine/pageControllers/__stubs__/request.js' import { FileStatus, @@ -267,4 +271,41 @@ describe('getPersonalisation', () => { expect(parsedBody.meta.referenceNumber).toBe('foobar') expect(parsedBody.data).toEqual(expectedData) }) + + it('should return the machine output 2', () => { + const pageDef = definition.pages[2] + const controller = new SummaryPageController(model, pageDef) + + const summaryViewModel = controller.getSummaryViewModel(request, context) + + const items = getFormSubmissionData( + summaryViewModel.context, + summaryViewModel.details + ) + + const body = format(context, items, model, submitResponse, formStatus) + + const parsedBody = JSON.parse(body) + + const expectedData = { + main: { + orderType: 'delivery' + }, + repeaters: { + pizza: [ + { + quantity: 2, + toppings: 'Ham' + }, + { + quantity: 1, + toppings: 'Pepperoni' + } + ] + }, + files: {} + } + + expect(parsedBody.data).toEqual(expectedData) + }) }) From 1e4304103045445878c5396d0ff0896fd3d9c037 Mon Sep 17 00:00:00 2001 From: whitewater design Date: Tue, 23 Sep 2025 13:46:17 +0100 Subject: [PATCH 2/6] refactor: DF-380 - Export machine v2 categoriseData --- src/server/plugins/engine/outputFormatters/machine/v2.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/plugins/engine/outputFormatters/machine/v2.ts b/src/server/plugins/engine/outputFormatters/machine/v2.ts index a62a51ed3..6b7ccbe22 100644 --- a/src/server/plugins/engine/outputFormatters/machine/v2.ts +++ b/src/server/plugins/engine/outputFormatters/machine/v2.ts @@ -74,7 +74,7 @@ export function format( * } * } */ -function categoriseData(items: DetailItem[]) { +export function categoriseData(items: DetailItem[]) { const output: { main: Record repeaters: Record[]> From 1ec882a82d3ab0cbd3ae6eeca82890b6f221d050 Mon Sep 17 00:00:00 2001 From: whitewater design Date: Tue, 23 Sep 2025 13:53:36 +0100 Subject: [PATCH 3/6] feat: DF-380 - Use null instead of undefined for main->adapter v1 values --- .../outputFormatters/adapter/v1.test.ts | 40 +++++++++++++++++++ .../engine/outputFormatters/adapter/v1.ts | 31 +++++++------- src/server/plugins/engine/types.ts | 2 +- 3 files changed, 58 insertions(+), 15 deletions(-) diff --git a/src/server/plugins/engine/outputFormatters/adapter/v1.test.ts b/src/server/plugins/engine/outputFormatters/adapter/v1.test.ts index 7c3f115cd..4ce889e45 100644 --- a/src/server/plugins/engine/outputFormatters/adapter/v1.test.ts +++ b/src/server/plugins/engine/outputFormatters/adapter/v1.test.ts @@ -723,6 +723,46 @@ describe('Adapter v1 formatter', () => { expect(parsedBody.meta.versionMetadata).toBeUndefined() }) + it('should handle optional fields that are undefined', () => { + const formMetadata: Partial = { + id: 'form-123', + slug: 'test-form', + title: 'Test Form', + notificationEmail: 'test@example.com' + } as FormMetadata + + const formStatus = { + isPreview: false, + state: FormStatus.Live + } + + const dummyField: Field = { + getFormValueFromState: (_) => '' + } as Field + + const items: DetailItem[] = [ + { + name: 'exampleField3', + label: 'Example Field 3', + href: '/example-field-3', + title: 'Example Field 3 Title', + field: dummyField, + value: '' + } as DetailItemField + ] + + const body = format( + context, + items, + model, + submitResponse, + formStatus, + formMetadata as FormMetadata + ) + const parsedBody = JSON.parse(body) as FormAdapterSubmissionMessagePayload + expect(parsedBody.data.main).toEqual({ exampleField3: null }) + }) + describe('version metadata handling', () => { it('should include versionMetadata when context has submittedVersionNumber and formMetadata has versions', () => { const formMetadata: Partial = { diff --git a/src/server/plugins/engine/outputFormatters/adapter/v1.ts b/src/server/plugins/engine/outputFormatters/adapter/v1.ts index 4f528f88f..faa084b63 100644 --- a/src/server/plugins/engine/outputFormatters/adapter/v1.ts +++ b/src/server/plugins/engine/outputFormatters/adapter/v1.ts @@ -6,7 +6,7 @@ import { import { 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 { format as machineV2 } from '~/src/server/plugins/engine/outputFormatters/machine/v2.js' +import { categoriseData } from '~/src/server/plugins/engine/outputFormatters/machine/v2.js' import { FormAdapterSubmissionSchemaVersion } from '~/src/server/plugins/engine/types/enums.js' import { type FormAdapterSubmissionMessageData, @@ -25,20 +25,9 @@ export function format( formStatus: ReturnType, formMetadata?: FormMetadata ): string { - const v2DataString = machineV2( - context, - items, - model, - submitResponse, - formStatus - ) - const v2DataParsed = JSON.parse(v2DataString) as { - data: FormAdapterSubmissionMessageData - } - const csvFiles = extractCsvFiles(submitResponse) - const transformedData = v2DataParsed.data + const { main: v2Main, ...v2Data } = categoriseData(items) const versionMetadata = getVersionMetadata( context.submittedVersionNumber, @@ -60,7 +49,21 @@ export function format( if (versionMetadata) { meta.versionMetadata = versionMetadata } - const data: FormAdapterSubmissionMessageData = transformedData + + const main = Object.fromEntries( + Object.entries(v2Main).map(([key, value]) => { + if (!value) { + return [key, null] + } + + return [key, value] + }) + ) + + const data: FormAdapterSubmissionMessageData = { + main, + ...v2Data + } const result: FormAdapterSubmissionMessageResult = { files: csvFiles diff --git a/src/server/plugins/engine/types.ts b/src/server/plugins/engine/types.ts index 2b0bc0214..993fa148e 100644 --- a/src/server/plugins/engine/types.ts +++ b/src/server/plugins/engine/types.ts @@ -446,7 +446,7 @@ export type RichFormValue = | UkAddressState export interface FormAdapterSubmissionMessageData { - main: Record + main: Record repeaters: Record[]> files: Record } From 7e3d8e88d2f1c947b962e9db07551a46379626af Mon Sep 17 00:00:00 2001 From: whitewater design Date: Tue, 23 Sep 2025 14:11:16 +0100 Subject: [PATCH 4/6] build: DF-380 - Add modulePathIgnorePatterns to jest --- jest.config.cjs | 1 + 1 file changed, 1 insertion(+) diff --git a/jest.config.cjs b/jest.config.cjs index d48093fc7..d4bbf3a61 100644 --- a/jest.config.cjs +++ b/jest.config.cjs @@ -22,6 +22,7 @@ module.exports = { '/src/**/*.{cjs,js,mjs,ts}', '/scripts/**/*.{cjs,js,mjs}' ], + modulePathIgnorePatterns: ['/coverage/', '/.server/'], coveragePathIgnorePatterns: [ '/node_modules/', '/.server', From 38d8e958c81f5baa43f40a97c1414705359ec2d9 Mon Sep 17 00:00:00 2001 From: whitewater design Date: Tue, 23 Sep 2025 17:15:00 +0100 Subject: [PATCH 5/6] feat: DF-380 - Explicit check on undefined form values --- src/server/plugins/engine/outputFormatters/adapter/v1.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/plugins/engine/outputFormatters/adapter/v1.ts b/src/server/plugins/engine/outputFormatters/adapter/v1.ts index faa084b63..476ffe6fb 100644 --- a/src/server/plugins/engine/outputFormatters/adapter/v1.ts +++ b/src/server/plugins/engine/outputFormatters/adapter/v1.ts @@ -52,7 +52,7 @@ export function format( const main = Object.fromEntries( Object.entries(v2Main).map(([key, value]) => { - if (!value) { + if (value === undefined) { return [key, null] } From 39247ce371e7942a051f197da9332249116a84be Mon Sep 17 00:00:00 2001 From: whitewater design Date: Tue, 23 Sep 2025 17:17:05 +0100 Subject: [PATCH 6/6] feat: DF-380 - Explicit check on undefined form values --- src/server/plugins/engine/outputFormatters/adapter/v1.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/plugins/engine/outputFormatters/adapter/v1.test.ts b/src/server/plugins/engine/outputFormatters/adapter/v1.test.ts index 4ce889e45..5bd4ffe48 100644 --- a/src/server/plugins/engine/outputFormatters/adapter/v1.test.ts +++ b/src/server/plugins/engine/outputFormatters/adapter/v1.test.ts @@ -737,7 +737,7 @@ describe('Adapter v1 formatter', () => { } const dummyField: Field = { - getFormValueFromState: (_) => '' + getFormValueFromState: (_) => undefined } as Field const items: DetailItem[] = [