Finish the is_array-on-find sweep: 24 dead not-found guards and 22 entity-returning ': array' methods - #1694
Merged
Merged
Conversation
…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.
rubenvdlinde
requested review from
WilcoLouwerse,
bbrands02 and
rjzondervan
as code owners
September 2, 2026 20:25
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 20:30 UTC
Download the full PDF report from the workflow artifacts.
…ge 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.
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 20:41 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
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.
The follow-up sweep #1686 catalogued
PR #1686 fixed 12 sites of the class "
is_array()on the return ofObjectService::find(), which returns an ObjectEntity and never an array — the check is always false and the guarded path silently never runs", and catalogued roughly 20 more for a follow-up. This PR is that follow-up, swept repo-wide with a scanner rather than trusting the catalogue. The sweep found 24 broken find sites (the catalogue undercounted) plus 22 sibling saveObject-return sites in the same files.What was actually broken, per area
Bezwaar/ — 15 find sites in 6 files.* Every
is_array($x) === false → throw 'not found'guard fired on objects that EXIST: committee referral (AdvisoryCommitteeService), advice-request transitions, hearing attendance and minutes (HearingService), beroep filing, file-inspection requests and cascade (BeroepService), bezwaar creation linking (BezwaarCreationHook), decision publication (DecisionService). The inverted sites were quieter:PanelIndependenceChecker's author resolution always answered''(the independence check compared against an empty steller), and the hearing case resolver always fell back to the objection id.Subsidie/ — 3 find sites.*
publish(),transitionAanvraag()andfinalize()answered "niet gevonden" for records that exist.Singles — 6.
NoticeOfDefaultController::show()always 404'd;VoorstelBesluitController's IDOR gate resolved every voorstel to null (so registerBesluit always 404'd, BEFORE raising anything);DossierCompiler::compile()always threw;CaseEmailRepository::findTemplate()always null;BeroepEscalationListener's dwingend derivation never ran; andInspectionChecklistServicecalledget_object_vars()on an ObjectEntity — protected properties, so an empty array — which silently disarmed the photo-required guard.The adjacent drift in the same files: 22
: arraymethods returningsaveObject()'s ObjectEntity — a TypeError on every live call (Bezwaar 11, Subsidie 11, including BewijsstukService, TerugvorderingService and TussenrapportageService, which the honest fakes would have redded next). All now go throughsaveObjectAsArraywith a local-merge fallback, per #1686's bridge.Deliberate non-sites, verified and left alone
ZgwService(7),ZgwRulesBase(2),ZgwZrcRulesService,CaseSharingService,FederatedCaseShareService,CaseTransferService,DeliveryConcludedListenerandStatusTypeLookupall handle both shapes via an explicitjsonSerialize()fallback — the dead array branch there is defensive, not load-bearing.SelectionReassignmentServicereads the entity viagetObject().SearchesObjectsitself keeps its dual-shape normalisation by design.Tests that cannot lie again
The per-file fakes that kept this green are repinned to the real contract, exactly like the shared
FakeTermijnStore: entity-shaped returns fromfind()/saveObject(),DoesNotExistExceptionon a miss, the real argument orders. Repinned:BezwaarCreationHookTest(3 anonymous fakes),VaststellingServiceTest,DossierCompilerTest,VoorstelBesluitControllerContractTest(its miss case now throws instead of returning a null live never produces). AdvisoryCommittee/Beroep/PanelIndependence/Bezwaar-DecisionService/BeroepEscalation/NoticeOfDefault have no unit tests exercising these paths — their fixes ride on the swept pattern and the static chain.Verification
--scope-to-diffvs origin/development: 40/40 applicable gates green.🤖 Generated with Claude Code