Skip to content

refactor(transitions): the status engine asks two facades instead of fourteen collaborators - #3858

Merged
rubenvdlinde merged 8 commits into
developmentfrom
build/dq-l11-26
Oct 15, 2026
Merged

rubenvdlinde merged 8 commits into
developmentfrom
build/dq-l11-26

Conversation

@rubenvdlinde

@rubenvdlinde rubenvdlinde commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

method-decomposition slice 8h: StatusTransitionService.

Stacked on #3854 (decision 224), which is stacked on #3836. Three commits are this PR's own: the design, the characterisation test, the split.

What changes

The status engine coupled to 18 classes against a limit of 13 and took 14 constructor arguments. It carried two PHPMD suppressions for that. Both are gone.

The engine asked two kinds of question of fourteen collaborators. It now asks two classes.

The engine asks New class Over
What does this case declare about its status and its moves? Transitions/CaseDeclarations StatusDeclarations, TransitionDeclarations, OfferedTransitions, StatusChecklist, and the status row
What does a move write, and what did earlier moves write? Transitions/CaseMoveLedger CaseStatusStore, CaseResultWriter, SideEffectDispatcher

The engine keeps the decision: which transition, in which order the checks run, what is refused and what the caller hears. It takes nine arguments and couples to 12 classes.

The design is in openspec/changes/method-decomposition/design.md, "StatusTransitionService: two facades", written before the code. It ends with the two points where the build differs from the first draft, and why.

What does not change

  • The public methods and their answers: getAvailableTransitions(), execute(), executeFreeForm(), replay(), isAdmin(), ADMIN_GROUP_ID.
  • The 32 classes that inject the engine. The container builds both new classes by type.
  • The seven classes behind the facades, their constructors and their tests.

The constructor of the engine does change. Nothing outside the tests builds it by hand.

How it is proven

  1. Characterisation first, own commit (96fe20b), green on the single class: StatusTransitionServiceOrderTest, 10 tests. It pins what the five existing engine tests left open: the order of the five refusals, the conflict check, the one save of a closing move with the version it read, the timer after the save, the record that cannot be written back, the free-form refusals, and the shape of the available transitions. Seven mutations of the old engine turn it red.
  2. The split (a3a1c0a): that test and the five existing ones are green with only their constructor call changed. No assertion was edited.
  3. Own tests for the new classes: CaseDeclarationsTest (4), CaseMoveLedgerTest (6).

Debt

.debt-baseline.json: phpmd.suppressWarnings 182 to 180 (development's number after train 5, minus 2). Inherited: development itself measures 187 suppressions against its own 182 since #3884/#3888 etc., so the debt-ratchet check reads red on development and here (185 measured against 180).

Inherited, not fixed here

StatusTransitionService::writeMove() and a few listeners still cite openspec/changes/what-a-status-declares/..., archived on 7 October. The PR stacked on this one repoints all 206 of them.

Verified

On a3a1c0a, every composer check:strict stage, one at a time in the lane's check worktree (exit codes in dq-l11-logs/r6/s26.rc):

  • lint 0, refusal-grounds snapshot 0, phpcs 0, phpmd 0 (whole tree, with both suppressions removed), psalm 0, phpstan 0.
  • PHPUnit --no-coverage: 8,507 tests, 2 failures. Both are StubApiDriftTest (DecisionChangeRequestedEvent, DecisionVisibilityRequestedEvent), the stub drift train 4 fixes. Not in this diff.
  • The nine engine and facade test classes under a coverage driver with fail-on-risky (pcov, 52 tests): no risky test.
  • Hydra gates on the diff: 78 of 79 applicable ran, 8 failed, none in this diff (22 and 53 need node_modules; 23, 43, 51, 55, 104, 107 inherited). Gates 16 and 46 pass.
  • openspec validate method-decomposition: valid.

Not run: npm run lint and vitest. No JavaScript, no l10n and no register change.

Live pass (decision 139): move a case through one status, close one with a result, and do one free-form move as an administrator.

Merged with development (14 Oct), verified again

#3854 is merged in, with development after train 5 (b99db82). .debt-baseline.json was the only conflict.

On b99db82, every composer check:strict stage once, one at a time in the lane's check worktree (exit codes in dq-l11-logs/r7/s26.rc):

Decision 224. The case type delete check, drc-004 and drc-007 let a request
through when their lookup failed. They now refuse with 503
check-not-completed, name the rule, and log the rule and the object.
drc-003 gives the same answer instead of claiming a duplicate, and the
controller's relation check before a document delete (drc-008a) follows.

Two sites leave the catch-return-null list, ceiling 263 to 261.
… facades

method-decomposition slice 8h. The engine couples to 18 classes against 13
and takes 14 constructor arguments. The design gives it one facade over what
a case declares and one over what a move writes; the engine keeps the order
of the checks.
Characterisation before the StatusTransitionService split (method-decomposition
slice 8h), green on the single class: the order of the five refusals, the
conflict check, the one save of a closing move with the version it read, the
timer after the save, the record that cannot be written back, the free-form
refusals and the shape of the available transitions. Seven mutations of the
engine turn it red.
…fourteen collaborators

method-decomposition slice 8h. StatusTransitionService coupled to 18 classes
against 13 and took 14 constructor arguments. Transitions/CaseDeclarations
reads what a case declares about its status and its moves, and
Transitions/CaseMoveLedger holds what a move writes. The engine keeps the
decision and the order of the checks, takes nine arguments and couples to 12
classes. Both suppressions are gone; phpmd.suppressWarnings 183 to 181.

The public methods and their answers are unchanged. The characterisation
test and the five existing engine tests are green with only their
constructor call changed.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/dossiq @ 72291c0

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ❌
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-manifest ✅
check-vue3-compile ✅
test-l10n ✅
format ❌
check-schema-l10n ✅
check-l10n-js ✅
check-debt-ratchet ❌
check-types-e2e ✅
composer ✅ ✅ 109/109
npm ✅ ✅ 659/659
app:check-code ⏭️
info.xml ✅
REUSE ❌
lockfile sync ✅
PHPUnit ❌ 8660/8660 · 3 skipped (no test failed, the job failed for another reason — see its log)
Newman ⏭️
Playwright ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development.
Hydra gates ❌

Quality workflow — 2026-10-14 20:16 UTC

Download the full PDF report from the workflow artifacts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant