Repository navigation
Let a customer release a hold before it runs out - #1
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
DELETE /v1/holds/{holdID}.holdIDis thebooking.idof the hold. It gives a running hold back.{"hold_token": "..."}), the same wayPOST /v1/bookingstakes it.token.Matches).inTx, the call locks the row withSELECT ... FOR UPDATEand setsstatus = 'released'. It does not touch notifications.204on success, and also when the hold was already released.404 hold_not_foundfor an unknown or malformed id, or a token that is not this hold's. Confirm gives the same answer.409 already_confirmedfor a confirmed hold, including one that was confirmed and later cancelled.409 hold_expiredfor a hold that ran out.422 validation_failedwhenhold_tokenis missing.410 hold_expired.0003_released_holds.sqladdsreleasedtobookings_status_check. It also adds it to the statuses thatbookings_customerallows without contact details.bookings_no_overlapalready covers onlyheldandconfirmed, so it is unchanged. The staff day list leaves released holds out, as it already does with expired ones.console.error.fetchkeepalive, so closing the tab right after does not cancel it.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.docs/api.md,README.mdandREADME.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:
409. Replaying the confirm when the release finds the hold confirmed keeps the guarantee the README makes for dropped connections.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):ErrHoldNotFound: a wrong token, another hold's token, an unknown id, a malformed id. An empty token gives a validation error.ErrAlreadyConfirmed, and so do confirmed-then-cancelled ones.ErrHoldExpired, both before and after the sweeper runs.ErrHoldExpired.ErrSlotTakenappears.TestReleaseHold(HTTP) covers:204body;405withAllow: DELETEfor other methods.TestExclusionConstraintIgnoresInactiveRows: areleasedrow needs no customer details and does not block a new hold.keepalive.204. Another client could hold the same start at once (201).409on the DELETE and201on the replayed confirm. The confirmation and manage link showed, and the database held one confirmed booking.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.