Allow gateway-less issued invoices to be marked as paid (#4442)

🐛 Allow gateway-less issued invoices to be marked as paid

Reject disabled gateways in validateAdminMarkAsPaidRequest, harden the
Mark as Paid modal for the no-active-gateways case, and point locked
invoices at the payment path instead of the Manage tab.

Fixes #4409
This commit is contained in:
Adam Daley authored and GitHub committed 2026-10-01 20:01:20 +01:00
1 parent 9ec1a84b36
commit 3e3b778cb4
5 files changed
+144 -6

No files matched your search

+3 -1
View File
@@ -111,9 +111,11 @@ class Admin extends \FOSSBilling\Api\AbstractApi
/**
* Sets invoice status to paid. This method differs from invoice update method
* in a way that it sends notification to Events system, so emails are sent.
* The gateway must be enabled; when the invoice has no gateway, gateway_id
* is required and is saved on the invoice, including on locked issued invoices.
*
* @optional bool $execute - execute related tasks on invoice items. Default false.
* @optional int $gateway_id - Payment gateway to associate with the invoice
* @optional int $gateway_id - Payment gateway to associate with the invoice. Required when the invoice has no gateway.
* @optional string $transactionId - Custom transaction ID to use when the selected gateway is Custom
* @optional string $paid_at - payment date to record instead of now, e.g. "2026-09-01 14:00:00"
*
+5 -2
View File
@@ -1144,7 +1144,7 @@ class Service implements InjectionAwareInterface
$this->di['em']->flush();
}
if ($payGateway->getGateway() === 'Custom' && $payGateway->isEnabled()) {
if ($payGateway->getGateway() === 'Custom') {
$actuallyPaid = null;
$paid = $this->di['em']->wrapInTransaction(function () use ($invoice, $payGateway, $transactionId, $paidAt, &$actuallyPaid): bool {
// Re-validate under the invoice lock: the invoice may have
@@ -1251,7 +1251,10 @@ class Service implements InjectionAwareInterface
if ($payGateway === null) {
throw new InformationException('Payment gateway not found');
}
if ($payGateway->getGateway() === 'Custom' && $payGateway->isEnabled()) {
if (!$payGateway->isEnabled()) {
throw new InformationException('Payment gateway is not enabled');
}
if ($payGateway->getGateway() === 'Custom') {
$transactionId = trim((string) ($data['transactionId'] ?? ''));
if ($transactionId === '') {
throw new InformationException('Transaction ID is required when using the Custom payment gateway.');
@@ -220,11 +220,18 @@
<div class="modal-body">
<input type="hidden" name="id" value="{{ invoice.id }}">
<input type="hidden" name="execute" value="1">
{% set pay_gateways = guest.invoice_gateways %}
{% if pay_gateways|length == 0 %}
<div class="alert alert-warning" role="alert">
{{ 'No active payment gateways are configured. Configure one before recording a payment.'|trans }}
<a href="{{ 'invoice/gateways'|url }}">{{ 'Manage payment gateways'|trans }}</a>
</div>
{% else %}
<div class="d-flex flex-column gap-3">
<div>
<label class="form-label" for="mark-paid-gateway">{{ 'Payment Method'|trans }}</label>
<select class="form-select" name="gateway_id" id="mark-paid-gateway" onchange="onMarkPaidGatewayChange(this)">
{% for gtw in guest.invoice_gateways %}
{% for gtw in pay_gateways %}
<option value="{{ gtw.id }}" data-code="{{ gtw.code }}" {% if gtw.id == invoice.gateway_id %}selected{% endif %}>{{ gtw.title }}</option>
{% endfor %}
</select>
@@ -239,10 +246,11 @@
<input class="form-control" type="datetime-local" name="paid_at" id="mark-paid-date" value="{{ 'now'|date('Y-m-d\\TH:i') }}">
</div>
</div>
{% endif %}
</div>
<div class="modal-footer">
<a href="#" class="btn btn-link link-secondary" data-bs-dismiss="modal">{{ 'Cancel'|trans }}</a>
<input type="submit" class="btn btn-primary" value="{{ 'Submit'|trans }}">
<input type="submit" class="btn btn-primary" value="{{ 'Submit'|trans }}"{% if pay_gateways|length == 0 %} disabled{% endif %}>
</div>
</form>
</div>
@@ -253,7 +261,10 @@
var code = select.options[select.selectedIndex].dataset.code;
row.style.display = code === 'Custom' ? '' : 'none';
}
onMarkPaidGatewayChange(document.getElementById('mark-paid-gateway'));
var gatewaySelect = document.getElementById('mark-paid-gateway');
if (gatewaySelect) {
onMarkPaidGatewayChange(gatewaySelect);
}
</script>
{% endif %}
@@ -860,6 +871,9 @@
<div class="col-lg-6">
<label class="form-label">{{ 'Payment Method'|trans }}</label>
{{ mf.selectbox('gateway_id', guest.invoice_gateways({ "format": "pairs" }), invoice.gateway_id, 0, 'Select Payment Method'|trans) }}
{% if invoice.issued and invoice.status == 'unpaid' %}
<div class="form-text">{{ 'Issued invoices are locked: to record an offline payment, choose the payment method in Mark as Paid instead.'|trans }}</div>
{% endif %}
</div>
<div class="col-lg-6">
<label class="form-label" for="created_at">{{ 'Date Created'|trans }}</label>
@@ -303,6 +303,60 @@ test('canceled invoices refuse payment and issued invoices lock their identity',
}
});
test('a gateway-less issued invoice is payable via mark as paid without unlocking edits (GH-4409)', function (): void {
Tests\Helpers\ApiClient::resetCookies();
try {
['id' => $clientId] = refPathCreateClient();
$draft = Tests\Helpers\ApiClient::request('admin/invoice/prepare', [
'client_id' => $clientId,
'items' => [['title' => 'E2E service', 'price' => 30, 'quantity' => 1]],
]);
assertApiSuccess($draft);
assertApiResultIsInt($draft);
$invoiceId = (int) $draft->getResult();
$issued = Tests\Helpers\ApiClient::request('admin/invoice/issue', ['id' => $invoiceId]);
assertApiSuccess($issued);
$invoice = Tests\Helpers\ApiClient::request('admin/invoice/get', ['id' => $invoiceId]);
assertApiSuccess($invoice);
expect($invoice->getResult()['issued'])->toBeTrue();
expect($invoice->getResult()['editable'])->toBeFalse();
// Marking paid without a gateway is still refused.
$missingGateway = Tests\Helpers\ApiClient::request('admin/invoice/mark_as_paid', [
'id' => $invoiceId,
]);
expect($missingGateway->wasSuccessful())->toBeFalse();
expect($missingGateway->getErrorMessage())->toContain('Payment gateway is required');
// The Manage tab stays locked: a standalone gateway change is refused.
$updateBlocked = Tests\Helpers\ApiClient::request('admin/invoice/update', [
'id' => $invoiceId,
'gateway_id' => refPathCustomGatewayId(),
]);
expect($updateBlocked->wasSuccessful())->toBeFalse();
expect($updateBlocked->getErrorMessage())->toContain('can no longer be edited');
// The payment path carries the gateway instead: locked but payable.
$paid = Tests\Helpers\ApiClient::request('admin/invoice/mark_as_paid', [
'id' => $invoiceId,
'gateway_id' => refPathCustomGatewayId(),
'transactionId' => 'txn' . uniqid(),
]);
assertApiSuccess($paid);
$paidInvoice = Tests\Helpers\ApiClient::request('admin/invoice/get', ['id' => $invoiceId]);
assertApiSuccess($paidInvoice);
expect($paidInvoice->getResult()['status'])->toBe('paid');
expect((int) $paidInvoice->getResult()['gateway_id'])->toBe(refPathCustomGatewayId());
} finally {
refPathCleanupClient();
}
});
function refPathCreateProduct(float $price): int
{
$result = Tests\Helpers\ApiClient::request('admin/product/prepare', [
@@ -1574,6 +1574,71 @@ test('markAsPaidByAdmin rejects an invalid payment date before any write', funct
->toThrow(FOSSBilling\InformationException::class, 'Invalid date format for paid_at');
});
test('validateAdminMarkAsPaidRequest requires a gateway when neither request nor invoice has one', function (): void {
$invoiceModel = createEntity(Invoice::class);
$invoiceModel->status = Invoice::STATUS_UNPAID;
$em = Mockery::mock(EntityManagerInterface::class);
$em->shouldNotReceive('getRepository');
$di = container();
$di['em'] = $em;
$service = new Service();
$service->setDi($di);
expect(fn () => $service->validateAdminMarkAsPaidRequest([], $invoiceModel))
->toThrow(FOSSBilling\InformationException::class, 'Payment gateway is required when marking an invoice as paid.');
});
test('validateAdminMarkAsPaidRequest rejects a disabled gateway', function (): void {
$gatewayModel = createEntity(PayGateway::class, [
'id' => 5,
'gateway' => 'Stripe',
'enabled' => false,
]);
$invoiceModel = createEntity(Invoice::class);
$invoiceModel->status = Invoice::STATUS_UNPAID;
$em = Mockery::mock(EntityManagerInterface::class);
$em->shouldReceive('getRepository')->with(PayGateway::class)->andReturn($gatewayRepo = Mockery::mock(PayGatewayRepository::class));
$gatewayRepo->shouldReceive('find')->once()->with(5)->andReturn($gatewayModel);
$di = container();
$di['em'] = $em;
$service = new Service();
$service->setDi($di);
expect(fn () => $service->validateAdminMarkAsPaidRequest(['gateway_id' => 5], $invoiceModel))
->toThrow(FOSSBilling\InformationException::class, 'Payment gateway is not enabled');
});
test('validateAdminMarkAsPaidRequest accepts an explicit gateway for a gateway-less issued invoice', function (): void {
$gatewayModel = createEntity(PayGateway::class, [
'id' => 7,
'gateway' => 'Stripe',
'enabled' => true,
]);
$invoiceModel = createEntity(Invoice::class);
$invoiceModel->status = Invoice::STATUS_UNPAID;
$invoiceModel->issued = true;
$em = Mockery::mock(EntityManagerInterface::class);
$em->shouldReceive('getRepository')->with(PayGateway::class)->andReturn($gatewayRepo = Mockery::mock(PayGatewayRepository::class));
$gatewayRepo->shouldReceive('find')->once()->with(7)->andReturn($gatewayModel);
$di = container();
$di['em'] = $em;
$service = new Service();
$service->setDi($di);
expect($service->validateAdminMarkAsPaidRequest(['gateway_id' => 7], $invoiceModel))->toBe($gatewayModel);
});
test('admin mark as paid creates no transaction when the invoice is canceled under lock', function (): void {
$serviceMock = Mockery::mock(Service::class)->makePartial()->shouldAllowMockingProtectedMethods();
$serviceMock->shouldNotReceive('markAsPaid', 'getTotalWithTax');