From 872545f1c060e6f269ac36ef68b390c914d858ab Mon Sep 17 00:00:00 2001
From: Thomas McGowan <tpmcgowan@users.noreply.github.com>
Date: Sun, 12 May 2024 22:09:24 +0100
Subject: [PATCH 01/10] Add client method to get prison config

---
 server/data/orchestrationApiClient.test.ts | 15 ++++++++++
 server/data/orchestrationApiClient.ts      |  9 +++++-
 server/data/orchestrationApiTypes.ts       |  2 ++
 server/routes/testutils/testData.ts        | 35 +++++++++++++++++++++-
 4 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/server/data/orchestrationApiClient.test.ts b/server/data/orchestrationApiClient.test.ts
index c37eb5b9..0bdaee29 100644
--- a/server/data/orchestrationApiClient.test.ts
+++ b/server/data/orchestrationApiClient.test.ts
@@ -70,4 +70,19 @@ describe('orchestrationApiClient', () => {
       expect(result).toStrictEqual(visitors)
     })
   })
+
+  describe('getPrison', () => {
+    it('should get a prison by prisonCode', async () => {
+      const prison = TestData.prisonDto()
+
+      fakeOrchestrationApi
+        .get(`/config/prisons/prison/${prison.code}`)
+        .matchHeader('authorization', `Bearer ${token}`)
+        .reply(200, prison)
+
+      const result = await orchestrationApiClient.getPrison(prison.code)
+
+      expect(result).toStrictEqual(prison)
+    })
+  })
 })
diff --git a/server/data/orchestrationApiClient.ts b/server/data/orchestrationApiClient.ts
index 1d3eb72e..b83c89bc 100644
--- a/server/data/orchestrationApiClient.ts
+++ b/server/data/orchestrationApiClient.ts
@@ -1,6 +1,6 @@
 import RestClient from './restClient'
 import config, { ApiConfig } from '../config'
