#3614466: A suspended hold resumes with a brand new window, so cancelling a checkout repeatedly holds a sold-out slot indefinitely made a resumed hold get its own deadline back instead of a fresh window, so that going to payment and cancelling wins no extra time. That is right for a hold the checkout suspended. It is wrong for a hold that was already suspended before the checkout started, and in the ordinary workflow-driven booking that is every hold.
BookingWorkflowStarter suspends every held line when the workflow starts, well before the payment step, and says why: to hand hold expiry to the workflow so only its node timeouts govern the windows and yoyaku's hold cron cannot race them. The clock is deliberately stopped, and the visitor then spends real time inside the workflow.
By the time the payment step locks the order, the lines carry no live deadline to take, so the checkout suspension records nothing and the deadline kept is the one the workflow stored on the way in. Unlocking after a failed payment restores that, which is now well in the past, and the next sweep releases the places.
So a visitor whose card is declined loses their whole basket, at the worst possible moment, and the longer the workflow legitimately took the more certain it is. Before #3614466: A suspended hold resumes with a brand new window, so cancelling a checkout repeatedly holds a sold-out slot indefinitely they were given a fresh window and could simply try again.
Confirmed against the code: hold a line, age it so a little of its window is left, suspend it as the workflow starts, then lock and unlock the order. The deadline after the unlock is identical to the one from before the workflow, rather than left suspended or renewed.
The rule that is missing is the same one the checkout lock already applies to the order: put back only what you yourself took away. A hold that was already off the clock when the checkout locked belongs to whoever stopped it, and unlocking should leave it exactly as it found it, still suspended, for the workflow to confirm or release.
Proposal: make the stored deadline mean "the deadline the checkout took away", written only by the suspension the checkout performs and only when there is a live deadline to take. The plain suspendHold() goes back to simply taking the hold off the clock, which is what a handover of ownership wants, and is what the workflow start and the place step are doing. resumeHold() then restores a deadline only where one was recorded, and leaves a hold nobody recorded alone.
This keeps what #3614466: A suspended hold resumes with a brand new window, so cancelling a checkout repeatedly holds a sold-out slot indefinitely was for: a visitor who opens a payment and cancels still gets back exactly the deadline they had, and gains nothing.
Resolved in two parts on the same branch. The first is the rule above: record only the deadline the checkout itself took away, and restore only that, so a hold somebody else stopped stays stopped. The second removes the last case where that defensiveness is load-bearing: the workflow now suspends every line it owns, including lines held into the order after it started, so a basket can no longer carry a mixture of lines with and without a deadline. Inside a workflow nothing is recorded and nothing is restored, which means the record-and-restore added in #3614466: A suspended hold resumes with a brand new window, so cancelling a checkout repeatedly holds a sold-out slot indefinitely now only ever acts on a site running yoyaku without orchestra.
Issue fork yoyaku-3614475
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
mably commentedComment #4
mably commentedComment #6
mably commented