fix(procest): repair OR-API drift + dangling seed references

[rubenvdlinde]

Found during the local NC upgrade-and-audit pass — two production blockers:

1. Dangling seed references (config import fails on every install/upgrade)

Three parafeeractie seed objects in procest_register.json referenced voorstel-collegeadvies-0042 / voorstel-collegeadvies-0055, which have no matching voorstel seed object. OR validates parafeeractie.voorstel as a uuid-format relation and rejected the slug strings → Failed to import configuration for app procest: Property 'voorstel' should match format 'uuid'. The three orphaned objects are removed (42 → 39 objects).

2. Removed OR API: ObjectService::getObjects() → findAll()

OCA\OpenRegister\Service\ObjectService::getObjects() no longer exists. Migrated all 9 call sites to findAll(['filters' => ['register' => ..., 'schema' => ..., ...]]) which returns a flat array. This fixes the Could not seed bezwaar/beroep data: Call to undefined method ObjectService::getObjects() repair warning (via SeedDataService::findByFilter).

TenantService::getTenantByGroupId() still has one getObjects() call but that path short-circuits before reaching it (the tenant_schema config key is never set) and PR #411 removes the method entirely — left as-is to avoid a merge conflict.

Out of scope (follow-ups)

• The broken procest-main.js bundle ("Unexpected identifier 'Case'" JS syntax error — needs a rebuild)
• 25 additionalProperties validation errors in src/manifest.json page actions against canonical schema v1.4.0 (the schema's action def is additionalProperties:false and lacks handler/route — needs an action-shape design decision)

Test plan

• occ upgrade on a fresh NC dev env no longer emits the procest config-import warning
• Quality gate (phpcs/psalm/phpstan) green

[github-actions]

Quality Report — ConductionNL/procest @ bf3fcea

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	❌
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		❌
stylelint		❌
composer			✅	✅ 100/100
npm			✅	✅ 419/419
PHPUnit					⏭️
Newman					⏭️
Playwright					❌

Spec coverage: 5% (21 tests / 456 specs)

---

Quality workflow — 2026-05-12 05:00 UTC

Download the full PDF report from the workflow artifacts.

[github-actions]

Quality Report — ConductionNL/procest @ b6942cc

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	❌
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		❌
stylelint		❌
composer			✅	✅ 100/100
npm			✅	✅ 419/419
PHPUnit					⏭️
Newman					⏭️
Playwright					❌

---

Quality workflow — 2026-05-12 05:16 UTC

Download the full PDF report from the workflow artifacts.

[github-actions]

Quality Report — ConductionNL/procest @ aebea2a

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	❌
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		❌
stylelint		❌
composer			✅	✅ 100/100
npm			✅	✅ 419/419
PHPUnit					⏭️
Newman					⏭️
Playwright					❌

Spec coverage: 5% (21 tests / 456 specs)

---

Quality workflow — 2026-05-12 05:30 UTC

Download the full PDF report from the workflow artifacts.

[WilcoLouwerse]

[BLOCKER] Admin-only pages now accessible without page-level permission guard

Four pages that previously had "permission": "admin" at the page level have had that guard dropped during the reformatting pass:

• /settings/wms-layers (WmsLayers index)
• /settings/wms-layers/:id (Kaartlaag detail)
• /settings/lhs-matrices (Handhavingsstrategie index)
• /settings/lhs-matrices/:id (LHS Matrix detail)

The corresponding menu items still carry "permission": "admin" so non-admins won't see the links in the sidebar — but the manifest page objects themselves no longer enforce access. Any user who knows or guesses the URL can navigate directly to these admin-only pages, bypassing the menu-level guard. This is a privilege-escalation path that opens WMS layer management (external tile server credentials) and LHS enforcement-strategy matrices to all authenticated users.

Fix: restore "permission": "admin" as a top-level sibling of "route" on each of the four affected page objects, exactly as it appeared before this PR.

[WilcoLouwerse]

[BLOCKER] hasRow() called on flat findAll() array — shape mismatch may cause unchecked re-seeding

$existing is now the flat array returned by findAll() (a list of entity objects). The old getObjects() returned a paginated wrapper ['objects' => [...], 'total' => N]. The call $this->hasRow($existing) on line 61 is unchanged in this PR.

If hasRow() internally checks $existing['objects'] or treats a non-empty paginated wrapper as truthy, it will now receive an unexpected shape and silently return false even when an active LHS matrix already exists — causing the repair step to overwrite live matrix data on every run of php occ maintenance:repair.

The diff does not show the body of hasRow(). Before merging, verify that hasRow() correctly handles a flat PHP array. If it does not, inline the check as count($existing) > 0 or update hasRow() accordingly.

[WilcoLouwerse]

[CONCERN] PHP + operator silently discards caller-supplied register/schema overrides

The filter array is constructed as ['register' => $registerId, 'schema' => $schemaId] + $filters. PHP's + operator keeps left-hand keys on collision, so any register or schema key in $filters is silently discarded. The same pattern appears in LhsRecommendationService.php line 175.

This is likely intentional (callers should not override register/schema), but it creates a silent footgun: a caller passing those keys will see no error and get unexpected query results. At minimum, add a brief comment explaining the merge order is deliberate.

[WilcoLouwerse]

[CONCERN] Incomplete OR-API migration: TenantService::getTenantByGroupId() still uses getObjects()

The PR description acknowledges that TenantService::getTenantByGroupId() still calls the removed getObjects() API and claims it is safe because execution short-circuits before reaching it. This is a fragile assumption: any refactor of the short-circuit condition or the caller chain could cause a fatal runtime error in production.

Since this PR is scoped to repair OR-API drift, leaving a known broken call site in-tree is an incomplete fix. Add a // TODO(#411): migrate getObjects() — safe only while <condition> holds comment directly on the call site so the next contributor can see the risk, and make sure PR #411 is linked as a blocking follow-up.

[WilcoLouwerse]

[CONCERN] Seed deletion not verified for reverse references to deleted parafeeractie slugs

Three parafeeractie seed objects are removed because they contain dangling voorstel references. However, the PR does not confirm that no other remaining seed objects reference the deleted slugs themselves (parafeeractie-stap1-advies-2026-0042, parafeeractie-stap2-parafering-2026-0042, parafeeractie-stap2-geretourneerd-2026-0055).

If any surviving seed object references these slugs (e.g. a workflow step, a status record, or a voorstel body field), the deletion creates new dangling references in the opposite direction. Run grep -r 'parafeeractie-stap1-advies-2026-0042\|parafeeractie-stap2-parafering-2026-0042\|parafeeractie-stap2-geretourneerd-2026-0055' lib/Settings/ before merging.

[WilcoLouwerse]

[CONCERN] Manifest version not bumped despite schema-breaking structural changes

The manifest is described as 'conformed to canonical app-manifest schema v1.4.0', yet "version": "1.0.0" is unchanged. Actions arrays were changed from shorthand strings to full objects — a structural change. If the manifest version field is consumed by the loader or CI, leaving it at 1.0.0 is misleading. Consider bumping it to match the target schema version or the app's own semantic version for this release.

[WilcoLouwerse]

[CONCERN] 25 additionalProperties schema validation errors left unresolved

The PR description explicitly acknowledges 25 additionalProperties validation errors remain after the schema migration. The manifest declares a $schema URL but does not fully validate against it. Any schema-validation CI gate will continue to fail. These errors should be enumerated in a follow-up issue with an acceptance deadline, not left as open-ended known debt in the PR description.

[WilcoLouwerse]

[CONCERN] PR has merge conflicts with base branch development — must rebase before merge

The PR is currently marked CONFLICTING with development. The permission-guard omissions for WmsLayers and LhsMatrices pages may have been introduced accidentally during conflict resolution rather than being deliberate decisions. The author must rebase (not merge) against the current tip of development, resolve all conflicts, and re-request review. Approving a conflicting PR risks missing additional regressions introduced during the conflict resolution.

[WilcoLouwerse]

[CONCERN] VisualWorkflowEditor commented out without a tracking issue

VisualWorkflowEditor is unwired because @vue-flow/core v1.x requires Vue 3 and procest runs on Vue 2.7. The comment is informative but there is no linked GitHub issue to track resolution. Without a tracked issue, this can silently rot and the src/components/workflow/ files will confuse contributors.

Fix: open a tracking issue (e.g. 'Replace @vue-flow with Vue-2-compatible library or migrate to Vue 3'), reference the issue number in the comment, and optionally add a // @deprecated tag to the workflow component files.

[WilcoLouwerse]

Review

🔴 Blockers (2)

• Admin-only pages now accessible without page-level permission guard — src/manifest.json:1341
• hasRow() called on flat findAll() array — shape mismatch may cause unchecked re-seeding — lib/Repair/SeedLhsMatrix.php:61

🟡 Concerns (7)

• PHP + operator silently discards caller-supplied register/schema overrides — lib/Service/SeedDataService.php:154
• Incomplete OR-API migration: TenantService::getTenantByGroupId() still uses getObjects() — lib/Service/AppointmentService.php:75
• Seed deletion not verified for reverse references to deleted parafeeractie slugs — lib/Settings/procest_register.json:189
• Manifest version not bumped despite schema-breaking structural changes — src/manifest.json:273
• 25 additionalProperties schema validation errors left unresolved — src/manifest.json:273
• PR has merge conflicts with base branch development — must rebase before merge — src/manifest.json:273
• VisualWorkflowEditor commented out without a tracking issue — src/customComponents.js:251

🟢 Minor (1)

• Inconsistent (int) casting of register/schema across findAll() call sites (lib/BackgroundJob/AppointmentReminderJob.php:14)
  BackgroundJob files cast register and schema config values to (int) before passing them to findAll(). SeedLhsMatrix.php and LhsRecommendationService.php pass them as-is. If OR's findAll() API performs strict type comparison on filter values, mixed string/int inputs could produce silent empty results. Standardise the cast pattern across all call sites.

Reviewed by WilcoLouwerse via automated batch review.

[rubenvdlinde]

Auto-closed in error by a force-push bug in the quality-cleanup harness. Reopened as #491 with the same branch and content.