Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rubenvdlinde wants to merge 3 commits into
base: development
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ff3d370
fix(procest): repair OR-API drift + dangling seed references
rubenvdlinde May 12, 2026
a9fe6e5
fix(procest): unwire @vue-flow visual workflow editor (Vue-2 build br…
rubenvdlinde May 12, 2026
fa646c1
fix(procest): conform src/manifest.json to canonical app-manifest schema
rubenvdlinde May 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions lib/BackgroundJob/AppointmentReminderJob.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,11 @@ protected function run($argument): void

$tomorrow = (new \DateTime('+1 day'))->format('Y-m-d');

$result = $objectService->getObjects(
(int) $register,
(int) $schema,
['status' => 'scheduled'],
$appointments = $objectService->findAll(
['filters' => ['register' => (int) $register, 'schema' => (int) $schema, 'status' => 'scheduled']],
);

foreach (($result['objects'] ?? []) as $apt) {
foreach ($appointments as $apt) {
if (is_object($apt) === true) {
$data = $apt->jsonSerialize();
} else {
Expand Down
7 changes: 2 additions & 5 deletions lib/BackgroundJob/ShareMaintenanceJob.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,10 @@ protected function run($argument): void
}

try {
$result = $objectService->getObjects(
(int) $register,
(int) $schema,
[],
$shares = $objectService->findAll(
['filters' => ['register' => (int) $register, 'schema' => (int) $schema]],
);

$shares = ($result['objects'] ?? []);
$reminderDate = new \DateTime('+'.self::REMINDER_DAYS.' days');

foreach ($shares as $share) {
Expand Down
10 changes: 5 additions & 5 deletions lib/Repair/SeedLhsMatrix.php
View file
Open in desktop
Copy link
Copy Markdown

@WilcoLouwerse WilcoLouwerse May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,11 @@ public function run(IOutput $output): void
return;
}

$existing = $objectService->getObjects(
register: $register,
schema: $schema,
filters: ['active' => true],
limit: 1,
$existing = $objectService->findAll(
[
'filters' => ['register' => $register, 'schema' => $schema, 'active' => true],
'limit' => 1,
],
);
if ($this->hasRow($existing) === true) {
$output->info('Active LHS matrix already exists. Skipping seed.');
Expand Down
16 changes: 4 additions & 12 deletions lib/Service/AppointmentService.php
View file
Open in desktop
Copy link
Copy Markdown

@WilcoLouwerse WilcoLouwerse May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,9 @@ public function getAppointmentsForCase(string $caseId): array
$register = $this->settingsService->getConfigValue('register');
$schema = $this->settingsService->getConfigValue('appointment_schema');

$result = $objectService->getObjects(
(int) $register,
(int) $schema,
['caseId' => $caseId],
return $objectService->findAll(
['filters' => ['register' => (int) $register, 'schema' => (int) $schema, 'caseId' => $caseId]],
);

return $result['objects'] ?? [];
}//end getAppointmentsForCase()

/**
Expand All @@ -221,13 +217,9 @@ public function getAppointmentByToken(string $token): ?array
$register = $this->settingsService->getConfigValue('register');
$schema = $this->settingsService->getConfigValue('appointment_schema');

$result = $objectService->getObjects(
(int) $register,
(int) $schema,
['cancelToken' => $token],
$appointments = $objectService->findAll(
['filters' => ['register' => (int) $register, 'schema' => (int) $schema, 'cancelToken' => $token]],
);

$appointments = ($result['objects'] ?? []);
if (empty($appointments) === true) {
return null;
}
Expand Down
8 changes: 2 additions & 6 deletions lib/Service/BerichtenboxService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,9 @@ public function getMessagesForCase(string $caseId): array
$register = $this->settingsService->getConfigValue('register');
$schema = $this->settingsService->getConfigValue('berichtenbox_message_schema');

$result = $objectService->getObjects(
(int) $register,
(int) $schema,
['caseId' => $caseId],
return $objectService->findAll(
['filters' => ['register' => (int) $register, 'schema' => (int) $schema, 'caseId' => $caseId]],
);

return $result['objects'] ?? [];
}//end getMessagesForCase()

/**
Expand Down
8 changes: 2 additions & 6 deletions lib/Service/CaseSharingService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,13 +278,9 @@ public function validateToken(string $token, ?string $password=null): array
$register = $this->settingsService->getConfigValue('register');
$schema = $this->settingsService->getConfigValue('case_share_schema');

$result = $objectService->getObjects(
(int) $register,
(int) $schema,
['token' => $token],
$shares = $objectService->findAll(
['filters' => ['register' => (int) $register, 'schema' => (int) $schema, 'token' => $token]],
);

$shares = ($result['objects'] ?? []);
if (empty($shares) === true) {
return ['valid' => false, 'error' => 'Token niet gevonden'];
}
Expand Down
10 changes: 5 additions & 5 deletions lib/Service/SeedDataService.php
View file
Open in desktop
Copy link
Copy Markdown

@WilcoLouwerse WilcoLouwerse May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Original file line number Diff line number Diff line change
Expand Up @@ -408,11 +408,11 @@ private function findByFilter(
array $filters,
): ?object {
try {
$results = $objectService->getObjects(
register: $registerId,
schema: $schemaId,
filters: $filters,
limit: 1,
$results = $objectService->findAll(
[
'filters' => (['register' => $registerId, 'schema' => $schemaId] + $filters),
'limit' => 1,
],
);

if (is_array($results) === true && count($results) > 0) {
Expand Down
10 changes: 5 additions & 5 deletions lib/Service/Vth/LhsRecommendationService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,11 +253,11 @@ private function loadMatrix(?int $version): array
$filters = ($version === null) ? ['active' => true] : ['version' => $version];

try {
$results = $objectService->getObjects(
register: $register,
schema: $schema,
filters: $filters,
limit: 1,
$results = $objectService->findAll(
[
'filters' => (['register' => $register, 'schema' => $schema] + $filters),
'limit' => 1,
],
);
} catch (Throwable $e) {
$this->logger->error(
Expand Down
39 changes: 0 additions & 39 deletions lib/Settings/procest_register.json
View file
Open in desktop
Copy link
Copy Markdown

@WilcoLouwerse WilcoLouwerse May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Original file line number Diff line number Diff line change
Expand Up @@ -5326,45 +5326,6 @@
}
]
},
{
"@self": {
"register": "procest",
"schema": "parafeeractie",
"slug": "parafeeractie-stap1-advies-2026-0042"
},
"voorstel": "voorstel-collegeadvies-0042",
"step": 1,
"actor": "j.devries",
"actorType": "user",
"action": "advised",
"advice": "Akkoord, mits de bouwtekeningen worden bijgewerkt conform het welstandsadvies van 12 april 2026."
},
{
"@self": {
"register": "procest",
"schema": "parafeeractie",
"slug": "parafeeractie-stap2-parafering-2026-0042"
},
"voorstel": "voorstel-collegeadvies-0042",
"step": 2,
"actor": "m.bakker",
"actorType": "user",
"action": "parafered",
"comment": "Geparafeerd na beoordeling welstandsadvies. Tekeningen conform bijgewerkt."
},
{
"@self": {
"register": "procest",
"schema": "parafeeractie",
"slug": "parafeeractie-stap2-geretourneerd-2026-0055"
},
"voorstel": "voorstel-collegeadvies-0055",
"step": 2,
"actor": "p.janssen",
"actorType": "user",
"action": "returned",
"comment": "Financiële paragraaf ontbreekt. Kosten-batenanalyse moet worden toegevoegd voordat dit voorstel kan worden geparafeerd."
},
{
"@self": {
"register": "procest",
Expand Down
17 changes: 10 additions & 7 deletions src/customComponents.js
View file
Open in desktop
Copy link
Copy Markdown

@WilcoLouwerse WilcoLouwerse May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,14 @@ import CaseTasksTab from './components/tabs/CaseTasksTab.vue'
import CaseDecisionsTab from './components/tabs/CaseDecisionsTab.vue'
import CaseDocumentsTab from './components/tabs/CaseDocumentsTab.vue'

// --- Visual workflow editor (only legitimate custom-UI page). ---
// Custom because vue-flow drag-and-drop graph editing has no analogue in
// the manifest's built-in page/widget types. See
// openspec/changes/visual-workflow-editor/design.md.
import VisualWorkflowEditor from './components/workflow/VisualWorkflowEditor.vue'
// --- Visual workflow editor — TEMPORARILY UNWIRED. ---
// `@vue-flow/{core,controls,background}` v1.x are Vue-3-only (they import
// Fragment / Teleport / createElementVNode / toValue from 'vue'), which breaks
// the webpack build under procest's Vue 2.7 base. The component files remain in
// src/components/workflow/ but are no longer pulled into the bundle. Re-wire
// once @vue-flow is replaced with a Vue-2-compatible flow library (or procest
// migrates to Vue 3). See openspec/changes/visual-workflow-editor/design.md.
// import VisualWorkflowEditor from './components/workflow/VisualWorkflowEditor.vue'

// --- Shared map surface (case detail map tab, dashboard widget, public case page). ---
// Thin wrapper around CnMapWidget; registered here so manifest entries
Expand Down Expand Up @@ -71,8 +74,8 @@ export default {
CaseDecisionsTab, // decisions where decision.case === parent.id
CaseDocumentsTab, // documents where document.case === parent.id

// --- Visual workflow editor (drag-and-drop, no manifest analogue). ---
VisualWorkflowEditor,
// --- Visual workflow editor — temporarily unwired (see import comment above). ---
// VisualWorkflowEditor,

// --- Shared map surface — referenceable from manifest pages. ---
MapComponent,
Expand Down
Loading
Loading