feat(besluitvorming): a voorstel is a case, not a record of its own - #1791
Closed
rubenvdlinde wants to merge 6 commits into
Closed
feat(besluitvorming): a voorstel is a case, not a record of its own#1791rubenvdlinde wants to merge 6 commits into
rubenvdlinde wants to merge 6 commits into
Conversation
The dossiq menu carried a `Proposals` entry nobody could account for. Following it back found that a voorstel was never a thing in its own right: it is a case whose case type requires a decision, modelled a second time as a standalone object with its own list page, its own status vocabulary and its own sign-off engine. That second model retires completely. Gone are the `proposal`, `parafeerroute`, `parafeeractie` and `paraferingAuditEntry` schemas, the `/voorstellen` pages, the menu entry, the deepLink, the parafering runtime across 21 PHP classes, the frontend surface, and every test written against them. Nothing moved this here from decidiq, which was the working assumption. `voorstel` has been in this app since its earliest schema work under the old `procest` name. docs/decisions/besluitvorming-vs-decidesk.md is the reason it stayed: on 2026-06-22 it recommended keeping besluitvorming here and NOT folding it into decidesk. Five later changes moved the other way and nobody revoked the doc, so it kept telling readers the opposite of what the fleet was doing. It is marked superseded here. The abstraction was already shipped, in three pieces: caseType.decisionTypes says which decision a case type needs, caseType.workflowDefinition is the flow whose human steps are the sign-offs, and case.decisions holds what decidiq concluded, raised over the typed seam by DossiqRequestDecisionNode. The three besluitvorming case types ARE that abstraction. They survive untouched, keeping their `Voorstel opstellen` status and `Steller` role, because that word is the case type's own vocabulary. No gate-53 waiver is added, and none is owed: the PAGE is deleted rather than hidden, so there is no orphaned route for a `removals` entry to speak for. That answers the question src/menu-layout.json left open on 2026-09-02, and the answer is that the question did not apply. Two defects fell out of the retirement: - The `voorstelReminder` row action POSTed to /apps/dossiq/api/notifications/parafering-reminder, a URL that appears nowhere in appinfo/routes.php at any commit. Every click answered 404 into an empty catch. - The three bvw bundles declared a `voorstelStatus` guard. An unknown guard type evaluates as `passed: false`, so deleting the guard class alone would have stranded every besluitvorming case at `Parafering` on every install, with "Onbekende guard" as the only evidence. The bundles are amended in this same commit for that reason. Stored rows are not touched, and the Dutch-to-English rename steps keep their `voorstel` entries: they run over rows an upgrading install may still hold. Verified: PHPUnit 2911 green, vitest 359 green, ESLint 0 errors (839 warnings to 823), PHPCS 0 errors, PHPMD clean, Psalm and PHPStan no errors, all 75 applicable hydra gates green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…erated file The retirement commit edited docs/features.json by hand. That file is GENERATED: the pre-commit hook regenerates it from openspec/features.overlay.json whenever staged changes touch openspec/, and it silently put the entry straight back, inside the same commit that removed it. The hook printed one line saying so and exited 0. So the retirement shipped a capability list still advertising "Parafering en mandaat" as stable, with a docsUrl pointing at openspec/specs/parafeerroute-engine/spec.md, which does not exist and did not exist before this work either. Removed from the overlay, then regenerated with the hook's own script. 24 entries to 23 in both files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rubenvdlinde
requested review from
Rem-Dam,
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
September 3, 2026 20:06
development gained #1788, whose third part had just WIRED the parafering door this branch removes: a RaiseParaferingAction on the proposal lifecycle's startParafering transition, a reworked ParaferingRaiseService and a 257-line test. It landed twenty minutes before this branch pushed. Resolved toward the retirement, which is what the branch is for. The `proposal` schema conflict (deleted here, extended there) takes the deletion, and RaiseParaferingAction, its test and ParaferingRaiseService go with it. The rest of #1788 is untouched and still shipping: AdoptShippedFlowsCommand, ReportShippedFlowAdoption and its two info.xml registrations, and the behandelaars group seeding. Those were parts 1 and 2, and they have nothing to do with proposals. Also dropped `parafeerroute` from SeedPayloadSchemaConformanceTest's SEEDER_CONSUMED_KEYS, new in #1788. That list mirrors the seeder's unset() calls, and the unset went with the parafeerroute seeding. Verified on the merged tree: PHPUnit 2922 green, vitest 359 green, ESLint 0 errors, PHPCS 0 errors, Psalm and PHPStan no errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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-09-03 20:31 UTC
Download the full PDF report from the workflow artifacts.
…assing All three were green in my terminal and red in CI, each for a different reason, and all three were readable locally the whole time. I read the printed summary instead of the exit code. 1. eslint. Identical output in both places, "823 problems (0 errors)", and exit 2. ESLint keeps eslint-suppressions.json, and a suppression for a file that still gets linted but no longer has the problem is a hard error. Removing the voorstelReminder handler took the console call its src/customComponents.js suppression covered. Pruned: 11 entries, of which ten were already dead on development for files deleted earlier. Only the customComponents one was load-bearing, because ESLint never lints a file that does not exist and so never calls its suppression unused. That is why development stayed green carrying the same ten. 2. prettier. Dropping 'proposal' from EXPECTED_SEARCHABLE_SLUGS made the array short enough to fit one line, which prettier --check wants and I did not run. 3. PHPUnit, all six matrix cells, with ZERO failures and ZERO errors: Tests 2922, Assertions 17500, Risky 1. phpunit.xml sets failOnRisky="true" and beStrictAboutCoverageMetadata="true", so one risky test reddens the cell. SeedPayloadSchemaConformanceTest executes RegisterFragmentMerger in setUp() without listing it, and strict coverage metadata only fires when a coverage driver is loaded. There is none on this box, so the local run cannot see it at all. Added the @uses annotation. That third one is not mine: development is red on the same risky test in the same six cells, from #1786 onward. Fixed here because this PR is red on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
development moved again while this branch was being fixed, and the PR went back to CONFLICTING, which is why no quality workflow ran on the previous push: GitHub runs no pull_request workflows when it cannot compute a merge commit. Four checks still pass in that state (CodeQL, conflict markers), and on a dashboard that is indistinguishable from a full pass. One conflict, and it is the same fix twice. #1789 carried `test(seed): declare RegisterFragmentMerger as used`, the identical @uses tag added here one commit earlier. Took theirs: same tag, plus a docblock paragraph explaining why the test is risky only under a coverage driver. Verified by EXIT CODE this time, not by reading the summary, which is what missed three failures on the previous round: format 0, lint 0, stylelint 0, test:unit 0, phpcs 0, phpstan 0, psalm 0, hydra-gates 0, phpunit 0 excluding StubApiDriftTest. StubApiDriftTest is excluded from that local number on purpose. It compares tests/Stubs/ against the sibling ../openregister working tree, which another session is editing: it has now reported StoreDescriptor drift in both directions within an hour as that checkout changed branch. CI clones openregister fresh and passes it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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-09-03 21:23 UTC
Download the full PDF report from the workflow artifacts.
…l class has CI is the only place that measured this correctly, and it took two wrong turns to believe it. StubApiDriftTest compares tests/Stubs/ against a SIBLING CHECKOUT at ../openregister. In a shared dev workspace that is another session's live working tree, and tonight it moved twice. The first local run said the real class had five constructor arguments and the stub four, so the argument was added. Twenty minutes later the same test said the opposite, because that checkout had switched to a feature branch without the parameter. Reading a test that reverses itself as environment noise, the fix was reverted. CI then failed on it, on all six PHPUnit cells, because CI clones openregister FRESH. Checked against the canonical remote rather than either local state: gh api .../openregister/contents/lib/AppHost/Service/StoreDescriptor.php?ref=development The real class takes five: `public readonly array $types = []`. The stub now does too. The lesson is narrow and worth keeping: a sibling working tree in a shared workspace is not evidence about anything, in either direction. A test that answers differently twice is not flaky, it is reading a moving input, and the remote is the input that decides. This is not caused by this branch. openregister gained the parameter tonight, between development's last CI run and this one, so every dossiq PR opened after it is red on the same cell until the stub catches up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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-09-03 21:34 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Author
|
Superseded by #1796, which carries the identical tree (same tree hash) with the Closed rather than updated because rewriting history needs a force-push, which this workspace's |
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
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.
Why
The dossiq menu carried a
Proposalsentry nobody could account for. Following it back changed what the fix should be.A voorstel is not a thing in its own right. It is a case whose case type requires a decision. It was modelled a second time as a standalone object, with its own list page, its own status vocabulary and its own sign-off engine.
Nothing moved it here from decidiq, which was the working assumption.
voorstelhas been in this app since its earliest schema work, under the oldprocestname, long before decidiq existed as a target. docs/decisions/besluitvorming-vs-decidesk.md is the reason it stayed: on 2026-06-22 it recommended keeping besluitvorming here and explicitly not folding it into decidesk. Five later changes moved the other way and nobody revoked the doc, so it kept telling readers the opposite of what the fleet was doing. It is marked superseded here.The menu entry was a gate artefact, not a decision.
src/menu-layout.jsonsays so:Voorstellenhad been removed, and was restored on 2026-09-02 because the case-detail consolidation dropped thecase-voorstellenwidget, the only navigation edge gate-53 could follow to/voorstellen. That note asked whichever change retired the surface for real to re-remove the entry with a truthful waiver.What replaces it
The abstraction was already shipped, in three pieces.
Cases, narrowed by case type throughfolderSidebar.filterFieldCaseDetail, and itscase-decidesk-decisionsleafcaseType.decisionTypescase.decisionsThe three besluitvorming case types (College-besluit, Raadsbesluit, Mandaatbesluit) are that abstraction, and they survive untouched. They keep their
Voorstel opstellenstatus, theirStellerrole and their voorstel document types, because that word is the case type's own vocabulary. What retires is the pretence that a proposal is a record you file somewhere else.No gate-53 waiver, and none is owed
The page is deleted rather than hidden, so there is no orphaned route for a
removalsentry to speak for. That answers the question the 2026-09-02 note left open, and the answer is that the question did not apply. gate-53 reports 7 warnings, all pre-existing, none namingVoorstellen.Two defects this found
Neither is the point of the change, and both say something about how the surface was maintained.
A row action that had never worked.
voorstelReminderPOSTed to/apps/dossiq/api/notifications/parafering-reminder. That URL appears nowhere inappinfo/routes.php, at any commit. Every click answered 404 into an empty catch that logged to the console.A guard that would have stranded every besluitvorming case. The three bundles declared a
voorstelStatusguard. An unknown guard type evaluates aspassed: falseinGuardRegistry, so deleting the guard class alone would have locked every besluitvorming case atParaferingon every install, with "Onbekende guard" as the only evidence. The bundles are amended in the same commit.A third one, in this PR's own first commit
The retirement edited
docs/features.jsonby hand. That file is generated: the pre-commit hook rebuilds it fromopenspec/features.overlay.jsonand silently put the entry back inside the same commit that removed it, printing one line and exiting 0. The second commit fixes it at the source. Worth knowing about before the next person edits that file.What is deliberately not done
Stored rows are not deleted. Retiring a schema does not remove what an install already holds.
RenameDutchColumnsandRenameDutchSchemaSlugskeep theirvoorstelentries: they are historical migrations that run over exactly those rows.Five sibling changes are removed rather than left standing.
parafering-to-decidiq,parafering-runtime-to-decidiq,parafering-runs-as-a-flowandapproval-routes-are-flowsall existed to move the engine somewhere; deleting it reaches the same end state and asks nothing of decidiq.bw-voorstellen-viewexisted to build the list view/voorstellennever had, which is the one clear waste here.StubApiDriftTestis left alone. It comparestests/Stubs/against a sibling checkout at../openregister, which in a shared dev workspace is another session's live working tree. It was red on cleandevelopment, and then reported drift in the opposite direction twenty minutes later because that checkout switched branch. CI clones openregister fresh, so its verdict is the one that counts.Verification
--base origin/developmentPHPStan caught a real leftover on the way:
$bundlebecame an unused closure binding once the parafeerroute read went.Review this first
src/menu-layout.jsonandopenspec/changes/proposals-are-cases/proposal.mdcarry the reasoning. The 17,441 deleted lines are mostly the four schemas and their seed data.🤖 Generated with Claude Code