fix: five live defects that stopped the seeded case flow - #1625
Merged
Conversation
…d task is recognised
Three of the five live defects that stopped the seeded case flow:
1. Handlers ignored runAs. SetStatusHandler, DossiqAskPersonNode and
DossiqRequestDecisionNode did their storage work bare, so under
FlowRunWorker every write was refused as 'Anonymous' while the run
context carried runAs: admin all along. A new FlowRunAsScope resolves
the context's runAs to a real, enabled account and hands the work to
ObjectService::runAs() — the same seam OpenRegister's own object-write
and object-read nodes use. It narrows, never grants; an unresolvable
or disabled identity refuses loudly, and a context naming nobody runs
bare so the interactive transition path is unchanged.
2. persistTask() misread the save result. It accepted only an array while
ObjectService::saveObject() returns an ObjectEntity, so every
SUCCESSFUL save was followed by 'could not identify the task it
created': the run stopped instead of suspending, the resume slot was
never written, and the task sat orphaned. The result handling now
reads the entity's uuid (array shape still accepted for the
duck-typed service), and the unit fake now returns an ObjectEntity
like production — the old fake agreed with the caller and could not
fail.
3. ask-indiener's assignee template was never rendered. The engine
templates only inside its own nodes, so '{{ case.assignee }}' was
stored literally and FlowRunAssignee refused every real user. The
node now renders the value through FlowValueTemplate against the case
before stamping it, stores the RENDERED name in the resume slot the
guard reads back, and refuses loudly when the template resolves to
nothing — an unassigned task would be answerable by anyone.
Regression tests: testTheWriteRunsAsTheRunsActingIdentity,
testTheTaskIsWrittenAsTheRunsActingIdentity,
testTheDecisionIsRaisedAsTheRunsActingIdentity (each red when the wrap
is removed), testTheSavedTaskIsIdentifiedByItsEntityUuid,
testATemplatedAssigneeIsRenderedAgainstTheCase, plus refusal tests for
unresolvable identities and empty renderings.
… the engine reads them Defect 4: the engine's contract is node-level exits[] matched by an edge's fromExit — FlowTokenRouter never reads a condition from an edge. The shipped declaration put both completeness conditions on the edges (e-compleet, e-onvolledig), so the engine saw an unconditioned split, took the first enabled transition, and routed a COMPLETE case to 'Wacht op aanvulling'. The check-complete node now declares its three exits (compleet, onvolledig, and the unconditional gestrand else) and the three edges reference them by fromExit. This is the only flow declaration in the app that branches; the new declaration test scans EVERY x-openregister- flows declaration in lib/Settings (register.d included) and fails on any edge-level condition, so the wrong shape cannot come back anywhere — the class is fixed, not the instance. CaseFlowDeclarationTest and the case-flow e2e both asserted the wrong shape and are moved to the exits[] contract, plus a new pairing test: every exit is referenced by an edge and every branching edge names a declared exit. Regression tests: testNoShippedFlowPutsAConditionOnAnEdge, testEveryExitAndEveryBranchingEdgePairUp.
…nks survive a reload
Defect 5: a hard load of /apps/dossiq/cases/<id> answered 200 from the
server and was rewritten client-side to /apps/dossiq/ — only sidebar
navigation ever reached the detail page. The router based itself on
generateUrl('/apps/dossiq'), which returns exactly ONE of the two URL
forms Nextcloud serves the app under (front-controller vs pretty),
decided by instance config rather than by the URL in the address bar. A
hard load of the other form fell outside the base, matched only the
/:pathMatch(.*)* catch-all, and its redirect ate the deep link.
routerBase() takes the base from the document's own pathname — whatever
prefix the page was actually served under IS the base, by construction —
falling back to generateUrl() only when the path carries no app mount at
all. The marker must end the path or be followed by a slash, so an app
id that merely starts with ours can never match.
Regression tests: routerBase.spec.js (vitest, both URL forms plus the
prefix-collision and fallback cases) and the e2e 'a deep link to a case
survives a hard reload, under both URL forms' — which reloads the exact
URL the browser lands on AND its other server-accepted spelling, since
the reload of a deep link is the test.
The runAs seam, the value template and the entity-shaped save result push DossiqAskPersonNode one over the coupling threshold and add one static call; both are documented suppressions rather than refactors — the dependencies are the suspend/resume vocabulary the node exists to speak, and FlowValueTemplate is published as a static final class with no instance to inject.
rubenvdlinde
requested review from
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
September 1, 2026 13:17
Contributor
Quality Report — ConductionNL/dossiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ⏭️ | ||||
| phpcs | ⏭️ | ||||
| phpmd | ❌ | ||||
| psalm | ⏭️ | ||||
| phpstan | ⏭️ | ||||
| phpmetrics | ✅ | ||||
| eslint | ⏭️ | ||||
| stylelint | ⏭️ | ||||
| build | ⏭️ | ||||
| format | ✅ | ||||
| composer | ⏭️ | ⏭️ | |||
| npm | ⏭️ | ✅ 540/540 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ⏭️ | ||||
| REUSE | ⏭️ | ||||
| PHPUnit | ❌ | ||||
| Newman | ❌ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-09-01 13:19 UTC
Download the full PDF report from the workflow artifacts.
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-01 13:28 UTC
Download the full PDF report from the workflow artifacts.
CI's coverage guard measured the change adding 57 statements against the touched files and losing 0.01% on the kept-or-added set: the new scope's refusal branches (disabled account, missing seam) and createdTaskId's fallback shapes ran in no test. A dedicated FlowRunAsScopeTest now exercises all of them, asserting on every refusal that the operation never ran, and the ask node's suite covers the legacy-array result, the serialised-entity fallback, and the result that names no id at all.
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-01 13:40 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.
What broke
Running the seeded "Case behandeling" flow for real on a clean instance (dossiq 0.3.11-unstable, openregister 2.0.13-unstable) surfaced five defects. Together they meant the flow could not move a single case under the worker. Each fix ships with a regression test that goes red when the fix is reverted (verified by mutation).
The five defects
1. Handlers ignored the run's acting identity
SetStatusHandler, DossiqAskPersonNode and DossiqRequestDecisionNode did their storage work bare. Under FlowRunWorker the session carries nobody, so openregister refused every write as
User 'Anonymous' does not have permission. The run context carriedrunAs: adminthe whole time.A new
FlowRunAsScoperesolves the context'srunAsto a real, enabled account and hands the work toObjectService::runAs(). That is the same seam openregister's own object-write and object-read nodes use. It narrows, never grants. An unresolvable or disabled identity refuses loudly. A context naming nobody runs bare, so the interactive transition path is unchanged.Tests:
testTheWriteRunsAsTheRunsActingIdentity,testTheTaskIsWrittenAsTheRunsActingIdentity,testTheDecisionIsRaisedAsTheRunsActingIdentity, plus refusal tests for unresolvable identities.2. persistTask() misread the save result
It accepted only an array, while
ObjectService::saveObject()returns anObjectEntity. Every successful save was followed by "could not identify the task it created": the run stopped instead of suspending, the resume slot was never written, the task sat orphaned. The node now reads the entity's uuid. The unit fake now returns anObjectEntitylike production, because the old fake agreed with the caller and could not fail.Test:
testTheSavedTaskIsIdentifiedByItsEntityUuid.3. The assignee template was never rendered
ask-indienerdeclaresassignee: "{{ case.assignee }}". The engine templates only inside its own nodes, so the literal was stored and FlowRunAssignee refused every real user. The node now renders the value throughFlowValueTemplateagainst the case, stores the rendered name in the resume slot, and refuses loudly when the template resolves to nothing. An empty assignee would make the task answerable by anyone.Tests:
testATemplatedAssigneeIsRenderedAgainstTheCase,testAnAssigneeTemplateThatResolvesToNothingRefuses.4. Conditions sat on edges, where the engine never looks
FlowTokenRouterreads conditions from a node'sexits[], matched by the edge'sfromExit. The declaration put them on the edges (e-compleet,e-onvolledig), so the engine saw an unconditioned split and took the first enabled transition: a complete case went to "Wacht op aanvulling". The completeness check now declares its three exits (with the unconditionalgestrandelse) and the edges reference them byfromExit.CaseFlowDeclarationTestand the e2e both asserted the wrong shape and now assert the engine's contract.Tests:
testNoShippedFlowPutsAConditionOnAnEdgescans every flow declaration inlib/Settings(register.d included) and fails on any edge-level condition, so the shape cannot come back anywhere.testEveryExitAndEveryBranchingEdgePairUppins the pairing.5. Deep links landed on the dashboard
A hard load of
/apps/dossiq/cases/<id>answered 200 from the server and was rewritten client-side to/apps/dossiq/. The router based itself ongenerateUrl(), which returns only one of the two URL forms Nextcloud serves the app under. The other form fell outside the base and the catch-all redirect ate the deep link.routerBase()now derives the base from the document's own pathname, withgenerateUrl()as fallback.Tests:
routerBase.spec.js(vitest, both URL forms plus prefix-collision and fallback), and the e2ea deep link to a case survives a hard reload, under both URL forms. The reload of a deep link is the test.Checks
--scope-to-diff --base origin/development: all 40 applicable gates pass against the current canonical gate inputsKnown CI caveat, not caused by this change
Gate-53 will FAIL on this PR under hydra-gates v1.10.0. The tag's vendored manifest schema predates the
reportsandflowpage types (taught upstream in ConductionNL/.github 9d853e8f, 2026-09-01) and itsFLEET_APP_IDSpredates the iq renames (decidiqmissing; also fixed on main). Any dossiq PR touching a manifest input fails the same way. With both files at their current upstream state the gates pass 40/40 here. The fix is a hydra-gates release, not a dossiq change.