refactor(procest): TermijnReporting* -> DeadlineReporting* - #803
Merged
rubenvdlinde merged 4 commits intoAug 11, 2026
Conversation
…tionDispatchJob A code-only slice of openspec/changes/english-vocabulary, and the first procest slice — chosen because it is one of the very few procest identifiers that is neither ZGW protocol vocabulary nor wired by string. 'termijn' here is a STATUTORY DEADLINE being monitored, not a contract term, so it becomes Deadline. That reading is app-specific on purpose: hrmq uses 'termijn' for notice periods (aanzegtermijn) and decidesk for terms of office (TermijnRegeling). Three apps, three meanings, one Dutch word — it must never become a fleet word. Referenced only via ::class from TermijnNotificationService, so a missed reference is a compile error rather than a job that silently never dispatches. No route entry, no register-fragment wiring, no baseline suppression keyed to the old path — all four checked, not assumed. TermijnNotificationService itself keeps its name in this commit; it belongs with the termijnbewaking slice, where the Termijn* schemas (termijnDefinitie, termijnInstance, termijnGebeurtenis) move together with their register fragments. Archived changes under openspec/changes/archive/ keep the old name — they are a record of what was built at the time. php -l clean; class name matches file name; use statement still ordered.
TermijnReportingController -> DeadlineReportingController and TermijnReportingService -> DeadlineReportingService, with their two Dutch controller methods (kwartaalrapport -> quarterlyReport, jaarrekening -> annualStatement), every Dutch parameter and local ($periode -> $period, $afdeling -> $department, $jaar -> $year, $jaarPrefix -> $yearPrefix), the two named-argument call sites, the three route names, the test, and the Postman collection's description. Stacked on feat/english-vocabulary-deadline-job (#797) because that branch already renames TermijnNotificationDispatchJob; branching from development would have conflicted. FOUR DUTCH CONTRACTS ARE DELIBERATELY LEFT ALONE, and each is annotated in place so the next reader does not "finish the job" and break it: 1. $row['afdeling'] is a SCHEMA PROPERTY. OpenRegister materialises it as a real column, so it moves with a data migration, not with a rename. 2. 'periode', 'afdeling' and 'jaar' as RESPONSE KEYS are the published shape of the reports. src/views/dashboard/TermijnDashboard.vue reads annual.jaar directly, so they move only with a coordinated frontend change. 3. getParam('periode') and getParam('jaar') are QUERY-STRING parameter names — the request contract. 4. The /api/termijn/* URLs. Route NAMES follow the renamed class and methods; URLs do not. Moving them is a breaking change for every consumer and is a separate decision. NAMED ARGUMENTS. resolveQuarter(periode:) and aggregateByType(afdeling:) are called with named arguments, which PHP resolves at call time — a parameter rename that misses one is a runtime fatal that `php -l` reports as clean. Both moved with their declarations. VERIFIED - php -l clean on all four PHP files; the Postman collection still parses. - phpunit: 5 tests, 14 assertions green. - Route resolution: each 'deadlineReporting#<action>' route name was checked against the controller's actual public methods — all three resolve. Positive control: the same check against the OLD action names (kwartaalrapport, jaarrekening) reports MISSING METHOD, so it can distinguish a renamed route from a dangling one. - Residual grep over lib/, tests/, appinfo/ and src/: no live reference to either old class name or the old route prefix. PHPCS SCOPE, MEASURED RATHER THAN ASSUMED. phpcs.xml declares <file>lib</file> and nothing else, so tests/ and appinfo/ are outside the standard entirely — appinfo/routes.php carries 373 violations on development and every sibling test file is equally "dirty". Both files this change touches inside lib/ are CLEAN, including two pre-existing missing-@SPEC errors fixed here (each now anchors to openspec/specs/termijn-reporting/spec.md, the canonical spec, not the change directory the file headers point at). An earlier pass ran phpcbf over the test file and produced a 94-line diff for what is a 6-line rename. That was churn against a gate that never reads the file, so it was reverted; the test diff is now the rename alone.
rubenvdlinde
requested review from
Rem-Dam,
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
August 11, 2026 20:38
This was referenced Aug 11, 2026
Base automatically changed from
feat/english-vocabulary-deadline-job
to
development
August 11, 2026 21:46
…ocabulary-deadline-reporting
…ocabulary-deadline-reporting
Contributor
Quality Report — ConductionNL/procest @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-vue3-compile | ✅ | ||||
| test-l10n | ✅ | ||||
| composer | ❌ | ✅ 100/100 | |||
| npm | ✅ | ✅ 552/552 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-11 21:58 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Quality Report — ConductionNL/procest @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-vue3-compile | ✅ | ||||
| test-l10n | ✅ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ✅ | ✅ 552/552 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ✅ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-11 22:16 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 11, 2026
…ner -> Deadline* (#804) * refactor(procest): TermijnNotificationDispatchJob -> DeadlineNotificationDispatchJob A code-only slice of openspec/changes/english-vocabulary, and the first procest slice — chosen because it is one of the very few procest identifiers that is neither ZGW protocol vocabulary nor wired by string. 'termijn' here is a STATUTORY DEADLINE being monitored, not a contract term, so it becomes Deadline. That reading is app-specific on purpose: hrmq uses 'termijn' for notice periods (aanzegtermijn) and decidesk for terms of office (TermijnRegeling). Three apps, three meanings, one Dutch word — it must never become a fleet word. Referenced only via ::class from TermijnNotificationService, so a missed reference is a compile error rather than a job that silently never dispatches. No route entry, no register-fragment wiring, no baseline suppression keyed to the old path — all four checked, not assumed. TermijnNotificationService itself keeps its name in this commit; it belongs with the termijnbewaking slice, where the Termijn* schemas (termijnDefinitie, termijnInstance, termijnGebeurtenis) move together with their register fragments. Archived changes under openspec/changes/archive/ keep the old name — they are a record of what was built at the time. php -l clean; class name matches file name; use statement still ordered. * refactor(procest): TermijnReporting* -> DeadlineReporting* TermijnReportingController -> DeadlineReportingController and TermijnReportingService -> DeadlineReportingService, with their two Dutch controller methods (kwartaalrapport -> quarterlyReport, jaarrekening -> annualStatement), every Dutch parameter and local ($periode -> $period, $afdeling -> $department, $jaar -> $year, $jaarPrefix -> $yearPrefix), the two named-argument call sites, the three route names, the test, and the Postman collection's description. Stacked on feat/english-vocabulary-deadline-job (#797) because that branch already renames TermijnNotificationDispatchJob; branching from development would have conflicted. FOUR DUTCH CONTRACTS ARE DELIBERATELY LEFT ALONE, and each is annotated in place so the next reader does not "finish the job" and break it: 1. $row['afdeling'] is a SCHEMA PROPERTY. OpenRegister materialises it as a real column, so it moves with a data migration, not with a rename. 2. 'periode', 'afdeling' and 'jaar' as RESPONSE KEYS are the published shape of the reports. src/views/dashboard/TermijnDashboard.vue reads annual.jaar directly, so they move only with a coordinated frontend change. 3. getParam('periode') and getParam('jaar') are QUERY-STRING parameter names — the request contract. 4. The /api/termijn/* URLs. Route NAMES follow the renamed class and methods; URLs do not. Moving them is a breaking change for every consumer and is a separate decision. NAMED ARGUMENTS. resolveQuarter(periode:) and aggregateByType(afdeling:) are called with named arguments, which PHP resolves at call time — a parameter rename that misses one is a runtime fatal that `php -l` reports as clean. Both moved with their declarations. VERIFIED - php -l clean on all four PHP files; the Postman collection still parses. - phpunit: 5 tests, 14 assertions green. - Route resolution: each 'deadlineReporting#<action>' route name was checked against the controller's actual public methods — all three resolve. Positive control: the same check against the OLD action names (kwartaalrapport, jaarrekening) reports MISSING METHOD, so it can distinguish a renamed route from a dangling one. - Residual grep over lib/, tests/, appinfo/ and src/: no live reference to either old class name or the old route prefix. PHPCS SCOPE, MEASURED RATHER THAN ASSUMED. phpcs.xml declares <file>lib</file> and nothing else, so tests/ and appinfo/ are outside the standard entirely — appinfo/routes.php carries 373 violations on development and every sibling test file is equally "dirty". Both files this change touches inside lib/ are CLEAN, including two pre-existing missing-@SPEC errors fixed here (each now anchors to openspec/specs/termijn-reporting/spec.md, the canonical spec, not the change directory the file headers point at). An earlier pass ran phpcbf over the test file and produced a 94-line diff for what is a 6-line rename. That was churn against a gate that never reads the file, so it was reverted; the test diff is now the rename alone. * refactor(procest): Termijn{Escalation,Pause,Extension}Service + listener -> Deadline* Four more classes in the deadline cluster: TermijnEscalationService -> DeadlineEscalationService TermijnPauseService -> DeadlinePauseService TermijnExtensionService -> DeadlineExtensionService TermijnCaseCreatedListener -> DeadlineCaseCreatedListener plus the test file, across 11 files: the DI listener registration, the daily scan job and service, TermijnController, and three test suites. Stacked on feat/english-vocabulary-deadline-reporting (#803), which is itself stacked on #797. Merge order: 797 -> 803 -> this. THE ONE DEFECT THIS SLICE PRODUCED, AND WHAT CAUGHT IT. Renaming the test FILE TermijnPauseExtensionServiceTest.php left the class inside it still called TermijnPauseExtensionServiceTest — neither of the two service-name replacements matched that token, because it is a third name. PHPUnit refused to load it ("Class DeadlinePauseExtensionServiceTest cannot be found"). The basename == class invariant I ran before committing covered only the four lib/ files and would have missed it; it now covers the tests too. Same failure mode as hrmq's provider discovery, reached from the opposite direction. LISTENER WIRING IS SAFE BY CONSTRUCTION. WorkflowListenerRegistrar registers the listener as TermijnCaseCreatedListener::class, not as a string literal, so the rename is resolved at compile time rather than silently failing to match at runtime. Checked before renaming rather than after. VERIFIED - php -l clean on every changed file. - phpunit, whole unit suite: 1777 tests, 6160 assertions, 5 skipped — green. - Every lib/ file this change touches is phpcs CLEAN. Three pre-existing issues were fixed along the way: docblock parameter-type alignment in TermijnController and TermijnDailyScanService (shifted by the longer type names), and two missing class-level @SPEC tags. Those now anchor to canonical specs — termijn-binding + termijn-pause-extension for the controller, which spans both, and termijn-escalation for the daily scan — rather than to the change directories the file headers still point at. - Residual grep over lib/, tests/, appinfo/ and src/: no live reference to any of the four old names. phpcs.xml scopes to <file>lib</file> only, so tests/ and appinfo/ are outside the standard; no formatting was applied to them.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 11, 2026
…es (#806) * refactor(procest): TermijnNotificationDispatchJob -> DeadlineNotificationDispatchJob A code-only slice of openspec/changes/english-vocabulary, and the first procest slice — chosen because it is one of the very few procest identifiers that is neither ZGW protocol vocabulary nor wired by string. 'termijn' here is a STATUTORY DEADLINE being monitored, not a contract term, so it becomes Deadline. That reading is app-specific on purpose: hrmq uses 'termijn' for notice periods (aanzegtermijn) and decidesk for terms of office (TermijnRegeling). Three apps, three meanings, one Dutch word — it must never become a fleet word. Referenced only via ::class from TermijnNotificationService, so a missed reference is a compile error rather than a job that silently never dispatches. No route entry, no register-fragment wiring, no baseline suppression keyed to the old path — all four checked, not assumed. TermijnNotificationService itself keeps its name in this commit; it belongs with the termijnbewaking slice, where the Termijn* schemas (termijnDefinitie, termijnInstance, termijnGebeurtenis) move together with their register fragments. Archived changes under openspec/changes/archive/ keep the old name — they are a record of what was built at the time. php -l clean; class name matches file name; use statement still ordered. * refactor(procest): TermijnReporting* -> DeadlineReporting* TermijnReportingController -> DeadlineReportingController and TermijnReportingService -> DeadlineReportingService, with their two Dutch controller methods (kwartaalrapport -> quarterlyReport, jaarrekening -> annualStatement), every Dutch parameter and local ($periode -> $period, $afdeling -> $department, $jaar -> $year, $jaarPrefix -> $yearPrefix), the two named-argument call sites, the three route names, the test, and the Postman collection's description. Stacked on feat/english-vocabulary-deadline-job (#797) because that branch already renames TermijnNotificationDispatchJob; branching from development would have conflicted. FOUR DUTCH CONTRACTS ARE DELIBERATELY LEFT ALONE, and each is annotated in place so the next reader does not "finish the job" and break it: 1. $row['afdeling'] is a SCHEMA PROPERTY. OpenRegister materialises it as a real column, so it moves with a data migration, not with a rename. 2. 'periode', 'afdeling' and 'jaar' as RESPONSE KEYS are the published shape of the reports. src/views/dashboard/TermijnDashboard.vue reads annual.jaar directly, so they move only with a coordinated frontend change. 3. getParam('periode') and getParam('jaar') are QUERY-STRING parameter names — the request contract. 4. The /api/termijn/* URLs. Route NAMES follow the renamed class and methods; URLs do not. Moving them is a breaking change for every consumer and is a separate decision. NAMED ARGUMENTS. resolveQuarter(periode:) and aggregateByType(afdeling:) are called with named arguments, which PHP resolves at call time — a parameter rename that misses one is a runtime fatal that `php -l` reports as clean. Both moved with their declarations. VERIFIED - php -l clean on all four PHP files; the Postman collection still parses. - phpunit: 5 tests, 14 assertions green. - Route resolution: each 'deadlineReporting#<action>' route name was checked against the controller's actual public methods — all three resolve. Positive control: the same check against the OLD action names (kwartaalrapport, jaarrekening) reports MISSING METHOD, so it can distinguish a renamed route from a dangling one. - Residual grep over lib/, tests/, appinfo/ and src/: no live reference to either old class name or the old route prefix. PHPCS SCOPE, MEASURED RATHER THAN ASSUMED. phpcs.xml declares <file>lib</file> and nothing else, so tests/ and appinfo/ are outside the standard entirely — appinfo/routes.php carries 373 violations on development and every sibling test file is equally "dirty". Both files this change touches inside lib/ are CLEAN, including two pre-existing missing-@SPEC errors fixed here (each now anchors to openspec/specs/termijn-reporting/spec.md, the canonical spec, not the change directory the file headers point at). An earlier pass ran phpcbf over the test file and produced a 94-line diff for what is a 6-line rename. That was churn against a gate that never reads the file, so it was reverted; the test diff is now the rename alone. * refactor(procest): Termijn{Escalation,Pause,Extension}Service + listener -> Deadline* Four more classes in the deadline cluster: TermijnEscalationService -> DeadlineEscalationService TermijnPauseService -> DeadlinePauseService TermijnExtensionService -> DeadlineExtensionService TermijnCaseCreatedListener -> DeadlineCaseCreatedListener plus the test file, across 11 files: the DI listener registration, the daily scan job and service, TermijnController, and three test suites. Stacked on feat/english-vocabulary-deadline-reporting (#803), which is itself stacked on #797. Merge order: 797 -> 803 -> this. THE ONE DEFECT THIS SLICE PRODUCED, AND WHAT CAUGHT IT. Renaming the test FILE TermijnPauseExtensionServiceTest.php left the class inside it still called TermijnPauseExtensionServiceTest — neither of the two service-name replacements matched that token, because it is a third name. PHPUnit refused to load it ("Class DeadlinePauseExtensionServiceTest cannot be found"). The basename == class invariant I ran before committing covered only the four lib/ files and would have missed it; it now covers the tests too. Same failure mode as hrmq's provider discovery, reached from the opposite direction. LISTENER WIRING IS SAFE BY CONSTRUCTION. WorkflowListenerRegistrar registers the listener as TermijnCaseCreatedListener::class, not as a string literal, so the rename is resolved at compile time rather than silently failing to match at runtime. Checked before renaming rather than after. VERIFIED - php -l clean on every changed file. - phpunit, whole unit suite: 1777 tests, 6160 assertions, 5 skipped — green. - Every lib/ file this change touches is phpcs CLEAN. Three pre-existing issues were fixed along the way: docblock parameter-type alignment in TermijnController and TermijnDailyScanService (shifted by the longer type names), and two missing class-level @SPEC tags. Those now anchor to canonical specs — termijn-binding + termijn-pause-extension for the controller, which spans both, and termijn-escalation for the daily scan — rather than to the change directories the file headers still point at. - Residual grep over lib/, tests/, appinfo/ and src/: no live reference to any of the four old names. phpcs.xml scopes to <file>lib</file> only, so tests/ and appinfo/ are outside the standard; no formatting was applied to them. * refactor(procest): rename the daily-scan and seed-data deadline classes TermijnDailyScanService -> DeadlineDailyScanService TermijnbewakingSeedDataService -> DeadlineMonitoringSeedDataService DailyTermijnScanJob -> DailyDeadlineScanJob SeedTermijnbewakingData -> SeedDeadlineMonitoringData TermijnbewakingEndToEndTest -> DeadlineMonitoringEndToEndTest Stacked on feat/english-vocabulary-deadline-services (#804). Merge order: 797 -> 803 -> 804 -> this. TWO OF THESE ARE WIRED IN appinfo/info.xml, NOT BY IMPORT. The job is declared as <job>OCA\Procest\BackgroundJob\DailyTermijnScanJob</job> and the seeder as <step>OCA\Procest\Repair\SeedTermijnbewakingData</step>. These fail differently from each other, which is why both were checked rather than assumed: - an unresolvable JOB is silently reaped. Nextcloud's JobList::buildJob() logs "failed to create instance of background job" and calls removeById(), so the daily termijn sweep would simply stop running with no error surface. - an unresolvable REPAIR STEP throws during upgrade. Both entries move here. Verified by walking info.xml and resolving every class it names to a file on disk: all 13 registered jobs plus the repair step resolve. Positive control: moving DailyDeadlineScanJob.php away makes that same walk report MISSING, so the check can fail. TEST CLASS NAMES. Three of the four test classes are prefixed by the service name they cover, so the service replacement carried them; only TermijnbewakingEndToEndTest was an independent token and was renamed explicitly. The basename == class invariant was run over lib/ AND tests/ this time — the previous slice (#804) ran it over lib/ only and missed exactly this class of defect. VERIFIED - php -l clean on every changed file; info.xml parses. - phpunit, whole unit suite: 1777 tests, 6160 assertions, 5 skipped — green. - Every lib/ file this change touches is phpcs CLEAN. Pre-existing issues fixed along the way: docblock parameter-type alignment in the job and the repair step (shifted by the longer injected type names), and three missing @SPEC tags now anchored to canonical specs (termijn-escalation for the scan job, termijnbewaking-schemas for the seeder). - Residual grep over lib/, tests/, appinfo/ and src/: no live reference to any of the five old names. This caught two prose references in unrelated files (VthSeedDataRepairStep and KccWerkplekSeedDataService) that name the seeder in comments; both updated so the comments stay true.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 11, 2026
* refactor(procest): TermijnNotificationDispatchJob -> DeadlineNotificationDispatchJob A code-only slice of openspec/changes/english-vocabulary, and the first procest slice — chosen because it is one of the very few procest identifiers that is neither ZGW protocol vocabulary nor wired by string. 'termijn' here is a STATUTORY DEADLINE being monitored, not a contract term, so it becomes Deadline. That reading is app-specific on purpose: hrmq uses 'termijn' for notice periods (aanzegtermijn) and decidesk for terms of office (TermijnRegeling). Three apps, three meanings, one Dutch word — it must never become a fleet word. Referenced only via ::class from TermijnNotificationService, so a missed reference is a compile error rather than a job that silently never dispatches. No route entry, no register-fragment wiring, no baseline suppression keyed to the old path — all four checked, not assumed. TermijnNotificationService itself keeps its name in this commit; it belongs with the termijnbewaking slice, where the Termijn* schemas (termijnDefinitie, termijnInstance, termijnGebeurtenis) move together with their register fragments. Archived changes under openspec/changes/archive/ keep the old name — they are a record of what was built at the time. php -l clean; class name matches file name; use statement still ordered. * refactor(procest): TermijnReporting* -> DeadlineReporting* TermijnReportingController -> DeadlineReportingController and TermijnReportingService -> DeadlineReportingService, with their two Dutch controller methods (kwartaalrapport -> quarterlyReport, jaarrekening -> annualStatement), every Dutch parameter and local ($periode -> $period, $afdeling -> $department, $jaar -> $year, $jaarPrefix -> $yearPrefix), the two named-argument call sites, the three route names, the test, and the Postman collection's description. Stacked on feat/english-vocabulary-deadline-job (#797) because that branch already renames TermijnNotificationDispatchJob; branching from development would have conflicted. FOUR DUTCH CONTRACTS ARE DELIBERATELY LEFT ALONE, and each is annotated in place so the next reader does not "finish the job" and break it: 1. $row['afdeling'] is a SCHEMA PROPERTY. OpenRegister materialises it as a real column, so it moves with a data migration, not with a rename. 2. 'periode', 'afdeling' and 'jaar' as RESPONSE KEYS are the published shape of the reports. src/views/dashboard/TermijnDashboard.vue reads annual.jaar directly, so they move only with a coordinated frontend change. 3. getParam('periode') and getParam('jaar') are QUERY-STRING parameter names — the request contract. 4. The /api/termijn/* URLs. Route NAMES follow the renamed class and methods; URLs do not. Moving them is a breaking change for every consumer and is a separate decision. NAMED ARGUMENTS. resolveQuarter(periode:) and aggregateByType(afdeling:) are called with named arguments, which PHP resolves at call time — a parameter rename that misses one is a runtime fatal that `php -l` reports as clean. Both moved with their declarations. VERIFIED - php -l clean on all four PHP files; the Postman collection still parses. - phpunit: 5 tests, 14 assertions green. - Route resolution: each 'deadlineReporting#<action>' route name was checked against the controller's actual public methods — all three resolve. Positive control: the same check against the OLD action names (kwartaalrapport, jaarrekening) reports MISSING METHOD, so it can distinguish a renamed route from a dangling one. - Residual grep over lib/, tests/, appinfo/ and src/: no live reference to either old class name or the old route prefix. PHPCS SCOPE, MEASURED RATHER THAN ASSUMED. phpcs.xml declares <file>lib</file> and nothing else, so tests/ and appinfo/ are outside the standard entirely — appinfo/routes.php carries 373 violations on development and every sibling test file is equally "dirty". Both files this change touches inside lib/ are CLEAN, including two pre-existing missing-@SPEC errors fixed here (each now anchors to openspec/specs/termijn-reporting/spec.md, the canonical spec, not the change directory the file headers point at). An earlier pass ran phpcbf over the test file and produced a 94-line diff for what is a 6-line rename. That was churn against a gate that never reads the file, so it was reverted; the test diff is now the rename alone. * refactor(procest): Termijn{Escalation,Pause,Extension}Service + listener -> Deadline* Four more classes in the deadline cluster: TermijnEscalationService -> DeadlineEscalationService TermijnPauseService -> DeadlinePauseService TermijnExtensionService -> DeadlineExtensionService TermijnCaseCreatedListener -> DeadlineCaseCreatedListener plus the test file, across 11 files: the DI listener registration, the daily scan job and service, TermijnController, and three test suites. Stacked on feat/english-vocabulary-deadline-reporting (#803), which is itself stacked on #797. Merge order: 797 -> 803 -> this. THE ONE DEFECT THIS SLICE PRODUCED, AND WHAT CAUGHT IT. Renaming the test FILE TermijnPauseExtensionServiceTest.php left the class inside it still called TermijnPauseExtensionServiceTest — neither of the two service-name replacements matched that token, because it is a third name. PHPUnit refused to load it ("Class DeadlinePauseExtensionServiceTest cannot be found"). The basename == class invariant I ran before committing covered only the four lib/ files and would have missed it; it now covers the tests too. Same failure mode as hrmq's provider discovery, reached from the opposite direction. LISTENER WIRING IS SAFE BY CONSTRUCTION. WorkflowListenerRegistrar registers the listener as TermijnCaseCreatedListener::class, not as a string literal, so the rename is resolved at compile time rather than silently failing to match at runtime. Checked before renaming rather than after. VERIFIED - php -l clean on every changed file. - phpunit, whole unit suite: 1777 tests, 6160 assertions, 5 skipped — green. - Every lib/ file this change touches is phpcs CLEAN. Three pre-existing issues were fixed along the way: docblock parameter-type alignment in TermijnController and TermijnDailyScanService (shifted by the longer type names), and two missing class-level @SPEC tags. Those now anchor to canonical specs — termijn-binding + termijn-pause-extension for the controller, which spans both, and termijn-escalation for the daily scan — rather than to the change directories the file headers still point at. - Residual grep over lib/, tests/, appinfo/ and src/: no live reference to any of the four old names. phpcs.xml scopes to <file>lib</file> only, so tests/ and appinfo/ are outside the standard; no formatting was applied to them. * refactor(procest): rename the daily-scan and seed-data deadline classes TermijnDailyScanService -> DeadlineDailyScanService TermijnbewakingSeedDataService -> DeadlineMonitoringSeedDataService DailyTermijnScanJob -> DailyDeadlineScanJob SeedTermijnbewakingData -> SeedDeadlineMonitoringData TermijnbewakingEndToEndTest -> DeadlineMonitoringEndToEndTest Stacked on feat/english-vocabulary-deadline-services (#804). Merge order: 797 -> 803 -> 804 -> this. TWO OF THESE ARE WIRED IN appinfo/info.xml, NOT BY IMPORT. The job is declared as <job>OCA\Procest\BackgroundJob\DailyTermijnScanJob</job> and the seeder as <step>OCA\Procest\Repair\SeedTermijnbewakingData</step>. These fail differently from each other, which is why both were checked rather than assumed: - an unresolvable JOB is silently reaped. Nextcloud's JobList::buildJob() logs "failed to create instance of background job" and calls removeById(), so the daily termijn sweep would simply stop running with no error surface. - an unresolvable REPAIR STEP throws during upgrade. Both entries move here. Verified by walking info.xml and resolving every class it names to a file on disk: all 13 registered jobs plus the repair step resolve. Positive control: moving DailyDeadlineScanJob.php away makes that same walk report MISSING, so the check can fail. TEST CLASS NAMES. Three of the four test classes are prefixed by the service name they cover, so the service replacement carried them; only TermijnbewakingEndToEndTest was an independent token and was renamed explicitly. The basename == class invariant was run over lib/ AND tests/ this time — the previous slice (#804) ran it over lib/ only and missed exactly this class of defect. VERIFIED - php -l clean on every changed file; info.xml parses. - phpunit, whole unit suite: 1777 tests, 6160 assertions, 5 skipped — green. - Every lib/ file this change touches is phpcs CLEAN. Pre-existing issues fixed along the way: docblock parameter-type alignment in the job and the repair step (shifted by the longer injected type names), and three missing @SPEC tags now anchored to canonical specs (termijn-escalation for the scan job, termijnbewaking-schemas for the seeder). - Residual grep over lib/, tests/, appinfo/ and src/: no live reference to any of the five old names. This caught two prose references in unrelated files (VthSeedDataRepairStep and KccWerkplekSeedDataService) that name the seeder in comments; both updated so the comments stay true. * feat(repair): migrate procest's Dutch deadline columns to English Adds RenameDutchDeadlineColumns, the data-migration half of procest's English vocabulary slice. No property is renamed in this commit — the migration lands first, because TermijnService and TermijnController cannot be renamed until the stored data can follow them. Stacked on feat/english-vocabulary-deadline-scan (#806). Merge order: 797 -> 803 -> 804 -> 806 -> this. WHY A MIGRATION IS NEEDED AT ALL. OpenRegister does not store an object as a JSON blob keyed by property name. Each schema property is a real, snake_cased COLUMN in oc_openregister_table_{register}_{schema}. MagicMapper ADDS a column on sync and never renames. A register-only rename therefore leaves the data in the Dutch column while every read looks at the English one and finds null. THIS APP HAS TWO REGISTERS, AND THE OBVIOUS SCOPING MISSES ONE. Measured on the reference install: `procest` holds 1051 rows and `procest-default` holds 107. The sibling step in decidesk resolves a single exact slug. Applying that shape here migrates 40 columns and silently leaves 29 behind — measured both ways: exact slug = 'procest' -> 40 columns slug LIKE 'procest%' -> 69 columns so the register set is resolved by PREFIX and every match is migrated. The failure that was avoided is the worst kind available here: a step that reports success having covered only part of the data. zaaktype IS EXEMPT AND DELIBERATELY ABSENT FROM THE MAP. It is the field name in the statutory ZGW wire format this app both consumes and emits, so it is exempt under the fleet rule — even though at 14 shard tables it is the second most widespread Dutch column here, which is exactly why leaving it out needed saying rather than looking like an oversight. COLLISIONS ARE REFUSED, NOT MERGED. `omschrijving` and `beschrijving` both mean `description`. Measured: they do not co-occur in any shard table on this install, so the guard is a safety net rather than an active path. A later fragment could introduce a pair, and a silent merge would destroy one of two values, so the step migrates neither and logs. SOFT-DELETED ROWS ARE MIGRATED TOO. The back-fill does not filter on `_deleted` — a restored row must not come back with a null subject. 36 non-null values exist across the mapped columns, 35 of them live. VERIFIED - php -l clean; info.xml parses; phpcs CLEAN under the app's standard, including its @SPEC anchor requirement. - Dry run of the step's exact resolution: 40 renames under `procest`, 29 under `procest-default`, 0 back-fills, 0 ambiguous. zaaktype untouched on all 14 tables carrying it. NOT VERIFIED. The step has not been executed against a database — only its resolution and its statement set were computed. It runs on the next upgrade; its idempotence and its back-fill branch are reasoned from the code and the sibling steps, not observed. Say so rather than let "migration verified" stand in for it. * fix(repair): use information_schema, not IDBConnection introspection phpstan failed this branch with three errors, all in the new repair step: Call to an undefined method OCP\IDBConnection::getPrefix(). Call to an undefined method OCP\IDBConnection::getSchema(). (x2) They are real. Read from the running server's own lib/public/IDBConnection.php, the interface exposes getQueryBuilder, getTypedQueryBuilder, getError, getDatabasePlatform, getDatabaseProvider, getShardDefinition and getCrossShardMoveHelper — and nothing else beginning with "get". Both methods I called exist on the concrete OC\DB\Connection, not on the OCP interface this step is typed against. WHY EVERY OTHER CHECK PASSED. `php -l` parses a call to a method that does not exist, and phpcs is a style tool. A nonexistent method on an injected interface is invisible to both. Only phpstan sees it, and I did not run phpstan locally before opening the PR — I described the step as verified on the strength of lint + phpcs + a SQL dry run, none of which touch the PHP API surface. The dry run in particular measured what the STATEMENTS would do, which is exactly why it looked convincing while the code that would issue them could not run at all. THE FIX follows openregister's own RegisterService::magicTableNames(), which solves the same problem: query information_schema and anchor the match on the `openregister_table_` MARKER rather than a computed prefix. That file documents why the obvious alternative fails — getQueryBuilder()->getTableName('') returns the literal `*PREFIX*` placeholder, resolved only when a query executes through the NC DB layer, which a raw information_schema string never is; a LIKE built from it matches zero tables and silently reports every register empty. Column introspection moves to information_schema.columns for the same reason. Matching is tightened while here: the suffix after `openregister_table_<id>_` must be all digits, so register 17 cannot match register 170's shard tables. VERIFIED - phpstan: [OK] No errors, whole-project run, same command as CI. - php -l clean; no `db->getSchema()` or `db->getPrefix()` call remains. THE SAME DEFECT IS IN FOUR SIBLING PRs, all authored today and all failing phpstan for this identical reason: openbuild#176, opencatalogi#850, decidesk#467 and softwarecatalog#488. Each needs this same change. None can merge while red, so nothing shipped — but each of their PR bodies claims verification that never covered the API surface. * style(repair): split shardTables() and drop the short variable phpmd flagged two things the information_schema rewrite introduced: CyclomaticComplexity shardTables() is at 10; the threshold is 10. ShortVariable $at is below the 3-character minimum. The marker-matching loop moves into isShardOf(), which fixes both: the extraction takes shardTables() back under the threshold, and $at becomes $offset. Behaviour is unchanged — same marker anchoring, same digits-only suffix check that keeps register 17 from matching register 170's tables. Also capitalises one inline comment, which phpcs requires. ALL THREE GATES RUN LOCALLY THIS TIME, which is the point. The original defect in this file shipped because I ran php -l and phpcs and called it verified; phpstan was the only gate that could see a call to a nonexistent interface method, and I had not run it. This round: phpcs CLEAN phpstan [OK] No errors phpmd 0 findings on this file * test(repair): cover the migration's scoping decisions The PHPUnit job was failing on the COVERAGE RATCHET, not on a test: Coverage current: 30.52% (14092/46169 statements) Coverage merge base: 30.55% (14000/45824 statements) FAIL: coverage dropped by 0.03% against the merge base. This change adds 345 statements. Adding code without tests drops coverage. Both suite legs reported 1803 and 1777 tests with zero failures, so "PHPUnit FAILED" here meant "you added untested code" — which was true. The migration shipped with no test at all. These ten tests pin the two decisions that determine what the step touches — which shard tables are in scope, and when a rename is refused — plus two invariants that are load-bearing but were only asserted in prose: that `zaaktype` stays out of the map because it is the statutory ZGW wire name, and that every destination is snake_case (MagicMapper DROPS a camelCase column whose snake_case twin exists, so a camelCase destination would be deleted). The DDL/DML paths are deliberately not unit-tested: they need a live database. A POSITIVE CONTROL FOUND A FALSE CLAIM IN MY OWN COMMENT. The first version of the shard-matching test asserted that register 17 must not match register 170's tables, quoting the comment above the guard. Deleting the ctype_digit guard and re-running left that test GREEN — it could not fail, because the marker already ends in '_', so `openregister_table_17_` is not a substring of `openregister_table_170_85`. The trailing underscore handles 17-vs-170; the digits check guards something else entirely: derived tables like `…_17_85_backup` and non-shards like `…_17_audit`, both of which contain the marker and would otherwise receive an ALTER TABLE. The test now asserts that case and goes RED when the guard is removed. The code comment, which stated the wrong reason, is corrected in the same commit. VERIFIED - phpunit: 10 tests, 24 assertions green; whole unit suite 1787/6184. - Positive control: guard removed -> 1 failure; restored -> 10 pass. - phpcs clean, phpstan [OK] No errors.
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
TermijnReportingController→DeadlineReportingControllerandTermijnReportingService→DeadlineReportingService, with:kwartaalrapport→quarterlyReport,jaarrekening→annualStatement$periode/$afdeling/$jaar/$jaarPrefix→$period/$department/$year/$yearPrefixStacked on #797 (
feat/english-vocabulary-deadline-job), which already renamesTermijnNotificationDispatchJob— branching fromdevelopmentwould have conflicted. Merge #797 first.Four Dutch contracts are deliberately left alone
Each is annotated in place, because the next reader's instinct will be to finish the job:
$row['afdeling']'periode','afdeling','jaar'as response keyssrc/views/dashboard/TermijnDashboard.vuereadsannual.jaardirectlygetParam('periode'),getParam('jaar')/api/termijn/*URLsNamed arguments
resolveQuarter(periode:)andaggregateByType(afdeling:)are called with named arguments, which PHP resolves at call time — a parameter rename that misses one is a runtime fatal thatphp -lreports as clean. Both moved with their declarations.Verification
php -lclean on all four PHP files; the Postman collection still parsesdeadlineReporting#<action>name checked against the controller's actual public methods — all three resolve. Positive control: the same check against the old actions (kwartaalrapport,jaarrekening) reports MISSING METHOD, so it can tell a renamed route from a dangling onelib/,tests/,appinfo/,src/— no live reference to either old class name or the old route prefixphpcs scope, measured rather than assumed
phpcs.xmldeclares<file>lib</file>and nothing else — sotests/andappinfo/are outside the standard entirely.appinfo/routes.phpcarries 373 violations ondevelopmentand every sibling test file is equally "dirty" by that measure.Both files this change touches inside
lib/are clean, including two pre-existing missing-@specerrors fixed here. Each now anchors toopenspec/specs/termijn-reporting/spec.md— the canonical spec, not the change directory the file headers point at.An earlier pass ran phpcbf over the test file and produced a 94-line diff for a 6-line rename. That was churn against a gate that never reads the file, so it was reverted; the test diff is now the rename alone.