diff --git a/app/Http/Controllers/Company/Invoice/InvoicesController.php b/app/Http/Controllers/Company/Invoice/InvoicesController.php index 8ef66ff6..ee0d1f9d 100644 --- a/app/Http/Controllers/Company/Invoice/InvoicesController.php +++ b/app/Http/Controllers/Company/Invoice/InvoicesController.php @@ -4,6 +4,7 @@ namespace App\Http\Controllers\Company\Invoice; use App\Http\Controllers\Controller; use App\Http\Requests; +use App\Http\Requests\ChangeInvoiceStatusRequest; use App\Http\Requests\CreateCreditNoteRequest; use App\Http\Requests\DeleteInvoiceRequest; use App\Http\Requests\SendInvoiceRequest; @@ -238,7 +239,7 @@ class InvoicesController extends Controller ->setStatusCode(201); } - public function changeStatus(Request $request, Invoice $invoice) + public function changeStatus(ChangeInvoiceStatusRequest $request, Invoice $invoice) { $this->authorize('send invoice', $invoice); diff --git a/app/Http/Requests/ChangeInvoiceStatusRequest.php b/app/Http/Requests/ChangeInvoiceStatusRequest.php new file mode 100644 index 00000000..534303ed --- /dev/null +++ b/app/Http/Requests/ChangeInvoiceStatusRequest.php @@ -0,0 +1,34 @@ + [ + 'required', + Rule::in([ + Invoice::STATUS_SENT, + Invoice::STATUS_COMPLETED, + ]), + ], + ]; + } +} diff --git a/app/Services/Document/InvoiceService.php b/app/Services/Document/InvoiceService.php index 46f9a882..f140166e 100644 --- a/app/Services/Document/InvoiceService.php +++ b/app/Services/Document/InvoiceService.php @@ -477,10 +477,21 @@ class InvoiceService $invoice->sent = true; $invoice->save(); } elseif ($status == Invoice::STATUS_COMPLETED) { - $invoice->status = Invoice::STATUS_COMPLETED; - $invoice->paid_status = Invoice::STATUS_PAID; - $invoice->due_amount = 0; - $invoice->save(); + $paid = (int) $invoice->payments()->sum('amount'); + $credited = $this->creditNoteService->creditedTotal($invoice); + $outstanding = max(0, (int) $invoice->total - $paid - $credited); + + if ( + $outstanding !== 0 + || (int) $invoice->due_amount !== 0 + || (int) $invoice->base_due_amount !== 0 + ) { + throw ValidationException::withMessages([ + 'status' => ['invoice_must_be_settled_before_completion'], + ]); + } + + $invoice->changeInvoiceStatus((int) $invoice->due_amount); } } } diff --git a/lang/en.json b/lang/en.json index 54dd6deb..e457173e 100644 --- a/lang/en.json +++ b/lang/en.json @@ -1854,6 +1854,7 @@ "credit_quantity_invalid": "Enter a valid quantity greater than zero for every selected line.", "credit_note_cannot_be_cloned": "A credit note cannot be cloned.", "credit_note_cannot_be_converted_to_estimate": "A credit note cannot be converted to an estimate.", + "invoice_must_be_settled_before_completion": "Record a payment or create a credit note before completing this invoice.", "payment_amount_exceeds_invoice_due_amount": "The payment is more than the invoice's outstanding balance.", "payment_number_used": "The payment number has already been taken.", "name_already_taken": "The name has already been taken.", diff --git a/resources/scripts/utils/error-handling.ts b/resources/scripts/utils/error-handling.ts index dd2f285f..09f07c28 100644 --- a/resources/scripts/utils/error-handling.ts +++ b/resources/scripts/utils/error-handling.ts @@ -63,6 +63,8 @@ const ERROR_TRANSLATION_MAP: Record = { 'a_credit_note_cannot_be_cloned': 'errors.credit_note_cannot_be_cloned', 'a_credit_note_cannot_be_converted_to_an_estimate': 'errors.credit_note_cannot_be_converted_to_estimate', + 'invoice_must_be_settled_before_completion': + 'errors.invoice_must_be_settled_before_completion', 'The estimate number has already been taken.': 'errors.estimate_number_used', 'The payment number has already been taken.': 'errors.estimate_number_used', 'The invoice number has already been taken.': 'errors.invoice_number_used', diff --git a/tests/Feature/Admin/CreditNoteTest.php b/tests/Feature/Admin/CreditNoteTest.php index ed38911e..4f1fbc42 100644 --- a/tests/Feature/Admin/CreditNoteTest.php +++ b/tests/Feature/Admin/CreditNoteTest.php @@ -905,18 +905,24 @@ test('no surviving row keeps a dangling related invoice reference', function () expect(Invoice::find($creditNote->id)->related_invoice_id)->toBeNull(); }); -test('completing an uncredited invoice still zeroes its balance', function () { +test('completing a fully credited invoice is idempotent', function () { $invoice = creditableInvoice(); + postJson("api/v1/invoices/{$invoice->id}/credit-note") + ->assertStatus(201); + postJson("api/v1/invoices/{$invoice->id}/status", ['status' => Invoice::STATUS_COMPLETED]) ->assertOk(); $invoice->refresh(); - // The credit-note bookkeeping must not touch the manual status change. + // Completion verifies the recorded credit note and does not disturb the + // already-settled balance. expect((int) $invoice->due_amount)->toBe(0) + ->and((int) $invoice->base_due_amount)->toBe(0) ->and($invoice->status)->toBe(Invoice::STATUS_COMPLETED) - ->and($invoice->paid_status)->toBe(Invoice::STATUS_PAID); + ->and($invoice->paid_status)->toBe(Invoice::STATUS_PAID) + ->and($invoice->payments)->toHaveCount(0); }); test('renders a credit note pdf through the original invoice template family, not a hardcoded layout', function () { diff --git a/tests/Feature/Admin/InvoiceTest.php b/tests/Feature/Admin/InvoiceTest.php index a67c4add..cc33ae03 100644 --- a/tests/Feature/Admin/InvoiceTest.php +++ b/tests/Feature/Admin/InvoiceTest.php @@ -1,11 +1,13 @@ create([ + 'status' => Invoice::STATUS_SENT, + 'sent' => true, + 'paid_status' => Invoice::STATUS_UNPAID, + 'total' => $total, + 'due_amount' => $dueAmount, + 'base_total' => $total, + 'base_due_amount' => $baseDueAmount, + 'exchange_rate' => 1, + ]); +} + test('testGetInvoices', function () { $response = getJson('api/v1/invoices?page=1&type=OVERDUE&limit=20'); @@ -258,25 +277,113 @@ test('send invoice to customer', function () { Mail::assertSent(SendInvoiceMail::class); }); -test('invoice mark as paid', function () { - $invoice = Invoice::factory()->create([ - 'invoice_date' => '1988-07-18', - 'due_date' => '1988-08-18', +test('invoice status controller uses the change invoice status request', function () { + $this->assertActionUsesFormRequest( + InvoicesController::class, + 'changeStatus', + ChangeInvoiceStatusRequest::class + ); +}); + +test('cannot complete an outstanding invoice', function () { + $invoice = completionInvoice(); + + postJson("api/v1/invoices/{$invoice->id}/status", ['status' => Invoice::STATUS_COMPLETED]) + ->assertUnprocessable() + ->assertJsonPath('errors.status.0', 'invoice_must_be_settled_before_completion'); + + $invoice->refresh(); + + expect((int) $invoice->due_amount)->toBe(10000) + ->and((int) $invoice->base_due_amount)->toBe(10000) + ->and($invoice->status)->toBe(Invoice::STATUS_SENT) + ->and($invoice->paid_status)->toBe(Invoice::STATUS_UNPAID) + ->and($invoice->payments)->toHaveCount(0); +}); + +test('cannot complete a partially paid invoice', function () { + $invoice = completionInvoice(10000, 5000, 5000); + $payment = Payment::factory()->create([ + 'company_id' => $invoice->company_id, + 'customer_id' => $invoice->customer_id, + 'invoice_id' => $invoice->id, + 'amount' => 5000, ]); - $data = [ - 'status' => Invoice::STATUS_COMPLETED, - ]; + $invoice->update(['paid_status' => Invoice::STATUS_PARTIALLY_PAID]); - $response = postJson('api/v1/invoices/'.$invoice->id.'/status', $data); + postJson("api/v1/invoices/{$invoice->id}/status", ['status' => Invoice::STATUS_COMPLETED]) + ->assertUnprocessable() + ->assertJsonPath('errors.status.0', 'invoice_must_be_settled_before_completion'); - $response - ->assertOk() - ->assertJson([ - 'success' => true, - ]); + $invoice->refresh(); - $this->assertEquals(Invoice::find($invoice->id)->paid_status, Invoice::STATUS_PAID); + expect((int) $invoice->due_amount)->toBe(5000) + ->and($invoice->status)->toBe(Invoice::STATUS_SENT) + ->and($invoice->paid_status)->toBe(Invoice::STATUS_PARTIALLY_PAID) + ->and($invoice->payments->modelKeys())->toBe([$payment->id]); +}); + +test('cannot complete an invoice with an inconsistent zero stored due amount', function () { + $invoice = completionInvoice(10000, 0, 10000); + + postJson("api/v1/invoices/{$invoice->id}/status", ['status' => Invoice::STATUS_COMPLETED]) + ->assertUnprocessable() + ->assertJsonPath('errors.status.0', 'invoice_must_be_settled_before_completion'); + + $invoice->refresh(); + + expect((int) $invoice->due_amount)->toBe(0) + ->and((int) $invoice->base_due_amount)->toBe(10000) + ->and($invoice->status)->toBe(Invoice::STATUS_SENT) + ->and($invoice->payments)->toHaveCount(0); +}); + +test('completes a fully paid invoice idempotently without removing its payment', function () { + $invoice = completionInvoice(10000, 0, 0); + $payment = Payment::factory()->create([ + 'company_id' => $invoice->company_id, + 'customer_id' => $invoice->customer_id, + 'invoice_id' => $invoice->id, + 'amount' => 10000, + ]); + + postJson("api/v1/invoices/{$invoice->id}/status", ['status' => Invoice::STATUS_COMPLETED]) + ->assertOk(); + postJson("api/v1/invoices/{$invoice->id}/status", ['status' => Invoice::STATUS_COMPLETED]) + ->assertOk(); + + $invoice->refresh(); + + expect($invoice->status)->toBe(Invoice::STATUS_COMPLETED) + ->and($invoice->paid_status)->toBe(Invoice::STATUS_PAID) + ->and($invoice->overdue)->toBe(0) + ->and($invoice->payments->modelKeys())->toBe([$payment->id]); +}); + +test('completes a zero value invoice', function () { + $invoice = completionInvoice(0, 0, 0); + + postJson("api/v1/invoices/{$invoice->id}/status", ['status' => Invoice::STATUS_COMPLETED]) + ->assertOk(); + + $invoice->refresh(); + + expect($invoice->status)->toBe(Invoice::STATUS_COMPLETED) + ->and($invoice->paid_status)->toBe(Invoice::STATUS_PAID) + ->and($invoice->overdue)->toBe(0); +}); + +test('invoice status requires a supported value', function () { + $invoice = completionInvoice(); + + postJson("api/v1/invoices/{$invoice->id}/status") + ->assertUnprocessable() + ->assertJsonValidationErrors('status'); + + postJson("api/v1/invoices/{$invoice->id}/status", ['status' => 'DRAFT']) + ->assertUnprocessable() + ->assertJsonValidationErrors('status'); }); test('invoice mark as sent', function () {