Skip to content

[29.x] BE's PEPPOL "escompte" compensation - #11038

Merged
Joshua (mynjj) merged 4 commits into
releases/29.xfrom
bugs/be-peppol-escompte-compensation-29.x
Sep 11, 2026
Merged

[29.x] BE's PEPPOL "escompte" compensation#11038
Joshua (mynjj) merged 4 commits into
releases/29.xfrom
bugs/be-peppol-escompte-compensation-29.x

Conversation

@mynjj

@mynjj Joshua (mynjj) commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Backport of #11023 to releases/29.x.

In BE, there's this weird tradition that when you have a payment terms discount, the VAT is calculated with that discount as taxable base, even when the full amount (pre-discount) is shown in the total.

In our standard PEPPOL, payment discounts are added as AllowanceCharge, which is what is desired, but the totals then become less, which is not what they do in BE.

A previous attempt to fix this was to remove the AllowanceCharge for the VAT, which was incorrect, since the totals would now be mismatching. More over, the confusion of the recent move to this app from BaseApp, and branching of 28.x when the fix landed, made the fix unavailable for 28.4 (to be dealt in other PRs).

What the PR does

  • Modeled this situation as: the AllowanceCharge staying (so taxable base is with discount), but an additional AllowanceCharge was added to compensate and show the total as it's tradition (and legally mandated) in BE. This also implied adding another TaxSubtotal and a ChargeTotalAmount. The produced documents were validated against several online validators, and also match the approach that Scrada uses (from customer's examples).
  • Actually added the test app, before there was no app.json in the folder.
  • Analogous changes are in BaseApp (while that code goes away, already deprecated).

I didn't add BaseApp's test, since that code is going away anyway, in favor of the new place in E-Documents.

Fixes AB#649018

@mynjj
Joshua (mynjj) requested a review from a team September 4, 2026 07:43
@mynjj
Joshua (mynjj) requested review from a team as code owners September 4, 2026 07:43
@github-actions github-actions Bot added the AL: Apps (W1) Add-on apps for W1 label Sep 4, 2026
@github-actions github-actions Bot added this to the Version 29.1 milestone Sep 4, 2026
@github-actions github-actions Bot added the Team: Finance GitHub request for Finance area label Sep 4, 2026
@mynjj
Joshua (mynjj) force-pushed the bugs/be-peppol-escompte-compensation-29.x branch from 6b498eb to 00cfc1a Compare September 4, 2026 07:48
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 1

Recommendation: Request Changes

What this PR does

This change keeps Belgian escompte as a payment-discount allowance, adds an exempt compensating charge and tax subtotal, and adjusts PEPPOL monetary totals so the payable amount stays whole while VAT uses the discounted base. The invoice path covered by the test is coherent, and the new shared PEPPOL app hook reaches sales and service invoices and credit memos. The remaining problems are in mixed exempt-tax documents and the deprecated BaseApp credit memo path.

Problem-solution fit

Fit: Partial

The described invoice scenario is clear, and the main invoice behavior matches it. The change does not yet cover every related document path and cannot always assign the right exemption reason when an ordinary exempt subtotal is present.

Suggestions

S1 (🔴 High): Exemption reason is applied too broadly
The fallback uses only tax category E to choose the escompte reason. In a document that also has a normal exempt VAT line, the normal line and the compensation line cannot be separated here, so one of them can get the wrong exemption reason. Pass the current VAT amount line or another compensation marker into this decision.

S2 (🔴 High): BaseApp credit memos miss compensation
The BaseApp invoice XMLport adds the compensation line, but the matching credit memo XMLport still builds totals without it. Belgian credit memos with payment discounts can still export the old wrong totals and an empty reason-code element. Add the same compensation and empty-code handling to the credit memo XMLport.

Risk assessment and necessity

Risk: High. The diff changes PEPPOL tax subtotals, allowance and charge totals, and BaseApp export behavior. A mistake can create legal XML with wrong totals or the wrong exemption reason.

Necessity: The change is needed because Belgian escompte must keep VAT on the discounted base while the payable amount stays whole. The scope is mostly right, but it is incomplete.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11038 round=1 by=alexei-dobriansky at=2026-09-04T11:10:47Z lastSha=00cfc1a522adff274d27a6f44cfa7877c84896e3 reviewKey=58f8c1c6bd51bee89d6554115972c2ba786c36fafc6b530805b3a191cedc232a suggestions=S1@9bd62c83,S2@420035a2

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

⚠️ Stale Status Check Deleted

The Pull Request Build workflow run for this PR was older than 72 hours and has been deleted.

📋 Why was it deleted?

Status checks that are too old may no longer reflect the current state of the target branch. To ensure this PR is validated against the latest code and passes up-to-date checks, a fresh build is required.


🔄 How to trigger a new status check:

  1. 📤 Push a new commit to the PR branch, or
  2. 🔁 Close and reopen the PR

This will automatically trigger a new Pull Request Build workflow run.

Joshua Martínez Pineda added 2 commits September 8, 2026 13:43
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 2

Recommendation: Request Changes

What this PR does

The latest commits pass the current VAT amount line into exemption-reason selection, add tests for a genuine exempt breakdown next to the escompte compensation, and apply the same exemption-reason overload to the deprecated PEPPOL code. This fixes the mixed exempt-tax reason issue. The BaseApp credit memo export still does not add the compensation line before totals are rendered, so that path can still produce the old wrong totals.

Status of previous suggestions
ID Title Status Author response
S1 Exemption reason is applied too broadly Addressed Fixed by passing the VAT amount line into the exemption-reason decision and adding mixed exempt-line coverage.
S2 BaseApp credit memos miss compensation Not addressed No reply. The credit memo path now passes the VAT amount line for the exemption reason, but it still never adds the compensation VAT amount line before monetary totals are calculated.
New observations (commits since round 1)

None - the new commits only address the previous exemption-reason suggestion and add related tests.

Risk assessment and necessity

Risk: High. This area writes PEPPOL tax subtotals, allowance/charge totals, and payable amounts. The app path now separates ordinary exempt lines from the escompte compensation, but the deprecated credit memo path still has a financial XML regression surface.

Necessity: The change is needed because Belgian escompte must keep VAT on the discounted base while the payable amount stays whole. The scope is still incomplete until credit memos in the deprecated PEPPOL path get the same compensation handling as invoices.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11038 round=2 by=alexei-dobriansky at=2026-09-08T14:41:44Z lastSha=4ab7b0382ff257cc312d5be8f8899834655dfbcf reviewKey=990df1dd8895134250cba67082199bc2f8c392b03c9ad72be10de2da57aaa7ce suggestions=S1@9bd62c83:addressed,S2@420035a2:notaddressed parentRound=1

@mynjj
Joshua (mynjj) enabled auto-merge (squash) September 9, 2026 15:12
@mynjj
Joshua (mynjj) merged commit 3685c70 into releases/29.x Sep 11, 2026
327 of 332 checks passed
@mynjj
Joshua (mynjj) deleted the bugs/be-peppol-escompte-compensation-29.x branch September 11, 2026 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AL: Apps (W1) Add-on apps for W1 Team: Finance GitHub request for Finance area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants