Skip to content

Let a customer release a hold before it runs out - #1

Merged
sinnercode228 merged 6 commits into
mainfrom
feature/release-hold
Oct 15, 2026
Merged

sinnercode228 merged 6 commits into
mainfrom
feature/release-hold

Conversation

@sinnercode228

@sinnercode228 sinnercode228 commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

What

  • New call DELETE /v1/holds/{holdID}. holdID is the booking.id of the hold. It gives a running hold back.
    • The hold token goes in the JSON body ({"hold_token": "..."}), the same way POST /v1/bookings takes it.
    • The token's SHA-256 hash is compared with the stored one in constant time (token.Matches).
    • Inside inTx, the call locks the row with SELECT ... FOR UPDATE and sets status = 'released'. It does not touch notifications.
  • Responses:
    • 204 on success, and also when the hold was already released.
    • 404 hold_not_found for an unknown or malformed id, or a token that is not this hold's. Confirm gives the same answer.
    • 409 already_confirmed for a confirmed hold, including one that was confirmed and later cancelled.
    • 409 hold_expired for a hold that ran out.
    • 422 validation_failed when hold_token is missing.
    • Confirming a released hold answers 410 hold_expired.
  • Migration 0003_released_holds.sql adds released to bookings_status_check. It also adds it to the statuses that bookings_customer allows without contact details. bookings_no_overlap already covers only held and confirmed, so it is unchanged. The staff day list leaves released holds out, as it already does with expired ones.
  • Widget:
    • Leaving the contact form (the Back button or a summary chip) drops the hold from the widget state. The widget calls the release without waiting for the answer. A failed release is logged with console.error.
    • When the answer arrives, the time list reloads. A list fetched while the release was still running counted the hold as busy; the reload brings back the starts it blocked.
    • The request is sent with fetch keepalive, so closing the tab right after does not cancel it.
    • If the release answers 409 already_confirmed, an earlier confirm whose answer was lost went through after all. The widget sends that confirm again with the same idempotency key and details, then shows the booking with its manage link.
    • The in-browser mock behind the demo follows the same rules and returns the same codes.
  • docs/api.md, README.md and README.ru.md: the new call is documented, the "no call to release a hold" limitation is removed, and the test counts are updated.

Why

Before this change there was no way to give a hold back. A customer who went back from the contact form and picked another time left the first slot held for up to 10 minutes. During that time nobody could book it or any time overlapping it, not even the same customer.

Three behaviour changes come with it:

  • Going back to the time list no longer keeps the customer's own hold marked there. The hold is released, and picking that time again creates a new hold. This is also what brings back the neighbouring times, which the customer's own hold had made unavailable.
  • Before, a customer whose confirm answer was lost could go back and come back to the same hold. A retry then returned the first result. Releasing on Back would have lost that booking silently: the customer was told it was not confirmed, and the release only logged the 409. Replaying the confirm when the release finds the hold confirmed keeps the guarantee the README makes for dropped connections.
  • The widget does not release on page unload. It keeps a running hold across a reload (sessionStorage), and a page that is unloading cannot tell a reload from a closed tab, so releasing on unload would break reloads. A tab closed on the contact form still leaves its hold to run out. This is now listed under known limitations.

How it is tested

Each new test was written first and failed for the expected reason. On main the reasons were a missing CHECK value, route or method. For the two widget tests added last, the reason was missing behaviour or a mutation of it.

  • internal/booking/release_test.go (integration):
    • After a release, the time and a start overlapping it are offered and can be held at once, without moving the clock. The released hold has no outbox rows and is not in the day list.
    • Releasing twice succeeds, and so does releasing again after the 10 minutes would have run out.
    • Each of these gives ErrHoldNotFound: a wrong token, another hold's token, an unknown id, a malformed id. An empty token gives a validation error.
    • Confirmed holds give ErrAlreadyConfirmed, and so do confirmed-then-cancelled ones.
    • Lapsed holds give ErrHoldExpired, both before and after the sweeper runs.
    • Confirming a released hold gives ErrHoldExpired.
    • 40 rounds of a release racing a confirm of the same hold: exactly one of them wins each round. Without the row lock this fails in round 1.
    • 40 rounds of a release racing three holds on the same period: at most one hold wins, and no error other than ErrSlotTaken appears.
  • TestReleaseHold (HTTP) covers:
    • every status code and error code above, and the empty 204 body;
    • slots offered again right after the release;
    • 405 with Allow: DELETE for other methods.
  • TestExclusionConstraintIgnoresInactiveRows: a released row needs no customer details and does not block a new hold.
  • Web:
    • HTTP client: DELETE with the token in the body, and keepalive.
    • Five mock tests that mirror the Go cases.
    • Five widget tests:
      • Back releases the hold and restores the full time list, with nothing marked as the customer's own.
      • The time list reloads once a slow release has gone through. Without the reload this test fails.
      • Changing the specialist from a summary chip releases the hold.
      • Going back after a confirm whose answer was lost shows that booking. The test checks that the confirm is sent again with the same key and details.
      • A failed release is logged, and the customer can still pick a time.
    • The old test "keeps its own hold when going back" was replaced by these.
  • By hand in Chromium, against the real server on a freshly migrated and seeded database:
    • Back sent one DELETE, which got 204. Another client could hold the same start at once (201).
    • With the first confirm's answer dropped after the server committed it, Back got 409 on the DELETE and 201 on the replayed confirm. The confirmation and manage link showed, and the database held one confirmed booking.
    • In the mock demo with two tabs, the released time appeared again in the other tab.
  • CI steps, run from a clean tree with a new database:
    • gofmt, go vet, staticcheck 2025.1.1 and go test -race ./... all pass. That is 123 test functions and 92 subtests; 67 of the functions are integration tests.
    • npm ci, lint, the 131 Vitest tests and the build all pass.
    • The Docker image was not built because this machine has no Docker daemon. The Dockerfile's static Go build and web build both succeed.

@sinnercode228
sinnercode228 merged commit b9fd6d5 into main Oct 15, 2026
3 checks passed
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.

1 participant