Locking an order for checkout is meant to freeze its lines. Removing is not refused at all.
Correction to the first version of this summary. It claimed that adding was unguarded too. That is wrong: holdGroup() takes a new line into an order only while it is empty or pending (BookingManager.php:180-193), and its comment covers "a late hold (a stale cart, a request racing a payment)". I had read only the validation at the top of the method. The add side needs no change.
Removing. BookingManager::release() checks only that the line is held and cancel() only that it is confirmed. Neither looks at the owning order, so a held line can be taken out of an order that is mid-payment: the per-line operations in the admin UI (BookingOperationController), the bulk release and cancel actions on the bookings tab, the API client, the seat map's re-seating, and the quantity stepper on the slot booking page all reach it. The amount the visitor is paying was computed from those lines.
The visitor's own surfaces are the ones least exposed, because they reach their order through BookingCart::current(), which counts only the empty and pending states as editable. That is each surface remembering to do the right thing rather than an invariant, and LocalBookingClient takes ids straight from its caller.
The wrinkle: the guard cannot simply live in release() unconditionally, because the checkout's own settlement legitimately releases a locked order's lines. DefaultSettlementSubscriber::onFailed() releases them when the payment fails, BookingTerminationReleaser when the workflow instance ends, and the order-wide release and cancel paths do the same. What has to be refused is an edit, not a settlement.
The rule also has to be narrower than the one for adding. Cancelling one ticket of a confirmed order is ordinary operator work, and cancel() only accepts a confirmed line, which lives in a settled order by definition, so refusing everything outside empty and pending would break it. Only the window the checkout owns needs closing.
Proposal: one guard in the booking manager, so every surface inherits it, refusing a removal while the order is in checkout (locked or placed); the settlement paths opt out by name. Companion to #3614424: The checkout amount is computed before the order is locked, so the basket can still change between the two, the ordering half of the same invariant: the checkout amount is computed before the lock is taken.
Issue fork yoyaku-3614426
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 #2
mably commentedComment #4
mably commentedComment #7
mably commented