FIX never add a payment to an invoice that is already settled - #32
Open
Pichinov-Jose wants to merge 1 commit into
Open
Pichinov-Jose wants to merge 1 commit into
Pichinov-Jose wants to merge 1 commit into
Conversation
Source payment lines are paired with local payments by position, so a payment that is not linked to this invoice is invisible to the pairing. identifyExistingPayment() is meant to recover those, but it gives up in exactly the cases that produce them: getSimilarPayment() requires a non empty "number", which only gateway transactions carry, and the candidate must already carry invoice totals — so a payment linked to no invoice, the very case it should catch, is rejected. The line then falls through to createPaymentItem() and the invoice is settled twice. Compare against what the invoice already carries instead of enumerating the causes. Overpaying stays possible — a settlement in vouchers or cash rarely falls on the exact cent, and nothing covers the invoice when it arrives — what is refused is the second full settlement of an invoice already closed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
An invoice can be settled twice by a single sync.
setPaymentLineData()pairs source payment lines with local payments by position, taking them from$this->payments— whichloadPayments()fills from the payments linked to this invoice. A payment that exists in the database but is linked to no invoice is therefore invisible to the pairing, and its source line has nothing to pair with.identifyExistingPayment()is the safety net for exactly that, and it gives up in exactly the cases that produce it:getSimilarPayment()bails out onempty($lineData["number"]). Only gateway transactions carry a number. Vouchers, cheques, transfers and cash have none, so no lookup is even attempted.getPaymentInvoicesTotals()to be non-empty — so a payment carrying no invoice link, the very case it is meant to recover, is rejected.The line falls through to
createPaymentItem(), and the invoice ends up with two payments for one movement. The same happens whenever the announced payment method no longer matches the recorded one, sincesearchForSimilarPayment()matches on method + number + date + amount as an exact tuple.Observed in production on one shop: 12 duplicate payments, 1 514.76 € of bank entries with no money behind them, in two episodes — 8 left unlinked by a resync in 2025, and 4 attached to their invoice, which then showed twice its amount as settled.
Note that Dolibarr core offers no protection here either:
Paiement::create()computes$remaintopayonly to decide whether to close the invoice, and never checks the amount. The interactive form warns (PaymentHigherThanReminderToPay) but it is a confirmation dialog, not a refusal — so nothing stops a programmatic overpayment.Fix
Rather than enumerate the causes, compare against what the invoice already carries: payments, credit notes and deposits covering the total leave nothing to receive.
This deliberately does not forbid overpaying. A settlement in holiday vouchers or in cash rarely falls on the exact cent — 130.00 € of vouchers against a 126.18 € invoice is a real, legitimate payment, and it is accepted because nothing covers the invoice yet when it arrives. What is refused is the second full settlement of an invoice already closed.
Mirrors the philosophy of #25 and #30: local, recorded money outranks the source's view of it.
🤖 Generated with Claude Code