From ba09d78f9c1387fc0960e044c45af4d122c0bdba Mon Sep 17 00:00:00 2001 From: Darko Gjorgjijoski Date: Fri, 21 Aug 2026 01:38:55 +0200 Subject: [PATCH] refactor(sales): rewrite legacy-era regions of the credit-note, item and balance services and the provider in place --- .../Sales/Application/CreditNoteService.php | 48 ++++++++++++------- .../Sales/Application/DocumentItemService.php | 13 ++--- .../Application/InvoiceBalanceService.php | 8 +++- app/Domains/Sales/SalesServiceProvider.php | 23 ++++++--- 4 files changed, 60 insertions(+), 32 deletions(-) diff --git a/app/Domains/Sales/Application/CreditNoteService.php b/app/Domains/Sales/Application/CreditNoteService.php index 961c032e..c993b031 100644 --- a/app/Domains/Sales/Application/CreditNoteService.php +++ b/app/Domains/Sales/Application/CreditNoteService.php @@ -39,6 +39,12 @@ class CreditNoteService private readonly CustomFieldValueWriter $customFieldValueWriter, ) {} + /** + * What the freshly written credit note is re-read with, so the caller hands + * back a document the API resource can render whole. + */ + private const RESPONSE_RELATIONS = ['items', 'items.fields', 'items.fields.customField', 'customer', 'taxes', 'relatedInvoice']; + /** * Create a credit note reversing the given invoice. * @@ -84,14 +90,7 @@ class CreditNoteService $this->recalculateBalance($original); - return Invoice::with([ - 'items', - 'items.fields', - 'items.fields.customField', - 'customer', - 'taxes', - 'relatedInvoice', - ])->find($creditNote->id); + return Invoice::with(self::RESPONSE_RELATIONS)->find($creditNote->id); }); } @@ -182,6 +181,25 @@ class CreditNoteService ->setSequenceScope(['type' => Invoice::TYPE_CREDIT_NOTE]) ->setNextNumbers(); + // The builder resolved all three figures in the pass above; read them + // off it here so the document below stays plain data. + $number = $serial->getNextNumber(); + $sequence = $serial->nextSequenceNumber; + $customerSequence = $serial->nextCustomerSequenceNumber; + + // Columns the reversal inherits verbatim: it has to sit in the same + // currency, tax and discount regime as the document it undoes, or the + // two would not net out against each other. + $carriedOver = $invoice->only([ + 'discount', + 'discount_type', + 'tax_per_item', + 'discount_per_item', + 'currency_id', + 'sales_tax_type', + 'sales_tax_address_type', + ]); + // exchange_rate is a float multiplier, not a currency amount. The base_* // fields are pro-rated from the original's stored base_* integers by the // calculator, so they are negated as-is rather than recomputed through @@ -195,9 +213,9 @@ class CreditNoteService // A reversal is never owed, so it has no due date at all. Leaving it // null also keeps the credit note out of every due/aging query. 'due_date' => null, - 'invoice_number' => $serial->getNextNumber(), - 'sequence_number' => $serial->nextSequenceNumber, - 'customer_sequence_number' => $serial->nextCustomerSequenceNumber, + 'invoice_number' => $number, + 'sequence_number' => $sequence, + 'customer_sequence_number' => $customerSequence, 'reference_number' => $invoice->invoice_number, 'customer_id' => $invoice->customer_id, 'company_id' => $invoice->company_id, @@ -212,13 +230,9 @@ class CreditNoteService // surface as an open (negative) balance in any due/aging view. 'paid_status' => Invoice::STATUS_PAID, 'sub_total' => -$amounts['sub_total'], - 'discount' => $invoice->discount, - 'discount_type' => $invoice->discount_type, 'discount_val' => -$amounts['discount_val'], 'total' => -$amounts['total'], 'due_amount' => 0, - 'tax_per_item' => $invoice->tax_per_item, - 'discount_per_item' => $invoice->discount_per_item, 'tax' => -$amounts['tax'], 'tax_included' => $invoice->tax_included, 'notes' => $invoice->notes, @@ -228,9 +242,7 @@ class CreditNoteService 'base_total' => -$amounts['base_total'], 'base_tax' => -$amounts['base_tax'], 'base_due_amount' => 0, - 'currency_id' => $invoice->currency_id, - 'sales_tax_type' => $invoice->sales_tax_type, - 'sales_tax_address_type' => $invoice->sales_tax_address_type, + ...$carriedOver, ]); $creditNote->unique_hash = Hashids::connection(HashidConnection::Invoice->value)->encode($creditNote->id); diff --git a/app/Domains/Sales/Application/DocumentItemService.php b/app/Domains/Sales/Application/DocumentItemService.php index 31929656..415b6d45 100644 --- a/app/Domains/Sales/Application/DocumentItemService.php +++ b/app/Domains/Sales/Application/DocumentItemService.php @@ -79,9 +79,11 @@ class DocumentItemService } if (gettype($tax['amount']) !== 'NULL') { - if (array_key_exists('recurring_invoice_id', $tax)) { - unset($tax['recurring_invoice_id']); - } + // A row lifted off a recurring template still carries + // the template's key, which means nothing on the + // generated document. Dropping an absent key is a + // no-op, so it needs no guard. + unset($tax['recurring_invoice_id']); $createdItem->taxes()->create($tax); } @@ -114,9 +116,8 @@ class DocumentItemService } if (gettype($tax['amount']) !== 'NULL') { - if (array_key_exists('recurring_invoice_id', $tax)) { - unset($tax['recurring_invoice_id']); - } + // Same template key as in createItems(), dropped the same way. + unset($tax['recurring_invoice_id']); $document->taxes()->create($tax); } diff --git a/app/Domains/Sales/Application/InvoiceBalanceService.php b/app/Domains/Sales/Application/InvoiceBalanceService.php index 186eb4cd..f2baec76 100644 --- a/app/Domains/Sales/Application/InvoiceBalanceService.php +++ b/app/Domains/Sales/Application/InvoiceBalanceService.php @@ -30,8 +30,12 @@ class InvoiceBalanceService $invoice->base_due_amount = (int) round($due * $invoice->exchange_rate); if ($due === 0) { - $invoice->status = Invoice::STATUS_COMPLETED; - $invoice->paid_status = Invoice::STATUS_PAID; + // Nothing left outstanding, so the document closes out on both + // axes at once. + $invoice->forceFill([ + 'status' => Invoice::STATUS_COMPLETED, + 'paid_status' => Invoice::STATUS_PAID, + ]); $invoice->overdue = false; } else { $invoice->status = $invoice->getPreviousStatus(); diff --git a/app/Domains/Sales/SalesServiceProvider.php b/app/Domains/Sales/SalesServiceProvider.php index 31a98fe8..78a22723 100644 --- a/app/Domains/Sales/SalesServiceProvider.php +++ b/app/Domains/Sales/SalesServiceProvider.php @@ -26,6 +26,19 @@ use Illuminate\Support\ServiceProvider; class SalesServiceProvider extends ServiceProvider { + /** + * Abilities answered by a policy method rather than by a model instance, + * as ability name => [policy class, policy method]. + */ + private const POLICY_ABILITIES = [ + 'send invoice' => [InvoicePolicy::class, 'send'], + 'create credit note' => [CreditNotePolicy::class, 'create'], + 'send estimate' => [EstimatePolicy::class, 'send'], + 'delete multiple invoices' => [InvoicePolicy::class, 'deleteMultiple'], + 'delete multiple estimates' => [EstimatePolicy::class, 'deleteMultiple'], + 'delete multiple recurring invoices' => [RecurringInvoicePolicy::class, 'deleteMultiple'], + ]; + public function register(): void { $this->app->bind(EstimatePdfDataProvider::class, EstimateService::class); @@ -45,11 +58,9 @@ class SalesServiceProvider extends ServiceProvider Gate::policy(Estimate::class, EstimatePolicy::class); Gate::policy(Invoice::class, InvoicePolicy::class); Gate::policy(RecurringInvoice::class, RecurringInvoicePolicy::class); - Gate::define('send invoice', [InvoicePolicy::class, 'send']); - Gate::define('create credit note', [CreditNotePolicy::class, 'create']); - Gate::define('send estimate', [EstimatePolicy::class, 'send']); - Gate::define('delete multiple invoices', [InvoicePolicy::class, 'deleteMultiple']); - Gate::define('delete multiple estimates', [EstimatePolicy::class, 'deleteMultiple']); - Gate::define('delete multiple recurring invoices', [RecurringInvoicePolicy::class, 'deleteMultiple']); + + foreach (self::POLICY_ABILITIES as $ability => $handler) { + Gate::define($ability, $handler); + } } }