fix(approval-routes): advance stages through OR's patch path, and feed the type picker from the registry - #1104
Merged
Conversation
…d the type picker from the registry Two defects from the live rig proof. A partial DecisionStage payload saved with a uuid is a FULL REPLACE to OpenRegister, which validates it whole and 400s on the required sequence, stageType, decisionMakerType and label it omits — so no approval route raised over the dossiq seam could ever advance or conclude. Every stage transition now goes through RegisterObjectStore::patch(), a thin wrapper over OR's sanctioned patchObject() merge path. The sweep found the same class in AgendaService (advanceBobPhase, processHamerstukken, reorderItems all 400d the same way) and in ApprovalStageTaskProjector's taskUuid linkage; all are on the patch path now. The route-engine test fakes MERGED uuid-bearing saves, which live OR does not, so the suite was green while every advance 400d — they now replace and validate required properties exactly like live OR, and fail 9 tests against the old code. The create-proposal pickers hardcoded five decision types, so a type an administrator added to the decision_types registry validated fine at the write path and never appeared in any picker. A new GET /api/v1/decision-types endpoint serves the registry's vocabulary, both pickers build their schema from it through a shared proposalFormSchema(), and the thirteen shipped types carry translated labels in every locale. An unreachable registry degrades to the shipped seed instead of blocking creation.
rubenvdlinde
requested review from
Rem-Dam,
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
September 2, 2026 18:12
Contributor
Quality Report — ConductionNL/decidiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-nav-ceiling | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ❌ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 104/104 | |||
| npm | ✅ | ✅ 537/537 | |||
| 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-02 18:41 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Quality Report — ConductionNL/decidiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-nav-ceiling | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 104/104 | |||
| npm | ✅ | ✅ 537/537 | |||
| 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-02 19:14 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
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.
Fixes two defects from the live rig proof (DC#1101 seam, DC#1099 registry).
Defect: partial stage saves 400, routes can never advance (HIGH)
ApprovalRouteService::completeAndAdvanceand its siblings saved PARTIAL DecisionStage payloads (status/outcome/decidedAt) with a uuid. OpenRegister validates a uuid-bearing save as a FULL REPLACE, so every stage transition 400d on the requiredsequence,stageType,decisionMakerTypeandlabelit omitted — no approval route raised over the dossiq seam could advance or conclude, andPOST /api/approval-routes/actionsreturned 400 (reproduced twice on the rig).Fix chosen: OR's sanctioned patch path, not hand-rolled read-modify-write. The vendored contract (conduction/hydra-gates v1.11.0) and OR development both carry
patchObject(), the RFC 7386-shaped merge counterpart to the replacing save.RegisterObjectStore::patch()wraps it; every stage transition now goes through it, so schema validation, the audit trail and event dispatch still apply and an explicitnullstill clearsoutcome/decidedAton a rewind.Sweep (one fix is not the class). The same class was found and fixed at:
ApprovalRouteService: 5 sites (complete, advance, terminal return, rewind reset x2)ApprovalStageTaskProjector: 2 sites (taskUuid linkage set/clear)AgendaService: 3 sites —advanceBobPhase,processHamerstukkenandreorderItemsall 400d the same way (agenda-itemrequirestitle,itemType,orderNumber)Left deliberately on the replacing save: the declared-state upserts in
GovernanceBodyCommandService(upsert,upsertMembership) andApprovalRouteCommandService::holdRoute— their payloads carry every required property and the seam's documented semantics are "the producer states it", where a merge would keep stale fields a producer meant to clear.ApiController::writedocuments its PUT semantics on the public surface and is likewise intentional.Tests that can now fail. The route-engine fakes MERGED uuid-bearing saves — the exact behaviour live OR does not have — so the suite was green while every advance 400d in production. The fakes in
ApprovalRouteServiceTest,ParaferingRouteRuntimeTestandApprovalRouteCommandServiceTestnow REPLACE and validate required properties per schema exactly like live OR (and merge only throughpatchObject). Negative control: the old code fails 9 tests against the corrected fakes with the rig's own "required properties … are missing" refusal; the fixed code passes all of them, pinning route advance and conclusion end-to-end.Defect: type picker ignores the registry (MEDIUM)
CnDecisionsTabandCnDecisionsWidgethardcoded 5 decision types in the create-proposal picker; theDecisionTypeRegistry(13 seeded, admin-extensible) was consulted only for validation, so an admin-added type never appeared.GET /api/v1/decision-types(DecisionTypesController) serves the registry's vocabulary — the pickers render inside foreign apps' pages, where decidiq initial state never loads, so an endpoint is the only channel that reaches them. Same guarded pattern as the integration hub:#[NoAdminRequired]+ session check, 401 for anonymous callers.proposalFormSchema()indecisionLink.js; an unreachable registry degrades to the shipped 13-type seed instead of blocking creation.enumLabels) — 9 new catalogue keys translated into every locale the app ships, Dutch and English included; an admin-added type renders as its own slug.Verification
composer lint/phpcs/phpmd/psalm/phpstan: green (run individually)test:l10n,check:l10n-js,check:schema-l10n(ratchet lowered by 1),check:manifest, eslint (0 errors, warning count unchanged from development)--scope-to-diffvs origin/development: 56/56 applicable gates pass