Set flashMessages once per request instead of calling getMessages() ad hoc

src/apps/companies/apps/referrals/send-referral/controllers.js

@@ -26,7 +26,9 @@ function renderSendReferralForm(req, res) {
         companyId: id,
         cancelUrl: urls.companies.detail(id),
         sendingAdviserTeamName,
-        flashMessages: res.locals.getMessages(),
+
+        // TODO: This should not be necessary as flashMessages are included in global props
+        // flashMessages: res.locals.flashMessages,
       },
     }
   )

src/middleware/__test__/user-locals.test.js

@@ -1,93 +1,29 @@
-const proxyquire = require('proxyquire')
-const { faker } = require('@faker-js/faker')
-
-const words = faker.lorem.words
-const modulePath = '../user-locals'
-
-describe('user-locals middleware', () => {
-  let req, res, next
-  beforeEach(() => {
-    res = {
-      locals: {},
+const { parseFlashMessages } = require('../user-locals')
+
+describe('parseFlashMessages', () => {
+  it('valid JSON in success:with-body', () => {
+    const BODY = { heading: 'foo bar', body: 'baz bing' }
+    const RAW_FLASH_MESSAGES = {
+      success: ['foo', '{"test":1}'],
+      error: ['bar', 'baz'],
+      'success:with-body': [JSON.stringify(BODY)],
     }
-    next = sinon.spy()
-  })
 
-  describe('#getMessages', () => {
-    afterEach(() => {
-      expect(next).to.have.been.called
+    expect(parseFlashMessages(RAW_FLASH_MESSAGES)).to.deep.equal({
+      ...RAW_FLASH_MESSAGES,
+      'success:with-body': [BODY],
     })
+  })
 
-    context('With no messages in the flash', () => {
-      it('returns the values', () => {
-        const userLocals = require(modulePath)
-        const flash = sinon.stub().returns({})
-        req = {
-          path: '',
-          flash,
-        }
-        userLocals(req, res, next)
-
-        const messages = res.locals.getMessages()
-
-        expect(flash).to.have.been.calledWith()
-        expect(messages).to.deep.equal({})
-      })
-    })
-
-    context('With valid messages in the flash', () => {
-      it('returns the values', () => {
-        const userLocals = require(modulePath)
-        const flashData = {
-          success: [words(5), '{"test":1}'],
-          error: [words(5), words(5)],
-          'success:with-body': [
-            JSON.stringify({ heading: words(2), body: words(10) }),
-          ],
-        }
-        const flash = sinon.stub().returns(flashData)
-        req = {
-          path: '',
-          flash,
-        }
-
-        userLocals(req, res, next)
-
-        const messages = res.locals.getMessages()
-
-        expect(messages).to.equal(flashData)
-        expect(typeof messages['success:with-body'][0]).to.equal('object')
-        expect(typeof messages.success[1]).to.equal('string')
-        expect(flash).to.have.been.calledWith()
-      })
-    })
-
-    context('With invalid JSON messages in the flash', () => {
-      it('returns the string and reports the error', () => {
-        const captureException = sinon.spy()
-        const userLocals = proxyquire(modulePath, {
-          '../lib/reporter': {
-            captureException,
-          },
-        })
-        const flashData = {
-          'success:with-body': [`heading: ${words(2)}`],
-        }
-        const flash = sinon.stub().returns(flashData)
-        req = {
-          path: '',
-          flash,
-        }
-
-        userLocals(req, res, next)
-
-        const messages = res.locals.getMessages()
+  it('invalid JSON in success:with-body', () => {
+    const RAW_FLASH_MESSAGES = {
+      success: ['foo', '{"test":1}'],
+      error: ['bar', 'baz'],
+      'success:with-body': ["I'm no valid JSON"],
+    }
 
-        expect(messages).to.equal(flashData)
-        expect(typeof messages['success:with-body'][0]).to.equal('string')
-        expect(flash).to.have.been.calledWith()
-        expect(captureException).to.have.been.called
-      })
-    })
+    expect(parseFlashMessages(RAW_FLASH_MESSAGES)).to.deep.equal(
+      RAW_FLASH_MESSAGES
+    )
   })
 })

src/middleware/react-global-props.js

@@ -9,6 +9,7 @@ const transformModulePermissions = (modules) =>
 module.exports = () => {
   return function reactGlobalProps(req, res, next) {
     res.locals.globalProps = {
+      flashMessages: res.locals.flashMessages,
       sentryDsn: config.sentryDsn,
       sentryEnvironment: config.sentryEnvironment,
       csrfToken: req.csrfToken(),

src/middleware/user-locals.js

@@ -27,7 +27,15 @@
   }
 }
 
-module.exports = (req, res, next) => {
+const parseFlashMessages = (rawFlashMessages) =>
+  Object.fromEntries(
+    Object.entries(rawFlashMessages).map(([k, v]) => [
+      k,
+      k.endsWith(':with-body') ? v.map(convertValueToJson) : v,
+    ])
+  )
+
+const userLocals = (req, res, next) => {
   const userPermissions = get(res, 'locals.user.permissions')
   const userProfile = config.oauth.bypassSSO
     ? null
@@ -37,28 +45,22 @@
     filterNonPermittedItem(userPermissions)
   )
 
-  Object.assign(res.locals, {
-    PERMITTED_APPLICATIONS: config.oauth.bypassSSO
-      ? [{ key: 'datahub-crm' }]
-      : permittedApplications,
-    ALLOWED_APPS: permittedNavItems.reduce((apps, { headerKey }) => {
-      headerKey && apps.push(headerKey)
-      return apps
-    }, []),
-    ACTIVE_KEY: getActiveHeaderKey(req.path, permittedNavItems),
-
-    getMessages() {
-      const items = req.flash()
+  res.locals.PERMITTED_APPLICATIONS = config.oauth.bypassSSO
+    ? [{ key: 'datahub-crm' }]
+    : permittedApplications
 
-      for (const [key, values] of Object.entries(items)) {
-        if (key.endsWith(':with-body')) {
-          items[key] = values.map(convertValueToJson)
-        }
-      }
+  res.locals.ALLOWED_APPS = permittedNavItems.reduce((apps, { headerKey }) => {
+    headerKey && apps.push(headerKey)
+    return apps
+  }, [])
 
-      return items
-    },
-  })
+  res.locals.ACTIVE_KEY = getActiveHeaderKey(req.path, permittedNavItems)
+  res.locals.flashMessages = parseFlashMessages(req.flash())
 
   next()
 }
+
+module.exports = {
+  parseFlashMessages,
+  userLocals,
+}

src/server.js

@@ -20,7 +20,7 @@ const currentJourney = require('./modules/form/current-journey')
 const nunjucks = require('./config/nunjucks')
 const headers = require('./middleware/headers')
 const locals = require('./middleware/locals')
-const userLocals = require('./middleware/user-locals')
+const { userLocals } = require('./middleware/user-locals')
 const user = require('./middleware/user')
 const auth = require('./middleware/auth')
 const store = require('./middleware/store')

src/templates/_macros/common/local-header.njk

@@ -14,7 +14,7 @@
  #}
 {% macro LocalHeader(props) %}
   {% set breadcrumbs = props.breadcrumbs | default(getBreadcrumbs()) %}
-  {% set messages = props.messages | default(getMessages() if getMessages) %}
+  {% set messages = props.messages | default(flashMessages) %}
   {% set modifier = props.modifier | concat('') | reverse | join(' c-local-header--') if props.modifier %}
 
   {% if props %}