PolicyCheckpoint declares four cases. Registration has no caller anywhere in the module: the webform booking handler validates through OrderManager::validateOrder(), which is hardcoded to Confirm (BookingFormHandler.php:187, OrderManager.php:122). So the form's submit is evaluated as though it were the confirmation.

The documentation says otherwise. The checkpoint table in docs/constraint-policies.md lists Registration as "the booking form's submit", and the paragraph under it explains why that checkpoint deliberately holds no opinion about whether the basket is closed. Both describe a checkpoint nothing reaches.

What it costs

Only isLast() distinguishes the two. Confirm answers TRUE, which is what makes ConstraintPolicyManager::outcomeFor() log a warning for every policy it could not evaluate: "was not evaluated at the last checkpoint". At the booking form that sentence is false, because the confirmation will ask again. An operator reading the log is told a rule was permanently skipped when it was merely waiting.

A policy declaring PolicyFact::FROZEN_TRANSACTION is exactly the case: the order is still an editable basket at the form, so it is deferred there and the warning fires. No policy shipped with the module declares that fact today, so this is latent rather than visible in production, and a site policy that declares one meets it immediately.

Two smaller consequences. An evaluation is cached per order and per checkpoint, so a form submission and a confirmation inside one request share one cache entry instead of being told apart. And the checkpoint name exists to tell one evaluation from another in a log, which it cannot do while two moments report the same name.

Proposed resolution

Give the booking form the checkpoint it is documented to use. That needs a way to ask for one, since validateOrder() means the confirmation and its contract says so; the checkout gate added in #3614531: Nothing checks the constraint policies before payment, so a violating order can be charged and only refused afterwards took the same shape, adding checkoutViolations() rather than a checkpoint argument.

The alternative is to delete the Registration case and correct the table. Worth stating and rejecting: the form genuinely is an earlier moment with a different answer to isLast(), and it is the one surface where refusing costs the booker nothing.

Out of scope

Whether the form should enforce at all, and the Enforce constraints option that governs it. This issue only makes the checkpoint match what the documentation and the enum already say.

Issue fork yoyaku-3614899

Command icon 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

mably created an issue. See original summary.

  • mably committed e85b9bc0 on 1.x
    fix: #3614899 The booking form validates at the confirm checkpoint, so...
mably’s picture

Status: Active » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.