PP-13312 refactor format card types and errors

app/controllers/simplified-account/settings/card-types/card-types.controller.js

@@ -9,18 +9,12 @@ async function get (req, res, next) {
   const accountType = req.account.type
   const isAdminUser = req.user.isAdminUserForService(serviceExternalId)
   const messages = res.locals?.flash?.messages ?? []
-  let noCardTypesSelectedError
-  if (res.locals?.flash?.noCardTypesSelectedError) {
-    noCardTypesSelectedError = { summary: [{ text: res.locals?.flash?.noCardTypesSelectedError }] }
-  }
   try {
     const { card_types: allCards } = await getAllCardTypes()
     const { card_types: acceptedCards } = await getAcceptedCardTypesForServiceAndAccountType(serviceExternalId, accountType)
-    const cardTypesSelected = noCardTypesSelectedError ? [] : acceptedCards
     const currentAcceptedCardTypeIds = acceptedCards.map(card => card.id)
-    const cardTypes = formatCardTypesForTemplate(allCards, cardTypesSelected, req.account, isAdminUser)
+    const cardTypes = formatCardTypesForTemplate(allCards, acceptedCards, req.account, isAdminUser)
     response(req, res, 'simplified-account/settings/card-types/index', {
-      errors: noCardTypesSelectedError,
       messages,
       cardTypes,
       isAdminUser,
@@ -39,10 +33,15 @@ async function post (req, res, next) {
   const selectedCardTypeIds = [...selectedDebitCards, ...selectedCreditCards]
   const currentAcceptedCardTypeIds = req.body.currentAcceptedCardTypeIds ? req.body.currentAcceptedCardTypeIds.split(',') : []
   if (!selectedDebitCards.length && !selectedCreditCards.length) {
-    req.flash('noCardTypesSelectedError', 'You must choose at least one card')
-    return res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.cardTypes.index, serviceExternalId, accountType))
+    const { card_types: allCards } = await getAllCardTypes()
+    const cardTypes = formatCardTypesForTemplate(allCards, [], req.account, true)
+    return response(req, res, 'simplified-account/settings/card-types/index', {
+      errors: { summary: [{ text: 'You must choose at least one card' }] },
+      cardTypes,
+      isAdminUser: true,
+      currentAcceptedCardTypeIds
+    })
   }
-
   const noChangesToAcceptedCardTypes = (
     currentAcceptedCardTypeIds.length === selectedCardTypeIds.length &&
     currentAcceptedCardTypeIds.every(item => selectedCardTypeIds.includes(item))
@@ -52,7 +51,6 @@ async function post (req, res, next) {
       formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.cardTypes.index, serviceExternalId, accountType)
     )
   }
-
   try {
     const payload = {
       card_types: selectedCardTypeIds

app/controllers/simplified-account/settings/card-types/card-types.controller.test.js

@@ -95,6 +95,7 @@ describe('Controller: settings/card-types', () => {
       expect(mockResponse.args[0][3].cardTypes.debitCards[0]).to.deep.include({ text: 'Visa debit', checked: true })
       expect(mockResponse.args[0][3]).to.have.property('cardTypes').to.have.property('creditCards').length(2)
       expect(mockResponse.args[0][3].cardTypes.creditCards[0]).to.deep.include({ text: 'Visa credit', checked: false })
+      expect(mockResponse.args[0][3].cardTypes.creditCards[1]).to.deep.include({ text: 'American Express', checked: false })
       expect(mockResponse.args[0][3]).to.have.property('isAdminUser').to.equal(true)
       expect(mockResponse.args[0][3]).to.have.property('currentAcceptedCardTypeIds').length(1)
     })
@@ -119,10 +120,11 @@ describe('Controller: settings/card-types', () => {
     })
 
     it('should pass context data to the response method', () => {
-      expect(mockResponse.args[0][3]).to.have.property('cardTypes').to.have.property('Enabled debit cards').to.have.length(1).to.include('Visa debit')
-      expect(mockResponse.args[0][3].cardTypes).to.have.property('Not enabled debit cards').to.have.length(0)
-      expect(mockResponse.args[0][3].cardTypes).to.have.property('Enabled credit cards').to.have.length(0)
-      expect(mockResponse.args[0][3].cardTypes).to.have.property('Not enabled credit cards').to.have.length(2).to.include('Visa credit')
+      expect(mockResponse.args[0][3]).to.have.property('cardTypes').to.have.property('debit/enabled')
+        .to.have.property('cards').to.deep.equal(['Visa debit'])
+      expect(mockResponse.args[0][3].cardTypes).to.have.property('debit/disabled').to.have.property('cards').to.have.length(0)
+      expect(mockResponse.args[0][3].cardTypes).to.have.property('credit/enabled').to.have.property('cards').to.have.length(0)
+      expect(mockResponse.args[0][3].cardTypes['credit/disabled'].cards).to.deep.equal(['Visa credit', 'American Express'])
       expect(mockResponse.args[0][3]).to.have.property('isAdminUser').to.equal(false)
       expect(mockResponse.args[0][3]).to.have.property('currentAcceptedCardTypeIds').length(1)
     })
@@ -185,10 +187,15 @@ describe('Controller: settings/card-types', () => {
       expect(mockPostAcceptedCardsForServiceAndAccountType.called).to.be.false // eslint-disable-line no-unused-expressions
     })
 
-    it('should redirect to same page with an error', () => {
-      expect(req.flash).to.have.been.calledWith('noCardTypesSelectedError', 'You must choose at least one card') // eslint-disable-line no-unused-expressions
-      expect(res.redirect.calledOnce).to.be.true // eslint-disable-line no-unused-expressions
-      expect(res.redirect.args[0][0]).to.include(paths.simplifiedAccount.settings.cardTypes.index)
+    it('should should pass context data to the response method with an error', () => {
+      expect(mockResponse.args[0][3]).to.have.property('errors').to.deep.equal({ summary: [{ text: 'You must choose at least one card' }] })
+      expect(mockResponse.args[0][3]).to.have.property('cardTypes').to.have.property('debitCards').length(1)
+      expect(mockResponse.args[0][3].cardTypes.debitCards[0]).to.deep.include({ text: 'Visa debit', checked: false })
+      expect(mockResponse.args[0][3]).to.have.property('cardTypes').to.have.property('creditCards').length(2)
+      expect(mockResponse.args[0][3].cardTypes.creditCards[0]).to.deep.include({ text: 'Visa credit', checked: false })
+      expect(mockResponse.args[0][3].cardTypes.creditCards[1]).to.deep.include({ text: 'American Express', checked: false })
+      expect(mockResponse.args[0][3]).to.have.property('isAdminUser').to.equal(true)
+      expect(mockResponse.args[0][3]).to.have.property('currentAcceptedCardTypeIds').length(1)
     })
   })
 })

app/utils/simplified-account/format/format-card-types.js

@@ -56,14 +56,26 @@ const formatCardTypesForAdminTemplate = (allCards, acceptedCards, account) => {
 const formatCardTypesForNonAdminTemplate = (allCards, acceptedCards) => {
   const acceptedCardTypeIds = acceptedCards.map(card => card.id)
   const formattedCardTypes = {
-    'Enabled debit cards': [],
-    'Not enabled debit cards': [],
-    'Enabled credit cards': [],
-    'Not enabled credit cards': []
+    'debit/enabled': {
+      cards: [],
+      heading: 'Enabled debit cards'
+    },
+    'debit/disabled': {
+      cards: [],
+      heading: 'Not enabled debit cards'
+    },
+    'credit/enabled': {
+      cards: [],
+      heading: 'Enabled credit cards'
+    },
+    'credit/disabled': {
+      cards: [],
+      heading: 'Not enabled credit cards'
+    }
   }
   allCards.forEach(card => {
-    const cardIsEnabled = acceptedCardTypeIds.includes(card.id) ? 'Enabled' : 'Not enabled'
-    formattedCardTypes[`${cardIsEnabled} ${card.type.toLowerCase()} cards`].push(formatLabel(card))
+    const cardIsEnabled = acceptedCardTypeIds.includes(card.id) ? 'enabled' : 'disabled'
+    formattedCardTypes[`${card.type.toLowerCase()}/${cardIsEnabled}`].cards.push(formatLabel(card))
   })
   return formattedCardTypes
 }

app/utils/simplified-account/format/format-card-types.test.js

@@ -107,10 +107,10 @@ describe('format-card-types for template', () => {
       const acceptedCards = [...allCards].filter(card => card.id !== 'id-001' && card.id !== 'id-002')
       const account = { requires3ds: true }
       const cards = formatCardTypesForTemplate(allCards, acceptedCards, account, false)
-      expect(cards['Enabled debit cards']).to.have.length(2).to.deep.equal(['Mastercard debit', 'Maestro'])
-      expect(cards['Not enabled debit cards']).to.have.length(1).to.deep.equal(['Visa debit'])
-      expect(cards['Enabled credit cards']).to.have.length(2).to.deep.equal(['American Express', 'JCB'])
-      expect(cards['Not enabled credit cards']).to.have.length(1).to.deep.equal(['Visa credit'])
+      expect(cards['debit/enabled']).to.deep.equal({ cards: ['Mastercard debit', 'Maestro'], heading: 'Enabled debit cards' })
+      expect(cards['debit/disabled']).to.deep.equal({ cards: ['Visa debit'], heading: 'Not enabled debit cards' })
+      expect(cards['credit/enabled']).to.deep.equal({ cards: ['American Express', 'JCB'], heading: 'Enabled credit cards' })
+      expect(cards['credit/disabled']).to.deep.equal({ cards: ['Visa credit'], heading: 'Not enabled credit cards' })
     })
   })
 })

app/views/simplified-account/settings/card-types/index.njk

@@ -67,13 +67,13 @@
       }}
     </form>
   {% else %}
-    {% for category, items in cardTypes %}
-      {% if (items.length > 0) %}
+    {% for category, item in cardTypes %}
+      {% if (item.cards.length > 0) %}
         <h3 class="govuk-heading-m">
-          {{ category }}
+          {{ item.heading }}
         </h3>
         {% set cardList = [] %}
-        {% for card in items %}
+        {% for card in item.cards %}
           {% set cardListItem = {
             key: {
               text: card,