Add controls & validation to rtn reqs reason page (#688)

* Returns required journey - Select return reason page iteration 2 (2 of 7) https://eaflood.atlassian.net/browse/WATER-4265 This is the second page in the returns required journey. This page will have radio control options. This page will have validation to ensure that the user has selected a radio option. * feat(app): WATER-4265 Added controls and validation for step 2 of 7 This commit adds controls for the 2nd step in the no returns required journey. It also has validation which checks to ensure a value is selected on the form * fix(app): WATER-4265 fixed broken test test had wrong data inside. fixed with correct data * fix: fix broken test * fix: amend sonarcloud issues * fix: amend another sonarcloud issue * Thoughts and opinions Having completed the [Returns required journey - Select start date page iteration 2 (1 of 7)](#646) I was expecting the pattern implemented to follow that more than what we'd originally done for 'no-returns-required'. But with so few pages implemented with controls it's hard to see a pattern! So, that wasn't made clear enough. To help I've completed [Update no-returns-required to use submit service](#708) which updates the 'no-returns-required' page to follow the template laid out for 'start-date'. Essentially, validation and error handling will now be done in a `Submit[Page]Service` rather than reuse `[Page]Service`. And so again, I'll go through and contribute to this PR to highlight the changes and when possible, expand on the reasons why. * Housekeeping - clean up after merging in `main` * Correction - The module name does not match file * Correction - wrong page in comments That was me! Doh! * Consistency - add new submit service This is slightly pre-emptive but we know we will need to handle persisting valid payloads to the sessions record soon. Having a service separate from what we call on the GET request gives us an appropriate place to put that. It also allows us to simplify the controller logic in the meantime. * Remove error handling from the presenter Now handled by the submit service. * Remove optional error from the GET service We'll never pass an error in now because we'll never call this in the POST handler. * Update view to reference error Also bring it inline with no returns required and start date. * Update controller to use new service * Update reference in validator We changed the control to match the page and everything else so we need to update the validator to use the new reference. * Fix the tests * Remove unused static lookup * Make the validator more robust As our unit test confirmed it was checking for a value but would accept any value. Now it will only accept certain values. * Update no-rtns-req validator Now it is consistent with what we just did in the `ReasonValidator`. * Add missing JSDoc comments * Correct test title * Add test for new submit service --------- Co-authored-by: Alan Cruikshanks <[email protected]>
DEFRA · Feb 5, 2024 · 530cdd7 · 530cdd7
1 parent fbf8833
commit 530cdd7
Show file tree

Hide file tree

Showing 14 changed files with 512 additions and 16 deletions.
diff --git a/.gitignore b/.gitignore
@@ -43,3 +43,6 @@ lcov.info
 
 # Build assets
 app/public/build
+
+# ignore husky config
+.husky/
diff --git a/app/controllers/return-requirements.controller.js b/app/controllers/return-requirements.controller.js
@@ -6,9 +6,11 @@
  */
 
 const NoReturnsRequiredService = require('../services/return-requirements/no-returns-required.service.js')
+const SelectReasonService = require('../services/return-requirements/reason.service.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 SubmitReasonService = require('../services/return-requirements/submit-reason.service.js')
 const SubmitStartDateService = require('../services/return-requirements/submit-start-date.service.js')
 
 async function abstractionPeriod (request, h) {
@@ -130,12 +132,10 @@ async function purpose (request, h) {
 async function reason (request, h) {
   const { sessionId } = request.params
 
-  const session = await SessionModel.query().findById(sessionId)
+  const pageData = await SelectReasonService.go(sessionId)
 
   return h.view('return-requirements/reason.njk', {
-    activeNavBar: 'search',
-    pageTitle: 'Select the reason for the return requirement',
-    ...session
+    ...pageData
   })
 }
 
@@ -252,6 +252,12 @@ async function submitPurpose (request, h) {
 async function submitReason (request, h) {
   const { sessionId } = request.params
 
+  const pageData = await SubmitReasonService.go(sessionId, request.payload)
+
+  if (pageData.error) {
+    return h.view('return-requirements/reason.njk', pageData)
+  }
+
   return h.redirect(`/system/return-requirements/${sessionId}/setup`)
 }
 

diff --git a/app/presenters/return-requirements/reason.presenter.js b/app/presenters/return-requirements/reason.presenter.js
@@ -0,0 +1,19 @@
+'use strict'
+
+/**
+ * Formats data for the `/return-requirements/{sessionId}/reason` page
+ * @module SelectReasonPresenter
+ */
+
+function go (session) {
+  const data = {
+    id: session.id,
+    licenceRef: session.data.licence.licenceRef
+  }
+
+  return data
+}
+
+module.exports = {
+  go
+}
diff --git a/app/services/return-requirements/reason.service.js b/app/services/return-requirements/reason.service.js
@@ -0,0 +1,33 @@
+'use strict'
+
+/**
+ * Orchestrates fetching and presenting the data for `/return-requirements/{sessionId}/reason` page
+ * @module SelectReasonService
+ */
+const SelectReasonPresenter = require('../../presenters/return-requirements/reason.presenter.js')
+const SessionModel = require('../../models/session.model.js')
+
+/**
+ * Orchestrates fetching and presenting the data for `/return-requirements/{sessionId}/reason` page
+ *
+ * Supports generating the data needed for the select reason 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.
+ *
+ * @param {string} id - The UUID for return requirement setup session record
+ *
+ * @returns {Promise<Object>} page data needed by the view template
+ */
+async function go (sessionId) {
+  const session = await SessionModel.query().findById(sessionId)
+  const formattedData = SelectReasonPresenter.go(session)
+
+  return {
+    activeNavBar: 'search',
+    pageTitle: 'Select the reason for the return requirement',
+    ...formattedData
+  }
+}
+
+module.exports = {
+  go
+}
diff --git a/app/services/return-requirements/submit-no-returns-required.service.js b/app/services/return-requirements/submit-no-returns-required.service.js
@@ -21,7 +21,7 @@ const SessionModel = require('../../models/session.model.js')
  * @param {string} sessionId - The id of the current session
  * @param {Object} payload - The submitted form data
  *
- * @returns {Promise<Object>} The page data for the start date page
+ * @returns {Promise<Object>} The page data for the no returns required page
  */
 async function go (sessionId, payload) {
   const session = await SessionModel.query().findById(sessionId)

diff --git a/app/services/return-requirements/submit-reason.service.js b/app/services/return-requirements/submit-reason.service.js
@@ -0,0 +1,57 @@
+'use strict'
+
+/**
+ * Orchestrates validating the data for `/return-requirements/{sessionId}/reason` page
+ * @module StartDateService
+ */
+
+const ReasonPresenter = require('../../presenters/return-requirements/reason.presenter.js')
+const ReasonValidator = require('../../validators/return-requirements/reason.validator.js')
+const SessionModel = require('../../models/session.model.js')
+
+/**
+ * Orchestrates validating the data for `/return-requirements/{sessionId}/reason` 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<Object>} The page data for the reason page
+ */
+async function go (sessionId, payload) {
+  const session = await SessionModel.query().findById(sessionId)
+
+  const validationResult = _validate(payload)
+
+  const formattedData = ReasonPresenter.go(session, payload)
+
+  return {
+    activeNavBar: 'search',
+    error: validationResult,
+    pageTitle: 'Select the reason for the return requirement',
+    ...formattedData
+  }
+}
+
+function _validate (payload) {
+  const validation = ReasonValidator.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
@@ -7,11 +7,25 @@
 
 const Joi = require('joi')
 
+const VALID_VALUES = [
+  'abstraction_below_100_cubic_metres_per_day',
+  'returns_exception',
+  'transfer_licence'
+]
+
+/**
+ * Validates data submitted for the `/return-requirements/{sessionId}/no-returns-required` page
+ *
+ * @param {Object} payload - The payload from the request to be validated
+ *
+ * @returns {Object} the result from calling Joi's schema.validate(). It will be an object with a `value:` property. If
+ * any errors are found the `error:` property will also exist detailing what the issues were
+ */
 function go (data) {
   const schema = Joi.object({
     'no-returns-required': Joi.string()
       .required()
-      .valid('abstraction_below_100_cubic_metres_per_day', 'returns_exception', 'transfer_licence')
+      .valid(...VALID_VALUES)
       .messages({
         'any.required': 'Select the reason for the return requirement',
         'any.only': 'Select the reason for the return requirement',

diff --git a/app/validators/return-requirements/reason.validator.js b/app/validators/return-requirements/reason.validator.js
@@ -0,0 +1,49 @@
+'use strict'
+
+/**
+ * Validates data submitted for the `/return-requirements/{sessionId}/reason` page
+ * @module ReasonValidator
+ */
+
+const Joi = require('joi')
+
+const VALID_VALUES = [
+  'change_to_special_agreement',
+  'name_or_address_change',
+  'transfer_and_now_chargeable',
+  'extension_of_licence_validity',
+  'major_change',
+  'minor_change',
+  'new_licence_in_part_succession_or_licence_apportionment',
+  'new_licence',
+  'new_special_agreement',
+  'succession_or_transfer_of_licence',
+  'succession_to_remainder_licence_or_licence_apportionment'
+]
+
+/**
+ * Validates data submitted for the `/return-requirements/{sessionId}/reason` page
+ *
+ * @param {Object} payload - The payload from the request to be validated
+ *
+ * @returns {Object} the result from calling Joi's schema.validate(). It will be an object with a `value:` property. If
+ * any errors are found the `error:` property will also exist detailing what the issues were
+ */
+function go (data) {
+  const schema = Joi.object({
+    reason: Joi.string()
+      .required()
+      .valid(...VALID_VALUES)
+      .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'
+      })
+  })
+
+  return schema.validate(data, { abortEarly: false })
+}
+
+module.exports = {
+  go
+}
diff --git a/app/views/return-requirements/reason.njk b/app/views/return-requirements/reason.njk
@@ -1,28 +1,104 @@
 {% extends 'layout.njk' %}
 {% from "govuk/components/back-link/macro.njk" import govukBackLink %}
 {% from "govuk/components/button/macro.njk" import govukButton %}
-
-{% set rootLink = "/system/return-requirements/" + id %}
+{% from "govuk/components/error-summary/macro.njk" import govukErrorSummary %}
+{% from "govuk/components/radios/macro.njk" import govukRadios %}
 
 {% block breadcrumbs %}
   {# Back link #}
   {{
     govukBackLink({
       text: 'Back',
-      href: rootLink + "/start-date"
+      href: '/system/return-requirements/' + id + '/start-date'
     })
   }}
 {% endblock %}
 
 {% block content %}
-  {# Main heading #}
+  {% if error %}
+    {{ govukErrorSummary({
+      titleText: "There is a problem",
+      errorList: [
+        {
+          text: error.text,
+          href: "#reason-error"
+        }
+      ]
+      }) }}
+  {% endif %}
+
   <div class="govuk-body">
-    <h1 class="govuk-heading-xl govuk-!-margin-bottom-3">{{ pageTitle }}</h1>
-  </div>
+    <form method="post">
+      {{ govukRadios({
+        name: "reason",
+        errorMessage: error,
+        fieldset: {
+          legend: {
+            html: '<span class="govuk-caption-l">Licence ' + licenceRef + '</span>' + pageTitle,
+            isPageHeading: true,
+            classes: "govuk-fieldset__legend--l govuk-!-margin-bottom-6"
+          }
+        },
+        items: [
+          {
+            text: 'Change to special agreement',
+            value: 'change_to_special_agreement',
+            checked: false
+          },
+          {
+            text: 'Licence holder name or address change',
+            value: 'name_or_address_change',
+            checked: false
+          },
+          {
+            text: 'Licence transferred and now chargeable',
+            value: 'transfer_and_now_chargeable',
+            checked: false
+          },
+          {
+            text: 'Limited extension of licence validity (LEV)',
+            value: 'extension_of_licence_validity',
+            checked: false
+          },
+          {
+            text: 'Major change',
+            value: 'major_change',
+            checked: false
+          },
+          {
+            text: 'Minor change',
+            value: 'minor_change',
+            checked: false
+          },
+          {
+            text: 'New licence in part succession or licence apportionment',
+            value: 'new_licence_in_part_succession_or_licence_apportionment',
+            checked: false
+          },
+          {
+            text: 'New licence',
+            value: 'new_licence',
+            checked: false
+          },
+          {
+            text: 'New special agreement',
+            value: 'new_special_agreement',
+            checked: false
+          },
+          {
+            text: 'Succession or transfer of licence',
+            value: 'succession_or_transfer_of_licence',
+            checked: false
+          },
+          {
+            text: 'Succession to remainder licence or licence apportionment',
+            value: 'succession_to_remainder_licence_or_licence_apportionment',
+            checked: false
+          }
+        ]
+      }) }}
 
-  <form method="post">
-    <div class="govuk-body">
       {{ govukButton({ text: "Continue" }) }}
-    </div>
-  </form>
+    </form>
+  </div>
 {% endblock %}
diff --git a/test/controllers/return-requirements.controller.test.js b/test/controllers/return-requirements.controller.test.js
@@ -11,6 +11,7 @@ const { expect } = Code
 // Things we need to stub
 const NoReturnsRequiredService = require('../../app/services/return-requirements/no-returns-required.service.js')
 const StartDateService = require('../../app/services/return-requirements/start-date.service.js')
+const SelectReasonService = require('../../app/services/return-requirements/reason.service.js')
 
 // For running our service
 const { init } = require('../../app/server.js')
@@ -151,6 +152,11 @@ describe('Return requirements controller', () => {
   })
 
   describe('GET /return-requirements/{sessionId}/reason', () => {
+    beforeEach(async () => {
+      Sinon.stub(SelectReasonService, 'go').resolves({
+        id: '8702b98f-ae51-475d-8fcc-e049af8b8d38', pageTitle: 'Select the reason for the return requirement'
+      })
+    })
     describe('when the request succeeds', () => {
       it('returns the page successfully', async () => {
         const response = await server.inject(_options('reason'))