feat(parafering): an approval route is a flow, not a schema - #1582
Merged
Conversation
An approval route is a sequence of manual sign-offs that must be taken before a decision may be attached to a case. That is a flow with a person at each step. It was modelled instead as a schema, twice: dossiq's `parafeerroute` and decidiq's `ApprovalRoute`. The second was mine, added last week in decidiq#1028, and it was the wrong shape. This supersedes it. Each step becomes a `dossiq.askPerson` node rather than a bare `openregister.awaitSignal`. The difference is the one an approver notices: askPerson raises a real dossiq TASK against them and waits for the answer in the same node, so the step lands in the work queue they already read. A raw await signal waits for an answer nobody was asked for. The chain ends at `dossiq.requestDecision`, because that is what an approval route is FOR. The case schema already carries `decisions` as a relation to decidiq Decision objects, so the destination was already there. A route whose step names no actor is refused whole rather than projected partly: askPerson rejects an empty assignee, and a flow that quietly omits a sign-off somebody expects is worse than no flow. Verified against the live instance: 7 real routes, 2 to 6 steps each, all projected; a re-run updates 7 and creates 0. "Collegeadvies - Omgevingsvergunning" came out as juridische dienst, teamleider VTH, afdelingshoofd VTH, portefeuillehouder, then the decision. The projections arrive DISABLED. The routes still drive parafering, and two live copies would ask every approver twice. Retiring the route, the schema and the settings page is the next change, once these have been checked against real work. The visible symptom this exists to fix is a settings menu with too many entries, and this change removes none of them; it makes removing one possible. Also: WorkflowTemplateFlowMigrator and this one shared 133 lines of identical machinery, now the ProjectsOntoFlows trait. Extracting it broke the working migrator — I dropped the `\Flow\` segment from the FlowService id, and every unit test stayed green because the container is a mock that answers to any id. The live dry-run caught it, reporting "OpenRegister exposes no FlowService" and exiting 0. The id is now a named constant with a test that asserts the string.
rubenvdlinde
requested review from
Rem-Dam,
WilcoLouwerse,
bbrands02 and
rjzondervan
as code owners
August 31, 2026 20:01
Contributor
Quality Report — ConductionNL/dossiq @
|
| 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 | ✅ | ||||
| composer | ✅ | ✅ 106/106 | |||
| npm | ✅ | ✅ 540/540 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ❌ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-31 20:06 UTC
Download the full PDF report from the workflow artifacts.
Two CI findings on the previous commit, both real. psalm: "Cannot add an array to a non-array" on `$summary[$row['outcome']] + 1`. The summary held counts and the row LIST in one array, so incrementing by key reads as "add 1 to whatever is at that key", and nothing stopped the key being `rows`. An outcome named `rows` would have appended to the row list instead of counting. Counts and rows are now separate structures, merged only in the return. Coverage ratchet: the change added 216 statements, 157 of them an occ command with no test. This repo had no command tests at all, so there was nothing to mirror; there is now a CommandTester suite covering what the command actually owns, which the migrator's own tests do not reach: it refuses without --user because a flow's owner is permanent, it refuses an unknown uid rather than guessing, it exits non-zero on a partial run, and `--dry-run` reaches the migrator AS a dry run. That last one earned its place — I pinned `$dryRun = false` and it went red. A command that accepted the flag and passed false would write while reporting it had not.
Contributor
Quality Report — ConductionNL/dossiq @
|
| 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 | ✅ | ||||
| composer | ✅ | ✅ 106/106 | |||
| npm | ✅ | ✅ 540/540 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-31 21:18 UTC
Download the full PDF report from the workflow artifacts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An approval route is a sequence of manual sign-offs that must be taken before a decision may be attached to a case. That is a flow with a person at each step.
It was modelled instead as a schema, twice: dossiq's
parafeerrouteand decidiq'sApprovalRoute. The second was mine, added last week in decidiq#1028, and it was the wrong shape. This supersedes it.Why askPerson and not awaitSignal
Each step becomes a
dossiq.askPersonnode. The difference is the one an approver actually notices: askPerson raises a real dossiq task against them and waits for the answer in the same node, so the step lands in the work queue they already read. A rawopenregister.awaitSignalwaits for an answer nobody was asked for.The chain ends at
dossiq.requestDecision, because that is what an approval route is for. The Case schema already carriesdecisionsas a relation to decidiq Decision objects (referenceType: decision,x-allow-create: true), so the destination was already modelled.Verified against real data
7 real routes on the dev instance, 2 to 6 steps each, all projected. A re-run updates 7 and creates 0. One came out as:
A route whose step names no actor is refused whole rather than projected partly. askPerson rejects an empty assignee, and a flow that quietly omits a sign-off somebody expects is worse than no flow.
What this does NOT do
The projections arrive disabled. The routes still drive parafering through
BesluitvormingParafeerService, and two live copies would ask every approver twice. Retiring the route, theparafeerrouteschema, decidiq'sApprovalRouteand the settings page is the next change, once these have been checked against real work.Worth stating plainly: the visible symptom this work exists to fix is a settings menu with too many entries, and this change removes none of them. It makes removing one possible.
The bug the live run caught
WorkflowTemplateFlowMigratorand this one shared 133 lines of identical machinery, now theProjectsOntoFlowstrait. Extracting it broke the working migrator: I dropped the\Flow\segment from the FlowService container id.Every unit test stayed green, because the container is a mock that answers to any id — a fake that accepts any argument cannot catch a wrong one. What caught it was running the command for real: it reported "OpenRegister exposes no FlowService on this instance" and exited 0, which reads as "nothing to do" rather than "I looked in the wrong place".
The id is now a named constant, and
testItAsksTheContainerForTheRealFlowServiceIdasserts the exact string. I put the wrong id back and confirmed it goes red.Verification
phpcs,phpmd,phpstan: cleanordersort turns it red