Four naming problems in one neighbourhood of the engine, found while fixing #3619446: A partial area reduction writes the booking's quantity outside the engine's locks. Each is small; together they make the one guard that protects a paying visitor's order hard to read.
The engine speaks the payment layer's word
CHECKOUT_STATES, isInCheckout() and assertNotInCheckout() live in the core module's src/, and the word checkout appears there 93 times across 18 files. Core depends on nothing but drupal:options; the modules that own a checkout are yoyaku_payment, with 79 mentions, and yoyaku_cart, with 24.
What core actually owns is two states, STATE_LOCKED and STATE_PLACED, and it already knows the right word for them. BookingTransactionInterface explains CHECKOUT_STATES as "An order in one of these is frozen", and isInCheckout() as "so its bookings are frozen". PolicyRequirement::FROZEN_TRANSACTION is already 'frozen_transaction'. The prose and one constant say frozen; the API says checkout.
The engine does not need to know a checkout exists. It needs to know the order is frozen and not the basket's to edit.
A guard wearing test vocabulary
assertNotInCheckout() is public on HoldEngine and throws BookingException. In Drupal core, assert* is test vocabulary: 726 such methods in the test code against 20 in lib, and several of those 20 are test-support helpers that happen to live there. Core's words for a guard that throws are validate*, with 97, check*, with 54, and ensure*, with 23. This module's own word is refusal, in BookerRefusal and getRefusalForThrowable().
Methods named after their caller
releaseForSettlement() and cancelForSettlement() are release() and cancel() minus the guard. The suffix says who is allowed to call them, which cannot be guessed from the name: a reader has to find assertNotInCheckout()'s docblock to learn that an edit is refused while a settlement is not. Its exception message has to explain the convention out loud: "Settlement uses the ForSettlement methods."
Core names the inner, unguarded operation with a do* prefix (doSave, doDelete, doCreate) and uses For in a method name for the subject of the work (getActionsForRoute, getExtensionsForMimeType), never for the caller's identity.
Settlement means two unrelated things
There is the post-event honoring sweep: SETTLEMENT_MANUAL, SETTLEMENT_COMPLETE, SETTLEMENT_UNUSED and settlement_grace on the resource, deciding whether a booker turned up. And there is the checkout letting go of what it holds, which is what releaseForSettlement() and yoyaku_payment's DefaultSettlementSubscriber mean.
TransactionManager::cancelTransaction() uses both senses within ten lines: it refuses an order that has reached a settled outcome in the first sense, and calls releaseForSettlement() in the second.
Proposed
Let the engine say frozen, which is the word its own prose already uses, and leave checkout to the modules that have one.
- CHECKOUT_STATES becomes FROZEN_STATES, and isInCheckout() becomes isFrozen().
- The guard becomes HoldEngine::ensureOrderNotFrozen(), which says what it refuses and no longer borrows the test suite's assert*.
- releaseForSettlement() and cancelForSettlement() become doRelease() and doCancel(), which is core's prefix for the inner operation a guarded one wraps, so the exception message stops explaining a convention out loud.
A second frozen, which the proposal did not foresee
PolicyRequirement::FROZEN_TRANSACTION already exists, and it does not mean the two states above. Its resolver answers the negation of isOpenForEditing(), so it holds for confirmed, partial, canceled, completed and unused as well, and its own docblock says so: "the negation of open for editing rather than membership of CHECKOUT_STATES". Giving the narrow span the word frozen would leave two frozens with different memberships in one codebase, and a policy declaring the requirement and then reading isFrozen() would get opposite answers at the review step, which is the checkpoint that requirement exists to serve.
So the wider one is renamed instead: PolicyRequirement::SEALED_TRANSACTION, with SealedTransactionRequirementResolver behind it. Frozen is the narrow span, held against a payment and reversible; sealed is every state that takes no more bookings.
CheckoutInteractionInterface, decided
It keeps its home in the engine and loses its name. It is BookerQuestionInterface now, with buildQuestions().
The home is not a matter of taste. The two sides of the seam are in different submodules: yoyaku_placement implements it and yoyaku_cart draws it, and placement does not depend on the cart. ModuleBoundariesTest states the rule the module already lives by, that the lower module owns the interface and the optional higher module implements it, so an interface in the cart would mean every module with a question to ask depends on the cart, and the surface that commits an order is not always one. PlaceSpacingInterface is the same rule read the other way: its implementer and its reader are both inside yoyaku_placement, so that is where it lives.
The name was the actual defect, and it was wrong from the day it was written: the questions are drawn on the cart page, immediately above the button that commits, and never on the payment page, so the interface was named after a moment it is not even shown at. Nothing it does is about a checkout: a policy hands back render elements and whatever surface is committing the order shows them. The module's own tests already call them questions.
PolicyCheckpoint::Checkout keeps its name
A checkpoint names a moment in the booker's journey rather than a state of the engine, and Registration is the same shape: the engine has no registration either, and the module that has one is the module evaluating there. The reason is written beside the enum so the next naming pass does not churn it.
Not in scope
The word checkout inside yoyaku_payment and yoyaku_cart, where it is correct. The post-event sense of settlement, which keeps the word.
AI-Generated: Yes (Claude Code found these while implementing #3619446: A partial area reduction writes the booking's quantity outside the engine's locks, took the core measurements quoted above by counting method-name shapes in Drupal core's lib and test code, and wrote the code and tests on the merge request. I reviewed and ran the work myself before posting it.)
Issue fork yoyaku-3619680
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