Skip to content

FIX never bring a validated invoice back to draft when it must stay unalterable - #38

Open
Pichinov-Jose wants to merge 1 commit into
SplashSync:2.0from
Pichinov-Jose:fix/never-draft-unalterable-invoice
Open

Pichinov-Jose wants to merge 1 commit into
SplashSync:2.0from
Pichinov-Jose:fix/never-draft-unalterable-invoice

Conversation

@Pichinov-Jose

@Pichinov-Jose Pichinov-Jose commented Sep 13, 2026

Copy link
Copy Markdown

Bug

StatusTrait::setStatusFields() calls setStatusDraft() unconditionally:

  • Unknown / PaymentDraft"Whatever => Set Draft";
  • PaymentDue on a paid or canceled invoice → "If Already Paid => Set Draft", then validate again.

Back to draft, the connector rewrites the lines, deletes the payment and validates again under the same reference. A source reports Unknown for any status it does not map — a custom WooCommerce status (e.g. a partial refund one) is enough.

Seen in production (Dolibarr 23, Splash 2.23.3, blockedlog active): a discount typed on a completed WooCommerce order turned an invoice validated and paid at 250.00 into 137.50, same reference; its card payment was deleted and replaced by a 137.50 one, while the gateway had actually captured 250.00. The unalterable log shows 20 invoices validated again with another amount.

With the unalterable log active — mandatory in France — a validated invoice must never be edited. On an invoice carrying real payments, rewriting it silently drops recorded money. A change made in the source after invoicing must become a credit note.

Mapping every source status is not a mitigation. A second production instance maps all its custom WooCommerce statuses, which closes the Unknown path — yet its unalterable log still shows invoices rewritten by the Splash webservice user (validated at one amount by a human, then validated again at a lower amount by Splash). The other two paths stay open whatever the mapping: on-hold / failedPaymentDraft (unconditional draft), and the PaymentDue family on an already paid invoice. The guard has to live in the Dolibarr side.

Fix

Before any back-to-draft transition (both paths above), isDraftForbidden() refuses when:

The source request is ignored and logged (dol_syslog warning + Splash warning). Draft invoices and validated invoices without payments on instances without blockedlog behave exactly as before.

This guard alone does not stop the line rewrite. The connector writes invoice lines with FactureLigne::update() / insert() and update_price(), which do not check the invoice status. Once the back-to-draft is refused, the source's lines are still written into the validated invoice, with no trace in the unalterable log. That gap is closed by #40, which should go together with this PR. (Corrected on 13/09: an earlier version of this description said the rewrite would fail on Dolibarr's validated-status checks. A production test showed it does not.)

Complements #30 (never unpay an invoice with payments) and #31 (never close an uncovered invoice): same principle — recorded money and issued documents outrank the source's opinion.

Tests

Run against real invoices, in a rolled-back transaction, Splash triggers disabled:

Case Before After
Paid invoice, PaymentDraft back to draft (2 → 0) stays closed
Validated, no payment, no blockedlog, PaymentDraft draft draft (unchanged behaviour)
Validated, no payment, blockedlog active, PaymentDraft back to draft stays validated
Paid invoice, PaymentDue reopened (2 → 1) stays closed

Deployed in production on the instance where the issue was found.

🤖 Generated with Claude Code

…nalterable

setStatusFields() calls setStatusDraft() unconditionally: "Unknown" and
"PaymentDraft" set draft "whatever", and "PaymentDue" sets a paid or
canceled invoice back to draft before validating it again. The connector
then rewrites the lines, deletes the payment and validates again under
the same reference.

A source reports "Unknown" for any status it does not map (a custom
WooCommerce status such as a partial-refund one is enough). On a real
shop, a discount typed on a completed order turned an invoice validated
and paid at 250.00 into 137.50 under the same reference, and its card
payment was replaced by 137.50 — while the gateway had captured 250.00.
The unalterable log recorded twenty invoices validated again with
another amount.

With the unalterable log (blockedlog) active, mandatory in France, a
validated invoice must never be edited; on an invoice carrying real
payments, rewriting it drops recorded money. A change made in the
source after invoicing must become a credit note.

Before any back-to-draft transition, refuse when blockedlog is active or
when the invoice carries payment lines (supplier payment table handled).
The source request is ignored and logged; a line rewrite that follows
then fails on the validated status, so the sync reports it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Pichinov-Jose

Copy link
Copy Markdown
Author

Correction to the description above: I claimed that, once the back-to-draft is refused, a following line rewrite would fail on Dolibarr's validated-status checks. It does not.

The connector writes lines through FactureLigne::update() / insert() and Facture::update_price(), which never look at the invoice status. A production test on 13/09, with this guard deployed and blockedlog active, confirmed it:

  • a 10% discount was typed on two lines of a completed WooCommerce order;
  • the next sync rewrote the validated invoice from 950.00 to 900.00 under the same reference;
  • the unalterable log recorded nothing.

The payments were untouched.

#40 closes that gap: the lines of an invoice validated when loaded, and still not a draft when its lines arrive, are kept as issued. This PR and #40 should be reviewed together. The description is updated accordingly.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant