Review comments + implement message for overall overbooking

integration_tests/pages/manage/occupancyDayView.ts

@@ -2,7 +2,7 @@ import { Cas1PremisesBasicSummary, Cas1PremisesDaySummary } from '@approved-prem
 import Page from '../page'
 import { DateFormats } from '../../../server/utils/dateUtils'
 import paths from '../../../server/paths/manage'
-import { daySummaryRows, generateDaySummaryText } from '../../../server/utils/premises/occupancy'
+import { daySummaryRows } from '../../../server/utils/premises/occupancy'
 
 export default class OccupancyDayViewPage extends Page {
   constructor(private pageTitle: string) {
@@ -18,11 +18,6 @@ export default class OccupancyDayViewPage extends Page {
     this.shouldContainSummaryListItems(daySummaryRows(premisesDaySummary).rows)
   }
 
-  shouldShowDaySummaryWarningContent(premisesDaySummary: Cas1PremisesDaySummary) {
-    const warningContent = generateDaySummaryText(premisesDaySummary.capacity.characteristicAvailability)
-    this.shouldShowBanner(warningContent)
-  }
-
   shouldNavigateToDay(linkLabel: string, date: string) {
     cy.contains(linkLabel).click()
     cy.get('h1').contains(DateFormats.isoDateToUIDate(date))

server/controllers/manage/premises/apOccupancyViewController.test.ts

@@ -143,31 +143,11 @@ describe('AP occupancyViewController', () => {
           previousDayLink: paths.premises.occupancy.day({ premisesId, date: '2024-12-31' }),
           nextDayLink: paths.premises.occupancy.day({ premisesId, date: '2025-01-02' }),
           daySummaryRows: daySummaryRows(premisesDaySummary),
-          daySummaryText: generateDaySummaryText(premisesDaySummary.capacity.characteristicAvailability),
+          daySummaryText: generateDaySummaryText(premisesDaySummary),
         }),
       )
       expect(premisesService.find).toHaveBeenCalledWith(token, premisesId)
       expect(premisesService.getDaySummary).toHaveBeenCalledWith({ token, premisesId, date })
     })
-
-    it('should render error if date is invalid', async () => {
-      request = createMock<Request>({
-        user: { token },
-        params: { premisesId, date: '2023-02-29' },
-        flash: jest.fn(),
-      })
-      const requestHandler = occupancyViewController.dayView()
-      await requestHandler(request, response, next)
-
-      expect(response.render).toHaveBeenCalledWith(
-        'manage/premises/occupancy/dayView',
-        expect.objectContaining({
-          backLink: paths.premises.occupancy.view({ premisesId }),
-          errorSummary: [{ text: 'Date "2023-02-29" is invalid.', href: '#date' }],
-        }),
-      )
-      expect(premisesService.find).toHaveBeenCalledWith(token, premisesId)
-      expect(premisesService.getDaySummary).not.toHaveBeenCalled()
-    })
   })
 })

server/controllers/manage/premises/apOccupancyViewController.ts

@@ -1,7 +1,6 @@
 import type { Request, RequestHandler, Response } from 'express'
 
 import { ObjectWithDateParts } from '@approved-premises/ui'
-import { Cas1PremisesDaySummary } from '@approved-premises/api'
 import { PremisesService } from '../../../services'
 
 import paths from '../../../paths/manage'
@@ -73,18 +72,8 @@ export default class ApOccupancyViewController {
     return async (req: Request, res: Response) => {
       const { token } = req.user
       const { premisesId, date } = req.params
-      const { errorSummary } = fetchErrorsAndUserInput(req)
-      try {
-        DateFormats.isoToDateObj(date)
-      } catch (e) {
-        const dateError = { date: `Date "${date}" is invalid.` }
-        errorSummary.push(generateErrorSummary(dateError)[0])
-      }
       const premises = await this.premisesService.find(token, premisesId)
-      let daySummary: Cas1PremisesDaySummary
-      if (!errorSummary.length) {
-        daySummary = await this.premisesService.getDaySummary({ token, premisesId, date })
-      }
+      const daySummary = await this.premisesService.getDaySummary({ token, premisesId, date })
 
       return res.render('manage/premises/occupancy/dayView', {
         premises,
@@ -93,8 +82,7 @@ export default class ApOccupancyViewController {
         previousDayLink: daySummary && paths.premises.occupancy.day({ premisesId, date: daySummary.previousDate }),
         nextDayLink: daySummary && paths.premises.occupancy.day({ premisesId, date: daySummary.nextDate }),
         daySummaryRows: daySummary && daySummaryRows(daySummary),
-        daySummaryText: daySummary && generateDaySummaryText(daySummary.capacity.characteristicAvailability),
-        errorSummary,
+        daySummaryText: daySummary && generateDaySummaryText(daySummary),
       })
     }
   }

server/utils/premises/occupancy.test.ts

@@ -57,8 +57,8 @@ describe('apOccupancy utils', () => {
   })
 
   describe('generateDaySummaryText', () => {
-    const buildAvailability = (overbookedCharacteristics: Array<Cas1SpaceBookingCharacteristic>) =>
-      Object.keys(occupancyCriteriaMap).map(characteristic =>
+    const buildDaySummary = (overbookedCharacteristics: Array<Cas1SpaceBookingCharacteristic>, overbook = false) => {
+      const characteristicAvailability = Object.keys(occupancyCriteriaMap).map(characteristic =>
         overbookedCharacteristics.includes(characteristic as Cas1SpaceBookingCharacteristic)
           ? premiseCharacteristicAvailability
               .strictlyOverbooked()
@@ -67,23 +67,46 @@ describe('apOccupancy utils', () => {
               .available()
               .build({ characteristic: characteristic as Cas1SpaceBookingCharacteristic }),
       )
+      const capacityForDay = overbook
+        ? cas1PremiseCapacityForDayFactory.overbooked().build({
+            characteristicAvailability,
+          })
+        : cas1PremiseCapacityForDayFactory.available().build({
+            characteristicAvailability,
+          })
+      return cas1PremisesDaySummaryFactory.build({
+        capacity: capacityForDay,
+      })
+    }
 
     it('should generate the text for an premises day with an overbooking on a single characteristic', () => {
-      const characteristicAvailability = buildAvailability(['isSingle'])
+      const characteristicAvailability = buildDaySummary(['isSingle'])
       expect(generateDaySummaryText(characteristicAvailability)).toEqual(
-        'This AP is overbooked on spaces with the following criterion: single room',
+        'This AP is overbooked on spaces with the following criterion: single room.',
       )
     })
 
     it('should generate the text for an premises day with an overbooking on multiple characteristics', () => {
-      const characteristicAvailability = buildAvailability(['isSingle', 'isArsonSuitable', 'isWheelchairDesignated'])
+      const characteristicAvailability = buildDaySummary(['isSingle', 'isArsonSuitable', 'isWheelchairDesignated'])
+      expect(generateDaySummaryText(characteristicAvailability)).toEqual(
+        'This AP is overbooked on spaces with the following criteria: wheelchair accessible, single room, designated arson room.',
+      )
+    })
+
+    it('should generate the text for an premises day with an overall overbooking but no overbooked characteristics', () => {
+      const characteristicAvailability = buildDaySummary([], true)
+      expect(generateDaySummaryText(characteristicAvailability)).toEqual('This AP is at overcapacity.')
+    })
+
+    it('should generate the text for an premises day with an overall overbooking and overbooked characteristics', () => {
+      const characteristicAvailability = buildDaySummary(['isSingle', 'isArsonSuitable'], true)
       expect(generateDaySummaryText(characteristicAvailability)).toEqual(
-        'This AP is overbooked on spaces with the following criteria: wheelchair accessible, single room, designated arson room',
+        'This AP is at overcapacity and is overbooked on spaces with the following criteria: single room, designated arson room.',
       )
     })
 
     it('should generate empty text for an premises day with no overbooked characteristics', () => {
-      const characteristicAvailability = buildAvailability([])
+      const characteristicAvailability = buildDaySummary([])
       expect(generateDaySummaryText(characteristicAvailability)).toEqual('')
     })
   })

server/utils/premises/occupancy.ts

@@ -1,14 +1,9 @@
-import {
-  Cas1PremiseCapacityForDay,
-  Cas1PremiseCharacteristicAvailability,
-  Cas1PremisesDaySummary,
-  Cas1SpaceBookingCharacteristic,
-} from '@approved-premises/api'
-import { SelectOption, SummaryListItem } from '@approved-premises/ui'
+import { Cas1PremiseCapacityForDay, Cas1PremisesDaySummary } from '@approved-premises/api'
+import { SelectOption } from '@approved-premises/ui'
 import { DateFormats } from '../dateUtils'
-import { textValue } from '../applications/helpers'
 import { occupancyCriteriaMap } from '../match/occupancy'
 import managePaths from '../../paths/manage'
+import { summaryListItem } from '../formUtils'
 
 type CalendarDayStatus = 'available' | 'full' | 'overbooked'
 
@@ -72,38 +67,34 @@ export const durationSelectOptions = (durationDays?: string): Array<SelectOption
     selected: value === durationDays || undefined,
   }))
 
-export const generateDaySummaryText = (
-  allCharacteristicAvailability: Array<Cas1PremiseCharacteristicAvailability>,
-): string => {
-  const overbookedCriteria: Array<Cas1SpaceBookingCharacteristic> = []
-  allCharacteristicAvailability.forEach((characteristicAvailability: Cas1PremiseCharacteristicAvailability) => {
-    const { characteristic, availableBedsCount, bookingsCount } = characteristicAvailability
-
-    if (bookingsCount > availableBedsCount) {
-      overbookedCriteria.push(characteristic)
-    }
-  })
-  return overbookedCriteria.length
-    ? `This AP is overbooked on spaces with the following ${overbookedCriteria.length > 1 ? 'criteria' : 'criterion'}: ${overbookedCriteria.map(characteristic => occupancyCriteriaMap[characteristic].toLowerCase()).join(', ')}`
-    : ''
+export const generateDaySummaryText = (daySummary: Cas1PremisesDaySummary): string => {
+  const {
+    capacity: { characteristicAvailability, availableBedCount, bookingCount },
+  } = daySummary
+  const overbookedCriteria = characteristicAvailability
+    .map(({ characteristic, availableBedsCount, bookingsCount }) =>
+      bookingsCount > availableBedsCount ? characteristic : undefined,
+    )
+    .filter(Boolean)
+  const messages: Array<string> = []
+  if (bookingCount > availableBedCount) messages.push('is at overcapacity')
+  if (overbookedCriteria.length)
+    messages.push(
+      `is overbooked on spaces with the following ${overbookedCriteria.length > 1 ? 'criteria' : 'criterion'}: ${overbookedCriteria.map(characteristic => occupancyCriteriaMap[characteristic].toLowerCase()).join(', ')}`,
+    )
+  return messages.length ? `This AP ${messages.join(' and ')}.` : ''
 }
 
-const summaryRow = (key: string, value: string): SummaryListItem =>
-  value && {
-    key: textValue(key),
-    value: textValue(value),
-  }
-
 export const daySummaryRows = (daySummary: Cas1PremisesDaySummary) => {
   const {
     capacity: { totalBedCount, bookingCount, availableBedCount },
   } = daySummary
   return {
     rows: [
-      summaryRow('Capacity', String(totalBedCount)),
-      summaryRow('Booked spaces', String(bookingCount)),
-      summaryRow('Out of service beds', String(totalBedCount - availableBedCount)),
-      summaryRow('Available spaces', String(availableBedCount - bookingCount)),
+      summaryListItem('Capacity', String(totalBedCount)),
+      summaryListItem('Booked spaces', String(bookingCount)),
+      summaryListItem('Out of service beds', String(totalBedCount - availableBedCount)),
+      summaryListItem('Available spaces', String(availableBedCount - bookingCount)),
     ],
   }
 }

server/views/manage/premises/occupancy/dayView.njk

@@ -26,7 +26,6 @@
 {% endset %}
 
 {% block content %}
-    {{ showErrorSummary(errorSummary) }}
     <span class="govuk-caption-l">{{ premises.name }}</span>
     <h1 class="govuk-heading-l">{{ pageHeading }}</h1>
     <div class="prev-next">