-import { AuthDetailDto, BookerReference, PrisonerInfoDto, VisitorInfoDto } from './orchestrationApiTypes'
+import { AuthDetailDto, BookerReference, PrisonDto, PrisonerInfoDto, VisitorInfoDto } from './orchestrationApiTypes'
 
 export default class OrchestrationApiClient {
   private restClient: RestClient
@@ -9,6 +9,8 @@ export default class OrchestrationApiClient {
     this.restClient = new RestClient('orchestrationApiClient', config.apis.orchestration as ApiConfig, token)
   }
 
+  // public-booker-controller
+
   async getBookerReference(authDetailDto: AuthDetailDto): Promise<BookerReference> {
     return this.restClient.put({
       path: '/public/booker/register/auth',
@@ -23,4 +25,9 @@ export default class OrchestrationApiClient {
   async getVisitors(bookerReference: string, prisonerNumber: string): Promise<VisitorInfoDto[]> {
     return this.restClient.get({ path: `/public/booker/${bookerReference}/prisoners/${prisonerNumber}/visitors` })
   }
+
+  // orchestration-prisons-config-controller
+  async getPrison(prisonCode: string): Promise<PrisonDto> {
+    return this.restClient.get({ path: `/config/prisons/prison/${prisonCode}` })
+  }
 }
diff --git a/server/data/orchestrationApiTypes.ts b/server/data/orchestrationApiTypes.ts
index 7e1b5266..cdd92bd6 100644
--- a/server/data/orchestrationApiTypes.ts
+++ b/server/data/orchestrationApiTypes.ts
@@ -3,6 +3,8 @@ import { components } from '../@types/orchestration-api'
 export type AuthDetailDto = components['schemas']['AuthDetailDto']
 export type BookerReference = components['schemas']['BookerReference']
 
+export type PrisonDto = components['schemas']['PrisonDto']
+
 export type PrisonerInfoDto = components['schemas']['PrisonerInfoDto']
 
 export type VisitorInfoDto = components['schemas']['VisitorInfoDto']
diff --git a/server/routes/testutils/testData.ts b/server/routes/testutils/testData.ts
index 9b80ddf9..28c538b7 100644
--- a/server/routes/testutils/testData.ts
+++ b/server/routes/testutils/testData.ts
@@ -1,4 +1,10 @@
-import type { AuthDetailDto, BookerReference, PrisonerInfoDto, VisitorInfoDto } from '../../data/orchestrationApiTypes'
+import type {
+  AuthDetailDto,
+  BookerReference,
+  PrisonDto,
+  PrisonerInfoDto,
+  VisitorInfoDto,
+} from '../../data/orchestrationApiTypes'
 
 export default class TestData {
   static authDetailDto = ({
@@ -9,6 +15,33 @@ export default class TestData {
 
   static bookerReference = ({ value = 'aaaa-bbbb-cccc' }: Partial<BookerReference> = {}): BookerReference => ({ value })
 
+  static prisonDto = ({
+    code = 'HEI',
+    prisonName = 'Hewell (HMP)',
+    active = true,
+    policyNoticeDaysMax = 28,
+    policyNoticeDaysMin = 2,
+    maxTotalVisitors = 6,
+    maxAdultVisitors = 2,
+    maxChildVisitors = 4,
+    adultAgeYears = 16,
+    excludeDates = [],
+    clients = [],
+  }: Partial<PrisonDto> = {}): PrisonDto =>
+    ({
+      code,
+      prisonName,
+      active,
+      policyNoticeDaysMax,
+      policyNoticeDaysMin,
+      maxTotalVisitors,
+      maxAdultVisitors,
+      maxChildVisitors,
+      adultAgeYears,
+      excludeDates,
+      clients,
+    }) as PrisonDto
+
   static prisonerInfoDto = ({
     prisonerNumber = 'A1234BC',
     firstName = 'JOHN',

From f6c48237d6d0b05e1cba7debe37360ae70ca0e88 Mon Sep 17 00:00:00 2001
From: Thomas McGowan <tpmcgowan@users.noreply.github.com>
Date: Sun, 12 May 2024 22:19:05 +0100
Subject: [PATCH 02/10] Add prison service

---
 server/services/index.ts              |  6 ++++-
 server/services/prisonService.test.ts | 37 +++++++++++++++++++++++++++
 server/services/prisonService.ts      | 16 ++++++++++++
 server/services/testutils/mocks.ts    |  5 ++--
 4 files changed, 61 insertions(+), 3 deletions(-)
 create mode 100644 server/services/prisonService.test.ts
 create mode 100644 server/services/prisonService.ts

diff --git a/server/services/index.ts b/server/services/index.ts
index bfe5fed5..c000a42c 100644
--- a/server/services/index.ts
+++ b/server/services/index.ts
@@ -1,17 +1,21 @@
 import { dataAccess } from '../data'
 import BookerService from './bookerService'
+import PrisonService from './prisonService'
 
 export const services = () => {
   const { applicationInfo, hmppsAuthClient, orchestrationApiClientBuilder } = dataAccess()
 
   const bookerService = new BookerService(orchestrationApiClientBuilder, hmppsAuthClient)
 
+  const prisonService = new PrisonService(orchestrationApiClientBuilder, hmppsAuthClient)
+
   return {
     applicationInfo,
     bookerService,
+    prisonService,
   }
 }
 
 export type Services = ReturnType<typeof services>
 
-export { BookerService }
+export { BookerService, PrisonService }
diff --git a/server/services/prisonService.test.ts b/server/services/prisonService.test.ts
new file mode 100644
index 00000000..3ecb7788
--- /dev/null
+++ b/server/services/prisonService.test.ts
@@ -0,0 +1,37 @@
+import TestData from '../routes/testutils/testData'
+import { createMockHmppsAuthClient, createMockOrchestrationApiClient } from '../data/testutils/mocks'
+import PrisonService from './prisonService'
+
+const token = 'some token'
+
+describe('Prison service', () => {
+  const hmppsAuthClient = createMockHmppsAuthClient()
+
+  const orchestrationApiClient = createMockOrchestrationApiClient()
+  const orchestrationApiClientFactory = jest.fn()
+
+  let prisonService: PrisonService
+
+  beforeEach(() => {
+    hmppsAuthClient.getSystemClientToken.mockResolvedValue(token)
+
+    orchestrationApiClientFactory.mockReturnValue(orchestrationApiClient)
+    prisonService = new PrisonService(orchestrationApiClientFactory, hmppsAuthClient)
+  })
+
+  afterEach(() => {
+    jest.resetAllMocks()
+  })
+
+  describe('getPrison', () => {
+    it('should return prison config for given prison code', async () => {
+      const prison = TestData.prisonDto()
+      orchestrationApiClient.getPrison.mockResolvedValue(prison)
+
+      const results = await prisonService.getPrison(prison.code)
+
+      expect(orchestrationApiClient.getPrison).toHaveBeenCalledWith(prison.code)
+      expect(results).toStrictEqual(prison)
+    })
+  })
+})
diff --git a/server/services/prisonService.ts b/server/services/prisonService.ts
new file mode 100644
index 00000000..97852291
--- /dev/null
+++ b/server/services/prisonService.ts
@@ -0,0 +1,16 @@
+import { HmppsAuthClient, OrchestrationApiClient, RestClientBuilder } from '../data'
+import { PrisonDto } from '../data/orchestrationApiTypes'
+
+export default class PrisonService {
+  constructor(
+    private readonly orchestrationApiClientFactory: RestClientBuilder<OrchestrationApiClient>,
+    private readonly hmppsAuthClient: HmppsAuthClient,
+  ) {}
+
+  async getPrison(prisonCode: string): Promise<PrisonDto> {
+    const token = await this.hmppsAuthClient.getSystemClientToken()
+    const orchestrationApiClient = this.orchestrationApiClientFactory(token)
+
+    return orchestrationApiClient.getPrison(prisonCode)
+  }
+}
diff --git a/server/services/testutils/mocks.ts b/server/services/testutils/mocks.ts
index 1c1fa021..ad4071c2 100644
--- a/server/services/testutils/mocks.ts
+++ b/server/services/testutils/mocks.ts
@@ -17,9 +17,10 @@ jest.mock('../../applicationInfo', () => {
   return jest.fn(() => testAppInfo)
 })
 
-import { BookerService } from '..'
+import { BookerService, PrisonService } from '..'
 
 jest.mock('..')
 
-// eslint-disable-next-line import/prefer-default-export
 export const createMockBookerService = () => new BookerService(null, null) as jest.Mocked<BookerService>
+
+export const createMockPrisonService = () => new PrisonService(null, null) as jest.Mocked<PrisonService>

From 4bfa369d69395d1b4880f3ce8a95ce4d88adb5ee Mon Sep 17 00:00:00 2001
From: Thomas McGowan <tpmcgowan@users.noreply.github.com>
Date: Mon, 13 May 2024 07:27:49 +0100
Subject: [PATCH 03/10] Get prison visitor config for Select visitors page

---
 cypress.config.ts                             |  1 +
 integration_tests/e2e/bookingJourney.cy.ts    |  7 +++-
 integration_tests/mockApis/orchestration.ts   | 36 ++++++++++++++++++-
 integration_tests/pages/home.ts               |  4 ++-
 server/@types/bapv.d.ts                       |  7 +++-
 server/routes/bookingJourney/index.ts         |  2 +-
 .../selectVisitorsController.ts               | 20 +++++++----
 .../pages/bookingJourney/selectVisitors.njk   | 15 ++++++--
 8 files changed, 77 insertions(+), 15 deletions(-)

diff --git a/cypress.config.ts b/cypress.config.ts
index 7bf79f7f..c91dee99 100644
--- a/cypress.config.ts
+++ b/cypress.config.ts
@@ -14,6 +14,7 @@ export default defineConfig({
     configFile: 'reporter-config.json',
   },
   taskTimeout: 60000,
+  viewportHeight: 1400,
   e2e: {
     setupNodeEvents(on) {
       on('task', {
diff --git a/integration_tests/e2e/bookingJourney.cy.ts b/integration_tests/e2e/bookingJourney.cy.ts
index cd6b2e15..9e2dfa2c 100644
--- a/integration_tests/e2e/bookingJourney.cy.ts
+++ b/integration_tests/e2e/bookingJourney.cy.ts
@@ -14,9 +14,14 @@ context('Booking journey', () => {
     cy.task('stubGetPrisoners', { prisoners: [TestData.prisonerInfoDto()] })
     cy.signIn()
 
+    // Home page - prisoner shown
     const homePage = Page.verifyOnPage(HomePage)
     homePage.prisonerName().contains('John Smith')
-    homePage.startButton().contains('Start')
+
+    // Start booking journey
+    cy.task('stubGetPrison')
+    cy.task('stubGetVisitors', {})
+    homePage.startBooking()
 
     // TODO add to this test as booking journey implemented
   })
diff --git a/integration_tests/mockApis/orchestration.ts b/integration_tests/mockApis/orchestration.ts
index 94ef800f..8aa3ace2 100644
--- a/integration_tests/mockApis/orchestration.ts
+++ b/integration_tests/mockApis/orchestration.ts
@@ -1,7 +1,7 @@
 import { SuperAgentRequest } from 'superagent'
 import { stubFor } from './wiremock'
 import TestData from '../../server/routes/testutils/testData'
-import { BookerReference, PrisonerInfoDto } from '../../server/data/orchestrationApiTypes'
+import { BookerReference, PrisonDto, PrisonerInfoDto, VisitorInfoDto } from '../../server/data/orchestrationApiTypes'
 
 export default {
   stubGetBookerReference: (bookerReference = TestData.bookerReference()): SuperAgentRequest =>
@@ -36,6 +36,40 @@ export default {
       },
     }),
 
+  stubGetVisitors: ({
+    bookerReference = TestData.bookerReference(),
+    prisonerNumber = TestData.prisonerInfoDto().prisonerNumber,
+    visitors = [TestData.visitorInfoDto()],
+  }: {
+    bookerReference: BookerReference
+    prisonerNumber: string
+    visitors: VisitorInfoDto[]
+  }): SuperAgentRequest =>
+    stubFor({
+      request: {
+        method: 'GET',
+        url: `/orchestration/public/booker/${bookerReference.value}/prisoners/${prisonerNumber}/visitors`,
+      },
+      response: {
+        status: 200,
+        headers: { 'Content-Type': 'application/json;charset=UTF-8' },
+        jsonBody: visitors,
+      },
+    }),
+
+  stubGetPrison: (prisonDto: PrisonDto = TestData.prisonDto()): SuperAgentRequest =>
+    stubFor({
+      request: {
+        method: 'GET',
+        url: `/orchestration/config/prisons/prison/${prisonDto.code}`,
+      },
+      response: {
+        status: 200,
+        headers: { 'Content-Type': 'application/json;charset=UTF-8' },
+        jsonBody: prisonDto,
+      },
+    }),
+
   stubOrchestrationPing: (status = 200): SuperAgentRequest =>
     stubFor({
       request: {
diff --git a/integration_tests/pages/home.ts b/integration_tests/pages/home.ts
index 7e353bbb..acaf3673 100644
--- a/integration_tests/pages/home.ts
+++ b/integration_tests/pages/home.ts
@@ -7,5 +7,7 @@ export default class HomePage extends Page {
 
   prisonerName = (): PageElement => cy.get('[data-test=prisoner-name]')
 
-  startButton = (): PageElement => cy.get('[data-test="start-booking"]')
+  startBooking = (): void => {
+    cy.get('[data-test="start-booking"]').click()
+  }
 }
diff --git a/server/@types/bapv.d.ts b/server/@types/bapv.d.ts
index 06f66b62..e6d60239 100644
--- a/server/@types/bapv.d.ts
+++ b/server/@types/bapv.d.ts
@@ -1,4 +1,4 @@
-import { PrisonerInfoDto, VisitorInfoDto } from '../data/orchestrationApiTypes'
+import { PrisonDto, PrisonerInfoDto, VisitorInfoDto } from '../data/orchestrationApiTypes'
 
 export type Booker = {
   reference: string
@@ -9,8 +9,13 @@ export type Booker = {
 export type BookingJourneyData = {
   // selected prisoner for this visit
   prisoner: PrisonerInfoDto
+
+  // prison for this visit
+  prison?: PrisonDto
+
   // all possible visitors for this visit
   allVisitors?: VisitorInfoDto[]
+
   // selected visitors for this visit
   selectedVisitors?: VisitorInfoDto[]
 }
diff --git a/server/routes/bookingJourney/index.ts b/server/routes/bookingJourney/index.ts
index 84818743..87a3383e 100644
--- a/server/routes/bookingJourney/index.ts
+++ b/server/routes/bookingJourney/index.ts
@@ -16,7 +16,7 @@ export default function routes(services: Services): Router {
     router.post(path, ...validationChain, asyncMiddleware(handler))
 
   const selectPrisonerController = new SelectPrisonerController()
-  const selectVisitorsController = new SelectVisitorsController(services.bookerService)
+  const selectVisitorsController = new SelectVisitorsController(services.bookerService, services.prisonService)
   const dateAndTimeController = new DateAndTimeController(services.bookerService)
 
   // TODO need session checks for each stage to validate what is in session - add middleware here to apply to all booking journey routes?
diff --git a/server/routes/bookingJourney/selectVisitorsController.ts b/server/routes/bookingJourney/selectVisitorsController.ts
index 768b4d12..e5b23b71 100644
--- a/server/routes/bookingJourney/selectVisitorsController.ts
+++ b/server/routes/bookingJourney/selectVisitorsController.ts
@@ -1,20 +1,26 @@
 import type { RequestHandler } from 'express'
 import { ValidationChain, body, validationResult } from 'express-validator'
-import { BookerService } from '../../services'
+import { BookerService, PrisonService } from '../../services'
 
 export default class SelectVisitorsController {
-  public constructor(private readonly bookerService: BookerService) {}
+  public constructor(
+    private readonly bookerService: BookerService,
+    private readonly prisonService: PrisonService,
+  ) {}
 
   public view(): RequestHandler {
     return async (req, res) => {
       const { booker, bookingJourney } = req.session
-      if (!bookingJourney.allVisitors) {
-        bookingJourney.allVisitors = await this.bookerService.getVisitors(
-          booker.reference,
-          booker.prisoners[0].prisonerNumber,
-        )
+
+      if (!bookingJourney.prison) {
+        ;[bookingJourney.prison, bookingJourney.allVisitors] = await Promise.all([
+          this.prisonService.getPrison(bookingJourney.prisoner.prisonCode),
+          this.bookerService.getVisitors(booker.reference, booker.prisoners[0].prisonerNumber),
+        ])
       }
+
       res.render('pages/bookingJourney/selectVisitors', {
+        prison: bookingJourney.prison,
         visitors: bookingJourney.allVisitors,
       })
     }
diff --git a/server/views/pages/bookingJourney/selectVisitors.njk b/server/views/pages/bookingJourney/selectVisitors.njk
index 578327ab..25b1fcbf 100644
--- a/server/views/pages/bookingJourney/selectVisitors.njk
+++ b/server/views/pages/bookingJourney/selectVisitors.njk
@@ -13,10 +13,19 @@
 
       <h1 class="govuk-heading-l">Who is going on the visit?</h1>
 
-      <p>Up to [x] people can visit someone at [prison name]. This includes</p>
+      <p>
+        Up to {{ prison.maxTotalVisitors }} {{ "person" | pluralise(prison.maxTotalVisitors, "people") }}
+        can visit someone at {{ prison.prisonName }}. This includes
+      </p>
       <ul class="govuk-list govuk-list--bullet">
-        <li>[x] people [x] years old or older</li>
-        <li>[x] people under [x] years old.</li>
+        <li>
+          {{ prison.maxAdultVisitors }} {{ "person" | pluralise(prison.maxAdultVisitors, "people") }}
+          {{ prison.adultAgeYears }} {{ "year" | pluralise(prison.adultAgeYears) }} old or older
+        </li>
+        <li>
+          {{ prison.maxChildVisitors }} {{ "person" | pluralise(prison.maxChildVisitors, "people") }}
+          under {{ prison.adultAgeYears }} {{ "year" | pluralise(prison.adultAgeYears) }} old.
+        </li>
       </ul>
       <p>At least one visitor must be 18 years or older</p>
 

From adf81c24115c38b2151a9c95ea1fa147563c1565 Mon Sep 17 00:00:00 2001
From: Thomas McGowan <tpmcgowan@users.noreply.github.com>
Date: Mon, 13 May 2024 07:59:42 +0100
Subject: [PATCH 04/10] Add form error handling and error component

---
 server/@types/express/index.d.ts              |  1 +
 .../selectVisitorsController.ts               | 11 +++--
 server/utils/nunjucksSetup.test.ts            | 48 +++++++++++++++++++
 server/utils/nunjucksSetup.ts                 | 23 +++++++++
 .../pages/bookingJourney/selectVisitors.njk   |  6 ++-
 server/views/partials/errorSummary.njk        | 14 ++++++
 6 files changed, 99 insertions(+), 4 deletions(-)
 create mode 100644 server/views/partials/errorSummary.njk

diff --git a/server/@types/express/index.d.ts b/server/@types/express/index.d.ts
index 890a0448..1733f933 100644
--- a/server/@types/express/index.d.ts
+++ b/server/@types/express/index.d.ts
@@ -25,6 +25,7 @@ export declare global {
 
     interface Request {
       id: string
+      flash(type: 'errors', message: FlashErrorMessage): number
       logout(done: (err: unknown) => void): void
     }
 
diff --git a/server/routes/bookingJourney/selectVisitorsController.ts b/server/routes/bookingJourney/selectVisitorsController.ts
index e5b23b71..f2367b27 100644
--- a/server/routes/bookingJourney/selectVisitorsController.ts
+++ b/server/routes/bookingJourney/selectVisitorsController.ts
@@ -19,7 +19,11 @@ export default class SelectVisitorsController {
         ])
       }
 
+      // TODO pre-populate form (e.g. if coming from Back link or Change answers)
+
       res.render('pages/bookingJourney/selectVisitors', {
+        errors: req.flash('errors'),
+        formValues: req.flash('formValues')?.[0] || {},
         prison: bookingJourney.prison,
         visitors: bookingJourney.allVisitors,
       })
@@ -29,9 +33,10 @@ export default class SelectVisitorsController {
   public submit(): RequestHandler {
     return async (req, res) => {
       const errors = validationResult(req)
-
       if (!errors.isEmpty()) {
-        throw new Error(JSON.stringify(errors.array())) // TODO add error messages to form
+        req.flash('errors', errors.array())
+        req.flash('formValues', req.body)
+        return res.redirect('/book-a-visit/select-visitors')
       }
 
       const { bookingJourney } = req.session
@@ -41,7 +46,7 @@ export default class SelectVisitorsController {
 
       bookingJourney.selectedVisitors = selectedVisitors
 
-      res.redirect('/book-a-visit/select-date-and-time')
+      return res.redirect('/book-a-visit/select-date-and-time')
     }
   }
 
diff --git a/server/utils/nunjucksSetup.test.ts b/server/utils/nunjucksSetup.test.ts
index d89dddf8..e211ec2f 100644
--- a/server/utils/nunjucksSetup.test.ts
+++ b/server/utils/nunjucksSetup.test.ts
@@ -1,4 +1,5 @@
 import express from 'express'
+import { FieldValidationError } from 'express-validator'
 import nunjucksSetup from './nunjucksSetup'
 
 describe('Nunjucks Filters', () => {
@@ -37,6 +38,53 @@ describe('Nunjucks Filters', () => {
     })
   })
 
+  describe('errorSummaryList', () => {
+    it('should map errors to text and href', () => {
+      const errors = <FieldValidationError[]>[
+        { msg: 'Field 1 message', path: 'field1' },
+        { msg: 'Field 2 message', path: 'field2' },
+      ]
+      const expectedResult = [
+        { text: 'Field 1 message', href: '#field1-error' },
+        { text: 'Field 2 message', href: '#field2-error' },
+      ]
+
+      const result = njk.getFilter('errorSummaryList')(errors)
+      expect(result).toEqual(expectedResult)
+    })
+
+    it('should map error with no path to text with no href', () => {
+      const errors = <FieldValidationError[]>[{ msg: 'Field 1 message with no path' }]
+      const expectedResult = [{ text: 'Field 1 message with no path' }]
+
+      const result = njk.getFilter('errorSummaryList')(errors)
+      expect(result).toEqual(expectedResult)
+    })
+
+    it('should handle empty errors object', () => {
+      const result = njk.getFilter('errorSummaryList')(undefined)
+      expect(result).toEqual([])
+    })
+  })
+
+  describe('findError', () => {
+    it('should find specified error and return errorMessage for GDS components', () => {
+      const errors = <FieldValidationError[]>[
+        { msg: 'Field 1 message', path: 'field1' },
+        { msg: 'Field 2 message', path: 'field2' },
+      ]
+      const expectedResult = { text: 'Field 2 message' }
+
+      const result = njk.getFilter('findError')(errors, 'field2')
+      expect(result).toEqual(expectedResult)
+    })
+
+    it('should handle empty errors object and missing form field', () => {
+      const result = njk.getFilter('findError')(undefined)
+      expect(result).toEqual(null)
+    })
+  })
+
   describe('pluralise', () => {
     describe('Regular plurals', () => {
       it('should return plural form when count is 0', () => {
diff --git a/server/utils/nunjucksSetup.ts b/server/utils/nunjucksSetup.ts
index 6bb03b84..d0e2e25c 100644
--- a/server/utils/nunjucksSetup.ts
+++ b/server/utils/nunjucksSetup.ts
@@ -3,6 +3,7 @@ import path from 'path'
 import nunjucks from 'nunjucks'
 import express from 'express'
 import { formatDuration, intervalToDuration, isAfter } from 'date-fns'
+import { FieldValidationError } from 'express-validator'
 import { formatDate, initialiseName } from './utils'
 import { ApplicationInfo } from '../applicationInfo'
 import config from '../config'
@@ -56,6 +57,28 @@ export default function nunjucksSetup(app: express.Express, applicationInfo: App
     return `${age} old`
   })
 
+  // convert errors to format for GOV.UK error summary component
+  njkEnv.addFilter('errorSummaryList', (errors = []) => {
+    return Object.keys(errors).map(error => {
+      return {
+        text: errors[error].msg,
+        href: errors[error].path ? `#${errors[error].path}-error` : undefined,
+      }
+    })
+  })
+
+  // find specific error and return errorMessage for field validation
+  njkEnv.addFilter('findError', (errors: FieldValidationError[], formFieldId) => {
+    if (!errors || !formFieldId) return null
+    const errorForMessage = errors.find(error => error.path === formFieldId)
+
+    if (errorForMessage === undefined) return null
+
+    return {
+      text: errorForMessage?.msg,
+    }
+  })
+
   njkEnv.addFilter('formatDate', formatDate)
 
   njkEnv.addFilter('initialiseName', initialiseName)
diff --git a/server/views/pages/bookingJourney/selectVisitors.njk b/server/views/pages/bookingJourney/selectVisitors.njk
index 25b1fcbf..52b02859 100644
--- a/server/views/pages/bookingJourney/selectVisitors.njk
+++ b/server/views/pages/bookingJourney/selectVisitors.njk
@@ -11,6 +11,8 @@
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds">
 
+      {% include "partials/errorSummary.njk" %}
+
       <h1 class="govuk-heading-l">Who is going on the visit?</h1>
 
       <p>
@@ -48,7 +50,9 @@
                 classes: "govuk-fieldset__legend--m"
               }
             },
-            items: visitorList
+            items: visitorList,
+            values: formValues.visitorIds,
+            errorMessage: errors | findError('visitorIds')
           }) }}
 
           {{ govukButton({
diff --git a/server/views/partials/errorSummary.njk b/server/views/partials/errorSummary.njk
new file mode 100644
index 00000000..13fafad4
--- /dev/null
+++ b/server/views/partials/errorSummary.njk
@@ -0,0 +1,14 @@
+{% from "govuk/components/error-summary/macro.njk" import govukErrorSummary %}
+
+{% if errors | length %}
+  {% set errorSummaryList = errors | errorSummaryList %}
+
+  <div class="govuk-grid-row">
+    <div class="govuk-grid-column-two-thirds">
+      {{ govukErrorSummary({
+        titleText: "There is a problem",
+        errorList:  errorSummaryList 
+      }) }}
+    </div>
+  </div>
+{% endif %}

From fd51100faf28ddf8021fa57c28bc49aeb872438b Mon Sep 17 00:00:00 2001
From: Thomas McGowan <tpmcgowan@users.noreply.github.com>
Date: Mon, 13 May 2024 14:03:10 +0100
Subject: [PATCH 05/10] Update appSetup.ts to handle sessionData

---
 package-lock.json                   | 87 -----------------------------
 package.json                        |  2 -
 server/routes/testutils/appSetup.ts | 20 +++++--
 3 files changed, 15 insertions(+), 94 deletions(-)

diff --git a/package-lock.json b/package-lock.json
index 894a304d..92c6002a 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -40,7 +40,6 @@
         "@types/bunyan-format": "^0.2.9",
         "@types/compression": "^1.7.5",
         "@types/connect-flash": "0.0.40",
-        "@types/cookie-session": "^2.0.49",
         "@types/csurf": "^1.11.5",
         "@types/express-session": "^1.18.0",
         "@types/http-errors": "^2.0.4",
@@ -58,7 +57,6 @@
         "axe-core": "^4.9.0",
         "cheerio": "^1.0.0-rc.12",
         "concurrently": "^8.2.2",
-        "cookie-session": "^2.1.0",
         "cypress": "^13.8.1",
         "cypress-axe": "^1.5.0",
         "cypress-multi-reporters": "^1.6.4",
@@ -1862,16 +1860,6 @@
         "@types/express": "*"
       }
     },
-    "node_modules/@types/cookie-session": {
-      "version": "2.0.49",
-      "resolved": "https://registry.npmjs.org/@types/cookie-session/-/cookie-session-2.0.49.tgz",
-      "integrity": "sha512-4E/bBjlqLhU5l4iGPR+NkVJH593hpNsT4dC3DJDr+ODm6Qpe13kZQVkezRIb+TYDXaBMemS3yLQ+0leba3jlkQ==",
-      "dev": true,
-      "dependencies": {
-        "@types/express": "*",
-        "@types/keygrip": "*"
-      }
-    },
     "node_modules/@types/cookiejar": {
       "version": "2.1.5",
       "resolved": "https://registry.npmjs.org/@types/cookiejar/-/cookiejar-2.1.5.tgz",
@@ -1987,12 +1975,6 @@
         "@types/node": "*"
       }
     },
-    "node_modules/@types/keygrip": {
-      "version": "1.0.6",
-      "resolved": "https://registry.npmjs.org/@types/keygrip/-/keygrip-1.0.6.tgz",
-      "integrity": "sha512-lZuNAY9xeJt7Bx4t4dx0rYCDqGPW8RXhQZK1td7d4H6E9zYbLoOtjBvfwdTKpsyxQI/2jv+armjX/RW+ZNpXOQ==",
-      "dev": true
-    },
     "node_modules/@types/methods": {
       "version": "1.1.4",
       "resolved": "https://registry.npmjs.org/@types/methods/-/methods-1.1.4.tgz",
@@ -3837,50 +3819,6 @@
         "node": ">= 0.6"
       }
     },
-    "node_modules/cookie-session": {
-      "version": "2.1.0",
-      "resolved": "https://registry.npmjs.org/cookie-session/-/cookie-session-2.1.0.tgz",
-      "integrity": "sha512-u73BDmR8QLGcs+Lprs0cfbcAPKl2HnPcjpwRXT41sEV4DRJ2+W0vJEEZkG31ofkx+HZflA70siRIjiTdIodmOQ==",
-      "dev": true,
-      "dependencies": {
-        "cookies": "0.9.1",
-        "debug": "3.2.7",
-        "on-headers": "~1.0.2",
-        "safe-buffer": "5.2.1"
-      },
-      "engines": {
-        "node": ">= 0.10"
-      }
-    },
-    "node_modules/cookie-session/node_modules/debug": {
-      "version": "3.2.7",
-      "resolved": "https://registry.npmjs.org/debug/-/debug-3.2.7.tgz",
-      "integrity": "sha512-CFjzYYAi4ThfiQvizrFQevTTXHtnCqWfe7x1AhgEscTz6ZbLbfoLRLPugTQyBth6f8ZERVUSyWHFD/7Wu4t1XQ==",
-      "dev": true,
-      "dependencies": {
-        "ms": "^2.1.1"
-      }
-    },
-    "node_modules/cookie-session/node_modules/safe-buffer": {
-      "version": "5.2.1",
-      "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.2.1.tgz",
-      "integrity": "sha512-rp3So07KcdmmKbGvgaNxQSJr7bGVSVk5S9Eq1F+ppbRo70+YeaDxkw5Dd8NPN+GD6bjnYm2VuPuCXmpuYvmCXQ==",
-      "dev": true,
-      "funding": [
-        {
-          "type": "github",
-          "url": "https://github.com/sponsors/feross"
-        },
-        {
-          "type": "patreon",
-          "url": "https://www.patreon.com/feross"
-        },
-        {
-          "type": "consulting",
-          "url": "https://feross.org/support"
-        }
-      ]
-    },
     "node_modules/cookie-signature": {
       "version": "1.0.6",
       "resolved": "https://registry.npmjs.org/cookie-signature/-/cookie-signature-1.0.6.tgz",
@@ -3891,19 +3829,6 @@
       "resolved": "https://registry.npmjs.org/cookiejar/-/cookiejar-2.1.4.tgz",
       "integrity": "sha512-LDx6oHrK+PhzLKJU9j5S7/Y3jM/mUHvD/DeI1WQmJn652iPC5Y4TBzC9l+5OMOXlyTTA+SmVUPm0HQUwpD5Jqw=="
     },
-    "node_modules/cookies": {
-      "version": "0.9.1",
-      "resolved": "https://registry.npmjs.org/cookies/-/cookies-0.9.1.tgz",
-      "integrity": "sha512-TG2hpqe4ELx54QER/S3HQ9SRVnQnGBtKUz5bLQWtYAQ+o6GpgMs6sYUvaiJjVxb+UXwhRhAEP3m7LbsIZ77Hmw==",
-      "dev": true,
-      "dependencies": {
-        "depd": "~2.0.0",
-        "keygrip": "~1.1.0"
-      },
-      "engines": {
-        "node": ">= 0.8"
-      }
-    },
     "node_modules/core-util-is": {
       "version": "1.0.2",
       "resolved": "https://registry.npmjs.org/core-util-is/-/core-util-is-1.0.2.tgz",
@@ -7721,18 +7646,6 @@
         "safe-buffer": "^5.0.1"
       }
     },
-    "node_modules/keygrip": {
-      "version": "1.1.0",
-      "resolved": "https://registry.npmjs.org/keygrip/-/keygrip-1.1.0.tgz",
-      "integrity": "sha512-iYSchDJ+liQ8iwbSI2QqsQOvqv58eJCEanyJPJi+Khyu8smkcKSFUCbPwzFcL7YVtZ6eONjqRX/38caJ7QjRAQ==",
-      "dev": true,
-      "dependencies": {
-        "tsscmp": "1.0.6"
-      },
-      "engines": {
-        "node": ">= 0.6"
-      }
-    },
     "node_modules/keyv": {
       "version": "4.5.4",
       "resolved": "https://registry.npmjs.org/keyv/-/keyv-4.5.4.tgz",
diff --git a/package.json b/package.json
index aec2f73b..c40e1f4e 100644
--- a/package.json
+++ b/package.json
@@ -123,7 +123,6 @@
     "@types/bunyan-format": "^0.2.9",
     "@types/compression": "^1.7.5",
     "@types/connect-flash": "0.0.40",
-    "@types/cookie-session": "^2.0.49",
     "@types/csurf": "^1.11.5",
     "@types/express-session": "^1.18.0",
     "@types/http-errors": "^2.0.4",
@@ -141,7 +140,6 @@
     "axe-core": "^4.9.0",
     "cheerio": "^1.0.0-rc.12",
     "concurrently": "^8.2.2",
-    "cookie-session": "^2.1.0",
     "cypress": "^13.8.1",
     "cypress-axe": "^1.5.0",
     "cypress-multi-reporters": "^1.6.4",
diff --git a/server/routes/testutils/appSetup.ts b/server/routes/testutils/appSetup.ts
index de04b73d..c4754aa5 100644
--- a/server/routes/testutils/appSetup.ts
+++ b/server/routes/testutils/appSetup.ts
@@ -15,8 +15,8 @@ jest.mock('../../applicationInfo', () => {
 })
 
 import express, { Express } from 'express'
-import cookieSession from 'cookie-session'
 import { NotFound } from 'http-errors'
+import type { Session, SessionData } from 'express-session'
 
 import routes from '../index'
 import nunjucksSetup from '../../utils/nunjucksSetup'
@@ -36,15 +36,23 @@ const bookerReference = TestData.bookerReference().value
 
 export const flashProvider = jest.fn()
 
-function appSetup(services: Services, production: boolean, userSupplier: () => Express.User): Express {
+function appSetup(
+  services: Services,
+  production: boolean,
+  userSupplier: () => Express.User,
+  sessionData: SessionData,
+): Express {
   const app = express()
 
   app.set('view engine', 'njk')
 
   nunjucksSetup(app, testAppInfo)
-  app.use(cookieSession({ keys: [''] }))
   app.use((req, res, next) => {
-    req.session.booker = { reference: bookerReference } // emulate populateCurrentBooker()
+    if (!sessionData.booker) {
+      // eslint-disable-next-line no-param-reassign
+      sessionData.booker = { reference: bookerReference } // emulate populateCurrentBooker()
+    }
+    req.session = sessionData as Session & Partial<SessionData>
     req.user = userSupplier()
     req.flash = flashProvider
     res.locals = {
@@ -65,10 +73,12 @@ export function appWithAllRoutes({
   production = false,
   services = {},
   userSupplier = () => user,
+  sessionData = {} as SessionData,
 }: {
   production?: boolean
   services?: Partial<Services>
   userSupplier?: () => Express.User
+  sessionData?: SessionData
 }): Express {
-  return appSetup(services as Services, production, userSupplier)
+  return appSetup(services as Services, production, userSupplier, sessionData)
 }

From 0ddebd578ceb80c90bb24fb5b414030ceeb137f5 Mon Sep 17 00:00:00 2001
From: Thomas McGowan <tpmcgowan@users.noreply.github.com>
Date: Mon, 13 May 2024 14:19:39 +0100
Subject: [PATCH 06/10] Improve Home page test

---
 server/routes/homeController.test.ts | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/server/routes/homeController.test.ts b/server/routes/homeController.test.ts
index 7ce49f39..ba780bb3 100644
--- a/server/routes/homeController.test.ts
+++ b/server/routes/homeController.test.ts
@@ -1,6 +1,7 @@
 import type { Express } from 'express'
 import request from 'supertest'
 import * as cheerio from 'cheerio'
+import { SessionData } from 'express-session'
 import { appWithAllRoutes } from './testutils/appSetup'
 import { createMockBookerService } from '../services/testutils/mocks'
 import TestData from './testutils/testData'
@@ -8,9 +9,11 @@ import TestData from './testutils/testData'
 let app: Express
 
 const bookerService = createMockBookerService()
+let sessionData: SessionData
 
 beforeEach(() => {
-  app = appWithAllRoutes({ services: { bookerService } })
+  sessionData = {} as SessionData
+  app = appWithAllRoutes({ services: { bookerService }, sessionData })
 })
 
 afterEach(() => {
@@ -19,7 +22,7 @@ afterEach(() => {
 
 describe('Home page', () => {
   // For MVP only one prisoner per booker supported; so only first rendered
-  it('should render the home page with the prisoner associated with the booker', () => {
+  it('should render the home page with the prisoner associated with the booker and store prisoners in session', () => {
     const bookerReference = TestData.bookerReference().value
     const prisoner = TestData.prisonerInfoDto()
     bookerService.getPrisoners.mockResolvedValue([prisoner])
@@ -37,6 +40,13 @@ describe('Home page', () => {
         expect($('[data-test="start-booking"]').text().trim()).toBe('Start')
 
         expect(bookerService.getPrisoners).toHaveBeenCalledWith(bookerReference)
+
+        expect(sessionData).toStrictEqual({
+          booker: {
+            reference: bookerReference,
+            prisoners: [prisoner],
+          },
+        } as SessionData)
       })
   })
 
@@ -56,6 +66,13 @@ describe('Home page', () => {
         expect($('[data-test=no-prisoners]').text()).toBe('No prisoner details found.')
 
         expect(bookerService.getPrisoners).toHaveBeenCalledWith(bookerReference)
+
+        expect(sessionData).toStrictEqual({
+          booker: {
+            reference: bookerReference,
+            prisoners: [],
+          },
+        } as SessionData)
       })
   })
 })

From c3d320adb8972ee416e5b8a6032e2e35f42310ea Mon Sep 17 00:00:00 2001
From: Thomas McGowan <tpmcgowan@users.noreply.github.com>
Date: Mon, 13 May 2024 16:54:58 +0100
Subject: [PATCH 07/10] Add tests for select prisoner

---
 .../selectPrisonerController.test.ts          | 68 +++++++++++++++++++
 .../selectPrisonerController.ts               |  3 +-
 2 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 server/routes/bookingJourney/selectPrisonerController.test.ts

diff --git a/server/routes/bookingJourney/selectPrisonerController.test.ts b/server/routes/bookingJourney/selectPrisonerController.test.ts
new file mode 100644
index 00000000..6fa023ec
--- /dev/null
+++ b/server/routes/bookingJourney/selectPrisonerController.test.ts
@@ -0,0 +1,68 @@
+import type { Express } from 'express'
+import request from 'supertest'
+import * as cheerio from 'cheerio'
+import { SessionData } from 'express-session'
+import { appWithAllRoutes } from '../testutils/appSetup'
+import TestData from '../testutils/testData'
+import { PrisonerInfoDto } from '../../data/orchestrationApiTypes'
+
+let app: Express
+let sessionData: SessionData
+
+afterEach(() => {
+  jest.resetAllMocks()
+})
+
+describe('Select prisoner', () => {
+  const bookerReference = TestData.bookerReference().value
+  const prisoner = TestData.prisonerInfoDto()
+
+  it('should clear any exiting bookingJourney session data, populate new data and redirect to select visitors page', () => {
+    sessionData = {
+      booker: { reference: bookerReference, prisoners: [prisoner] },
+      bookingJourney: { prisoner: { prisonerNumber: 'OLD JOURNEY DATA' } as PrisonerInfoDto },
+    } as SessionData
+
+    app = appWithAllRoutes({ services: {}, sessionData })
+
+    return request(app)
+      .post('/book-a-visit/select-prisoner')
+      .send({ prisonerNumber: prisoner.prisonerNumber })
+      .expect(302)
+      .expect('location', '/book-a-visit/select-visitors')
+      .expect(() => {
+        expect(sessionData).toStrictEqual({
+          booker: {
+            reference: bookerReference,
+            prisoners: [prisoner],
+          },
+          bookingJourney: {
+            prisoner,
+          },
+        } as SessionData)
+      })
+  })
+
+  it('should reject an invalid prisoner number', () => {
+    sessionData = {
+      booker: { reference: bookerReference, prisoners: [prisoner] },
+    } as SessionData
+
+    app = appWithAllRoutes({ services: {}, sessionData })
+
+    return request(app)
+      .post('/book-a-visit/select-prisoner')
+      .send({ prisonerNumber: 'INVALID' })
+      .expect(404)
+      .expect(res => {
+        const $ = cheerio.load(res.text)
+        expect($('h1').text()).toBe('Prisoner not found')
+        expect(sessionData).toStrictEqual({
+          booker: {
+            reference: bookerReference,
+            prisoners: [prisoner],
+          },
+        } as SessionData)
+      })
+  })
+})
diff --git a/server/routes/bookingJourney/selectPrisonerController.ts b/server/routes/bookingJourney/selectPrisonerController.ts
index af843477..9af58b8e 100644
--- a/server/routes/bookingJourney/selectPrisonerController.ts
+++ b/server/routes/bookingJourney/selectPrisonerController.ts
@@ -1,4 +1,5 @@
 import type { RequestHandler } from 'express'
+import { NotFound } from 'http-errors'
 
 export default class SelectPrisonerController {
   public constructor() {}
@@ -17,7 +18,7 @@ export default class SelectPrisonerController {
 
       const { prisonerNumber } = req.body
       if (prisonerNumber !== prisoner.prisonerNumber) {
-        throw new Error('Invalid prisoner selected')
+        throw new NotFound('Prisoner not found')
       }
 
       req.session.bookingJourney = { prisoner }

From fe7ed8edc786ac8c71496f9eb014a9e40130bf09 Mon Sep 17 00:00:00 2001
From: Thomas McGowan <tpmcgowan@users.noreply.github.com>
Date: Mon, 13 May 2024 18:47:46 +0100
Subject: [PATCH 08/10] Fix page titles

---
 server/routes/homeController.test.ts                 | 4 ++++
 server/views/autherror.test.ts                       | 1 +
 server/views/pages/bookingJourney/selectVisitors.njk | 2 +-
 server/views/pages/home.njk                          | 2 +-
 server/views/partials/layout.njk                     | 2 +-
 5 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/server/routes/homeController.test.ts b/server/routes/homeController.test.ts
index ba780bb3..4c2f239e 100644
--- a/server/routes/homeController.test.ts
+++ b/server/routes/homeController.test.ts
@@ -32,6 +32,7 @@ describe('Home page', () => {
       .expect('Content-Type', /html/)
       .expect(res => {
         const $ = cheerio.load(res.text)
+        expect($('title').text()).toMatch(/^Book a visit -/)
         expect($('[data-test="back-link"]').length).toBe(0)
         expect($('h1').text()).toBe('Book a visit')
         expect($('[data-test="prisoner-name"]').text()).toBe('John Smith')
@@ -59,6 +60,7 @@ describe('Home page', () => {
       .expect('Content-Type', /html/)
       .expect(res => {
         const $ = cheerio.load(res.text)
+        expect($('title').text()).toMatch(/^Book a visit -/)
         expect($('[data-test="back-link"]').length).toBe(0)
         expect($('h1').text()).toBe('Book a visit')
         expect($('[data-test="prisoner-name"]').length).toBe(0)
@@ -86,6 +88,8 @@ describe('Page header', () => {
       .expect('Content-Type', /html/)
       .expect(res => {
         const $ = cheerio.load(res.text)
+        expect($('title').text()).toBe('Book a visit - Visit someone in prison - GOV.UK')
+
         expect($('header .one-login-header').length).toBe(1)
         expect($('header.govuk-header').length).toBe(0)
         expect($('.service-header__heading').text()).toBe('Visit someone in prison')
diff --git a/server/views/autherror.test.ts b/server/views/autherror.test.ts
index 4c22ac65..50c54f1d 100644
--- a/server/views/autherror.test.ts
+++ b/server/views/autherror.test.ts
@@ -9,6 +9,7 @@ describe('Authorisation error page', () => {
   it('should render a default GOVUK Header on the authorisation error page', () => {
     app.render('autherror', (err: Error, html: string) => {
       const $ = cheerio.load(html)
+      expect($('head title').text()).toBe('Visit someone in prison - GOV.UK')
 
       expect($('header.govuk-header').length).toBe(1)
       expect($('header .one-login-header').length).toBe(0)
diff --git a/server/views/pages/bookingJourney/selectVisitors.njk b/server/views/pages/bookingJourney/selectVisitors.njk
index 52b02859..90b5a5f0 100644
--- a/server/views/pages/bookingJourney/selectVisitors.njk
+++ b/server/views/pages/bookingJourney/selectVisitors.njk
@@ -3,7 +3,7 @@
 {% from "govuk/components/checkboxes/macro.njk" import govukCheckboxes %}
 {% from "govuk/components/details/macro.njk" import govukDetails %}
 
-{% set pageTitle = applicationName + " - Home" %}
+{% set pageTitle = "Who is going on the visit?" %}
 
 {% set backLinkHref = "/" %}
 
diff --git a/server/views/pages/home.njk b/server/views/pages/home.njk
index 5105a44f..eab25133 100644
--- a/server/views/pages/home.njk
+++ b/server/views/pages/home.njk
@@ -1,7 +1,7 @@
 {% extends "../partials/layout.njk" %}
 {% from "govuk/components/button/macro.njk" import govukButton %}
 
-{% set pageTitle = applicationName + " - Home" %}
+{% set pageTitle = "Book a visit" %}
 
 {% block content %}
 
diff --git a/server/views/partials/layout.njk b/server/views/partials/layout.njk
index 19ef7f34..58921928 100644
--- a/server/views/partials/layout.njk
+++ b/server/views/partials/layout.njk
@@ -26,7 +26,7 @@
   <link href="/assets/stylesheets/application.css?{{ version }}" rel="stylesheet"/>
 {% endblock %}
 
-{% block pageTitle %}{{pageTitle | default(applicationName)}}{% endblock %}
+{% block pageTitle %}{{ pageTitle + " - " if pageTitle }}{{ applicationName }} - GOV.UK{% endblock %}
 
 {% block header %}
   {{ govukOneLoginServiceHeader({ 

From 546aa4419462e09dd01a090a38bd60e56fc26f7d Mon Sep 17 00:00:00 2001
From: Thomas McGowan <tpmcgowan@users.noreply.github.com>
Date: Tue, 14 May 2024 07:49:15 +0100
Subject: [PATCH 09/10] Add tests for select visitors controller

---
 server/@types/bapv.d.ts                       |   6 +
 .../selectVisitorsController.test.ts          | 227 ++++++++++++++++++
 .../selectVisitorsController.ts               |   2 +-
 server/routes/homeController.test.ts          |   2 +-
 .../pages/bookingJourney/selectVisitors.njk   |  35 +--
 5 files changed, 253 insertions(+), 19 deletions(-)
 create mode 100644 server/routes/bookingJourney/selectVisitorsController.test.ts

diff --git a/server/@types/bapv.d.ts b/server/@types/bapv.d.ts
index e6d60239..de035181 100644
--- a/server/@types/bapv.d.ts
+++ b/server/@types/bapv.d.ts
@@ -1,3 +1,4 @@
+import { FieldValidationError } from 'express-validator'
 import { PrisonDto, PrisonerInfoDto, VisitorInfoDto } from '../data/orchestrationApiTypes'
 
 export type Booker = {
@@ -19,3 +20,8 @@ export type BookingJourneyData = {
   // selected visitors for this visit
   selectedVisitors?: VisitorInfoDto[]
 }
+
+export type FlashData = {
+  errors?: FieldValidationError[]
+  formValues?: Record<string, string | string[]>
+}
diff --git a/server/routes/bookingJourney/selectVisitorsController.test.ts b/server/routes/bookingJourney/selectVisitorsController.test.ts
new file mode 100644
index 00000000..8dabce22
--- /dev/null
+++ b/server/routes/bookingJourney/selectVisitorsController.test.ts
@@ -0,0 +1,227 @@
+import type { Express } from 'express'
+import request from 'supertest'
+import * as cheerio from 'cheerio'
+import { SessionData } from 'express-session'
+import { FieldValidationError } from 'express-validator'
+import { appWithAllRoutes, flashProvider } from '../testutils/appSetup'
+import { createMockBookerService, createMockPrisonService } from '../../services/testutils/mocks'
+import TestData from '../testutils/testData'
+import { FlashData } from '../../@types/bapv'
+
+let app: Express
+
+const bookerService = createMockBookerService()
+const prisonService = createMockPrisonService()
+let sessionData: SessionData
+
+const url = '/book-a-visit/select-visitors'
+
+const bookerReference = TestData.bookerReference().value
+const prisoner = TestData.prisonerInfoDto()
+const prison = TestData.prisonDto()
+const visitors = [
+  TestData.visitorInfoDto({ personId: 1, firstName: 'Visitor', lastName: 'One', dateOfBirth: '1980-02-03' }),
+  TestData.visitorInfoDto({ personId: 2, firstName: 'Visitor', lastName: 'Two', dateOfBirth: '1990-09-03' }),
+  TestData.visitorInfoDto({ personId: 3, firstName: 'Visitor', lastName: 'Three', dateOfBirth: '2024-03-01' }),
+]
+
+afterEach(() => {
+  jest.resetAllMocks()
+  jest.useRealTimers()
+})
+
+describe('Select visitors page', () => {
+  let flashData: FlashData
+
+  describe(`GET ${url}`, () => {
+    beforeEach(() => {
+      jest.useFakeTimers({ advanceTimers: true, now: new Date('2024-05-01') })
+
+      bookerService.getVisitors.mockResolvedValue(visitors)
+      prisonService.getPrison.mockResolvedValue(prison)
+
+      flashData = {}
+      flashProvider.mockImplementation((key: keyof FlashData) => flashData[key])
+
+      sessionData = {
+        booker: { reference: bookerReference, prisoners: [prisoner] },
+        bookingJourney: { prisoner },
+      } as SessionData
+
+      app = appWithAllRoutes({ services: { bookerService, prisonService }, sessionData })
+    })
+
+    it('should render prison visitor rules, visitor list and save all visitors to session', () => {
+      return request(app)
+        .get(url)
+        .expect('Content-Type', /html/)
+        .expect(res => {
+          const $ = cheerio.load(res.text)
+          expect($('title').text()).toMatch(/^Who is going on the visit\? -/)
+          expect($('[data-test="back-link"]').attr('href')).toBe('/')
+          expect($('h1').text()).toBe('Who is going on the visit?')
+
+          expect($('[data-test=visitors-max-total]').text().trim().replace(/\s+/g, ' ')).toBe(
+            'Up to 6 people can visit someone at Hewell (HMP). This includes:',
+          )
+          expect($('[data-test=visitors-max-adults]').text().trim().replace(/\s+/g, ' ')).toBe(
+            '2 people 16 years old or older',
+          )
+          expect($('[data-test=visitors-max-child]').text().trim().replace(/\s+/g, ' ')).toBe(
+            '4 people under 16 years old',
+          )
+
+          expect($('form[method=POST]').attr('action')).toBe('/book-a-visit/select-visitors')
+          expect($('input[name=visitorIds]').length).toBe(3)
+          expect($('input[name=visitorIds][value=1] + label').text().trim()).toBe('Visitor One, (44 years old)')
+          expect($('input[name=visitorIds][value=2] + label').text().trim()).toBe('Visitor Two, (33 years old)')
+          expect($('input[name=visitorIds][value=3] + label').text().trim()).toBe('Visitor Three, (2 months old)')
+
+          expect($('[data-test="continue-button"]').text().trim()).toBe('Continue')
+
+          expect(bookerService.getVisitors).toHaveBeenCalledWith(bookerReference, prisoner.prisonerNumber)
+          expect(prisonService.getPrison).toHaveBeenCalledWith(prisoner.prisonCode)
+
+          expect(sessionData).toStrictEqual({
+            booker: {
+              reference: bookerReference,
+              prisoners: [prisoner],
+            },
+            bookingJourney: {
+              prisoner,
+              prison,
+              allVisitors: visitors,
+            },
+          } as SessionData)
+        })
+    })
+
+    it('should render validation errors', () => {
+      const validationError: FieldValidationError = {
+        type: 'field',
+        location: 'body',
+        path: 'visitorIds',
+        value: [],
+        msg: 'No visitors selected',
+      }
+
+      flashData = { errors: [validationError], formValues: { visitorIds: [] } }
+
+      return request(app)
+        .get(url)
+        .expect('Content-Type', /html/)
+        .expect(res => {
+          const $ = cheerio.load(res.text)
+          expect($('.govuk-error-summary a[href="#visitorIds-error"]').text()).toBe('No visitors selected')
+          expect($('#visitorIds-error').text()).toContain('No visitors selected')
+        })
+    })
+
+    it('should handle booker having no visitors for this prisoner', () => {
+      bookerService.getVisitors.mockResolvedValue([])
+
+      return request(app)
+        .get(url)
+        .expect('Content-Type', /html/)
+        .expect(res => {
+          const $ = cheerio.load(res.text)
+          expect($('title').text()).toMatch(/^Who is going on the visit\? -/)
+          expect($('[data-test="back-link"]').attr('href')).toBe('/')
+          expect($('h1').text()).toBe('Who is going on the visit?')
+
+          expect($('[data-test=visitors-max-total]').length).toBe(0)
+          expect($('[data-test=visitors-max-adults]').length).toBe(0)
+          expect($('[data-test=visitors-max-child]').length).toBe(0)
+
+          expect($('form[method=POST]').length).toBe(0)
+          expect($('input[name=visitorIds]').length).toBe(0)
+
+          expect($('[data-test="continue-button"]').length).toBe(0)
+
+          expect(bookerService.getVisitors).toHaveBeenCalledWith(bookerReference, prisoner.prisonerNumber)
+          expect(prisonService.getPrison).toHaveBeenCalledWith(prisoner.prisonCode)
+
+          expect(sessionData).toStrictEqual({
+            booker: {
+              reference: bookerReference,
+              prisoners: [prisoner],
+            },
+            bookingJourney: {
+              prisoner,
+              prison,
+              allVisitors: [],
+            },
+          } as SessionData)
+        })
+    })
+  })
+
+  describe(`POST ${url}`, () => {
+    beforeEach(() => {
+      sessionData = {
+        booker: { reference: bookerReference, prisoners: [prisoner] },
+        bookingJourney: { prisoner, prison, allVisitors: visitors },
+      } as SessionData
+
+      app = appWithAllRoutes({ sessionData })
+    })
+
+    it('should should save selected visitors to session and redirect to select date and time page', () => {
+      return request(app)
+        .post(url)
+        .send({ visitorIds: [1, 3] })
+        .expect(302)
+        .expect('Location', '/book-a-visit/select-date-and-time')
+        .expect(() => {
+          expect(sessionData).toStrictEqual({
+            booker: {
+              reference: bookerReference,
+              prisoners: [prisoner],
+            },
+            bookingJourney: {
+              prisoner,
+              prison,
+              allVisitors: visitors,
+              selectedVisitors: [visitors[0], visitors[2]],
+            },
+          } as SessionData)
+        })
+    })
+
+    it('should set a validation error and redirect to original page when no visitors selected', () => {
+      const expectedFlashData: FlashData = {
+        errors: [
+          {
+            type: 'field',
+            location: 'body',
+            path: 'visitorIds',
+            value: [],
+            msg: 'No visitors selected',
+          },
+        ],
+        formValues: { visitorIds: [] },
+      }
+
+      return request(app)
+        .post(url)
+        .expect(302)
+        .expect('Location', url)
+        .expect(() => {
+          expect(flashProvider).toHaveBeenCalledWith('errors', expectedFlashData.errors)
+          expect(flashProvider).toHaveBeenCalledWith('formValues', expectedFlashData.formValues)
+
+          expect(sessionData).toStrictEqual({
+            booker: {
+              reference: bookerReference,
+              prisoners: [prisoner],
+            },
+            bookingJourney: {
+              prisoner,
+              prison,
+              allVisitors: visitors,
+            },
+          } as SessionData)
+        })
+    })
+  })
+})
diff --git a/server/routes/bookingJourney/selectVisitorsController.ts b/server/routes/bookingJourney/selectVisitorsController.ts
index f2367b27..6c12ab85 100644
--- a/server/routes/bookingJourney/selectVisitorsController.ts
+++ b/server/routes/bookingJourney/selectVisitorsController.ts
@@ -22,7 +22,7 @@ export default class SelectVisitorsController {
       // TODO pre-populate form (e.g. if coming from Back link or Change answers)
 
       res.render('pages/bookingJourney/selectVisitors', {
-        errors: req.flash('errors'),
+        errors: req.flash('errors'), // TODO need to add "Error: " to page title if errors?
         formValues: req.flash('formValues')?.[0] || {},
         prison: bookingJourney.prison,
         visitors: bookingJourney.allVisitors,
diff --git a/server/routes/homeController.test.ts b/server/routes/homeController.test.ts
index 4c2f239e..65ad3d60 100644
--- a/server/routes/homeController.test.ts
+++ b/server/routes/homeController.test.ts
@@ -36,8 +36,8 @@ describe('Home page', () => {
         expect($('[data-test="back-link"]').length).toBe(0)
         expect($('h1').text()).toBe('Book a visit')
         expect($('[data-test="prisoner-name"]').text()).toBe('John Smith')
-        expect($('input[name=prisonerNumber]').val()).toBe(prisoner.prisonerNumber)
         expect($('form[method=POST]').attr('action')).toBe('/book-a-visit/select-prisoner')
+        expect($('input[name=prisonerNumber]').val()).toBe(prisoner.prisonerNumber)
         expect($('[data-test="start-booking"]').text().trim()).toBe('Start')
 
         expect(bookerService.getPrisoners).toHaveBeenCalledWith(bookerReference)
diff --git a/server/views/pages/bookingJourney/selectVisitors.njk b/server/views/pages/bookingJourney/selectVisitors.njk
index 90b5a5f0..e0c81aa0 100644
--- a/server/views/pages/bookingJourney/selectVisitors.njk
+++ b/server/views/pages/bookingJourney/selectVisitors.njk
@@ -15,23 +15,24 @@
 
       <h1 class="govuk-heading-l">Who is going on the visit?</h1>
 
-      <p>
-        Up to {{ prison.maxTotalVisitors }} {{ "person" | pluralise(prison.maxTotalVisitors, "people") }}
-        can visit someone at {{ prison.prisonName }}. This includes
-      </p>
-      <ul class="govuk-list govuk-list--bullet">
-        <li>
-          {{ prison.maxAdultVisitors }} {{ "person" | pluralise(prison.maxAdultVisitors, "people") }}
-          {{ prison.adultAgeYears }} {{ "year" | pluralise(prison.adultAgeYears) }} old or older
-        </li>
-        <li>
-          {{ prison.maxChildVisitors }} {{ "person" | pluralise(prison.maxChildVisitors, "people") }}
-          under {{ prison.adultAgeYears }} {{ "year" | pluralise(prison.adultAgeYears) }} old.
-        </li>
-      </ul>
-      <p>At least one visitor must be 18 years or older</p>
-
       {% if visitors | length %}
+
+        <p data-test="visitors-max-total">
+          Up to {{ prison.maxTotalVisitors }} {{ "person" | pluralise(prison.maxTotalVisitors, "people") }}
+          can visit someone at {{ prison.prisonName }}. This includes:
+        </p>
+        <ul class="govuk-list govuk-list--bullet">
+          <li data-test="visitors-max-adults">
+            {{ prison.maxAdultVisitors }} {{ "person" | pluralise(prison.maxAdultVisitors, "people") }}
+            {{ prison.adultAgeYears }} {{ "year" | pluralise(prison.adultAgeYears) }} old or older
+          </li>
+          <li data-test="visitors-max-child">
+            {{ prison.maxChildVisitors }} {{ "person" | pluralise(prison.maxChildVisitors, "people") }}
+            under {{ prison.adultAgeYears }} {{ "year" | pluralise(prison.adultAgeYears) }} old
+          </li>
+        </ul>
+        <p>At least one visitor must be 18 years or older</p>
+
         {% set visitorList = [] %}
         {% for visitor in visitors %}
           {% set visitorList = (visitorList.push({
@@ -65,7 +66,7 @@
 
       {% else %}
         {# TODO check what the message/behaviour in this case should be #}
-        <p>No visitor details found.</p>
+        <p data-test="no-visitors">No visitor details found.</p>
       {% endif %}
 
       {# TODO Get correct link for form #}

From c90fa5f661a2ade00b68de3d7588bd1b6eb937f1 Mon Sep 17 00:00:00 2001
From: Thomas McGowan <tpmcgowan@users.noreply.github.com>
Date: Tue, 14 May 2024 08:05:40 +0100
Subject: [PATCH 10/10] Fix typo

---
 server/@types/express/index.d.ts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/server/@types/express/index.d.ts b/server/@types/express/index.d.ts
index 1733f933..d1105c3a 100644
--- a/server/@types/express/index.d.ts
+++ b/server/@types/express/index.d.ts
@@ -1,3 +1,4 @@
+import { ValidationError } from 'express-validator'
 import { Booker, BookingJourneyData } from '../bapv'
 
 export default {}
@@ -25,7 +26,7 @@ export declare global {
 
     interface Request {
       id: string
-      flash(type: 'errors', message: FlashErrorMessage): number
+      flash(type: 'errors', message: ValidationError[]): number
       logout(done: (err: unknown) => void): void
     }