Interaction, webform and operation: an operator's step is not opened to a bearer link, a refused submission is reported, and a migrated run shows its new messages
The orchestra_interaction, orchestra_interaction_webform and
orchestra_interaction_operation slice of #3620619. One commit per finding, each
behaviour change carrying a test run red without the fix and green with it.
The later commits are three audit passes over the earlier ones; they are kept as their own commits because two of them found real defects in the first round's work rather than polish.
Behaviour
- A bearer capability link dispatched an assigned step's interactive surface. Nothing on the dispatch path asked whether the step is assigned, so a link holder was sent to an operator's own surface with every outcome link and the submit rendered, and every one of them refused on click. Four docblocks say the bearer doorway shows such a step's status. The rule now lives in one predicate that all three dispatch sites share: the grant's own step, the step a consumed branch link advanced to, and the step a stale link catches up to. The second of those is the ordinary case, since a visitor's link names a consumed token the moment they act.
- A refused first save of a webform step said nothing at all.
preSave()declines to bind a submission whose submitter may not complete the step;postSave()then returned without a warning, a log line, or anything on the confirmation page. The re-edit path already warned in the same situation. - A run's source view could show a later step's submission as the requester's. It read the latest bound submission, and a run through two webform steps binds one per step. It reads the one that started the run now.
- "Review (editable)" promised a reopen Webform will not perform. It needs the
form's
token_updatesetting, off by default; without it the party gets a blank form and a second submission binds, abandoning the one they were sent to amend. The interaction refuses the pairing at authoring time, naming the setting. - The editable-review error could throw out of the validation handler.
setError()reads#parentsoff the element it is given, and only the settings container carries them before the form builder has run. It also had to be named relative to the subform, becauseSubformState::setErrorByName()prefixes the subform's own path itself. - Reading a chain's messages loaded the whole chain. Removing the flag took the query's narrowing with it. A chain is not a handful of tokens: it holds everything between two interactive steps, so a loop between them piles every iteration into one, and this is read on the visitor's page. The narrowing is back, taken from the pinned definition rather than from anything stamped on a token, so it cannot go stale; a workflow declaring no message costs no query.
- A token's
has_interactionflag went stale after a version migration, so a node that gained a message in the target version had it skipped for the rest of the run, and the instance trace lost its "Open" link on the same tokens. The flag is gone; both readers ask the pinned definition.chainandchain_predecessorstay: they record what the run did, which a migration cannot invalidate. - The trace link reached its instance by dereferencing the token. That hangs the loaded instance on the field item for the rest of the request, so a token loaded before a migration answers from the version it was pinned to then, which is the same staleness the flag was removed for. It loads through the storage now, which attaches nothing and is already statically cached.
Documentation and translations
Several corrected claims turned out to live in more places than the code did.
The reopen promise was in four files and had been fixed in one; the
"work-item-free" claim was in five, the last of them phrased as "no work item"
so a search for the original wording missed it; the neutral-page rule was in
two. The sweep covers the module READMEs, docs/interaction.md,
docs/webform.md, docs/interaction-chains.md, docs/external-interaction-internals.md
and docs/architecture.md.
refuseUnviewableStep()justified its scope with an entity access hook that exists nowhere in the project, and pointed at amayView()that does not exist.- The source provider claimed webform access-checked rendering; a view builder checks nothing, and the run page is the gate.
- The examples README named a Request form the module does not ship, and its link-variant instruction produced a run whose every step reopens a form it has no submission of.
- The interaction operation was documented, described and translated as "work-item-free", and mints one like every other human node. Its corrected paragraph then over-claimed the other way: the outcome and the completer reach a process variable only where the node names one, and a node naming neither keeps no record of either.
- The module description rendered "pending-actions list" as a second French for a
term this project already writes
liste d'actions à traiter, including two entries further down the same catalog.en attenteis this project's French for a parked token, so the synonym also collided with it.
Outside the three modules
Three files, each forced by a fix above: docs/interaction-chains.md loses the
section describing the removed flag, translations/untranslated.txt gains the one
new log message, and .cspell-project-words.txt gains dispatchable.
Note for merging: this branch merges cleanly onto the current 1.x
(git merge-tree exits 0 against be5c94b9). !438 (merged) also appends to
translations/untranslated.txt and is currently in conflict against 1.x in its
own right, since !435 (merged) landed in that file first; it needs a rebase regardless of
the order the two are taken in.
What the audit passes found
Five passes over the earlier commits, kept as their own commits. Two found real defects rather than polish: the assigned-step guard covered one of the three dispatch sites, and the trace link above swapped a stale stamped flag for a stale attached entity. Two more closed coverage gaps where a fix was proven on one of its consumers and not the other.
Deploying this to a running site
Two structural changes, neither of which announces itself in a diff a reviewer skims, and both of which a fresh install or a test run hides:
OrchestraInteractionOperationsHooksgoes from one constructor argument to three. It is an autowired#[Hook]class, so nothing in aservices.ymlrecords it and a site still holding the compiled container passes the old argument list. The symptom is anArgumentCountErroron the instance trace, not at deploy time. A container rebuild clears it.- The
has_interactionbase field is gone. A site that installed an earlier alpha still has the column and the installed field-storage definition, soentityDefinitionUpdateManager()->getChangeSummary()reports it as pending until the field storage is uninstalled or the site is reinstalled. Pre-1.0 this project ships no update hooks, so reinstalling is the intended answer.
Verified
Every behaviour change above has a test that fails with the fix reverted. The
suite is CI's: the phpunit job is green, and the next-major (Drupal 12) lane is
played by hand each push, with its log read to confirm the classes ran rather than
skipped. Lint jobs are allow_failure here, so each is read individually; cspell
was red under a green badge on the first push and is green now.