Skip to content

fix(bookings): link catering and equipment to native room bookings - #545

Merged
MrYuion merged 2 commits into
developfrom
fix/native-booking-linked-orders
Oct 8, 2026
Merged

MrYuion merged 2 commits into
developfrom
fix/native-booking-linked-orders

Conversation

@camreeves

Copy link
Copy Markdown
Contributor

What was broken

With app.events.use_bookings on, a room booking is a staff-api booking rather than a calendar event. The meeting form still created its catering orders and equipment requests with ?event_id=<booking id>, which staff-api resolves against calendar event metadata, so every order was refused:

POST /api/staff/v1/bookings  (booking_type: catering-order)
-> 422 {"error":"error linking booking to event",
        "failures":[{"field":"event_id","reason":"Could not find metadata for event ARRAY['1138']"}]}

The room booking was then rolled back, so catering and equipment could not be ordered on a native room booking at all. staff-api has supported linking a child booking to its parent through parent_id since 2023.

The asset path already had a native branch, but it keyed on from_booking while the meeting form passes a CalendarEvent whose flag is from_bookings.

Change

  • createBookingsForEvent: for a native event, send parent_id (as an integer, which is what staff-api reads) and no event_id or ical_uid query; look existing linked bookings up without the event_id filter
  • validateAssetRequestsForResource: treat from_bookings like from_booking, and send parent_id as an integer

Unit tests added in libs/bookings and libs/assets. The #497 e2e specs ROOM-22 (catering) and ROOM-24 (equipment) pass against this change.

With `app.events.use_bookings` on, the meeting form created its catering
orders and equipment requests with `?event_id=<booking id>`. staff-api
resolves that against calendar event metadata, refused the order with
422 "error linking booking to event", and the room booking was rolled
back, so neither could be ordered on a native room booking.

Link them by `parent_id` instead, which staff-api reads as an integer,
and look existing linked bookings up without the `event_id` filter. The
asset path already had a native branch keyed on `from_booking`; the
meeting form passes an event whose flag is `from_bookings`, so honour
both.
@vercel

vercel Bot commented Oct 7, 2026

Copy link
Copy Markdown

Deployment failed for project frontend-templates with the following error:

Resource is limited - try again in 24 hours (more than 100, code: "api-deployments-free-per-day").

Learn More: https://vercel.com/placeos?upgradeToPro=build-rate-limit

@MrYuion

MrYuion commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

The parent_id link fixes the 422 on create. But edits of a native booking can now delete equipment bookings that belong to other bookings, and can create duplicate linked bookings.

1. Equipment edit deletes asset bookings of other bookings (data loss)

libs/assets/src/lib/assets.fn.ts:385-396

For a native booking, the "existing bookings" query sends email: host and booking_id: id. staff-api has no booking_id filter on GET /bookings, so it ignores it. The result is every asset request of the host on that day, not only the requests linked to this booking.

Effects:

  • With reset_state (time change), lines 411-416 add the request_id of every approved or rejected request in that list to changed. The returned function then calls removeBooking on them. This includes requests of the host's other bookings.
  • queryGroupAvailability(..., bookings.map((_) => _.id)) ignores all of those bookings. An asset that another booking of the host holds at the same time shows as free.

I reproduced the delete with a temporary unit test: a native edit with reset_state: true deleted an approved asset booking with extension_data.parent_id: '9999'.

Fix: keep only the bookings linked to this parent, and remove the booking_id parameter because it has no effect:

const bookings = (await queryBookings({ ... })).filter(
    (_) => !native || _.extension_data?.parent_id === id,
);

2. Native linked-booking lookup only finds the current user's bookings

libs/bookings/src/lib/bookings.fn.ts:761-768

For a native event, the query has no event_id, email or zones. In that case staff-api returns only the current user's bookings (bookings.cr index: user_id.nil? && zones.empty? && user_email.nil?). It does not include booked_by. Linked bookings get user_email: event.host and no user_id. When the editor is not the host, existing is empty. This happens for a delegate (now supported, see d095db0) or for an admin. Each save then creates a second set of visitor and catering bookings, and the old set stays. A removed catering order is not deleted.

Fix: for native events, read the children from event.linked_bookings. staff-api returns the children on every booking response (model to_json sets linked_bookings), and newCalendarEventFromBooking keeps them on created_event. This needs no query and no owner or period filter:

