Ensure all date errors returned at once

Refactor of booking post handler to follow usual try/catch pattern
ministryofjustice · Jan 15, 2025 · fe539b9 · fe539b9
1 parent c86e131
commit fe539b9
Show file tree

Hide file tree

Showing 3 changed files with 164 additions and 86 deletions.
diff --git a/server/controllers/match/placementRequests/occupancyViewController.test.ts b/server/controllers/match/placementRequests/occupancyViewController.test.ts
@@ -23,6 +23,7 @@ import {
   dayAvailabilitySummaryListItems,
 } from '../../../utils/match/occupancy'
 import { placementRequestSummaryList } from '../../../utils/placementRequests/placementRequestSummaryList'
+import { ValidationError } from '../../../utils/errors'
 
 describe('OccupancyViewController', () => {
   const token = 'SOME_TOKEN'
@@ -48,7 +49,7 @@ describe('OccupancyViewController', () => {
   })
 
   beforeEach(() => {
-    jest.resetAllMocks()
+    jest.clearAllMocks()
 
     occupancyViewController = new OccupancyViewController(placementRequestService, premisesService)
     request = createMock<Request>({
@@ -64,14 +65,11 @@ describe('OccupancyViewController', () => {
     premisesService.find.mockResolvedValue(premises)
     premisesService.getCapacity.mockResolvedValue(premiseCapacity)
 
-    jest.spyOn(validationUtils, 'fetchErrorsAndUserInput')
-    when(validationUtils.fetchErrorsAndUserInput as jest.Mock)
-      .calledWith(request)
-      .mockReturnValue({
-        errors: {},
-        errorSummary: [],
-        userInput: {},
-      })
+    jest.spyOn(validationUtils, 'fetchErrorsAndUserInput').mockReturnValue({
+      errors: {},
+      errorSummary: [],
+      userInput: {},
+    })
   })
 
   describe('view', () => {
@@ -160,13 +158,11 @@ describe('OccupancyViewController', () => {
         'departureDate-year': '2026',
       }
 
-      when(validationUtils.fetchErrorsAndUserInput as jest.Mock)
-        .calledWith(request)
-        .mockReturnValue({
-          errors: expectedErrors,
-          errorSummary: expectedErrorSummary,
-          userInput: expectedUserInput,
-        })
+      jest.spyOn(validationUtils, 'fetchErrorsAndUserInput').mockReturnValue({
+        errors: expectedErrors,
+        errorSummary: expectedErrorSummary,
+        userInput: expectedUserInput,
+      })
 
       const requestHandler = occupancyViewController.view()
 
@@ -272,6 +268,8 @@ describe('OccupancyViewController', () => {
   })
 
   describe('bookSpace', () => {
+    const params = { id: placementRequestDetail.id, premisesId: premises.id }
+
     const startDate = '2025-08-15'
     const durationDays = '22'
 
@@ -286,8 +284,6 @@ describe('OccupancyViewController', () => {
     }
 
     it(`should redirect to space-booking confirmation page when date validation succeeds`, async () => {
-      const params = { id: placementRequestDetail.id, premisesId: premises.id }
-
       const requestHandler = occupancyViewController.bookSpace()
       await requestHandler({ ...request, params, body: validBookingBody }, response, next)
 
@@ -299,37 +295,125 @@ describe('OccupancyViewController', () => {
       )
     })
 
-    it(`should redirect to occupancy view with existing query string and add errors messages when date validation fails`, async () => {
-      jest.spyOn(validationUtils, 'addErrorMessageToFlash')
-      const body = {
-        ...validBookingBody,
-        'arrivalDate-day': '',
-        criteria: 'isWheelchairDesignated,isSuitedForSexOffenders',
-      }
-      const query = {
-        startDate,
-        durationDays,
-      }
-      const params = { id: placementRequestDetail.id, premisesId: premises.id }
+    describe('when there are errors', () => {
+      beforeEach(() => {
+        jest.spyOn(validationUtils, 'catchValidationErrorOrPropogate').mockReturnValue(undefined)
+      })
 
-      const requestHandler = occupancyViewController.bookSpace()
+      it(`should redirect to occupancy view when dates are empty`, async () => {
+        const body = {}
 
-      await requestHandler({ ...request, params, query, body }, response, next)
+        const requestHandler = occupancyViewController.bookSpace()
+        await requestHandler({ ...request, params, body }, response, next)
 
-      expect(validationUtils.addErrorMessageToFlash).toHaveBeenCalledWith(
-        request,
-        'You must enter an arrival date',
-        'arrivalDate',
-      )
+        const expectedErrorData = {
+          arrivalDate: 'You must enter an arrival date',
+          departureDate: 'You must enter a departure date',
+        }
 
-      const expectedQueryString = `startDate=${startDate}&durationDays=${durationDays}&criteria=isWheelchairDesignated&criteria=isSuitedForSexOffenders`
+        expect(validationUtils.catchValidationErrorOrPropogate).toHaveBeenCalledWith(
+          request,
+          response,
+          new ValidationError({}),
+          matchPaths.v2Match.placementRequests.search.occupancy({
+            id: placementRequestDetail.id,
+            premisesId: premises.id,
+          }),
+        )
 
-      expect(response.redirect).toHaveBeenCalledWith(
-        `${matchPaths.v2Match.placementRequests.search.occupancy({
-          id: placementRequestDetail.id,
-          premisesId: premises.id,
-        })}?${expectedQueryString}`,
-      )
+        const errorData = (validationUtils.catchValidationErrorOrPropogate as jest.Mock).mock.lastCall[2].data
+
+        expect(errorData).toEqual(expectedErrorData)
+      })
+
+      it(`should redirect to occupancy view when dates are invalid`, async () => {
+        const body = {
+          'arrivalDate-day': '31',
+          'arrivalDate-month': '02',
+          'arrivalDate-year': '2025',
+          'departureDate-day': '34',
+          'departureDate-month': '05',
+          'departureDate-year': '19999',
+        }
+
+        const requestHandler = occupancyViewController.bookSpace()
+        await requestHandler({ ...request, params, body }, response, next)
+
+        const expectedErrorData = {
+          arrivalDate: 'The arrival date is an invalid date',
+          departureDate: 'The departure date is an invalid date',
+        }
+
+        expect(validationUtils.catchValidationErrorOrPropogate).toHaveBeenCalledWith(
+          request,
+          response,
+          new ValidationError({}),
+          matchPaths.v2Match.placementRequests.search.occupancy({
+            id: placementRequestDetail.id,
+            premisesId: premises.id,
+          }),
+        )
+
+        const errorData = (validationUtils.catchValidationErrorOrPropogate as jest.Mock).mock.lastCall[2].data
+
+        expect(errorData).toEqual(expectedErrorData)
+      })
+
+      it(`should redirect to occupancy view when the departure date is before the arrival date`, async () => {
+        const body = {
+          'arrivalDate-day': '28',
+          'arrivalDate-month': '01',
+          'arrivalDate-year': '2025',
+          'departureDate-day': '27',
+          'departureDate-month': '01',
+          'departureDate-year': '2025',
+        }
+
+        const requestHandler = occupancyViewController.bookSpace()
+        await requestHandler({ ...request, params, body }, response, next)
+
+        const expectedErrorData = {
+          departureDate: 'The departure date must be after the arrival date',
+        }
+
+        expect(validationUtils.catchValidationErrorOrPropogate).toHaveBeenCalledWith(
+          request,
+          response,
+          new ValidationError({}),
+          matchPaths.v2Match.placementRequests.search.occupancy({
+            id: placementRequestDetail.id,
+            premisesId: premises.id,
+          }),
+        )
+
+        const errorData = (validationUtils.catchValidationErrorOrPropogate as jest.Mock).mock.lastCall[2].data
+
+        expect(errorData).toEqual(expectedErrorData)
+      })
+
+      it(`should redirect to occupancy view with the existing query string`, async () => {
+        const body = {}
+        const query = {
+          startDate,
+          durationDays,
+          criteria: ['isWheelchairDesignated', 'isSuitedForSexOffenders'],
+        }
+
+        const requestHandler = occupancyViewController.bookSpace()
+        await requestHandler({ ...request, params, query, body }, response, next)
+
+        const expectedQueryString = `startDate=${startDate}&durationDays=${durationDays}&criteria=isWheelchairDesignated&criteria=isSuitedForSexOffenders`
+
+        expect(validationUtils.catchValidationErrorOrPropogate).toHaveBeenCalledWith(
+          request,
+          response,
+          new ValidationError({}),
+          `${matchPaths.v2Match.placementRequests.search.occupancy({
+            id: placementRequestDetail.id,
+            premisesId: premises.id,
+          })}?${expectedQueryString}`,
+        )
+      })
     })
   })
 

diff --git a/server/controllers/match/placementRequests/occupancyViewController.ts b/server/controllers/match/placementRequests/occupancyViewController.ts
@@ -10,7 +10,7 @@ import {
   validateSpaceBooking,
 } from '../../../utils/match'
 import {
-  addErrorMessageToFlash,
+  catchValidationErrorOrPropogate,
   fetchErrorsAndUserInput,
   generateErrorMessages,
   generateErrorSummary,
@@ -28,6 +28,7 @@ import { convertKeyValuePairToCheckBoxItems } from '../../../utils/formUtils'
 import { OccupancySummary } from '../../../utils/match/occupancySummary'
 import paths from '../../../paths/match'
 import { placementRequestSummaryList } from '../../../utils/placementRequests/placementRequestSummaryList'
+import { ValidationError } from '../../../utils/errors'
 
 type CriteriaQuery = Array<Cas1SpaceBookingCharacteristic> | Cas1SpaceBookingCharacteristic
 
@@ -171,28 +172,15 @@ export default class {
     return async (req: Request, res: Response) => {
       const { body } = req
       const { criteria: criteriaBody } = body
-      const criteria = criteriaBody.split(',')
+      const criteria = criteriaBody?.split(',')
 
-      const errors = validateSpaceBooking(body)
+      try {
+        const errors = validateSpaceBooking(body)
 
-      if (this.hasErrors(errors)) {
-        if (errors.arrivalDate) {
-          addErrorMessageToFlash(req, errors.arrivalDate, 'arrivalDate')
-        }
-        if (errors.departureDate) {
-          addErrorMessageToFlash(req, errors.departureDate, 'departureDate')
+        if (Object.keys(errors).length) {
+          throw new ValidationError(errors)
         }
 
-        const { startDate, durationDays } = req.query
-        const redirectUrl = occupancyViewLink({
-          placementRequestId: req.params.id,
-          premisesId: req.params.premisesId,
-          startDate: startDate as string,
-          durationDays: durationDays as string,
-          spaceCharacteristics: criteria,
-        })
-        res.redirect(redirectUrl)
-      } else {
         const { arrivalDate } = DateFormats.dateAndTimeInputsToIsoString(
           body as ObjectWithDateParts<'arrivalDate'>,
           'arrivalDate',
@@ -209,15 +197,21 @@ export default class {
           departureDate,
           criteria,
         })
-        res.redirect(redirectUrl)
+        return res.redirect(redirectUrl)
+      } catch (error) {
+        const { startDate, durationDays, criteria: criteriaQuery } = req.query
+        const redirectUrl = occupancyViewLink({
+          placementRequestId: req.params.id,
+          premisesId: req.params.premisesId,
+          startDate: startDate as string,
+          durationDays: durationDays as string,
+          spaceCharacteristics: criteria || criteriaQuery,
+        })
+        return catchValidationErrorOrPropogate(req, res, error, redirectUrl)
       }
     }
   }
 
-  private hasErrors(errors: Record<string, string>): boolean {
-    return errors && Object.keys(errors).length > 0
-  }
-
   viewDay(): TypedRequestHandler<Request> {
     return async (req: ViewDayRequest, res: Response) => {
       const { token } = req.user

diff --git a/server/utils/match/validateSpaceBooking.ts b/server/utils/match/validateSpaceBooking.ts
@@ -3,32 +3,32 @@ import { DateFormats, dateAndTimeInputsAreValidDates, dateIsBlank, datetimeIsInT
 
 export const validateSpaceBooking = (body: ObjectWithDateParts<string>): Record<string, string> => {
   const errors: Record<string, string> = {}
+
   if (dateIsBlank(body, 'arrivalDate')) {
     errors.arrivalDate = 'You must enter an arrival date'
-    return errors
-  }
-  if (!dateAndTimeInputsAreValidDates(body as ObjectWithDateParts<'arrivalDate'>, 'arrivalDate')) {
+  } else if (!dateAndTimeInputsAreValidDates(body as ObjectWithDateParts<'arrivalDate'>, 'arrivalDate')) {
     errors.arrivalDate = 'The arrival date is an invalid date'
-    return errors
   }
+
   if (dateIsBlank(body, 'departureDate')) {
     errors.departureDate = 'You must enter a departure date'
-    return errors
-  }
-  if (!dateAndTimeInputsAreValidDates(body as ObjectWithDateParts<'departureDate'>, 'departureDate')) {
+  } else if (!dateAndTimeInputsAreValidDates(body as ObjectWithDateParts<'departureDate'>, 'departureDate')) {
     errors.departureDate = 'The departure date is an invalid date'
-    return errors
   }
-  const { arrivalDate } = DateFormats.dateAndTimeInputsToIsoString(
-    body as ObjectWithDateParts<'arrivalDate'>,
-    'arrivalDate',
-  )
-  const { departureDate } = DateFormats.dateAndTimeInputsToIsoString(
-    body as ObjectWithDateParts<'departureDate'>,
-    'departureDate',
-  )
-  if (datetimeIsInThePast(departureDate, arrivalDate) || departureDate === arrivalDate) {
-    errors.departureDate = 'The departure date must be after the arrival date'
+
+  if (!Object.keys(errors).length) {
+    const { arrivalDate } = DateFormats.dateAndTimeInputsToIsoString(
+      body as ObjectWithDateParts<'arrivalDate'>,
+      'arrivalDate',
+    )
+    const { departureDate } = DateFormats.dateAndTimeInputsToIsoString(
+      body as ObjectWithDateParts<'departureDate'>,
+      'departureDate',
+    )
+    if (datetimeIsInThePast(departureDate, arrivalDate) || departureDate === arrivalDate) {
+      errors.departureDate = 'The departure date must be after the arrival date'
+    }
   }
+
   return errors
 }