Skip to content

fix(zgw): a failed ZGW read, update or delete says why, not always 404 (Q-dossiq-L11-2) - #3921

Merged
rubenvdlinde merged 12 commits into
developmentfrom
build/dq-l11-30
Oct 15, 2026
Merged

rubenvdlinde merged 12 commits into
developmentfrom
build/dq-l11-30

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Coordinator ruling on Q-dossiq-L11-2, following decision 224. Stacked on #3920 (Q-dossiq-L11-3): land after it.

What changes

A ZGW delete answered 404 "Not found" on any failure. An unknown id, a store that was down, a refused permission and a delete that happened but whose notification then failed all read the same. A show did the same, and an update answered 400.

One place now sorts a store failure, for every resource: Zgw/StoreFailureAnswer, behind ZgwService::storeFailureResponse().

The store says Answer
DoesNotExistException, or no object 404 {"detail": "Not found"}, as before
OpenRegister's NotAuthorizedException, or any failure with code 403 403, code permission_denied (the body the ZGW scope check already uses)
anything else 503, code store-not-reached, logged at error with the operation and the object

A refused permission never becomes 503, and a store that was down never becomes 404.

Who answers through it: ZgwService::handleShow(), handleUpdate() (the read of the existing object; a failing save keeps its 400) and handleDestroy(), plus BrcController::destroyDecision() and BrcDocumentLinkHandler::destroy().

A delete that happened answers 204 even when its notification fails; that failure is logged at error (ZgwService::notifyDeleted()). PATCH merges onto the object the update already read, instead of reading it a second time.

The cross-register writes of the Besluiten API (Zgw/BrcCrossRegisterSync: brc-006, brc-005a, brc-005b, brc-009) stay best effort, as ruled. A failure is now logged at error with the rule, the resource and the objects, instead of a warning with only the message.

Spec

REQ-ZBR-023 in openspec/changes/guards-refuse-when-unsure, tasks section 6.

Seen, not fixed

  • OpenRegister's scoped find() rewraps a database failure as DoesNotExistException (MagicMapper::findInRegisterSchemaTable). In that one path a store that is down still reads as "not found", and dossiq cannot tell. It needs an OpenRegister change.
  • ZrcController deletes a case on its own path (409 for the delete guard, 400 for anything else). It is not part of this change.

Verified (once, on the stack top 3bfc0b4, which holds #3858 and the four PRs above it)

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

  • lint 0, refusal-grounds snapshot 0, phpcs 0, psalm 0, phpstan 0.
  • phpmd 2: only the two BooleanArgumentFlag findings in lib/Service/Termijn/TermLetters.php (401, 425) that development carries since fix(letters): every date in a letter to the applicant reads the same way #3884/fix(portal): the Woo request form reads Dutch dates and names refused fields in Dutch #3895. Inherited.
  • PHPUnit --no-coverage: 8,676 tests, exit 0 (7 skipped, 1 warning). A first run found 3 reds of this stack (BrcCrossRegisterSyncTest still read the old warning lines), fixed in 28e4ad6, then the run above.
  • Mutations: the catch in ZgwBusinessRulesService, the throw in findBySchemaKey, the throw in fieldPairExists, the not-found branch, the not-allowed branch and the null check in handleDestroy each turn their tests red.
  • Not run locally: the touched test classes under pcov with fail-on-risky. The pcov image is gone and docker cannot pull, so the @covers/@uses of every touched test were checked by hand (each class a test can now execute is listed). CI is the check for that.
  • eslint and prettier on the 17 JS/Vue files the citation PR touches: 0. Vitest not run: comments only.
  • Hydra gates against refactor(transitions): the status engine asks two facades instead of fourteen collaborators #3858's head: 80 of 80 applicable ran, 8 failed, all inherited (22 and 53 on src/manifest.json, which this stack does not touch; 23, 43, 51, 55, 104, 107). Gates 5, 13, 16, 25 and 46 pass.
  • openspec validate guards-refuse-when-unsure and method-decomposition: valid.
  • Inherited: the debt ratchet reads red on development itself (phpmd.suppressWarnings 187 measured against 182) and eslint.inlineDisables 15 against 14.

No register change, so no info.version bump.

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.
…ments

The change what-a-status-declares was archived on 7 October. 206 @SPEC lines in 42 files still cited its path under openspec/changes/, which no longer exists. They now cite openspec/specs/status-transition-engine/spec.md (REQ-SDC-01 to 05) and openspec/specs/doorlooptijd-dashboard/spec.md (REQ-SDW-01), where the requirements live. Two lines that became a repeat of the line above them are removed.
… (Q-dossiq-L11-3)

The store reads the ZGW rules share answered a failed read as nothing found:
a uniqueness search that failed counted the identificatie as free, and a
zaaktype, informatieobjecttype or besluittype that could not be read counted
as absent. They now throw LookupNotCompleted, and ZgwBusinessRulesService
refuses the request for it once, with the 503 check-not-completed answer of
decision 224, logged at error with the rule and what was read.

Not found, and a read OpenRegister refuses (NotAuthorizedException), still
read as none. No store in the request, or an empty schema setting, still
skips the lookup (the coordinator's ruling on the question).

The lookups lost their logger, which only logged the swallowed failures.
catch-return-null: three sites left, ceiling 254 to 251. REQ-ZBR-022.
…4 (Q-dossiq-L11-2)

A ZGW delete answered 404 Not found on any failure: an unknown id, a store
that was down and a refused permission read the same, and so did a delete
that happened but whose notification failed. A show did the same, and an
update answered 400.

One place, StoreFailureAnswer behind ZgwService::storeFailureResponse(),
now sorts a store failure for every resource: DoesNotExistException or no
object is 404; OpenRegister's NotAuthorizedException or any failure with
code 403 is 403 permission_denied; anything else is 503 store-not-reached,
logged at error. ZgwService show, update and delete and the two BRC deletes
answer through it. A delete that happened answers 204 even when its
notification fails, and logs that at error. PATCH merges onto the object
the update already read instead of reading it a second time.

The cross-register writes of the Besluiten API stay best effort; a failure
is now logged at error with the rule, the resource and the objects.
REQ-ZBR-023.
The sync now logs a failed cross-register write at error with the rule,
the resource and the objects (Q-dossiq-L11-2); its own test still read the
old warning lines.
@rubenvdlinde
rubenvdlinde merged commit 28e4ad6 into development Oct 15, 2026
44 of 50 checks passed
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