diff --git a/app/controllers/return-requirements.controller.js b/app/controllers/return-requirements.controller.js index fc99a35cab..5707bd23d5 100644 --- a/app/controllers/return-requirements.controller.js +++ b/app/controllers/return-requirements.controller.js @@ -6,9 +6,9 @@ */ const NoReturnsRequiredService = require('../services/return-requirements/no-returns-required.service.js') -const NoReturnsRequiredValidator = require('../validators/return-requirements/no-returns-required.validator.js') const SessionModel = require('../models/session.model.js') const StartDateService = require('../services/return-requirements/start-date.service.js') +const SubmitNoReturnsRequiredService = require('../services/return-requirements/submit-no-returns-required.service.js') const SubmitStartDateService = require('../services/return-requirements/submit-start-date.service.js') async function abstractionPeriod (request, h) { @@ -227,10 +227,10 @@ async function submitFrequencyReported (request, h) { async function submitNoReturnsRequired (request, h) { const { sessionId } = request.params - const validation = NoReturnsRequiredValidator.go(request.payload) - if (validation.error) { - const pageData = await NoReturnsRequiredService.go(sessionId, validation.error) + const pageData = await SubmitNoReturnsRequiredService.go(sessionId, request.payload) + + if (pageData.error) { return h.view('return-requirements/no-returns-required.njk', pageData) } diff --git a/app/lib/static-lookups.lib.js b/app/lib/static-lookups.lib.js index df69811e0a..d79b1980bb 100644 --- a/app/lib/static-lookups.lib.js +++ b/app/lib/static-lookups.lib.js @@ -28,17 +28,10 @@ const sources = [ 'wrls' ] -const reasonNewRequirementsFields = [ - 'abstraction_below_100_cubic_metres_per_day', - 'returns_exception', - 'transfer_licence' -] - module.exports = { billRunTypes, companyTypes, contactTypes, organisationTypes, - reasonNewRequirementsFields, sources } diff --git a/app/presenters/return-requirements/no-returns-required.presenter.js b/app/presenters/return-requirements/no-returns-required.presenter.js index 98b8ab44a1..bbd681f0a0 100644 --- a/app/presenters/return-requirements/no-returns-required.presenter.js +++ b/app/presenters/return-requirements/no-returns-required.presenter.js @@ -5,53 +5,15 @@ * @module NoReturnsRequiredPresenter */ -const { reasonNewRequirementsFields } = require('../../lib/static-lookups.lib.js') - -function go (session, error = null) { +function go (session) { const data = { id: session.id, - errorMessage: _error(error), - licenceRef: session.data.licence.licenceRef, - radioItems: _radioItems(session) + licenceRef: session.data.licence.licenceRef } return data } -function _error (error) { - if (!error) { - return null - } - - const errorMessage = { - text: error.message - } - - return errorMessage -} - -function _radioItems (_session) { - const radioItems = [ - { - value: reasonNewRequirementsFields[0], - text: 'Abstraction amount below 100 cubic metres per day', - checked: false - }, - { - value: reasonNewRequirementsFields[1], - text: 'Returns exception', - checked: false - }, - { - value: reasonNewRequirementsFields[2], - text: 'Transfer licence', - checked: false - } - ] - - return radioItems -} - module.exports = { go } diff --git a/app/services/return-requirements/no-returns-required.service.js b/app/services/return-requirements/no-returns-required.service.js index 2cef85a989..f8e023a89a 100644 --- a/app/services/return-requirements/no-returns-required.service.js +++ b/app/services/return-requirements/no-returns-required.service.js @@ -13,18 +13,13 @@ const SessionModel = require('../../models/session.model.js') * Supports generating the data needed for the no returns required page in the return requirements setup journey. It * fetches the current session record and combines it with the radio buttons and other information needed for the form. * - * If a validation issue is found when the form is submitted, it will be called from the POST handler with the Joi - * validation error passed in. This extra information will be used to ensure the right error message is included in the - * data needed by the view. - * * @param {string} id - The UUID for return requirement setup session record - * @param {Object} [error] - A Joi validation error if an issue was found with the submitted form data * - * @returns {Object} page data needed by the view template + * @returns {Promise} The view data for the no returns required page */ -async function go (sessionId, error = null) { +async function go (sessionId) { const session = await SessionModel.query().findById(sessionId) - const formattedData = NoReturnsRequiredPresenter.go(session, error) + const formattedData = NoReturnsRequiredPresenter.go(session) return { activeNavBar: 'search', diff --git a/app/services/return-requirements/submit-no-returns-required.service.js b/app/services/return-requirements/submit-no-returns-required.service.js new file mode 100644 index 0000000000..7a7727fc5a --- /dev/null +++ b/app/services/return-requirements/submit-no-returns-required.service.js @@ -0,0 +1,57 @@ +'use strict' + +/** + * Orchestrates validating the data for `/return-requirements/{sessionId}/no-returns-required` page + * @module StartDateService + */ + +const NoReturnsRequiredPresenter = require('../../presenters/return-requirements/no-returns-required.presenter.js') +const NoReturnsRequiredValidator = require('../../validators/return-requirements/no-returns-required.validator.js') +const SessionModel = require('../../models/session.model.js') + +/** + * Orchestrates validating the data for `/return-requirements/{sessionId}/no-returns-required` page + * + * It first retrieves the session instance for the returns requirements journey in progress. + * + * The validation result is then combined with the output of the presenter to generate the page data needed by the view. + * If there was a validation error the controller will re-render the page so needs this information. If all is well the + * controller will redirect to the next page in the journey. + * + * @param {string} sessionId - The id of the current session + * @param {Object} payload - The submitted form data + * + * @returns {Promise} The page data for the start date page + */ +async function go (sessionId, payload) { + const session = await SessionModel.query().findById(sessionId) + + const validationResult = _validate(payload) + + const formattedData = NoReturnsRequiredPresenter.go(session, payload) + + return { + activeNavBar: 'search', + error: validationResult, + pageTitle: 'Why are no returns required?', + ...formattedData + } +} + +function _validate (payload) { + const validation = NoReturnsRequiredValidator.go(payload) + + if (!validation.error) { + return null + } + + const { message } = validation.error.details[0] + + return { + text: message + } +} + +module.exports = { + go +} diff --git a/app/validators/return-requirements/no-returns-required.validator.js b/app/validators/return-requirements/no-returns-required.validator.js index 3feb8cfef6..159c1be249 100644 --- a/app/validators/return-requirements/no-returns-required.validator.js +++ b/app/validators/return-requirements/no-returns-required.validator.js @@ -9,10 +9,12 @@ const Joi = require('joi') function go (data) { const schema = Joi.object({ - reasonNewRequirements: Joi.string() + 'no-returns-required': Joi.string() .required() + .valid('abstraction_below_100_cubic_metres_per_day', 'returns_exception', 'transfer_licence') .messages({ 'any.required': 'Select the reason for the return requirement', + 'any.only': 'Select the reason for the return requirement', 'string.empty': 'Select the reason for the return requirement' }) }) diff --git a/app/views/return-requirements/no-returns-required.njk b/app/views/return-requirements/no-returns-required.njk index 8b7a969416..b32d29a8b5 100644 --- a/app/views/return-requirements/no-returns-required.njk +++ b/app/views/return-requirements/no-returns-required.njk @@ -4,35 +4,34 @@ {% from "govuk/components/error-summary/macro.njk" import govukErrorSummary %} {% from "govuk/components/radios/macro.njk" import govukRadios %} -{% set rootLink = "/system/return-requirements/" + id %} {% block breadcrumbs %} {# Back link #} {{ govukBackLink({ text: 'Back', - href: rootLink + '/start-date' + href: "/system/return-requirements/" + id + '/start-date' }) }} {% endblock %} {% block content %} - {% if errorMessage %} + {% if error %} {{ govukErrorSummary({ titleText: "There is a problem", errorList: [ { - text: errorMessage.text, - href: "#reasonNewRequirements-error" + text: error.text, + href: "#no-returns-required-error" } ] }) }} {% endif %} -
- {{ govukRadios({ - idPrefix: "reasonNewRequirements", - name: "reasonNewRequirements", - errorMessage: errorMessage, +
+ + {{ govukRadios({ + name: "no-returns-required", + errorMessage: error, fieldset: { legend: { html: 'Licence ' + licenceRef + '' + pageTitle, @@ -40,12 +39,27 @@ classes: "govuk-fieldset__legend--l govuk-!-margin-bottom-6" } }, - items: radioItems - }) }} + items: [ + { + text: 'Abstraction amount below 100 cubic metres per day', + value: 'abstraction_below_100_cubic_metres_per_day', + checked: false + }, + { + text: 'Returns exception', + value: 'returns_exception', + checked: false + }, + { + text: 'Transfer licence', + value: 'transfer_licence', + checked: false + } + ] + }) }} -
{{ govukButton({ text: "Continue" }) }} -
- + +
{% endblock %} diff --git a/test/presenters/return-requirements/no-returns-required.presenter.test.js b/test/presenters/return-requirements/no-returns-required.presenter.test.js index b3ec444fd2..1ec2501336 100644 --- a/test/presenters/return-requirements/no-returns-required.presenter.test.js +++ b/test/presenters/return-requirements/no-returns-required.presenter.test.js @@ -31,38 +31,9 @@ describe('No Returns Required presenter', () => { const result = NoReturnsRequiredPresenter.go(session) expect(result).to.equal({ - errorMessage: null, id: 'f1288f6c-8503-4dc1-b114-75c408a14bd0', - licenceRef: '01/123', - radioItems: [ - { - checked: false, - text: 'Abstraction amount below 100 cubic metres per day', - value: 'abstraction_below_100_cubic_metres_per_day' - }, - { - checked: false, - text: 'Returns exception', - value: 'returns_exception' - }, - { - checked: false, - text: 'Transfer licence', - value: 'transfer_licence' - } - ] + licenceRef: '01/123' }) }) }) - - describe('when provided with an error', () => { - const error = new Error('Test error message') - - it('includes the error message in the presented data', () => { - const result = NoReturnsRequiredPresenter.go(session, error) - - expect(result.errorMessage).to.exist() - expect(result.errorMessage.text).to.equal(error.message) - }) - }) }) diff --git a/test/services/return-requirements/no-returns-required.service.test.js b/test/services/return-requirements/no-returns-required.service.test.js index 4bfb15d227..3b81b8c655 100644 --- a/test/services/return-requirements/no-returns-required.service.test.js +++ b/test/services/return-requirements/no-returns-required.service.test.js @@ -19,7 +19,18 @@ describe('No Returns Required service', () => { beforeEach(async () => { await DatabaseHelper.clean() - session = await SessionHelper.add({ data: { licence: { licenceRef: '01/123' } } }) + session = await SessionHelper.add({ + data: { + licence: { + id: '8b7f78ba-f3ad-4cb6-a058-78abc4d1383d', + currentVersionStartDate: '2023-01-01T00:00:00.000Z', + endDate: null, + licenceRef: '01/ABC', + licenceHolder: 'Turbo Kid', + startDate: '2022-04-01T00:00:00.000Z' + } + } + }) }) describe('when called', () => { @@ -29,33 +40,14 @@ describe('No Returns Required service', () => { expect(result.id).to.equal(session.id) }) - describe('without the optional error param', () => { - it('returns page data for the view', async () => { - const result = await NoReturnsRequiredService.go(session.id) - - expect(result.activeNavBar).to.exist() - expect(result.pageTitle).to.exist() - expect(result.licenceRef).to.exist() - expect(result.radioItems).to.exist() - - expect(result.errorMessage).to.be.null() - }) - }) - - describe('with the optional error param', () => { - const error = new Error('Test error message') - - it('returns page data for the view including the error message', async () => { - const result = await NoReturnsRequiredService.go(session.id, error) - - expect(result.activeNavBar).to.exist() - expect(result.pageTitle).to.exist() - expect(result.licenceRef).to.exist() - expect(result.radioItems).to.exist() + it('returns page data for the view', async () => { + const result = await NoReturnsRequiredService.go(session.id) - expect(result.errorMessage).to.exist() - expect(result.errorMessage.text).to.equal(error.message) - }) + expect(result).to.equal({ + activeNavBar: 'search', + pageTitle: 'Why are no returns required?', + licenceRef: '01/ABC' + }, { skip: ['id'] }) }) }) }) diff --git a/test/services/return-requirements/submit-no-returns-required.service.test.js b/test/services/return-requirements/submit-no-returns-required.service.test.js new file mode 100644 index 0000000000..ac3d463f90 --- /dev/null +++ b/test/services/return-requirements/submit-no-returns-required.service.test.js @@ -0,0 +1,97 @@ +'use strict' + +// Test framework dependencies +const Lab = require('@hapi/lab') +const Code = require('@hapi/code') + +const { describe, it, beforeEach } = exports.lab = Lab.script() +const { expect } = Code + +// Test helpers +const DatabaseHelper = require('../../support/helpers/database.helper.js') +const SessionHelper = require('../../support/helpers/session.helper.js') + +// Thing under test +const SubmitNoReturnsRequiredService = require('../../../app/services/return-requirements/submit-no-returns-required.service.js') + +describe('Submit No Returns Required service', () => { + let payload + let session + + beforeEach(async () => { + await DatabaseHelper.clean() + + session = await SessionHelper.add({ + data: { + licence: { + id: '8b7f78ba-f3ad-4cb6-a058-78abc4d1383d', + currentVersionStartDate: '2023-01-01T00:00:00.000Z', + endDate: null, + licenceRef: '01/ABC', + licenceHolder: 'Turbo Kid', + startDate: '2022-04-01T00:00:00.000Z' + }, + journey: 'no-returns-required' + } + }) + }) + + describe('when called', () => { + describe('with a valid payload', () => { + beforeEach(() => { + payload = { + 'no-returns-required': 'abstraction_below_100_cubic_metres_per_day' + } + }) + + it('fetches the current setup session record', async () => { + const result = await SubmitNoReturnsRequiredService.go(session.id, payload) + + expect(result.id).to.equal(session.id) + }) + + it('returns page data for the view', async () => { + const result = await SubmitNoReturnsRequiredService.go(session.id, payload) + + expect(result).to.equal({ + activeNavBar: 'search', + error: null, + pageTitle: 'Why are no returns required?', + licenceRef: '01/ABC' + }, { skip: ['id'] }) + }) + }) + + describe('with an invalid payload', () => { + describe('because the user has not selected anything', () => { + beforeEach(() => { + payload = {} + }) + + it('fetches the current setup session record', async () => { + const result = await SubmitNoReturnsRequiredService.go(session.id, payload) + + expect(result.id).to.equal(session.id) + }) + + it('returns page data for the view', async () => { + const result = await SubmitNoReturnsRequiredService.go(session.id, payload) + + expect(result).to.equal({ + activeNavBar: 'search', + pageTitle: 'Why are no returns required?', + licenceRef: '01/ABC' + }, { skip: ['id', 'error'] }) + }) + + it('returns page data with an error', async () => { + const result = await SubmitNoReturnsRequiredService.go(session.id, payload) + + expect(result.error).to.equal({ + text: 'Select the reason for the return requirement' + }) + }) + }) + }) + }) +}) diff --git a/test/validators/no-returns-required/no-returns-required.validator.test.js b/test/validators/no-returns-required/no-returns-required.validator.test.js index 9336c03cc7..028a09948f 100644 --- a/test/validators/no-returns-required/no-returns-required.validator.test.js +++ b/test/validators/no-returns-required/no-returns-required.validator.test.js @@ -7,28 +7,33 @@ const Code = require('@hapi/code') const { describe, it } = exports.lab = Lab.script() const { expect } = Code -// Test helpers -const { reasonNewRequirementsFields } = require('../../../app/lib/static-lookups.lib.js') - // Thing under test const NoReturnsRequiredValidator = require('../../../app/validators/return-requirements/no-returns-required.validator.js') describe('No Returns Required validator', () => { describe('when valid data is provided', () => { - reasonNewRequirementsFields.forEach(reason => { - it(`confirms the data is valid (reason ${reason})`, () => { - const result = NoReturnsRequiredValidator.go({ reasonNewRequirements: reason }) + it('confirms the data is valid', () => { + const result = NoReturnsRequiredValidator.go({ 'no-returns-required': 'transfer_licence' }) - expect(result.value).to.exist() - expect(result.error).not.to.exist() - }) + expect(result.value).to.exist() + expect(result.error).not.to.exist() }) }) describe('when valid data is provided', () => { describe("because no 'reason' is given", () => { it('fails validation', () => { - const result = NoReturnsRequiredValidator.go({ reasonNewRequirements: '' }) + const result = NoReturnsRequiredValidator.go({ 'no-returns-required': '' }) + + expect(result.value).to.exist() + expect(result.error).to.exist() + expect(result.error.details[0].message).to.equal('Select the reason for the return requirement') + }) + }) + + describe("because an unknown 'reason' is given", () => { + it('fails validation', () => { + const result = NoReturnsRequiredValidator.go({ 'no-returns-required': 'no-water' }) expect(result.value).to.exist() expect(result.error).to.exist()