From 2f9fb40b00a947536ef32ce9700e17437dc4cf99 Mon Sep 17 00:00:00 2001 From: Hugh FD Jackson Date: Tue, 28 Jan 2025 16:37:42 +0000 Subject: [PATCH 1/3] BAU: lift viewModel into a generic AnswerModel fn Ultimately, I'd like to see us: - define a view for each Answerodel - have an answer know how to populate that view (based on its configuration) As part of the transition, I'm: - Adding a viewModel to the base AnswerModel class - passing in the validation options (so the AnswerModel can be responsible for adding the errors itself). Once we have moved all models over to this way of doing things, we can: - removing passing the value & errorMessage in via the QuestionPageController - add the view template to the AnswerModel class itself - update layouts/questions.njk to look up the view & render it from the AnswerModel class Note that the viewModel for a class can return anything that satisfies its own template. There's no one-size-fits-all expectation for viewModels across types such as radio buttons, addresses, names, etc. Also note that most Answers won't need their own viewModels and templates - if they're merely configured version of more generic types. See RadioButton for an example of this. --- .../question-page-controller.js | 2 + .../question-page-controller.test.js | 4 +- .../common/model/answer/answer-model.js | 11 ++++ .../common/model/answer/answer-model.test.js | 6 ++ .../model/answer/radio-button/radio-button.js | 15 ++++- .../answer/radio-button/radio-button.njk | 2 +- .../answer/radio-button/radio-button.test.js | 58 +++++++++++-------- 7 files changed, 70 insertions(+), 28 deletions(-) diff --git a/src/server/common/controller/question-page-controller/question-page-controller.js b/src/server/common/controller/question-page-controller/question-page-controller.js index 0cb78b41..51b293ff 100644 --- a/src/server/common/controller/question-page-controller/question-page-controller.js +++ b/src/server/common/controller/question-page-controller/question-page-controller.js @@ -52,6 +52,7 @@ export class QuestionPageController extends GenericPageController { heading: this.page.heading, value: answer.value, answer, + viewModelOptions: { validate: false }, ...this.page.viewProps(req) }) } @@ -76,6 +77,7 @@ export class QuestionPageController extends GenericPageController { heading: this.page.heading, value: answer.value, answer, + viewModelOptions: { validate: true }, errors, errorMessages: Answer.errorMessages(errors), ...this.page.viewProps(req) diff --git a/src/server/common/controller/question-page-controller/question-page-controller.test.js b/src/server/common/controller/question-page-controller/question-page-controller.test.js index 0425e705..00d32226 100644 --- a/src/server/common/controller/question-page-controller/question-page-controller.test.js +++ b/src/server/common/controller/question-page-controller/question-page-controller.test.js @@ -465,7 +465,8 @@ describe('QuestionPageController', () => { heading: question, nextPage: 'redirect_uri', pageTitle: question, - value: undefined + value: undefined, + viewModelOptions: { validate: false } }) ) @@ -491,6 +492,7 @@ describe('QuestionPageController', () => { { href: `#${questionKey}`, text: 'There is a problem' } ], errors: { questionKey: { text: 'There is a problem' } }, + viewModelOptions: { validate: true }, heading: question, nextPage: 'test_next_page', pageTitle: `Error: ${question}`, diff --git a/src/server/common/model/answer/answer-model.js b/src/server/common/model/answer/answer-model.js index 0c0d35e4..a5ce72ca 100644 --- a/src/server/common/model/answer/answer-model.js +++ b/src/server/common/model/answer/answer-model.js @@ -68,6 +68,13 @@ export class AnswerModel { throw new NotImplementedError() } + /** + * @params {AnswerViewModelOptions} options + */ + viewModel(options) { + throw new NotImplementedError() + } + /** * @param {unknown} _data * @returns {unknown} @@ -91,3 +98,7 @@ export class AnswerModel { /** * @typedef {{[key:string]: string | undefined}} RawPayload */ + +/** + * @typedef {{ validate: boolean }} AnswerViewModelOptions + */ diff --git a/src/server/common/model/answer/answer-model.test.js b/src/server/common/model/answer/answer-model.test.js index 063d56e6..5ccbe861 100644 --- a/src/server/common/model/answer/answer-model.test.js +++ b/src/server/common/model/answer/answer-model.test.js @@ -28,6 +28,12 @@ describe('AnswerModel', () => { expect(() => AnswerModel.fromState({})).toThrow(notImplementedError) }) + it('should throw NotImplementedError when viewModel is called', () => { + expect(() => answer.viewModel({ validate: true })).toThrow( + notImplementedError + ) + }) + it('should seal the object to prevent property additions or deletions', () => { class AnswerModelBasic extends AnswerModel { _extractFields(data) { diff --git a/src/server/common/model/answer/radio-button/radio-button.js b/src/server/common/model/answer/radio-button/radio-button.js index c296ee01..64643c27 100644 --- a/src/server/common/model/answer/radio-button/radio-button.js +++ b/src/server/common/model/answer/radio-button/radio-button.js @@ -3,6 +3,8 @@ import { AnswerModel } from '../answer-model.js' import { validateAnswerAgainstSchema } from '../validation.js' import { NotImplementedError } from '../../../helpers/not-implemented-error.js' +/** @import {AnswerViewModelOptions} from '../answer-model.js' */ + /* eslint-disable jsdoc/require-returns-check */ /** @@ -93,7 +95,10 @@ export class RadioButtonAnswer extends AnswerModel { }) } - get viewModel() { + /** + * @param {AnswerViewModelOptions} options + */ + viewModel({ validate }) { const items = Object.entries(this.config.options).map(([key, value]) => ({ id: key, value: key, @@ -103,12 +108,18 @@ export class RadioButtonAnswer extends AnswerModel { } })) items[0].id = this.config.payloadKey - return { + + const model = { name: this.config.payloadKey, id: this.config.payloadKey, fieldset: {}, value: this.value, items } + + if (validate) { + model.errorMessage = this.validate().errors[this.config.payloadKey] + } + return model } } diff --git a/src/server/common/model/answer/radio-button/radio-button.njk b/src/server/common/model/answer/radio-button/radio-button.njk index 2a3560b6..be5d55c7 100644 --- a/src/server/common/model/answer/radio-button/radio-button.njk +++ b/src/server/common/model/answer/radio-button/radio-button.njk @@ -1,2 +1,2 @@ {% from "govuk/components/radios/macro.njk" import govukRadios %} -{{ govukRadios(assign({}, answer.viewModel, { errorMessage: errors[answer.config.payloadKey]})) }} +{{ govukRadios(answer.viewModel(viewModelOptions)) }} diff --git a/src/server/common/model/answer/radio-button/radio-button.test.js b/src/server/common/model/answer/radio-button/radio-button.test.js index 8c29df80..a801b822 100644 --- a/src/server/common/model/answer/radio-button/radio-button.test.js +++ b/src/server/common/model/answer/radio-button/radio-button.test.js @@ -141,31 +141,41 @@ describe('RadioButton', () => { }) describe('#RadioButton.viewModel', () => { - it('should return everything (except errors) to render in the template', () => { - const answer = new RadioButtonTest({ test_radio: 'value_1' }) - expect(answer.viewModel).toEqual({ - name: 'test_radio', - id: 'test_radio', - fieldset: {}, - value: answer.value, - items: [ - { - id: 'test_radio', - value: 'value_1', - text: 'test_label_1', - hint: { - text: undefined - } - }, - { - id: 'value_2', - value: 'value_2', - text: 'test_label_2', - hint: { - text: 'test_hint_2' - } + const answer = new RadioButtonTest({ test_radio: 'invalid_answer' }) + const defaultViewModel = { + name: 'test_radio', + id: 'test_radio', + fieldset: {}, + value: answer.value, + items: [ + { + id: 'test_radio', + value: 'value_1', + text: 'test_label_1', + hint: { + text: undefined + } + }, + { + id: 'value_2', + value: 'value_2', + text: 'test_label_2', + hint: { + text: 'test_hint_2' } - ] + } + ] + } + it('should return everything (except errors) to render in the template', () => { + expect(answer.viewModel({ validate: false })).toEqual(defaultViewModel) + }) + + it('should return everything (including errors) to render in the template', () => { + console.log(answer.viewModel({ validate: true })) + console.log(answer.validate().errors) + expect(answer.viewModel({ validate: true })).toEqual({ + ...defaultViewModel, + errorMessage: { text: 'Select an option' } }) }) }) From bba90c98d1d163f300edb4a63379f2f4508e08a3 Mon Sep 17 00:00:00 2001 From: Hugh FD Jackson Date: Tue, 28 Jan 2025 16:54:41 +0000 Subject: [PATCH 2/3] BAU: fix linting issues --- src/server/common/model/answer/answer-model.js | 2 +- .../common/model/answer/radio-button/radio-button.test.js | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/server/common/model/answer/answer-model.js b/src/server/common/model/answer/answer-model.js index a5ce72ca..4e0b44b9 100644 --- a/src/server/common/model/answer/answer-model.js +++ b/src/server/common/model/answer/answer-model.js @@ -69,7 +69,7 @@ export class AnswerModel { } /** - * @params {AnswerViewModelOptions} options + * @param {AnswerViewModelOptions} options */ viewModel(options) { throw new NotImplementedError() diff --git a/src/server/common/model/answer/radio-button/radio-button.test.js b/src/server/common/model/answer/radio-button/radio-button.test.js index a801b822..d78b4702 100644 --- a/src/server/common/model/answer/radio-button/radio-button.test.js +++ b/src/server/common/model/answer/radio-button/radio-button.test.js @@ -166,13 +166,12 @@ describe('RadioButton', () => { } ] } + it('should return everything (except errors) to render in the template', () => { expect(answer.viewModel({ validate: false })).toEqual(defaultViewModel) }) it('should return everything (including errors) to render in the template', () => { - console.log(answer.viewModel({ validate: true })) - console.log(answer.validate().errors) expect(answer.viewModel({ validate: true })).toEqual({ ...defaultViewModel, errorMessage: { text: 'Select an option' } From 8184ff7e953ef394b34dc2407c0b23f85f041a9d Mon Sep 17 00:00:00 2001 From: Hugh FD Jackson Date: Tue, 28 Jan 2025 17:11:07 +0000 Subject: [PATCH 3/3] BAU: update unused param to _options --- src/server/common/model/answer/answer-model.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/common/model/answer/answer-model.js b/src/server/common/model/answer/answer-model.js index 4e0b44b9..e07c29c9 100644 --- a/src/server/common/model/answer/answer-model.js +++ b/src/server/common/model/answer/answer-model.js @@ -69,9 +69,9 @@ export class AnswerModel { } /** - * @param {AnswerViewModelOptions} options + * @param {AnswerViewModelOptions} _options */ - viewModel(options) { + viewModel(_options) { throw new NotImplementedError() }