const existing = event.from_bookings
    ? (event.linked_bookings || [])
          .filter((_) => _.booking_type === type)
          .map((_) => new Booking(_ as any))
    : await linkedBookingsForEvent(event, type);

A smaller fix is to add email: event.host, but that still misses children after a host change.

Also update the doc comment at lines 750-753. It says the API ignores the period. That is not true for native events.

3. Rollback of a new native booking calls the events API

libs/events/src/lib/event-form.service.ts:1546 (outside the diff)

When a child booking fails on a new booking, _removeBookingAfterError calls removeEvent. That function sends DELETE /api/staff/v1/events/<id>. A native room booking is a staff-api booking, so I expect that this call fails and the room booking stays. This PR makes the path reachable for native bookings, for example on an asset clash (409). The PR description says the room booking was rolled back. The code does not do that for native bookings. Fix: call removeBooking(event.id) when event.from_bookings is true.

4. Type casts for parent_id

bookings.fn.ts:964 (Number(event.id) as any) and assets.fn.ts:509 ((asset_data as any).parent_id).

Booking.parent_id is string (libs/common/src/lib/types/booking.class.ts:87), but staff-api stores an Int64. Change the type to match the API in one place, then remove the casts. Also, Number('') gives 0 and a non-numeric id gives NaN, which serialises as null. Add an assert or a guard. The PR also adds from_bookings to an any parameter in validateAssetRequestsForResource. A Pick<CalendarEvent, 'id' | 'ical_uid' | 'from_bookings'> & { from_booking?: boolean } type catches the next flag rename. The from_booking/from_bookings mismatch that this PR fixes is that kind of bug.

5. Test gaps

The new native tests mock GET to return [], so they only cover the create path. Add:

  • An asset test where GET returns an approved request of another parent and reset_state is true. Expect no del call.
  • A createBookingsForEvent test for a native edit where the current user is not the host. Expect an update, not a second create.

assets and bookings unit tests pass at dc35caa.

Review by Claude Opus 5.5 in Claude Code (T3 Code).

Address review on #545:

- Asset requests of a native booking are filtered by
  `extension_data.parent_id`, so an edit no longer removes or ignores
  asset bookings of the host's other bookings. Drop the `booking_id`
  query, which staff-api ignores
- Read the children of a native booking from `linked_bookings`, so a
  delegate or admin edit updates them instead of creating duplicates
- Remove a new native room booking with `removeBooking` when a child
  booking fails, not the events API
- Type `parent_id` on the create path and drop the `any` casts

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@MrYuion

MrYuion commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

Fixed in 3d44184.

  1. Asset requests of other bookings: a native lookup now keeps only bookings where extension_data.parent_id matches the parent. The booking_id parameter is removed. This also fixes the desk booking path (from_booking).
  2. Linked-booking lookup: a native event now reads its children from event.linked_bookings and sends no query. replacedEventBookingIds returns nothing for a native event, because the booking id does not change on a host change. The doc comment is updated.
  3. Rollback: _removeBookingAfterError calls removeBooking(event.id) for a native booking.
  4. Type casts: the two casts are removed. createBooking takes a NewBookingData type with parent_id?: string | number, and parentBookingId() converts the id. validateAssetRequestsForResource now uses Partial<Pick<CalendarEvent, 'id' | 'ical_uid' | 'from_bookings'>> & { from_booking?: boolean }.
    • I did not change Booking.parent_id to number. About 15 places compare it with string booking ids, so that change belongs in its own PR.
    • parentBookingId() throws on an empty id. It does not reject non-numeric ids, because the mock API uses ids like -booking-123. Those are sent unchanged.
  5. Tests: added both requested tests. They fail on dc35caa and pass now. The assets, bookings and events unit tests pass.

All 17 affected apps build except stagehand. Its initial bundle is 1.01 kB over budget at dc35caa as well, so this change does not cause it.

Fixes by Claude Opus 5.5 in Claude Code (T3 Code).

@MrYuion
MrYuion merged commit 191acec into develop Oct 8, 2026
22 of 36 checks passed
@MrYuion
MrYuion deleted the fix/native-booking-linked-orders branch October 8, 2026 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants