-
Notifications
You must be signed in to change notification settings - Fork 12k
Description
Summary
Booking flow currently allows creating a booking on a reserved slot, leading to potential double-bookings.
Steps to Reproduce
- Create a reservation for a slot (reservationId not passed to booking endpoint).
- Attempt to book the same slot directly via handleNewBooking.
- Booking succeeds even though slot is already reserved.
Expected Result
Booking a reserved slot should be prevented, OR require conversion of the reservation → booking within a single transaction.
Actual Result
handleNewBooking does not check reserved slots. Bookings succeed on reserved slots, causing possible overlaps.
Impact
- Severity: S1 (Critical – risk of overbooking in production)
- Affects: round robin & collective events
- Risk: Double-bookings, inconsistent user experience
Notes
- Discussion confirms selectedSlots are not referenced in handleNewBooking.
- Booker may need to pass reservedSlotUid.
- Suggested fix:
- Update handleNewBooking to receive reservedSlotUid.
- Within a single transaction: create booking → delete reserved slot.
- Update Booker to persist and pass reservedSlotUid (stored in local storage).
Round robin & collective events: reservations not implemented at all (requires separate handling).
Implementation
Small refactors
- Naming reserved slot cookie - when user selects a slot in Booker a request is made to trpc / api to reserve the slot and response contains a cookie named
uidthat contains reserved slot uid. In reserveSlot.handler.ts create constantconst RESERVED_SLOT_UID_COOKIE_NAME = "uid"that is used when setting the cookie e.g in the reserve slot handler. Also the old api v2 controller uses the constant to set the cookie in response after reserving a slot slots-2024-04-15/controllers/slots.controller.ts. - Retrieving reserved slot cookie - before when a handler / api needed to extract the
"uid"cookie from the request it does so by accessing request object directly e.g.req?.cookies?.uid. We now have a function getReservedSlotUidFromCookies that can be used instead and that under the hood also uses the const mentioned aboveconst RESERVED_SLOT_UID_COOKIE_NAME. - Storing reserved slot uid in memory - embed can't store reserved slot uid in a cookie because embed is on third party websites, which is why we needed useSlotReservationId.ts hook that stored reserved uid in a constant in memory. Instead, add new properties to Booker/store.ts
reservedSlotUidandsetReservedSlotUidto store reserved slot uid in the booker context. That way we don't have a standalone const that really can be within the context state.
The flow of frontend reserving slot and passing reserved slot uid when booking
- Store reserved slot - in web app components/hooks/useSlots.ts calls
trpc.viewer.slots.reserveSlotand then stores the reserved slot uid in the booker store context usingsetReservedSlotUid. We do the same in the platform booker atom atoms/hooks/useSlots.ts. - Pass it to request body - we have updated bookingCreateBodySchema.ts to now, in addition to reserved slot uid being a cookie, also always pass the uid in the request body when making the booking for predictable behaviour. Both web app booker and the platform booker atom use the atoms/hooks/bookings/useHandleBookEvent.ts to pass booking mutations (be it trpc or hooks) and then it extracts
reservedSlotUidfrom the booker context and passes it to booking-to-mutation-input-mapper.tsx creating a booking request body withreservedSlotUidin it. - Access reserved slot uid in api layer when booking happens:
- If the booking happens within web app or booker embed on 3rd party website, then api/book/event.ts for normal booking and api/book/recurring-event.ts for recurring booking are called by the web app booker. These handlers use the newly created
getReservedSlotUidFromRequestfunction (see code in this file) that extract the reserved slot uid either from cookies or from request body so that both web app storing uid in cookie and body and 3rd party websites where reserved slot uid is stored in body. - Platform booker atom makes request to the old bookings api v2 endpoint 2024-04-15/controllers/bookings.controller.ts which also uses the
getReservedSlotUidFromRequestfunction to extract reserved slot uid. - The new api v2 bookings endpoint also ues the new function
getReservedSlotUidFromRequestin 2024-08-13/services/input.service.ts to construct a booking request to core handlers containingreservedSlotUid
- If the booking happens within web app or booker embed on 3rd party website, then api/book/event.ts for normal booking and api/book/recurring-event.ts for recurring booking are called by the web app booker. These handlers use the newly created
- Access reserved slot uid in service layer. After api layer has extracted the reserved slot uid it is not time to make within a single transaction a booking and deleting the reserved slot if there is no other reserved slot first that was reserved sooner. Notably, this does not work for team event types like collective or round robin and seated event types because that is out of scope for this ticket and requires extra logic aka for those events all is as previously. If reservedSlotUid is not provided for api requests then we book as usual without checking if slot is first in line or not for it to not to be a breaking change. Instant bookings don't need to handle this because slots are not reserved for them at all.
- Regular bookings use RegularBookingService.ts service:
- I did mini refactor by storing
const bookingStartUtcas constant instead of copying the logicnew Date(dayjs(reqBody.start).utc().format())every where. - On line 1735 you see that if it is not a team event type and not a seated one then AND the
reservedSlotUidis passed then we call the newly created createBookingWithReservedSlot.ts function that within a transaction:- Calls newly created validateReservedSlot.ts - it finds the oldest active reserved slot (slot start time equals booking start time and slot end time equals booking end time) and if it does not find any it passes, if the
reservedSlotUidequals to the oldest active slot then it passes. It throws an error only if there are 2 or more reserved slots andreservedSlotUidis not the oldest. - Then it calls
createBookingfunction (it is the same one we have been calling in the RegularBookingService.ts and now still call if noreservedSlotUidis passed). - Delete reserved slot by
reservedSlotUidfor specific event type and start and end times. We do this because booker can re-use samereservedSlotUidwhen reserving slots for different event types so we can't just delete byreservedSlotUid.
- Calls newly created validateReservedSlot.ts - it finds the oldest active reserved slot (slot start time equals booking start time and slot end time equals booking end time) and if it does not find any it passes, if the
- I did mini refactor by storing
- Recurring bookings use RecurringBookingService.ts that then calls RecurringBookingService.ts for each individual booking.
- If it is recurring round robin booking we pass
reserveSlotUidas undefined because this PR does not handle team event types. - We do pass
reserveSlotUidif it is not team event type and is the first recurrence. This is because right now when user selects a slot for recurring events only 1 slot is reserved, but in theory we should reserve a slot for each recurrence. This also is out of scope of this PR so we only check the first recurrence reserved slot because only for that one a slot is reserved.
- If it is recurring round robin booking we pass
- Regular bookings use RegularBookingService.ts service:
Here is how the error looks in Booker
Tests - frontend layer
We test that frontend components pass reservedSlotUid within request body:
- booker booking-pages.e2e.ts tests that
- when slot is selected reserved slot uid is returned by
slots/reserveSlot, let's name it A - before booking is made reserved slot with uid A exists in database
- when booking is made (request to api/book/event) that the request body contains
reservedSlotUidand that it is equal to A - after booking is made that reserved slot uid in request body equals A and that after the booking the A reserved slot does not exist in database.
- when slot is selected reserved slot uid is returned by
- booker embed embed-pages.e2e.ts test that when booking is made (request to api/book/event) that the request body contains
reservedSlotUid - booker atom booker-atom.e2e.ts that when booking is made (request to api/book/event) that the request body contains
reservedSlotUid
Tests - backend layer
We test that if reservedSlotUid is received in the backend that uses services above that booking succeeds:
- Old api v2 bookings controller 2024-04-15/controllers/reserved-slot-bookings.e2e-spec.ts test:
- Reserve slot and then create booking by passing reserved slot uid in cookie
- Reserve slot and then create booking by passing reserved slot uid in request body
- Reserve slot A and B and then book with B and booking should fail because A is the first reservation in line that is not expired
- In database create expired reservation A and B that is still valid and then book with B and booking should succeed because A is the first reservation in line but has expired - check that A is still in database and B is deleted
- Do the same with recurring events
- Do the same tests for new api v2 bookings controller 2024-08-13/controllers/e2e/reserved-slot-bookings.e2e-spec.ts