refactor(procest): Ingebrekestelling{Service,Controller} -> NoticeOfDefault* - #810
Open
rubenvdlinde wants to merge 1 commit into
Open
Conversation
…efault* IngebrekestellingService -> NoticeOfDefaultService IngebrekestellingController -> NoticeOfDefaultController $ingebrekestellingId -> $noticeOfDefaultId plus the two route names addressing the controller. Six files. Stacked on feat/english-vocabulary-deadline-params (#809). Merge order: 797 -> 803 -> 804 -> 806 -> 807 -> 808 -> 809 -> this. WHAT STAYS DUTCH, AND WHY IT IS NOT AN OVERSIGHT. Three things in these files are the data contract rather than identifiers, and each would break silently if renamed without a register change and a migration: - 'relevantIngbrekes' — a TermijnInstance property (note the existing typo in the schema; it is reproduced exactly because it is the column name). This is the one-dwangsom guard: if the read returns null the guard stops firing and a SECOND DwangsomBerekening is spawned per notice, which is a money bug. - 'ingebrekestelling' — a DwangsomBerekening property. - The /api/termijn/ingebrekestellingen URLs — the published contract. Route NAMES follow the renamed controller; the URLs deliberately do not. The public method names were already English (register, show), so no method rename was needed; registerIngebrekestelling() on the service is left for the slice that renames the domain noun itself. APPLIED THE #809 LESSON. That commit renamed a promoted constructor property by searching `$name` and missed all 47 `$this->name` accesses, because the sigil belongs to $this rather than to the member. Here the property-access spelling was checked explicitly BEFORE editing (`this->ingebrekestelling` — none) and the residual grep at the end matches the bare word, not the sigilled form. VERIFIED - php -l clean on all six files; the Postman collection still parses. - phpunit, whole unit suite: 1777 tests, 6160 assertions, 5 skipped — green, identical to baseline. - basename == class across lib/ AND tests/ for all three renamed files. - Route resolution: both noticeOfDefault#<action> names resolve to real public methods; control confirms the old class name is gone. - Every touched lib/ file is phpcs CLEAN. - Residual grep (bare word) over lib/, tests/, appinfo/ and src/: none.
rubenvdlinde
requested review from
Rem-Dam,
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
August 11, 2026 21:24
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
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
IngebrekestellingServiceNoticeOfDefaultServiceIngebrekestellingControllerNoticeOfDefaultController$ingebrekestellingId$noticeOfDefaultIdplus the two route names addressing the controller. Six files.
Stacked on #809. Merge order: #797 → #803 → #804 → #806 → #807 → #808 → #809 → this.
What stays Dutch, and why it isn't an oversight
Three things here are the data contract, not identifiers. Each would break silently if renamed without a register change and a migration:
'relevantIngbrekes'— aTermijnInstanceproperty. Note the existing typo in the schema; it's reproduced exactly, because it is the column name. This is the one-dwangsom guard: if the read returns null the guard stops firing and a secondDwangsomBerekeningis spawned per notice. That's a money bug.'ingebrekestelling'— aDwangsomBerekeningproperty./api/termijn/ingebrekestellingen— the published URLs. Route names follow the renamed controller; the URLs deliberately do not.The controller's public methods were already English (
register,show), so no method rename was needed.registerIngebrekestelling()on the service is left for the slice that renames the domain noun itself.Applied the #809 lesson
#809 renamed a promoted constructor property by searching
$nameand missed all 47$this->nameaccesses — the sigil belongs to$this, not the member.Here the property-access spelling was checked before editing (
this->ingebrekestelling— none present), and the residual grep matches the bare word, not the sigilled form.Verification
php -lclean on all six files; the Postman collection still parseslib/andtests/for all three renamed filesnoticeOfDefault#<action>names resolve to real public methods; control confirms the old class name is gonelib/file is phpcs CLEANlib/,tests/,appinfo/,src/— none