The OpenRegister calls that only the fakes believed: saveObject's retired arg order and the id filter that matches nothing - #1686
Merged
rubenvdlinde merged 4 commits intoSep 2, 2026
Conversation
… filters the fakes kept green Two OpenRegister contract drifts shipped as green-CI/dead-runtime defects (live-rig proof, DQ#1663): - saveObject: 29 call sites still used the retired (register, schema, object) positional order; the real signature takes the object FIRST, so every one of them threw at runtime and, among other things, no beslistermijn timer ever armed. All 29 now go through named arguments or the SearchesObjects saveObjectAsArray bridge (which gains a uuid pass-through for update sites). - searchObjects: 7 call sites filtered on a top-level 'id' key, which addresses a schema property no schema declares and silently matches zero rows (the parafering raise never found its voorstel). They now use the resolving get-by-id path (ObjectService::find via findObjectAsArray, plus a StufRegisterAccess::findById for the StUF wrapper). - The adjacent shape drift the honest fakes then exposed: find() returns an ObjectEntity, never an array, so an is_array() check after it is a dead path. Fixed at the 12 sites the reworked fakes cover; the remaining sites of that class are catalogued for a follow-up. Tests that cannot lie again: FakeTermijnStore (and the per-test fakes it exposed) now declare the REAL saveObject signature - the retired order fatals in tests the way it does live - and their searches resolve ids only through @self/find, returning zero rows for top-level id filters exactly like the live service. OrContractRegressionTest pins both contracts explicitly. Also folded in: the live-journey e2e list assertion was pagination-blind (identifier-asc, 20 per page - the created case lands on the LAST page of any lived-in rig); it now uses the ?title= deep-link filter.
rubenvdlinde
requested review from
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
September 2, 2026 18:22
…t-drift-save-and-uuid-filters # Conflicts: # tests/e2e/case-flow-live-journeys.spec.ts
The merged repair test's fake declared saveObject without the uuid parameter, so the trait's named-argument call errored and zero writes landed.
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-02 19:48 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
added a commit
that referenced
this pull request
Sep 2, 2026
…tity-returning ': array' methods (#1694) * fix(or-contract): finish the is_array-on-find sweep the honest fakes catalogued PR #1686 fixed 12 sites of the class 'is_array() on the return of ObjectService::find(), which returns an ObjectEntity and never an array' and catalogued the rest for a follow-up. This is that follow-up, swept repo-wide rather than trusting the catalogue: - Bezwaar/* (15 find sites): AdvisoryCommitteeService, BeroepService, BezwaarCreationHook, DecisionService, HearingService and PanelIndependenceChecker all guarded find() results with is_array(), so every 'not found' guard fired on FOUND objects (committee referral, advice-request transitions, hearing attendance/minutes, beroep filing and cascade, decision publication) — the features always threw, or the dead true-branch silently skipped the work (panel independence author resolution, hearing case resolution). - Subsidie/* (3 find sites): BeschikkingService.publish, SubsidieService.transitionAanvraag and VaststellingService.finalize answered 'niet gevonden' for records that exist. - Singles (6): NoticeOfDefaultController.show (always 404), VoorstelBesluitController's IDOR gate (every voorstel unresolvable), DossierCompiler.compile (always 'Case not found'), CaseEmailRepository.findTemplate (always null), BeroepEscalationListener (dwingend derivation never ran), and InspectionChecklistService, whose get_object_vars() on an ObjectEntity read an empty array so the photo-required guard never fired. All go through the SearchesObjects bridge (findObjectAsArray + null check), as #1686 chose. The same files carried the adjacent drift #1686 named: ': array' methods returning saveObject()'s ObjectEntity — a TypeError on every live call. 22 such sites (Bezwaar 11, Subsidie 11) now go through saveObjectAsArray with an honest local fallback. Deliberate non-sites, left alone: ZgwService/ZgwRulesBase/ ZgwZrcRulesService and the sharing/transfer services handle both shapes via an explicit jsonSerialize() fallback, and SelectionReassignmentService reads the entity via getObject(). The per-file fakes that kept this green are repinned to the real contract (entity-shaped returns, DoesNotExistException on a miss, real argument orders) in BezwaarCreationHookTest, VaststellingServiceTest, DossierCompilerTest and VoorstelBesluitControllerContractTest, matching the shared FakeTermijnStore. PHPUnit 2771 green; php -l, PHPCS, Psalm, PHPStan, PHPMD (both rulesets, per subdir, fresh pdepend cache) all clean. * test(beroep): cover the repaired find/save contract, which the coverage ratchet demanded The sweep added statements to files with zero unit coverage (the Bezwaar services), and the changed-files coverage ratchet correctly read that as 'adding code without tests': 18.64% head vs 18.84% base, scoped to the 22 changed files. BeroepService was the largest untested surface (7 repaired sites), so it gets the contract pins: register() resolves an EXISTING contested beslissing (pre-fix: always threw), a truly missing one still refuses, addFileInspectionRequest() appends to the existing beroep, and the reopen_objection cascade resolves the source bezwaar and fires the beroep-reopen transition. All four run against the shared entity-shaped FakeTermijnStore, so an is_array regression fails the way it fails live.
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.
What broke, live
The live-rig proof (DQ#1663) surfaced two green-CI/dead-runtime defect classes, both of the same family: the test fake agreed with the caller, so the suite could not fail.
1. saveObject in the retired argument order (29 sites, 4 were reported). OpenRegister's
ObjectService::saveObject()takes the object FIRST (saveObject($object, $extend, $register, $schema, $uuid, …)). The reported four sites plus a sweep found 29 call sites still passing($register, $schema, $object): the string register lands in thearray|ObjectEntity $objectslot and the call throws. Caught-and-logged as 'persist failed', which among other things meant no beslistermijn timer ever armed on a live pair. All 29 now use named arguments or theSearchesObjects::saveObjectAsArray()bridge, which gains auuidpass-through for the update sites.2.
filters: ['id' => $uuid]matches nothing (7 sites, all reported). The current search resolves object identity only through the@selfmetadata block; a top-levelidkey addresses a schema property no schema declares and silently returns zero rows, so the parafering raise never found its voorstel and the StUF adapter never found its endpoint. All 7 now use the resolving get-by-id path:findObjectAsArray()(backed byObjectService::find(), verified to resolve slugs and uuids on OR development) and a newStufRegisterAccess::findById()for the StUF wrapper.3. The adjacent drift the honest fakes then exposed:
find()returns an ObjectEntity, never an array, so anis_array()check after it is a dead path (the bezwaar register/resolve flows, termijn instance reads, dwangsom lookups). Fixed at the 12 sites the reworked fakes cover; roughly 20 more sites of this class in files outside this blast radius (Bezwaar/, Subsidie/, ZGW rules) are left for a follow-up sweep so this PR stays reviewable.Tests that cannot lie again
FakeTermijnStore(shared by ~19 test files) now declares the REALsaveObject()signature, returns entity-shaped objects, throwsDoesNotExistExceptionfromfind(), and returns zero rows for a top-levelid/uuidfilter, exactly like live. Test seeding moved to an explicitseed()helper (49 sites).OrContractRegressionTestpins both contracts explicitly: the termijn persist works through the real signature, the retired positional order fatals, and a top-level id filter resolves nothing while the get-by-id path finds the row.Also folded in
The live-journey e2e list assertion was pagination-blind (case list sorts identifier-asc, pages at 20, so the created case lands on the LAST page of any rig with >20 cases). It now uses the
?title=deep-link filter (CnIndexPage's route-query filter contract), with the dashboard-boot retry kept.Verification
playwright test --listcollects 9 tests.--scope-to-diff: 42/42 applicable gates green.🤖 Generated with Claude Code