Roll back auto bill regression

This commit is contained in:
David Bomba
2026-04-16 11:14:32 +10:00
parent a6a925299f
commit d28fcd119f
2 changed files with 71 additions and 5 deletions

View File

@@ -58,8 +58,11 @@ class AutoBillInvoice extends AbstractService
/* @var \App\Modesl\Client $client */
$is_partial = false;
/* Mark the invoice as sent */
$this->invoice = $this->invoice->service()->markSent()->save();
/* Mark the invoice as paid if there is no balance */
if ($this->invoice->balance == 0 && !$this->invoice->is_deleted && ($this->invoice->status_id == Invoice::STATUS_DRAFT || $this->invoice->status_id == Invoice::STATUS_SENT)) {
if (floatval($this->invoice->balance) == 0) {
return $this->invoice->service()->markPaid()->save();
}
@@ -68,10 +71,6 @@ class AutoBillInvoice extends AbstractService
return $this->invoice;
}
/* Mark the invoice as sent */
$this->invoice = $this->invoice->service()->markSent()->save();
//if the credits cover the payments, we stop here, build the payment with credits and exit early
if ($this->client->getSetting('use_credits_payment') != 'off') {
$this->applyCreditPayment();

View File

@@ -159,4 +159,71 @@ class AutoBillInvoiceApiTest extends TestCase
$payment_count_after = Payment::where('client_id', $this->client->id)->count();
$this->assertEquals($payment_count_before, $payment_count_after, 'No payment should be created when no payment source is available');
}
/**
* POST /api/v1/invoices?auto_bill=true (without mark_sent)
*
* Thesis: when auto_bill=true is used on a draft invoice (without
* mark_sent=true), a manual payment is applied because the invoice
* starts in draft status.
*
* Actual behavior: AutoBillInvoice::run() calls markSent() first
* (line 62), transitioning the draft to SENT and setting balance =
* amount. Then it tries to collect via credits, unapplied payments,
* and gateway tokens. Without a payment source, the exception is
* caught silently and the invoice remains SENT/unpaid — the user's
* draft invoice was promoted to SENT as a side effect of auto_bill.
*/
public function testAutoBillDraftInvoiceCreatesManualPayment(): void
{
$settings = ClientSettings::defaults();
$settings->use_credits_payment = 'off';
$settings->use_unapplied_payment = 'off';
$this->client->settings = $settings;
$this->client->save();
$payment_count_before = Payment::where('client_id', $this->client->id)->count();
// Prove that a plain draft has balance=0 even though amount > 0
$response = $this->withHeaders([
'X-API-SECRET' => config('ninja.api_secret'),
'X-API-TOKEN' => $this->token,
])->postJson('/api/v1/invoices', $this->invoiceData(100));
$response->assertStatus(200);
$arr = $response->json();
$draft = Invoice::find($this->decodePrimaryKey($arr['data']['id']));
$this->assertEquals(Invoice::STATUS_DRAFT, $draft->status_id);
$this->assertEquals(100, (float) $draft->amount, 'Draft amount is calculated');
$this->assertEquals(0, (float) $draft->balance, 'Draft balance is 0 (not calculated for drafts)');
// Now create with auto_bill=true but WITHOUT mark_sent=true
$response2 = $this->withHeaders([
'X-API-SECRET' => config('ninja.api_secret'),
'X-API-TOKEN' => $this->token,
])->postJson('/api/v1/invoices?auto_bill=true', $this->invoiceData(100));
$response2->assertStatus(200);
$arr2 = $response2->json();
$invoice = Invoice::find($this->decodePrimaryKey($arr2['data']['id']));
// Side effect: AutoBillInvoice::run() calls markSent() before
// attempting to collect payment, so the draft is promoted to SENT
$this->assertEquals(Invoice::STATUS_SENT, $invoice->status_id, 'Draft was promoted to SENT by auto_bill');
$this->assertEquals(100, (float) $invoice->balance, 'Balance equals amount after markSent');
$this->assertEquals(0, (float) $invoice->paid_to_date, 'No payment was collected');
// No payment was created — the auto_bill failed silently because
// there is no gateway token, credits, or unapplied payments
$payment = Payment::query()
->whereHas('invoices', fn ($q) => $q->where('invoices.id', $invoice->id))
->first();
$this->assertNull($payment, 'No payment linked to this invoice');
$payment_count_after = Payment::where('client_id', $this->client->id)->count();
$this->assertEquals($payment_count_before, $payment_count_after, 'No new payments created');
}
}