refactor: Extract assessment answer presentation

This change extracts the handling of rendering assessment answers within another component, for example, a list of panels with a tag into another presenter. This will allow it to be shared by other parts of the app and give the ability to present the assessment answers in separate ways more easily. This change does still contain a style of loading settings internally that we would like to get rid of (see #696) as the refactor would be a much bigger and more time consuming change at this stage.
ministryofjustice · Jul 29, 2020 · 02b513a · 02b513a
1 parent 9e9850a
commit 02b513a
Show file tree

Hide file tree

Showing 11 changed files with 474 additions and 452 deletions.
diff --git a/app/move/controllers/view.js b/app/move/controllers/view.js
@@ -49,9 +49,14 @@ module.exports = function view(req, res) {
   const urls = {
     update: updateUrls,
   }
+  const assessment = presenters
+    .assessmentAnswersByCategory(assessmentAnswers)
+    .filter(category => category.key !== 'court')
+    .map(presenters.assessmentCategoryToPanelComponent)
 
   const locals = {
     move,
+    assessment,
     personEscortRecord,
     personEscortRecordIsEnabled,
     personEscortRecordIsComplete,
@@ -62,9 +67,6 @@ module.exports = function view(req, res) {
     moveSummary: presenters.moveToMetaListComponent(move, updateActions),
     personalDetailsSummary: presenters.personToSummaryListComponent(person),
     tagList: presenters.assessmentToTagList(assessmentAnswers),
-    assessment: presenters
-      .assessmentAnswersByCategory(assessmentAnswers)
-      .map(presenters.assessmentCategoryToPanelComponent),
     canCancelMove:
       (userPermissions.includes('move:cancel') &&
         move.status === 'requested' &&

diff --git a/app/move/controllers/view.test.js b/app/move/controllers/view.test.js
@@ -25,6 +25,20 @@ const pathStubs = {
 const controller = proxyquire('./view', pathStubs)
 
 const mockAssessmentAnswers = []
+const mockAssessmentByCategory = [
+  {
+    key: 'risk',
+    answers: [],
+  },
+  {
+    key: 'health',
+    answers: [],
+  },
+  {
+    key: 'court',
+    answers: [],
+  },
+]
 
 const mockMove = {
   id: 'moveId',
@@ -99,7 +113,7 @@ describe('Move controllers', function () {
         .returns('__frameworkFlagsToTagList__')
       sinon.stub(presenters, 'personToSummaryListComponent').returnsArg(0)
       sinon.stub(presenters, 'assessmentToTagList').returnsArg(0)
-      sinon.stub(presenters, 'assessmentAnswersByCategory').returnsArg(0)
+      sinon.stub(presenters, 'assessmentAnswersByCategory').returns([])
       sinon.stub(presenters, 'assessmentCategoryToPanelComponent').returnsArg(0)
       sinon.stub(presenters, 'assessmentToSummaryListComponent').returnsArg(0)
       sinon
@@ -123,6 +137,7 @@ describe('Move controllers', function () {
 
     context('by default', function () {
       beforeEach(function () {
+        presenters.assessmentAnswersByCategory.returns(mockAssessmentByCategory)
         controller(req, res)
         params = res.render.args[0][1]
       })
@@ -231,9 +246,36 @@ describe('Move controllers', function () {
         ).to.be.calledOnceWithExactly(mockAssessmentAnswers)
       })
 
+      it('should call assessmentCategoryToPanelComponent presenter with correct categories', function () {
+        expect(presenters.assessmentCategoryToPanelComponent).to.be.calledTwice
+        expect(presenters.assessmentCategoryToPanelComponent).to.be.calledWith(
+          {
+            answers: [],
+            key: 'health',
+          },
+          1
+        )
+        expect(presenters.assessmentCategoryToPanelComponent).to.be.calledWith(
+          {
+            answers: [],
+            key: 'risk',
+          },
+          0
+        )
+      })
+
       it('should contain assessment param', function () {
         expect(params).to.have.property('assessment')
-        expect(params.assessment).to.deep.equal(mockAssessmentAnswers)
+        expect(params.assessment).to.deep.equal([
+          {
+            answers: [],
+            key: 'risk',
+          },
+          {
+            answers: [],
+            key: 'health',
+          },
+        ])
       })
 
       it('should call assessmentToSummaryListComponent presenter with correct args', function () {

diff --git a/app/move/views/_includes/assessment.njk b/app/move/views/_includes/assessment.njk
@@ -7,12 +7,7 @@
     </h2>
 
     {% for panel in assessmentCategory.panels %}
-      {% call appPanel(panel) %}
-        {{ appMetaList({
-          classes: "app-meta-list--divider",
-          items: panel.items
-        }) }}
-      {% endcall %}
+      {{ appPanel(panel) }}
     {% else %}
       {{ appMessage({
         classes: "app-message--muted govuk-!-margin-top-2",

diff --git a/common/presenters/assessment-answers-by-category.js b/common/presenters/assessment-answers-by-category.js
@@ -1,20 +1,23 @@
 const { sortBy, mapValues } = require('lodash')
 
-const { TAG_CATEGORY_WHITELIST } = require('../../config')
-const { filterExpired } = require('../helpers/reference-data')
+const { ASSESSMENT_ANSWERS_CATEGORY_SETTINGS } = require('../../config')
+const referenceDataHelpers = require('../helpers/reference-data')
 
 module.exports = function assessmentAnswersByCategory(assessmentAnswers = []) {
-  const mapped = mapValues(TAG_CATEGORY_WHITELIST, (params, category) => {
-    const answers = assessmentAnswers
-      .filter(answer => answer.category === category)
-      .filter(filterExpired)
+  const mapped = mapValues(
+    ASSESSMENT_ANSWERS_CATEGORY_SETTINGS,
+    (params, category) => {
+      const answers = assessmentAnswers
+        .filter(answer => answer.category === category)
+        .filter(referenceDataHelpers.filterExpired)
 
-    return {
-      ...params,
-      answers,
-      key: category,
+      return {
+        ...params,
+        answers,
+        key: category,
+      }
     }
-  })
+  )
 
   return sortBy(mapped, 'sortOrder')
 }
diff --git a/common/presenters/assessment-answers-by-category.test.js b/common/presenters/assessment-answers-by-category.test.js
@@ -1,13 +1,12 @@
 const proxyquire = require('proxyquire')
 
-const {
-  data: mockProfile,
-} = require('../../test/fixtures/api-client/profile.create.json')
+const referenceDataHelpers = require('../helpers/reference-data')
+
 const assessmentAnswersByCategory = proxyquire(
   './assessment-answers-by-category',
   {
     '../../config': {
-      TAG_CATEGORY_WHITELIST: {
+      ASSESSMENT_ANSWERS_CATEGORY_SETTINGS: {
         health: {
           tagClass: '',
           sortOrder: 2,
@@ -18,108 +17,77 @@ const assessmentAnswersByCategory = proxyquire(
         },
       },
     },
-    '../../common/helpers/reference-data': {
-      filterExpired: sinon.stub().returnsArg(0),
-    },
   }
 )
 
 describe('Presenters', function () {
   describe('#assessmentAnswersByCategory()', function () {
-    context('when provided with mock moves response', function () {
-      let transformedResponse
+    let transformedResponse
+
+    beforeEach(function () {
+      sinon.stub(referenceDataHelpers, 'filterExpired').returnsArg(0)
+    })
+
+    context('when answers exist for categories', function () {
+      const mockAnswers = [
+        {
+          title: 'Violent',
+          category: 'risk',
+        },
+        {
+          title: 'Escape',
+          category: 'risk',
+        },
+        {
+          title: 'Must be held separately',
+          category: 'risk',
+        },
+        {
+          title: 'Medication',
+          category: 'health',
+        },
+        {
+          title: 'Solicitor or other legal representation',
+          category: 'court',
+        },
+      ]
 
       beforeEach(function () {
-        transformedResponse = assessmentAnswersByCategory(
-          mockProfile.attributes.assessment_answers
-        )
+        transformedResponse = assessmentAnswersByCategory(mockAnswers)
       })
 
       it('should return the correct number of categories', function () {
         expect(transformedResponse.length).to.equal(2)
       })
 
       it('should correctly order categories', function () {
-        expect(transformedResponse[0].key).to.equal('risk')
+        const keys = transformedResponse.map(it => it.key)
+        expect(keys).to.deep.equal(['risk', 'health'])
       })
 
-      it('should correctly order categories', function () {
-        expect(transformedResponse[1].key).to.equal('health')
-      })
-
-      it('should contain correct number of types', function () {
+      it('should contain correct number of answers', function () {
         expect(Object.keys(transformedResponse[0].answers)).to.have.length(3)
-        expect(Object.keys(transformedResponse[1].answers)).to.have.length(3)
+        expect(Object.keys(transformedResponse[1].answers)).to.have.length(1)
       })
 
-      it('should group risk answers correctly', function () {
-        expect(transformedResponse[0].answers).to.deep.equal([
-          {
-            title: 'Violent',
-            comments: 'Karate black belt',
-            date: null,
-            expiry_date: null,
-            assessment_question_id: '7448fb0c-d10e-4cab-9354-1732a4d17a30',
-            category: 'risk',
-            key: 'violent',
-          },
-          {
-            title: 'Escape',
-            comments: 'Large poster in cell',
-            date: null,
-            expiry_date: null,
-            assessment_question_id: 'd61c0068-cdb0-43de-88e5-c0a09adbf726',
-            category: 'risk',
-            key: 'escape',
-          },
-          {
-            title: 'Must be held separately',
-            comments: 'Incitement to riot',
-            date: null,
-            expiry_date: null,
-            assessment_question_id: '745a50e8-ea8d-4667-8ba0-c09c62ee3fc5',
-            category: 'risk',
-            key: 'hold_separately',
-          },
-        ])
+      it('should include category params', function () {
+        transformedResponse.forEach(it => {
+          expect(Object.keys(it)).to.have.length(4)
+          expect(Object.keys(it)).to.deep.equal([
+            'tagClass',
+            'sortOrder',
+            'answers',
+            'key',
+          ])
+        })
       })
 
-      it('should group health answers correctly', function () {
-        expect(transformedResponse[1].answers).to.deep.equal([
-          {
-            title: 'Special diet or allergy',
-            comments: 'Vegan',
-            date: null,
-            expiry_date: null,
-            assessment_question_id: 'ff77c212-1244-4b2b-a441-81922d5c2ed8',
-            category: 'health',
-            key: 'special_diet_or_allergy',
-          },
-          {
-            title: 'Health issue',
-            comments: 'Claustophobic',
-            date: null,
-            expiry_date: null,
-            assessment_question_id: 'c1c8e0c1-4376-4f80-8515-dfb7d454a95c',
-            category: 'health',
-            key: 'health_issue',
-          },
-          {
-            title: 'Medication',
-            comments: 'Heart medication needed twice daily',
-            date: null,
-            expiry_date: null,
-            assessment_question_id: '044cca13-b632-4f65-8111-8902e4913cfe',
-            category: 'health',
-            key: 'medication',
-          },
-        ])
+      it('should call expiry filter on all answers', function () {
+        expect(referenceDataHelpers.filterExpired.callCount).to.equal(4)
       })
     })
 
-    context('when no items exist for whitelisted category', function () {
-      let transformedResponse
-
+    context('when no answers exist for categories', function () {
       beforeEach(function () {
         transformedResponse = assessmentAnswersByCategory([])
       })
@@ -129,22 +97,18 @@ describe('Presenters', function () {
       })
 
       it('should correctly order categories', function () {
-        expect(transformedResponse[0].key).to.equal('risk')
-      })
-
-      it('should correctly order categories', function () {
-        expect(transformedResponse[1].key).to.equal('health')
+        const keys = transformedResponse.map(it => it.key)
+        expect(keys).to.deep.equal(['risk', 'health'])
       })
 
-      it('should contain correct number of types', function () {
-        expect(Object.keys(transformedResponse[0].answers)).to.have.length(0)
-        expect(Object.keys(transformedResponse[1].answers)).to.have.length(0)
+      it('should contain empty answers for each', function () {
+        transformedResponse.forEach(it => {
+          expect(it.answers).to.have.length(0)
+        })
       })
     })
 
     context('when called with no arguments', function () {
-      let transformedResponse
-
       beforeEach(function () {
         transformedResponse = assessmentAnswersByCategory()
       })
@@ -154,16 +118,14 @@ describe('Presenters', function () {
       })
 
       it('should correctly order categories', function () {
-        expect(transformedResponse[0].key).to.equal('risk')
-      })
-
-      it('should correctly order categories', function () {
-        expect(transformedResponse[1].key).to.equal('health')
+        const keys = transformedResponse.map(it => it.key)
+        expect(keys).to.deep.equal(['risk', 'health'])
       })
 
-      it('should contain correct number of types', function () {
-        expect(Object.keys(transformedResponse[0].answers)).to.have.length(0)
-        expect(Object.keys(transformedResponse[1].answers)).to.have.length(0)
+      it('should contain empty answers for each', function () {
+        transformedResponse.forEach(it => {
+          expect(it.answers).to.have.length(0)
+        })
       })
     })
   })