Issue #3622388: Refuse a version migration the manager cannot place, before the operator maps anything

The version migration surfaces previewed a migration the manager would refuse, and one of those refusals was the manager's own bug.

WorkflowVersionManager remaps the flow a token arrived on as well as its node, because a synchronizing join tallies arrived arcs by their ids: a flow id the target version does not have counts toward nothing, so the join waits for an arc that can never arrive and the run never finishes. To remap it, the manager reads the arc's two endpoints out of the definition the run is executing — and read that definition as getDefinition((int) $instance->getDefinitionVersion()). On an unpinned run the version is NULL, so it asked for revision 0, got nothing, and refused a run whose arc plainly still connected the same two nodes. An unpinned run is exactly the one a migration exists for: getMigratableInstanceIds() goes out of its way to include it.

Above that, previewMigration() asked only which nodes are missing. So both surfaces reported the runs as movable, WorkflowMigrateForm asked the operator to map each live node, and only on submit did the manager refuse the arc — after the work, and after earlier instances in the batch had already migrated and committed.

And the form did not check the version belongs to this workflow at all. The target is a number in the route's path, the one thing on that page not derived from the run. Pointed at another workflow's version it reported this workflow's runs as movable and then asked the operator to map their live nodes onto the other workflow's node ids and labels, because the mapping select is built from the target's own nodes. The Drush command already refuses that up front, naming both workflows.

What changed

  • getSourceDefinition() resolves the definition a run is executing — its pinned version, else the live workflow — which is the answer DefinitionResolver::getEffectiveDefinition() gives the rest of the engine. That alone fixes the unpinned run.
  • resolveFlowMap() is the flow remapping, lifted out of planMigration() and returning both the rewrites to apply and the arcs the target has no counterpart for. The migration refuses those, now naming the instance and every unsettleable arc in one exception instead of the first token it happened to reach.
  • getUnmatchedFlows() is what the preview reports, and it is deliberately narrower: only an arc whose two endpoints both exist in the target. Where an endpoint is a node the target lacks, mapping that node settles the arc as well, and the preview asks for that mapping already — reporting it as unmatched would be telling the operator to change the workflow when the form is about to ask them for the one thing that works.
  • MigrationPreview carries $unmatchedFlows, and isReady() accounts for it. The property is a defaulted third argument, so nothing constructing a preview outside this class breaks.
  • WorkflowMigrateForm refuses a foreign version and an unmatched arc before it builds the mapping form or the migrate button, and says which arc and which workflow.
  • OrchestraVersionCommands reports unmatched arcs in --dry-run and refuses otherwise, worded so the operator changes the workflow rather than the --map.
  • French for all four new strings, each in its own project's catalog.

Tests

  • WorkflowVersioningTest::testAnUnpinnedRunHasItsArcMovedToo() starts an unpinned run on a reshaped workflow and asserts the token's source_flow becomes the target's arc. Without the fix the migration throws instead.
  • WorkflowVersioningTest::testThePreviewReportsAnArcNoMappingCanSettle() asserts the preview names the arc, that isReady() is FALSE, and that migrate() refuses with that arc in the message — the preview and the migration agreeing is the point.
  • WorkflowMigrateFormTest::testAnotherWorkflowsVersionIsRefusedUpFront() and ::testAnArcWithNoCounterpartIsReportedInsteadOfTheMappingForm() build the form against a stubbed manager and assert actions and map are absent and the note names the cause. Both stub previewMigration() so a regression walks past the refusal and fails on the button, rather than erroring earlier on a NULL preview.

Checks

phpcs Drupal (1,056 files, 0) and DrupalPractice (0), phpstan level 5 [OK] No errors, cspell clean, scripts/check-translations.php exit 0 with no missing French and no orphans, and both kernel classes green against this branch in a lean root.

Merge request reports

Loading