Update no-returns-required to use submit service (#708)

https://eaflood.atlassian.net/browse/WATER-4262

When we added validation and controls to the 'no returns required' page of the returns requirements setup journey it was the first page that we did so. It just has 3 radio buttons and no business logic so validation of the submitted form was quite simple. Therefore, so was our solution.

The [Select a start date page](#646) on the overhand was a different story. Not only did we have to cover date validation for the first time, but we also had to combine the date entry with radio buttons and business logic. It was at this point we decided a whole new `Submit[Page]Service` was needed to manage handling the validation.

In both cases, we will also need to handle persisting valid results into the session so a service separate from what we use during the `GET` request starts to make a lot of sense.

This change revisits the 'no returns required' page to update it to use a `SubmitNoReturnsRequiredService`. Hopefully, this will make clear our patterns and conventions going forward.

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)
   }
 

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
 }

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
 }

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<Object>} 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',

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<Object>} 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
+}

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'
       })
   })

app/views/return-requirements/no-returns-required.njk

@@ -4,48 +4,62 @@
 {% 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 %}
 
-  <form method="post" class="form">
-    {{ govukRadios({
-        idPrefix: "reasonNewRequirements",
-        name: "reasonNewRequirements",
-        errorMessage: errorMessage,
+  <div class="govuk-body">
+    <form method="post">
+      {{ govukRadios({
+        name: "no-returns-required",
+        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: 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
+          }
+        ]
+      }) }}
 
-    <div class="govuk-body">
       {{ govukButton({ text: "Continue" }) }}
-    </div>
-  </form>
+    </form>
+  </div>
 
 {% endblock %}

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)
-    })
-  })
 })

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'] })
     })
   })
 })