refactor(sales): rewrite legacy-era regions of the credit-note, item and balance services and the provider in place

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);

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);
             }

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();

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);
+        }
     }
 }