Select the points for the requirements for returns page

app/controllers/return-requirements.controller.js

@@ -7,7 +7,7 @@
 
 const CheckYourAnswersService = require('../services/return-requirements/check-your-answers.service.js')
 const NoReturnsRequiredService = require('../services/return-requirements/no-returns-required.service.js')
-const SelectPointsService = require('../services/return-requirements/points.service.js')
+const PointsService = require('../services/return-requirements/points.service.js')
 const SelectPurposeService = require('../services/return-requirements/purpose.service.js')
 const SelectReasonService = require('../services/return-requirements/reason.service.js')
 const SessionModel = require('../models/session.model.js')
@@ -128,7 +128,7 @@ async function noReturnsRequired (request, h) {
 async function points (request, h) {
   const { sessionId } = request.params
 
-  const pageData = await SelectPointsService.go(sessionId)
+  const pageData = await PointsService.go(sessionId)
 
   return h.view('return-requirements/points.njk', {
     ...pageData

app/presenters/return-requirements/points.presenter.js

@@ -30,25 +30,21 @@ function _licencePoints (pointsData, payload) {
   // for the return requirement.
   // If it is not set then it is because the presenter has been called from 'PointsService' and it's the first
   // load. Else it has been called by the 'SubmitPointsService' and the user has not checked a point from the list.
-  // Either way, we use it to tell us wether there is anything in the payload worth transforming.
+  // Either way, we use it to tell us whether there is anything in the payload worth transforming.
   const points = payload.points
 
   if (points) {
     return points
   }
 
-  if (!pointsData || pointsData === undefined) {
-    return {
-      abstractionPoints: null
-    }
-  }
-
   const abstractionPoints = []
 
+  if (!pointsData) {
+    return abstractionPoints
+  }
+
   pointsData.forEach((pointDetail) => {
-    if (pointDetail) {
-      abstractionPoints.push(_generateAbstractionContent(pointDetail))
-    }
+    abstractionPoints.push(_generateAbstractionContent(pointDetail))
   })
 
   return abstractionPoints

app/presenters/return-requirements/purpose.presenter.js

@@ -30,7 +30,7 @@ function _licencePurposes (purposesData, payload) {
   // for the return requirement.
   // If it is not set then it is because the presenter has been called from 'PurposeService' and it's the first
   // load. Else it has been called by the 'SubmitPurposeService' and the user has not checked a purpose from the list.
-  // Either way, we use it to tell us wether there is anything in the payload worth transforming.
+  // Either way, we use it to tell us whether there is anything in the payload worth transforming.
   const purposes = payload.purposes
 
   if (!purposes) {

app/services/return-requirements/fetch-points.service.js

@@ -12,7 +12,7 @@ const LicenceModel = require('../../models/licence.model.js')
 /**
  * Fetches points details needed for `/return-requirements/{sessionId}/points` page
  *
- * @param {string} licenceId The UUID for the licence to fetch
+ * @param {string} licenceId - The UUID for the licence to fetch
  *
  * @returns {Promise<Object>} The points details for the matching licenceId
  */

app/services/return-requirements/points.service.js

@@ -15,7 +15,7 @@ const SessionModel = require('../../models/session.model.js')
  * Supports generating the data needed for the points page in the return requirements setup journey. It fetches the
  * current session record and combines it with the checkboxes and other information needed for the form.
  *
- * @param {string} sessionId - The id of the current session
+ * @param {string} sessionId - The UUID of the current session
  *
  * @returns {Promise<Object>} The view data for the points page
 */

app/services/return-requirements/submit-points.service.js

@@ -6,7 +6,7 @@
  */
 
 const FetchPointsService = require('../../services/return-requirements/fetch-points.service.js')
-const PointValidation = require('../../validators/return-requirements/point.validator.js')
+const PointsValidation = require('../../validators/return-requirements/points.validator.js')
 const SelectPointsPresenter = require('../../presenters/return-requirements/points.presenter.js')
 const SessionModel = require('../../models/session.model.js')
 
@@ -15,11 +15,11 @@ const SessionModel = require('../../models/session.model.js')
  *
  * It first retrieves the session instance for the returns requirements journey in progress.
  *
- * The user input is then validated and the 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.
+ * The user input is then validated and the 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 {string} sessionId - The UUID of the current session
  * @param {Object} payload - The submitted form data
  *
  * @returns {Promise<Object>} The page data for the start date page
@@ -40,7 +40,7 @@ async function go (sessionId, payload) {
 }
 
 function _validate (payload) {
-  const validation = PointValidation.go(payload)
+  const validation = PointsValidation.go(payload)
 
   if (!validation.error) {
     return null

app/validators/return-requirements/point.validator.js → app/validators/return-requirements/points.validator.js

@@ -10,16 +10,18 @@ const Joi = require('joi')
 /**
  * Validates data submitted for the `/return-requirements/{sessionId}/points` page
  *
- * When setting up a requirement users must specify a points for the return requirement.
- * Users must select one or more points linked to the licence. If these requirements are not met
- * the validation will return an error.
+ * When setting up a requirement users must specify points for the return requirement. Users must select one or more
+ * points linked to the licence. If these requirements are not met the validation will return an error.
  *
  * @param {Object} payload - The payload from the request to be validated
  *
- * @returns {Object} The result from calling Joi's schema.validate(). If any errors are found the
- * `error:` property will also exist detailing what the issue is.
+ * @returns {Object} The result from calling Joi's schema.validate(). If any errors are found the `error:` property will
+ * also exist detailing what the issue is.
  */
 function go (payload) {
+  // NOTE: When a single point is checked by a user, it returns as a string. When multiple points are checked, the
+  // 'payload' is returned as an array. To make Joi validation straightforward, if the 'payload.points' is a string, it is
+  // turned into an array and validated as such.
   let points = payload.points
 
   if (!Array.isArray(points)) {

app/views/return-requirements/points.njk

@@ -36,12 +36,11 @@
     <span class="govuk-caption-l"> Licence {{ licenceRef }} </span>
     <h1 class="govuk-heading-xl govuk-!-margin-bottom-3">{{ pageTitle }}</h1>
 
-
     <div class="govuk-hint"> Select all that apply </div>
   </div>
 
-<div>
-  <form method="post">
+  <div>
+    <form method="post">
       {% set checkBoxItems = [] %}
       {% for point in licencePoints %}
 
@@ -51,7 +50,7 @@
           checked: false
         } %}
 
-      {% set checkBoxItems = (checkBoxItems.push(checkBoxItem), checkBoxItems) %}
+        {% set checkBoxItems = (checkBoxItems.push(checkBoxItem), checkBoxItems) %}
 
       {% endfor %}
         {{ govukCheckboxes({
@@ -61,8 +60,6 @@
         }) }}
 
     {{ govukButton({ text: "Continue" }) }}
-  </form>
-</div>
-
-
+    </form>
+  </div>
 {% endblock %}

test/controllers/return-requirements.controller.test.js

@@ -11,7 +11,7 @@ const { expect } = Code
 // Things we need to stub
 const CheckYourAnswersService = require('../../app/services/return-requirements/check-your-answers.service.js')
 const NoReturnsRequiredService = require('../../app/services/return-requirements/no-returns-required.service.js')
-const SelectPointsService = require('../../app/services/return-requirements/points.service.js')
+const PointsService = require('../../app/services/return-requirements/points.service.js')
 const SelectPurposeService = require('../../app/services/return-requirements/purpose.service.js')
 const SelectReasonService = require('../../app/services/return-requirements/reason.service.js')
 const SetupService = require('../../app/services/return-requirements/setup.service.js')
@@ -153,10 +153,11 @@ describe('Return requirements controller', () => {
 
   describe('GET /return-requirements/{sessionId}/points', () => {
     beforeEach(async () => {
-      Sinon.stub(SelectPointsService, 'go').resolves({
+      Sinon.stub(PointsService, 'go').resolves({
         id: '8702b98f-ae51-475d-8fcc-e049af8b8d38', pageTitle: 'Select the points for the requirements for returns'
       })
     })
+
     describe('when the request succeeds', () => {
       it('returns the page successfully', async () => {
         const response = await server.inject(_options('points'))

test/presenters/return-requirements/points.presenter.test.js

@@ -8,7 +8,7 @@ const { describe, it, beforeEach } = exports.lab = Lab.script()
 const { expect } = Code
 
 // Thing under test
-const SelectPointsPresenter = require('../../../app/presenters/return-requirements/points.presenter.js')
+const PointsPresenter = require('../../../app/presenters/return-requirements/points.presenter.js')
 
 describe('Select Points presenter', () => {
   let session
@@ -65,7 +65,7 @@ describe('Select Points presenter', () => {
   describe('when provided with a populated session', () => {
     describe('and no payload', () => {
       it('correctly presents the data', () => {
-        const result = SelectPointsPresenter.go(session, pointsData)
+        const result = PointsPresenter.go(session, pointsData)
 
         expect(result).to.equal({
           id: '61e07498-f309-4829-96a9-72084a54996d',
@@ -88,7 +88,7 @@ describe('Select Points presenter', () => {
     }
 
     it('correctly presents the data', () => {
-      const result = SelectPointsPresenter.go(session, pointsData, payload)
+      const result = PointsPresenter.go(session, pointsData, payload)
 
       expect(result).to.equal({
         id: '61e07498-f309-4829-96a9-72084a54996d',

test/services/return-requirements/fetch-points.service.test.js

@@ -16,7 +16,7 @@ const RegionHelper = require('../../support/helpers/region.helper.js')
 // Thing under test
 const FetchPointsService = require('../../../app/services/return-requirements/fetch-points.service.js')
 
-describe('Fetch points service', () => {
+describe('Return requirements Fetch Points service', () => {
   let licence
   let region
 
@@ -61,23 +61,25 @@ describe('Fetch points service', () => {
     })
   })
 
-  it('returns result', async () => {
-    const result = await FetchPointsService.go(licence.id)
+  describe('when called', () => {
+    it('returns result', async () => {
+      const result = await FetchPointsService.go(licence.id)
 
-    expect(result).to.equal([{
-      NGR1_EAST: '69212',
-      NGR2_EAST: 'null',
-      NGR3_EAST: 'null',
-      NGR4_EAST: 'null',
-      LOCAL_NAME: 'RIVER MEDWAY AT YALDING INTAKE',
-      NGR1_NORTH: '50394',
-      NGR1_SHEET: 'TQ',
-      NGR2_NORTH: 'null',
-      NGR2_SHEET: 'null',
-      NGR3_NORTH: 'null',
-      NGR3_SHEET: 'null',
-      NGR4_NORTH: 'null',
-      NGR4_SHEET: 'null'
-    }])
+      expect(result).to.equal([{
+        NGR1_EAST: '69212',
+        NGR2_EAST: 'null',
+        NGR3_EAST: 'null',
+        NGR4_EAST: 'null',
+        LOCAL_NAME: 'RIVER MEDWAY AT YALDING INTAKE',
+        NGR1_NORTH: '50394',
+        NGR1_SHEET: 'TQ',
+        NGR2_NORTH: 'null',
+        NGR2_SHEET: 'null',
+        NGR3_NORTH: 'null',
+        NGR3_SHEET: 'null',
+        NGR4_NORTH: 'null',
+        NGR4_SHEET: 'null'
+      }])
+    })
   })
 })

test/services/return-requirements/submit-points.service.test.js

@@ -14,7 +14,7 @@ const SessionHelper = require('../../support/helpers/session.helper.js')
 
 // Things we need to stub
 const FetchPointsService = require('../../../app/services/return-requirements/fetch-points.service.js')
-const PointsValidation = require('../../../app/validators/return-requirements/point.validator.js')
+const PointsValidation = require('../../../app/validators/return-requirements/points.validator.js')
 
 // Thing under test
 const SubmitPointsService = require('../../../app/services/return-requirements/submit-points.service.js')
@@ -54,38 +54,7 @@ describe('Submit Points service', () => {
           ]
         }
 
-        Sinon.stub(FetchPointsService, 'go').resolves([
-          {
-            NGR1_EAST: '69212',
-            NGR2_EAST: 'null',
-            NGR3_EAST: 'null',
-            NGR4_EAST: 'null',
-            LOCAL_NAME: 'RIVER MEDWAY AT YALDING INTAKE',
-            NGR1_NORTH: '50394',
-            NGR1_SHEET: 'TQ',
-            NGR2_NORTH: 'null',
-            NGR2_SHEET: 'null',
-            NGR3_NORTH: 'null',
-            NGR3_SHEET: 'null',
-            NGR4_NORTH: 'null',
-            NGR4_SHEET: 'null'
-          },
-          {
-            NGR1_EAST: '68083',
-            NGR2_EAST: 'null',
-            NGR3_EAST: 'null',
-            NGR4_EAST: 'null',
-            LOCAL_NAME: 'BEWL WATER RESERVOIR',
-            NGR1_NORTH: '33604',
-            NGR1_SHEET: 'TQ',
-            NGR2_NORTH: 'null',
-            NGR2_SHEET: 'null',
-            NGR3_NORTH: 'null',
-            NGR3_SHEET: 'null',
-            NGR4_NORTH: 'null',
-            NGR4_SHEET: 'null'
-          }
-        ])
+        Sinon.stub(FetchPointsService, 'go').resolves(_testFetchPointsService())
 
         Sinon.stub(PointsValidation, 'go').resolves(null)
       })
@@ -118,38 +87,7 @@ describe('Submit Points service', () => {
       beforeEach(() => {
         payload = {}
 
-        Sinon.stub(FetchPointsService, 'go').resolves([
-          {
-            NGR1_EAST: '69212',
-            NGR2_EAST: 'null',
-            NGR3_EAST: 'null',
-            NGR4_EAST: 'null',
-            LOCAL_NAME: 'RIVER MEDWAY AT YALDING INTAKE',
-            NGR1_NORTH: '50394',
-            NGR1_SHEET: 'TQ',
-            NGR2_NORTH: 'null',
-            NGR2_SHEET: 'null',
-            NGR3_NORTH: 'null',
-            NGR3_SHEET: 'null',
-            NGR4_NORTH: 'null',
-            NGR4_SHEET: 'null'
-          },
-          {
-            NGR1_EAST: '68083',
-            NGR2_EAST: 'null',
-            NGR3_EAST: 'null',
-            NGR4_EAST: 'null',
-            LOCAL_NAME: 'BEWL WATER RESERVOIR',
-            NGR1_NORTH: '33604',
-            NGR1_SHEET: 'TQ',
-            NGR2_NORTH: 'null',
-            NGR2_SHEET: 'null',
-            NGR3_NORTH: 'null',
-            NGR3_SHEET: 'null',
-            NGR4_NORTH: 'null',
-            NGR4_SHEET: 'null'
-          }
-        ])
+        Sinon.stub(FetchPointsService, 'go').resolves(_testFetchPointsService())
       })
 
       it('fetches the current setup session record', async () => {
@@ -183,3 +121,38 @@ describe('Submit Points service', () => {
     })
   })
 })
+
+function _testFetchPointsService () {
+  return [
+    {
+      NGR1_EAST: '69212',
+      NGR2_EAST: 'null',
+      NGR3_EAST: 'null',
+      NGR4_EAST: 'null',
+      LOCAL_NAME: 'RIVER MEDWAY AT YALDING INTAKE',
+      NGR1_NORTH: '50394',
+      NGR1_SHEET: 'TQ',
+      NGR2_NORTH: 'null',
+      NGR2_SHEET: 'null',
+      NGR3_NORTH: 'null',
+      NGR3_SHEET: 'null',
+      NGR4_NORTH: 'null',
+      NGR4_SHEET: 'null'
+    },
+    {
+      NGR1_EAST: '68083',
+      NGR2_EAST: 'null',
+      NGR3_EAST: 'null',
+      NGR4_EAST: 'null',
+      LOCAL_NAME: 'BEWL WATER RESERVOIR',
+      NGR1_NORTH: '33604',
+      NGR1_SHEET: 'TQ',
+      NGR2_NORTH: 'null',
+      NGR2_SHEET: 'null',
+      NGR3_NORTH: 'null',
+      NGR3_SHEET: 'null',
+      NGR4_NORTH: 'null',
+      NGR4_SHEET: 'null'
+    }
+  ]
+}