From 57cf704530f62dda86c065f5105df550654c1386 Mon Sep 17 00:00:00 2001 From: Sylvester Damgaard Date: Mon, 27 Jul 2026 12:47:14 +0200 Subject: [PATCH] fix(invoicing): write the refunded status the approval screen promises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `InvoiceStatus::Refunded` existed in the enum and was assigned nowhere in the app. Its only other appearance was RefundInvoiceAction's approval preview, which shows the operator `status → refunded` before they approve — the refund then executed and left the invoice `paid`. The console promised a transition the code never performed, to the one person whose job was to check it. Set cumulatively rather than per-refund: several partial refunds that together cover the invoice leave it as fully reversed just as one full refund does. A partial refund still leaves it `Paid`, which is correct — it IS paid, with part reversed, and the credit notes carry the detail. NEXUS CONSEQUENCE, handled deliberately rather than inherited. The economic-nexus ledger filters on [Open, Paid]. Until now a fully-refunded invoice stayed `Paid` and was measured; writing the real status would have silently dropped it, REDUCING measured US sales as a side effect of an unrelated fix — the kind of change that surfaces years later as a missed registration. `Refunded` is therefore counted explicitly, which is also the safer reading: thresholds are generally stated on gross sales, and the errors are asymmetric — over-counting prompts a registration that may not be needed, under-counting misses one that is, which is back taxes plus penalties. Where a state permits netting returns, that is a per-state refinement against the dataset's rules, not a blanket exclusion. Both tests verified to fail with the transition removed. --- app/Billing/Invoicing/InvoiceOperations.php | 40 ++++++++++++++++++- app/Billing/Nexus/InvoiceSalesLedger.php | 12 +++++- tests/Feature/InvoiceLifecycleConsoleTest.php | 28 +++++++++++++ 3 files changed, 77 insertions(+), 3 deletions(-) diff --git a/app/Billing/Invoicing/InvoiceOperations.php b/app/Billing/Invoicing/InvoiceOperations.php index 07c106f..99c75ce 100644 --- a/app/Billing/Invoicing/InvoiceOperations.php +++ b/app/Billing/Invoicing/InvoiceOperations.php @@ -12,6 +12,7 @@ use App\Billing\Seller\SellerCatalog; use App\Billing\Support\WeightedAllocator; use App\Billing\Tax\TaxContextFactory; +use App\Models\CreditNote; use App\Models\Invoice; use App\Models\InvoiceLine; use App\Models\Organization; @@ -86,8 +87,43 @@ public function refund(Invoice $invoice, ?int $netMinor, RefundReason $reason, s : RefundRequest::partial($actionId, $invoice->organization_id, $engineInvoice, Money::ofMinor($netMinor, $invoice->currency), $reason, $at, $invoice->gateway_reference); // The refunder's ledger post, refund-record save and CreditNoteIssued listener - // (which writes the durable credit note) all commit together. - return $this->db->transaction(fn (): Refund => $this->refunder->refund($request)); + // (which writes the durable credit note) all commit together — and, once the credit + // notes cover the invoice, so does the status transition to Refunded. + return $this->db->transaction(function () use ($request, $invoice): Refund { + $refund = $this->refunder->refund($request); + + $this->settleRefundedStatus($invoice); + + return $refund; + }); + } + + /** + * Move a fully-reversed invoice to {@see InvoiceStatus::Refunded}. + * + * `Refunded` existed in the enum and was assigned NOWHERE in the app: its only other + * appearance was the approval preview, which showed the operator `status → refunded` before + * they approved. The refund then executed and left the invoice `paid`, so the console + * promised a transition the code never performed. + * + * Cumulative, not per-refund: several partial refunds that together cover the invoice leave + * it as fully reversed just as a single full one does. A partial refund correctly leaves the + * invoice `Paid` — it *is* still paid, with part reversed, and the credit notes carry the + * detail. + */ + private function settleRefundedStatus(Invoice $invoice): void + { + if ($invoice->status === InvoiceStatus::Refunded) { + return; + } + + $refunded = (int) CreditNote::query() + ->where('invoice_id', $invoice->id) + ->sum('gross_minor'); + + if ($refunded >= $invoice->total_minor && $invoice->total_minor > 0) { + $invoice->forceFill(['status' => InvoiceStatus::Refunded])->save(); + } } public function markPaid(Invoice $invoice, ?string $reference): void diff --git a/app/Billing/Nexus/InvoiceSalesLedger.php b/app/Billing/Nexus/InvoiceSalesLedger.php index 34c715a..83b4d75 100644 --- a/app/Billing/Nexus/InvoiceSalesLedger.php +++ b/app/Billing/Nexus/InvoiceSalesLedger.php @@ -167,7 +167,17 @@ private function platformActivity(SubdivisionCode $state, Carbon $windowStart, C $base = Invoice::query() ->where('seller', $this->sellers->default()->id) - ->whereIn('status', [InvoiceStatus::Open, InvoiceStatus::Paid]) + // `Refunded` is counted, deliberately. Until the refund path started writing that + // status, a fully-reversed invoice stayed `Paid` and was measured here; dropping it + // now would silently REDUCE measured sales as a side effect of an unrelated fix. + // + // Counting it is also the safer reading: most post-Wayfair thresholds are stated on + // GROSS sales, and the two errors are not symmetric — over-counting prompts a + // registration the seller may not strictly need, while under-counting misses one + // they do, which is back taxes plus penalties. Where a state permits netting + // returns, that is a per-state refinement to make against the dataset's own rules, + // not a blanket exclusion to assume here. + ->whereIn('status', [InvoiceStatus::Open, InvoiceStatus::Paid, InvoiceStatus::Refunded]) ->where('issued_at', '>=', $windowStart) ->where('issued_at', '<', $windowEnd) ->whereIn('organization_id', $organizationIds); diff --git a/tests/Feature/InvoiceLifecycleConsoleTest.php b/tests/Feature/InvoiceLifecycleConsoleTest.php index 5115fde..13b5aa7 100644 --- a/tests/Feature/InvoiceLifecycleConsoleTest.php +++ b/tests/Feature/InvoiceLifecycleConsoleTest.php @@ -218,4 +218,32 @@ public function test_credit_note_shows_on_invoice_and_in_the_list(): void $this->withSession($this->session)->get('/credit-notes/'.$note->id) ->assertOk()->assertSee($invoice->number); } + + public function test_a_full_refund_leaves_the_invoice_in_the_status_the_approval_preview_promised(): void + { + $invoice = $this->invoicedOrg('org_refund_full'); + $invoice->forceFill(['status' => 'paid', 'paid_at' => now()])->save(); + + $this->withSession($this->session)->post('/invoices/'.$invoice->id.'/refund', [ + 'mode' => 'full', 'reason' => 'goodwill', 'idempotency_key' => 'k-full-status', + ])->assertRedirect()->assertSessionHas('status'); + + // RefundInvoiceAction's approval preview shows the operator `status -> refunded` before + // they approve. Before this, the refund executed and the invoice stayed `paid`, so the + // console promised a transition the code never performed. + $this->assertSame(InvoiceStatus::Refunded, $invoice->refresh()->status); + } + + public function test_a_partial_refund_leaves_the_invoice_paid(): void + { + $invoice = $this->invoicedOrg('org_refund_part'); + $invoice->forceFill(['status' => 'paid', 'paid_at' => now()])->save(); + + $this->withSession($this->session)->post('/invoices/'.$invoice->id.'/refund', [ + 'mode' => 'partial', 'amount' => '10.00', 'reason' => 'goodwill', 'idempotency_key' => 'k-part-status', + ])->assertRedirect()->assertSessionHas('status'); + + // Still paid — it IS paid, with part reversed; the credit note carries the detail. + $this->assertSame(InvoiceStatus::Paid, $invoice->refresh()->status); + } }