Skip to content

Fix the request root key for reservations_requests create/patch - #148

Merged
JureCindro merged 1 commit into
masterfrom
fix-reservations-requests-root-key
Aug 10, 2026
Merged

Fix the request root key for reservations_requests create/patch#148
JureCindro merged 1 commit into
masterfrom
fix-reservations-requests-root-key

Conversation

@JureCindro

Copy link
Copy Markdown
Contributor

The bug

create_reservation_request and patch_reservation_request send the request body under a requests root key:

post("reservations/requests", requests: [options])
patch("reservations/requests/#{id}", requests: [options])

Core rejects that. Api::V3::Reservations::RequestsController#create calls require_params_for(:reservations_request), and require_params_for pluralises its argument (resource_name.to_s.pluralize) to reservations_requests. When that key is absent it raises InvalidPayloadFormatError, which the V3 base controller renders as 400 Bad Request.

So as released in 1.3.0, neither method can succeed against a real Core — every create and every update 400s.

How it was found

Integrating this endpoint in bsa-booking (CMB-7366) against a live local Core. We hit the 400 on the first real call and probed both keys directly: requests → 400, reservations_requests → 201. The cassette recorded from that run shows the accepted body:

POST /api/v3/reservations/requests
{"reservations_requests":[{"conversation_id":19,"expires_at":"...","booking_payload":{...},"client_id":19,"rental_id":1}]}

Why the existing specs didn't catch it

The cassettes added in #146 look hand-written from the API design document rather than recorded against a server — they carry the doc's example values (conversation_id: 123, rental_id: 456, client_id: 789, final_price: "950.00"), round timestamps and production URLs. They encode the same assumption as the client code, so the specs agree with the bug rather than catching it.

I've corrected the two request bodies in those cassettes to match what Core actually accepts. Their responses are unchanged — see below.

Note on the asymmetry

The request root is reservations_requests, but Core's response root is requests. That is not a typo:

{"links":{"requests.rental":"..."},"requests":[{"links":{...},"id":60,...}]}

Response#resources_key derives the key from the response body, not from the request, so the two sides are independent and .pop keeps working unchanged. Verified against a live Core response.

Changes

  • create_reservation_request / patch_reservation_request now send reservations_requests: [options]
  • The two spec body assertions updated to match
  • The two cassettes' request bodies updated; responses untouched
  • CHANGELOG entry

bundle exec rspec spec/bookingsync/api/client/reservations_requests_spec.rb → 8 examples, 0 failures.

Worth flagging for the other channel apps: BSA Airbnb and VRBO integrate the same endpoint and will hit the identical 400.

(Unrelated, noticed while running the suite locally: the specs don't boot on Ruby 3.4 because base64 left the default gems and isn't declared in the Gemfile. I left that out to keep this PR focused.)

Core's Api::V3::Reservations::RequestsController calls
require_params_for(:reservations_request), which pluralises to
`reservations_requests`. Sending `requests` fails the key check and
returns 400 Bad Request, so create_reservation_request and
patch_reservation_request could never succeed against a real Core.

The response root stays `requests` — that is what Core actually emits,
and Response#resources_key reads it off the response body, so the two
sides legitimately differ. Only the outbound bodies change here.
@JureCindro
JureCindro requested a review from Robgra13 August 10, 2026 10:47

@Robgra13 Robgra13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hand-written from the API design document rather than recorded against a server

that is true, great find

@JureCindro
JureCindro merged commit c23225c into master Aug 10, 2026
3 checks passed
@JureCindro JureCindro mentioned this pull request Aug 10, 2026
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