From cea26452c33191c67466bce20b6456034718aea6 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Tue, 11 Nov 2025 10:25:10 +0000 Subject: [PATCH 1/7] Handles Declaration field output --- .../components/helpers/components.test.ts | 85 +++++++++++++++++++ .../engine/components/helpers/components.ts | 3 + .../engine/models/SummaryViewModel.test.ts | 4 +- .../plugins/engine/models/SummaryViewModel.ts | 7 +- .../pageControllers/RepeatPageController.ts | 2 +- 5 files changed, 96 insertions(+), 5 deletions(-) diff --git a/src/server/plugins/engine/components/helpers/components.test.ts b/src/server/plugins/engine/components/helpers/components.test.ts index 03e7a07fd..8e748997d 100644 --- a/src/server/plugins/engine/components/helpers/components.test.ts +++ b/src/server/plugins/engine/components/helpers/components.test.ts @@ -1,5 +1,6 @@ import { ComponentType, + type DeclarationFieldComponent, type EastingNorthingFieldComponent, type LatLongFieldComponent, type NationalGridFieldNumberFieldComponent, @@ -11,6 +12,7 @@ import { getAnswerMarkdown } from '~/src/server/plugins/engine/components/helpers/components.js' import { + DeclarationField, EastingNorthingField, LatLongField, NationalGridFieldNumberField, @@ -216,6 +218,89 @@ describe('Location field formatting', () => { }) }) + describe('DeclarationField', () => { + let field: DeclarationField + + beforeEach(() => { + const def: DeclarationFieldComponent = { + type: ComponentType.DeclarationField, + name: 'declField', + title: 'Declaration title', + content: '# markup heading', + options: {} + } + field = new DeclarationField(def, { model }) + }) + + it('formats correctly when agreed', () => { + const state = { + declField: true + } + + const answer = getAnswer(field, state, { format: 'email' }) + expect(answer).toBe('I understand and agree\n') + }) + + it('formats correctly when not agreed', () => { + const state = { + declField: false + } + + const answer = getAnswer(field, state, { format: 'email' }) + expect(answer).toBe('\n') + }) + + it('formats for data output when agreed', () => { + const state = { + declField: true + } + + const answer = getAnswer(field, state, { format: 'data' }) + expect(answer).toBe('true') + }) + + it('formats for data output when not agreed', () => { + const state = { + declField: false + } + + const answer = getAnswer(field, state, { format: 'data' }) + expect(answer).toBe('') + }) + + it('formats for data output when no value', () => { + const state = {} + + const answer = getAnswer(field, state, { format: 'data' }) + expect(answer).toBe('') + }) + + it('formats for summary display when agreed', () => { + const state = { + declField: true + } + + const answer = getAnswer(field, state, { format: 'summary' }) + expect(answer).toBe('I understand and agree') + }) + + it('formats for summary display when not agreed', () => { + const state = { + declField: false + } + + const answer = getAnswer(field, state, { format: 'summary' }) + expect(answer).toBe('') + }) + + it('formats for summary display when no value', () => { + const state = {} + + const answer = getAnswer(field, state, { format: 'summary' }) + expect(answer).toBe('') + }) + }) + describe('getAnswerMarkdown', () => { it('formats EastingNorthingField correctly', () => { const def: EastingNorthingFieldComponent = { diff --git a/src/server/plugins/engine/components/helpers/components.ts b/src/server/plugins/engine/components/helpers/components.ts index 880df9812..1597eb263 100644 --- a/src/server/plugins/engine/components/helpers/components.ts +++ b/src/server/plugins/engine/components/helpers/components.ts @@ -216,6 +216,9 @@ export function getAnswer( // Use context value for submission data if (options.format === 'data') { const context = field.getContextValueFromState(state) + if (field instanceof Components.DeclarationField) { + return context ? context.toString() : '' + } return context?.toString() ?? '' } diff --git a/src/server/plugins/engine/models/SummaryViewModel.test.ts b/src/server/plugins/engine/models/SummaryViewModel.test.ts index 65ebd13ab..30769b72c 100644 --- a/src/server/plugins/engine/models/SummaryViewModel.test.ts +++ b/src/server/plugins/engine/models/SummaryViewModel.test.ts @@ -66,7 +66,7 @@ describe('SummaryViewModel', () => { 'Pizzas', 'Pizza' ], - values: ['Collection', 'Not supplied'], + values: ['Collection', 'Not provided'], answers: ['Collection', ''], names: ['orderType', 'pizza'] }, @@ -342,7 +342,7 @@ describe('SummaryPageController', () => { ) }) - it('should display default page title for v2 form when title not supplied', () => { + it('should display default page title for v2 form when title not provided', () => { const state: FormState = { $$__referenceNumber: 'foobar', orderType: 'collection', diff --git a/src/server/plugins/engine/models/SummaryViewModel.ts b/src/server/plugins/engine/models/SummaryViewModel.ts index 41c6c682e..898e1f07d 100644 --- a/src/server/plugins/engine/models/SummaryViewModel.ts +++ b/src/server/plugins/engine/models/SummaryViewModel.ts @@ -104,7 +104,7 @@ export class SummaryViewModel { }, value: { classes: 'app-prose-scope', - html: item.value || 'Not supplied' + html: item.value || 'Not provided' }, actions: { items @@ -216,7 +216,10 @@ function ItemField( return { name: field.name, label: field.title, - title: field.label, + title: + field.options.required === false + ? `${field.label} (optional)` + : field.label, error: field.getFirstError(options.errors), value: getAnswer(field, state), href: getPageHref(page, options.path, { diff --git a/src/server/plugins/engine/pageControllers/RepeatPageController.ts b/src/server/plugins/engine/pageControllers/RepeatPageController.ts index 4368e24a8..3c6296c4b 100644 --- a/src/server/plugins/engine/pageControllers/RepeatPageController.ts +++ b/src/server/plugins/engine/pageControllers/RepeatPageController.ts @@ -422,7 +422,7 @@ export class RepeatPageController extends QuestionPageController { text: `${title} ${index + 1}` }, value: { - text: itemDisplayText || 'Not supplied' + text: itemDisplayText || 'Not provided' }, actions: { items From 71cef2698842d1ec61c835fac25881188dcc11ae Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Tue, 11 Nov 2025 11:47:09 +0000 Subject: [PATCH 2/7] Extra coverage --- .../engine/models/SummaryViewModel.test.ts | 49 +++++++++++++++++++ .../plugins/engine/models/SummaryViewModel.ts | 2 +- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/server/plugins/engine/models/SummaryViewModel.test.ts b/src/server/plugins/engine/models/SummaryViewModel.test.ts index 30769b72c..b9c5d254b 100644 --- a/src/server/plugins/engine/models/SummaryViewModel.test.ts +++ b/src/server/plugins/engine/models/SummaryViewModel.test.ts @@ -1,3 +1,9 @@ +import { + type FormDefinition, + type PageQuestion, + type RadiosFieldComponent +} from '@defra/forms-model' + import { FORM_PREFIX } from '~/src/server/constants.js' import { FormModel, @@ -281,6 +287,49 @@ describe('SummaryViewModel', () => { }) } ) + + it('should use correct summary labels', () => { + request.query.force = '' // Preview URL '?force' + const state = { + $$__referenceNumber: 'foobar', + orderType: 'collection', + pizza: [] + } satisfies FormState + + // Setup an optional question + const definitionOptional = structuredClone(definition) as FormDefinition + const firstPage = definitionOptional.pages[0] as PageQuestion + const firstComponent = firstPage.components[0] as RadiosFieldComponent + firstComponent.options.required = false + + const model = new FormModel(definitionOptional, { + basePath: `${FORM_PREFIX}/test` + }) + + context = model.getFormContext(request, state) + + const page = createPage(model, definition.pages[2]) + + summaryViewModel = new SummaryViewModel(request, page, context) + + expect(summaryViewModel.details).toHaveLength(2) + + const [details1, details2] = summaryViewModel.details + + expect(details1.items[0]).toMatchObject({ + name: 'orderType', + value: 'Collection', + title: 'How you would like to receive your pizza (optional)', + label: 'How would you like to receive your pizza?' + }) + + expect(details2.items[0]).toMatchObject({ + name: 'pizza', + value: '', + title: 'Pizzas', + label: 'Pizza' + }) + }) }) describe('SummaryPageController', () => { diff --git a/src/server/plugins/engine/models/SummaryViewModel.ts b/src/server/plugins/engine/models/SummaryViewModel.ts index 898e1f07d..0e90f9271 100644 --- a/src/server/plugins/engine/models/SummaryViewModel.ts +++ b/src/server/plugins/engine/models/SummaryViewModel.ts @@ -204,7 +204,7 @@ function ItemRepeat( * Creates a form field detail item * @see {@link DetailItemField} */ -function ItemField( +export function ItemField( page: PageControllerClass, state: FormState, field: Field, From 807b33556ec1f861562f351ad3cf0ab3433bd108 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Tue, 11 Nov 2025 15:29:35 +0000 Subject: [PATCH 3/7] Reverted to use TRUE/FALSE in CSV --- .../plugins/engine/components/DeclarationField.test.ts | 2 +- .../plugins/engine/components/DeclarationField.ts | 2 +- .../engine/components/helpers/components.test.ts | 10 +++++----- .../plugins/engine/components/helpers/components.ts | 3 --- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/server/plugins/engine/components/DeclarationField.test.ts b/src/server/plugins/engine/components/DeclarationField.test.ts index bfb8a41a4..fdcd5ed71 100644 --- a/src/server/plugins/engine/components/DeclarationField.test.ts +++ b/src/server/plugins/engine/components/DeclarationField.test.ts @@ -186,7 +186,7 @@ describe('DeclarationField', () => { const answer2 = getAnswer(field, state2) expect(answer1).toBe('I understand and agree') - expect(answer2).toBe('') + expect(answer2).toBe('Not provided') }) it('returns payload from state', () => { diff --git a/src/server/plugins/engine/components/DeclarationField.ts b/src/server/plugins/engine/components/DeclarationField.ts index f328abc19..b37a15946 100644 --- a/src/server/plugins/engine/components/DeclarationField.ts +++ b/src/server/plugins/engine/components/DeclarationField.ts @@ -98,7 +98,7 @@ export class DeclarationField extends FormComponent { } getDisplayStringFromFormValue(value: FormValue | FormPayload): string { - return value ? this.declarationConfirmationLabel : '' + return value ? this.declarationConfirmationLabel : 'Not provided' } getViewModel(payload: FormPayload, errors?: FormSubmissionError[]) { diff --git a/src/server/plugins/engine/components/helpers/components.test.ts b/src/server/plugins/engine/components/helpers/components.test.ts index 8e748997d..096f14595 100644 --- a/src/server/plugins/engine/components/helpers/components.test.ts +++ b/src/server/plugins/engine/components/helpers/components.test.ts @@ -247,7 +247,7 @@ describe('Location field formatting', () => { } const answer = getAnswer(field, state, { format: 'email' }) - expect(answer).toBe('\n') + expect(answer).toBe('Not provided\n') }) it('formats for data output when agreed', () => { @@ -265,14 +265,14 @@ describe('Location field formatting', () => { } const answer = getAnswer(field, state, { format: 'data' }) - expect(answer).toBe('') + expect(answer).toBe('false') }) it('formats for data output when no value', () => { const state = {} const answer = getAnswer(field, state, { format: 'data' }) - expect(answer).toBe('') + expect(answer).toBe('false') }) it('formats for summary display when agreed', () => { @@ -290,14 +290,14 @@ describe('Location field formatting', () => { } const answer = getAnswer(field, state, { format: 'summary' }) - expect(answer).toBe('') + expect(answer).toBe('Not provided') }) it('formats for summary display when no value', () => { const state = {} const answer = getAnswer(field, state, { format: 'summary' }) - expect(answer).toBe('') + expect(answer).toBe('Not provided') }) }) diff --git a/src/server/plugins/engine/components/helpers/components.ts b/src/server/plugins/engine/components/helpers/components.ts index 1597eb263..880df9812 100644 --- a/src/server/plugins/engine/components/helpers/components.ts +++ b/src/server/plugins/engine/components/helpers/components.ts @@ -216,9 +216,6 @@ export function getAnswer( // Use context value for submission data if (options.format === 'data') { const context = field.getContextValueFromState(state) - if (field instanceof Components.DeclarationField) { - return context ? context.toString() : '' - } return context?.toString() ?? '' } From 7c2f2ae52befa6b8ebd9ec48ef2da7009092dffb Mon Sep 17 00:00:00 2001 From: Alex Luckett Date: Thu, 13 Nov 2025 13:39:07 +0000 Subject: [PATCH 4/7] Clean up declaration form value and state state is either true or false, never undefined form value is either 'true' or 'unchecked', never '' or undefined --- src/server/plugins/engine/components/DeclarationField.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/server/plugins/engine/components/DeclarationField.ts b/src/server/plugins/engine/components/DeclarationField.ts index b37a15946..0f425056c 100644 --- a/src/server/plugins/engine/components/DeclarationField.ts +++ b/src/server/plugins/engine/components/DeclarationField.ts @@ -68,12 +68,12 @@ export class DeclarationField extends FormComponent { getFormValueFromState(state: FormSubmissionState) { const { name } = this - return state[name] === true ? 'true' : undefined + return state[name] === true ? 'true' : 'unchecked' } getFormDataFromState(state: FormSubmissionState): FormPayload { const { name } = this - return { [name]: state[name] === true ? 'true' : undefined } + return { [name]: state[name] === true ? 'true' : 'unchecked' } } getStateFromValidForm(payload: FormPayload): FormState { @@ -98,7 +98,7 @@ export class DeclarationField extends FormComponent { } getDisplayStringFromFormValue(value: FormValue | FormPayload): string { - return value ? this.declarationConfirmationLabel : 'Not provided' + return value === 'true' ? this.declarationConfirmationLabel : 'Not provided' } getViewModel(payload: FormPayload, errors?: FormSubmissionError[]) { From ceacc7133659ecf5f8db590614fd85e6de21bead Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Fri, 14 Nov 2025 09:21:56 +0000 Subject: [PATCH 5/7] Fixed checkbox checked retention --- .../components/DeclarationField.test.ts | 43 ++++++++++++++++--- .../engine/components/DeclarationField.ts | 5 ++- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/server/plugins/engine/components/DeclarationField.test.ts b/src/server/plugins/engine/components/DeclarationField.test.ts index fdcd5ed71..c650c09d8 100644 --- a/src/server/plugins/engine/components/DeclarationField.test.ts +++ b/src/server/plugins/engine/components/DeclarationField.test.ts @@ -246,7 +246,6 @@ describe('DeclarationField', () => { label: { text: def.title }, name: 'myComponent', attributes: {}, - values: [], content: 'Lorem ipsum dolar sit amet', id: 'myComponent', fieldset: { @@ -257,7 +256,8 @@ describe('DeclarationField', () => { items: [ { value: 'true', - text: 'I understand and agree' + text: 'I understand and agree', + checked: true } ] }) @@ -276,10 +276,16 @@ describe('DeclarationField', () => { expect(viewModel).toEqual( expect.objectContaining({ - values: ['true'], hint: { text: 'Please read and confirm the following' - } + }, + items: [ + { + value: 'true', + text: 'I understand and agree', + checked: true + } + ] }) ) }) @@ -301,14 +307,39 @@ describe('DeclarationField', () => { collection = new ComponentCollection([def], { model }) field = collection.fields[0] - const viewModel = field.getViewModel(getFormData(['unchecked', 'true'])) + const viewModel = field.getViewModel(getFormData(['true'])) + + expect(viewModel).toEqual( + expect.objectContaining({ + items: [ + { + value: 'true', + text: 'I consent to the processing of my personal data', + checked: true + } + ] + }) + ) + }) + + it('sets checkbox as unchecked', () => { + def = { + ...def, + hint: 'Please read and confirm the following' + } satisfies DeclarationFieldComponent + + collection = new ComponentCollection([def], { model }) + field = collection.fields[0] + + const viewModel = field.getViewModel(getFormData(undefined)) expect(viewModel).toEqual( expect.objectContaining({ items: [ { value: 'true', - text: 'I consent to the processing of my personal data' + text: 'I understand and agree', + checked: false } ] }) diff --git a/src/server/plugins/engine/components/DeclarationField.ts b/src/server/plugins/engine/components/DeclarationField.ts index b37a15946..f08eb8752 100644 --- a/src/server/plugins/engine/components/DeclarationField.ts +++ b/src/server/plugins/engine/components/DeclarationField.ts @@ -110,6 +110,7 @@ export class DeclarationField extends FormComponent { content, declarationConfirmationLabel = defaultDeclarationConfirmationLabel } = this + const isChecked = !!payload[this.name] return { ...super.getViewModel(payload, errors), hint: hint ? { text: hint } : undefined, @@ -119,11 +120,11 @@ export class DeclarationField extends FormComponent { } }, content, - values: payload[this.name], items: [ { text: declarationConfirmationLabel, - value: 'true' + value: 'true', + checked: isChecked } ] } From 3f1e2c204d0fdf6f8fb817d27c6d95f1213deb42 Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Fri, 14 Nov 2025 09:29:48 +0000 Subject: [PATCH 6/7] Fixed test --- src/server/plugins/engine/components/DeclarationField.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/plugins/engine/components/DeclarationField.test.ts b/src/server/plugins/engine/components/DeclarationField.test.ts index c650c09d8..be89fd504 100644 --- a/src/server/plugins/engine/components/DeclarationField.test.ts +++ b/src/server/plugins/engine/components/DeclarationField.test.ts @@ -197,7 +197,7 @@ describe('DeclarationField', () => { const payload2 = field.getFormDataFromState(state2) expect(payload1).toEqual(getFormData('true')) - expect(payload2).toEqual(getFormData()) + expect(payload2).toEqual(getFormData('unchecked')) }) it('returns value from state', () => { @@ -208,7 +208,7 @@ describe('DeclarationField', () => { const value2 = field.getFormValueFromState(state2) expect(value1).toBe('true') - expect(value2).toBeUndefined() + expect(value2).toBe('unchecked') }) it('returns context for conditions and form submission', () => { From 0082190a7334282c7a076136b019f8f589675b9c Mon Sep 17 00:00:00 2001 From: Jez Barnsley Date: Fri, 14 Nov 2025 09:43:50 +0000 Subject: [PATCH 7/7] Updated forms-model version --- package-lock.json | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index aabae2381..5d375a04c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "hasInstallScript": true, "license": "SEE LICENSE IN LICENSE", "dependencies": { - "@defra/forms-model": "^3.0.575", + "@defra/forms-model": "^3.0.579", "@defra/hapi-tracing": "^1.26.0", "@elastic/ecs-pino-format": "^1.5.0", "@hapi/boom": "^10.0.1", @@ -2298,9 +2298,9 @@ } }, "node_modules/@defra/forms-model": { - "version": "3.0.575", - "resolved": "https://registry.npmjs.org/@defra/forms-model/-/forms-model-3.0.575.tgz", - "integrity": "sha512-U6eJ34ukuaZaWs8jkGxi/lVwV36R+KlN1zAoz/IuW0KDehrXbSOmaI7NJEFEu4vSniCWctImjxVkNbsJ8h1xGg==", + "version": "3.0.579", + "resolved": "https://registry.npmjs.org/@defra/forms-model/-/forms-model-3.0.579.tgz", + "integrity": "sha512-X5kqcIlCzYjKkeUatdTZqL5bAPyJMXdKVJWU44GhktH26P28GlQdspAF9OWBz4oNF0cegDiCOLTR3DKyk6zvaw==", "license": "OGL-UK-3.0", "dependencies": { "@joi/date": "^2.1.1", diff --git a/package.json b/package.json index aa53da8f4..0b044e31c 100644 --- a/package.json +++ b/package.json @@ -70,7 +70,7 @@ }, "license": "SEE LICENSE IN LICENSE", "dependencies": { - "@defra/forms-model": "^3.0.575", + "@defra/forms-model": "^3.0.579", "@defra/hapi-tracing": "^1.26.0", "@elastic/ecs-pino-format": "^1.5.0", "@hapi/boom": "^10.0.1",