From 26b5edd7730384f140148496c3f50903d2fcbdf4 Mon Sep 17 00:00:00 2001 From: Niels Drost Date: Sat, 4 Jul 2026 14:24:02 +0200 Subject: [PATCH] feat(core): duplicate-number prevention via observers (#513, #514) Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 30 +++++++++++++++++++ Modules/Invoices/Models/Invoice.php | 23 -------------- .../Invoices/Observers/InvoiceObserver.php | 4 +-- .../InvoiceDuplicateNumberPreventionTest.php | 3 -- Modules/Quotes/Models/Quote.php | 19 ------------ Modules/Quotes/Observers/QuoteObserver.php | 10 ++----- .../Providers/QuotesServiceProvider.php | 3 ++ .../QuoteDuplicateNumberPreventionTest.php | 3 -- 8 files changed, 38 insertions(+), 57 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9dfe654aa..f44ff2160 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -195,6 +195,36 @@ DB_DATABASE=:memory: --- +## Observers + +All model lifecycle business logic (duplicate checks, validation, side effects on save/create/update/delete) goes in `Observers/`, **not** inline `booted()` methods on the model. + +### Convention + +- Create `Modules//Observers/Observer.php` extending `Modules\Core\Observers\AbstractObserver` +- Register in the module's ServiceProvider `boot()`: + ```php + Model::observe(ModelObserver::class); + ``` +- Always use `Model::withoutGlobalScopes()` inside observers — global scopes (e.g. `BelongsToCompany`) may not be active in all contexts (unauthenticated, queued jobs, tests) +- **Do not** add `static::creating()` / `static::saving()` callbacks inside `booted()` on the model — the Observer handles this + +### Example + +```php +// Modules/Invoices/Observers/InvoiceObserver.php +class InvoiceObserver extends AbstractObserver { + public function saving(Invoice $invoice): void { + // saving fires before both creating and updating — covers all writes + } +} + +// Modules/Invoices/Providers/InvoicesServiceProvider.php boot() +Invoice::observe(InvoiceObserver::class); +``` + +--- + ## Key model notes - `Company::$primaryKey` = `id` (standard); URL slug is `search_code` (10 chars, unique, e.g. `ivplv2`) diff --git a/Modules/Invoices/Models/Invoice.php b/Modules/Invoices/Models/Invoice.php index c99012f39..806fa2a18 100644 --- a/Modules/Invoices/Models/Invoice.php +++ b/Modules/Invoices/Models/Invoice.php @@ -27,7 +27,6 @@ use Modules\Invoices\Enums\InvoiceStatus; use Modules\Payments\Models\Payment; use Modules\Quotes\Models\Quote; -use RuntimeException; /** * @property int $id @@ -229,28 +228,6 @@ public function delete(): bool return parent::delete(); } - protected static function booted(): void - { - static::creating(function (self $invoice) { - if ($invoice->invoice_number === null) { - return; - } - - $query = static::withoutGlobalScopes() - ->where('company_id', $invoice->company_id) - ->where('invoice_number', $invoice->invoice_number); - - // A credit note is allowed to share the same number as its parent invoice - if ($invoice->creditinvoice_parent_id) { - $query->where('id', '!=', $invoice->creditinvoice_parent_id); - } - - if ($query->exists()) { - throw new RuntimeException("Duplicate invoice number '{$invoice->invoice_number}'"); - } - }); - } - /* |-------------------------------------------------------------------------- | Factory diff --git a/Modules/Invoices/Observers/InvoiceObserver.php b/Modules/Invoices/Observers/InvoiceObserver.php index 3d96f1804..28e08cf34 100644 --- a/Modules/Invoices/Observers/InvoiceObserver.php +++ b/Modules/Invoices/Observers/InvoiceObserver.php @@ -17,7 +17,7 @@ class InvoiceObserver extends AbstractObserver public function saving(Invoice $invoice): void { if ($invoice->invoice_number !== null) { - $query = Invoice::query() + $query = Invoice::withoutGlobalScopes() ->where('company_id', $invoice->company_id) ->where('invoice_number', $invoice->invoice_number) ->where('id', '!=', $invoice->id ?? 0); @@ -36,7 +36,7 @@ public function saving(Invoice $invoice): void } if ($query->exists()) { - throw new RuntimeException("Duplicate invoice number '{$invoice->invoice_number}' for company ID {$invoice->company_id}"); + throw new RuntimeException("Duplicate invoice number '{$invoice->invoice_number}'"); } } } diff --git a/Modules/Invoices/Tests/Feature/InvoiceDuplicateNumberPreventionTest.php b/Modules/Invoices/Tests/Feature/InvoiceDuplicateNumberPreventionTest.php index e7e1c2fe9..19d39ff24 100644 --- a/Modules/Invoices/Tests/Feature/InvoiceDuplicateNumberPreventionTest.php +++ b/Modules/Invoices/Tests/Feature/InvoiceDuplicateNumberPreventionTest.php @@ -14,7 +14,6 @@ class InvoiceDuplicateNumberPreventionTest extends AbstractAdminPanelTestCase { #[Test] - #[Group('failing')] public function it_prevents_duplicate_invoice_numbers_within_same_company(): void { /* Arrange */ @@ -63,7 +62,6 @@ public function it_allows_same_invoice_number_in_different_companies(): void } #[Test] - #[Group('failing')] public function it_allows_multiple_null_invoice_numbers_for_drafts(): void { /* Arrange */ @@ -153,7 +151,6 @@ public function it_allows_creating_a_credit_note_with_the_same_number_as_its_par } #[Test] - #[Group('failing')] public function it_allows_updating_invoice_without_changing_number(): void { /* Arrange */ diff --git a/Modules/Quotes/Models/Quote.php b/Modules/Quotes/Models/Quote.php index 408ae21eb..6a3843cfc 100644 --- a/Modules/Quotes/Models/Quote.php +++ b/Modules/Quotes/Models/Quote.php @@ -19,7 +19,6 @@ use Modules\Invoices\Models\Invoice; use Modules\Quotes\Database\Factories\QuoteFactory; use Modules\Quotes\Enums\QuoteStatus; -use RuntimeException; /** * @property int $id @@ -171,24 +170,6 @@ public function scopeRecent($query, $limit = 25) ->limit($quoteLimit); } - protected static function booted(): void - { - static::creating(function (self $quote) { - if ($quote->quote_number === null) { - return; - } - - $exists = static::withoutGlobalScopes() - ->where('company_id', $quote->company_id) - ->where('quote_number', $quote->quote_number) - ->exists(); - - if ($exists) { - throw new RuntimeException("Duplicate quote number '{$quote->quote_number}'"); - } - }); - } - /* |-------------------------------------------------------------------------- | Factory diff --git a/Modules/Quotes/Observers/QuoteObserver.php b/Modules/Quotes/Observers/QuoteObserver.php index 68a00d9df..172d5c687 100644 --- a/Modules/Quotes/Observers/QuoteObserver.php +++ b/Modules/Quotes/Observers/QuoteObserver.php @@ -16,18 +16,14 @@ class QuoteObserver extends AbstractObserver public function saving(Quote $quote): void { if ($quote->quote_number !== null) { - $duplicate = Quote::query()->where('company_id', $quote->company_id) + $duplicate = Quote::withoutGlobalScopes() + ->where('company_id', $quote->company_id) ->where('quote_number', $quote->quote_number) ->where('id', '!=', $quote->id ?? 0) ->exists(); if ($duplicate) { - throw new RuntimeException( - trans('quotes.errors.duplicate_quote_number', [ - 'quote_number' => $quote->quote_number, - 'company_id' => $quote->company_id, - ]) - ); + throw new RuntimeException("Duplicate quote number '{$quote->quote_number}'"); } } } diff --git a/Modules/Quotes/Providers/QuotesServiceProvider.php b/Modules/Quotes/Providers/QuotesServiceProvider.php index 530164401..5f69011ef 100644 --- a/Modules/Quotes/Providers/QuotesServiceProvider.php +++ b/Modules/Quotes/Providers/QuotesServiceProvider.php @@ -4,6 +4,8 @@ use Illuminate\Support\Facades\Blade; use Illuminate\Support\ServiceProvider; +use Modules\Quotes\Models\Quote; +use Modules\Quotes\Observers\QuoteObserver; use Nwidart\Modules\Traits\PathNamespace; use RecursiveDirectoryIterator; use RecursiveIteratorIterator; @@ -24,6 +26,7 @@ public function boot(): void $this->registerConfig(); $this->registerViews(); $this->loadMigrationsFrom(module_path($this->name, 'Database/Migrations')); + Quote::observe(QuoteObserver::class); } public function register(): void diff --git a/Modules/Quotes/Tests/Feature/QuoteDuplicateNumberPreventionTest.php b/Modules/Quotes/Tests/Feature/QuoteDuplicateNumberPreventionTest.php index 231597813..67dc567b3 100644 --- a/Modules/Quotes/Tests/Feature/QuoteDuplicateNumberPreventionTest.php +++ b/Modules/Quotes/Tests/Feature/QuoteDuplicateNumberPreventionTest.php @@ -16,7 +16,6 @@ class QuoteDuplicateNumberPreventionTest extends AbstractAdminPanelTestCase { #[Test] - #[Group('failing')] public function it_prevents_duplicate_quote_numbers_within_same_company(): void { /* Arrange */ @@ -65,7 +64,6 @@ public function it_allows_same_quote_number_in_different_companies(): void } #[Test] - #[Group('failing')] public function it_allows_multiple_null_quote_numbers_for_drafts(): void { /* Arrange */ @@ -101,7 +99,6 @@ public function it_allows_multiple_null_quote_numbers_for_drafts(): void } #[Test] - #[Group('failing')] public function it_allows_updating_quote_without_changing_number(): void { /* Arrange */