Skip to content

[Master]-]VAT Settlement G/L Entries Do Not Populate Source Currency Amount and Source VAT Currency Amount After Running Calculate and Post VAT Settlement - #11186

Open
Fixes4BC (neeleshsinghal) wants to merge 27 commits into
mainfrom
bugs/Bug-648151-VAT-Settle-GL-Entries-populate-Source-Currency-Values11
Open

[Master]-]VAT Settlement G/L Entries Do Not Populate Source Currency Amount and Source VAT Currency Amount After Running Calculate and Post VAT Settlement#11186
Fixes4BC (neeleshsinghal) wants to merge 27 commits into
mainfrom
bugs/Bug-648151-VAT-Settle-GL-Entries-populate-Source-Currency-Values11

Conversation

@neeleshsinghal

@neeleshsinghal Fixes4BC (neeleshsinghal) commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fixes AB#648151

@neeleshsinghal
Fixes4BC (neeleshsinghal) requested a review from a team September 8, 2026 05:21
@neeleshsinghal
Fixes4BC (neeleshsinghal) requested a review from a team as a code owner September 8, 2026 05:21
@github-actions github-actions Bot added the Team: Finance GitHub request for Finance area label Sep 8, 2026
@github-actions github-actions Bot added this to the Version 30.0 milestone Sep 8, 2026
Comment thread src/Layers/AU/BaseApp/Finance/VAT/Reporting/CalcandPostVATSettlement.Report.al Outdated
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 1

Recommendation: Request Changes

What this PR does

The change updates the VAT settlement reports so source currency amounts use LCY values when Additional Reporting Currency is blank, and ACY values when it is set. This matches the reported blank-source-currency problem for the normal settlement paths, but the NO proportional deduction paths still set source amounts from values that do not match the final posted amount.

Problem-solution fit

Fit: Partial

The reported scenario is clear and the main approach is needed. The solution is incomplete because one localization can still post source currency amounts that do not match the G/L entry amount in proportional deduction VAT flows.

Suggestions

S1 (🔴 High): Use the posted NO settlement amount
In the NO report, the settlement line posts VATAmount + VATAmountOfPropDedSettled, but the new helper receives only VATAmount. When Additional Reporting Currency is blank, this can leave Source Currency Amount lower than the posted amount for proportional deduction VAT. Pass the same amount that is posted.

S2 (🔴 High): Update NO source amount after overwrite
This helper stores the source amount before the caller changes GenJnlLine2.Amount to VATAmountOfPropDed. When Additional Reporting Currency is blank, the posted line can keep the old source amount. Set the source amount from the final line amount before posting.

S3 (🔴 High): Add VAT settlement posting coverage
Please add a regression test that posts VAT settlement with Additional Reporting Currency blank and checks the source currency fields on the created G/L entries. This is a financial posting path, so the regression can otherwise return and persist wrong ledger data.

S4 (🟠 Moderate): Remove the extra AU BOM
The AU file now starts with two UTF-8 BOM sequences. Remove the extra hidden character so source tools do not see an unexpected character before the first comment.

Risk assessment and necessity

Risk: This is a high-risk financial posting area because report 20 creates G/L entries and persists source currency amounts. The NO proportional deduction paths can still write source currency amounts that differ from the final posted amounts, and the AU header change adds a small tooling risk.

Necessity: The change is needed because VAT settlement entries should keep source currency values when Additional Reporting Currency is blank. The scope is right, but the NO edge cases and missing posting coverage should be fixed before merge.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11186 round=1 by=alexei-dobriansky at=2026-09-08T07:20:41Z lastSha=cfbc7e8e3f5113b65581f5a8637b8603a5192c02 reviewKey=1e2111868368b2181c5a5396b0486beada9de7617e6a2fd3b8ad6a67a63e4fb2 suggestions=S1@a2b5b091,S2@856aab21,S3@4762d6d3,S4@61c2a50d

Comment thread src/Layers/IT/BaseApp/Finance/VAT/Reporting/CalcandPostVATSettlement.Report.al Outdated
Comment thread src/Layers/NO/BaseApp/Finance/VAT/Reporting/CalcandPostVATSettlement.Report.al Outdated
Comment thread src/Layers/IT/BaseApp/Finance/VAT/Reporting/CalcandPostVATSettlement.Report.al Outdated
…lement.Report.al

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Comment thread src/Layers/ES/Tests/ERM-Application/ERMVATSettlementwithApply.Codeunit.al Outdated
Comment thread src/Layers/RU/Tests/ERM-Application/ERMVATSettlementwithApply.Codeunit.al Outdated
Comment thread src/Layers/US/Tests/ERM-Application/ERMVATSettlementwithApply.Codeunit.al Outdated
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 5

Recommendation: Request Changes

What this PR does

The new commit removes the key-unsafe Source Code filter from the RU regression test helper. It does not change the posting logic. The main LCY/ACY source amount fallback is still the right direction for normal VAT settlement entries, but one NO proportional deduction path can still post a source amount calculated from an old line amount. BE and US validation also still fail.

Status of previous suggestions
ID Title Status Author response
S1 Use the posted NO settlement amount Addressed The settlement-account line still passes the final VATAmount + VATAmountOfPropDedSettled amount to the source amount helper.
S2 Update NO source amount after overwrite Not addressed The reverse charge proportional deduction line still calls CreateGenJnlLine first and then overwrites GenJnlLine2.Amount, so Source Currency Amount can stay based on the earlier amount.
S3 Add VAT settlement posting coverage Not addressed The tests cover the general flow, but they do not cover the NO proportional deduction branch and they can pass without proving that the settlement source amounts were populated.
S4 Remove the extra AU BOM Addressed The AU file remains normalized.
S5 Pass the BE settlement document number Not addressed The BE test now passes DocNo, but the new test does not declare that variable. The BE build reports AL0118.
S6 Avoid the G/L Entry Source Code filter Not addressed The RU helper was fixed in this round, but the US helper still filters G/L Entry by Source Code. The US build reports AA0210.
New observations (commits since round 4)

None - the new commit only partially addresses S6 by fixing RU. US still has the same validation issue.

Risk assessment and necessity

Risk: This is still a high-risk financial posting path because report 20 creates G/L entries and stores source currency fields. The remaining NO path can persist a source amount that does not match the final posted amount, and the BE/US builds are still failing.

Necessity: The change is needed because VAT settlement entries should keep source currency values when Additional Reporting Currency is blank. The scope is appropriate, but the remaining posting mismatch and validation failures should be fixed before merge.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11186 round=5 by=alexei-dobriansky at=2026-09-09T19:19:45Z lastSha=585f1286d6054de87852619ea97633606d4163f9 reviewKey=76e415f484b0dd74614b3e716e6fec7fbdf743263eb6267c438a5eaca59066f9 suggestions=S1@a2b5b091:addressed,S2@856aab21:notaddressed,S3@4762d6d3:notaddressed,S4@61c2a50d:addressed,S5@c1bf37f9:notaddressed,S6@e60f04dc:notaddressed parentRound=4

Comment thread src/Layers/US/Tests/ERM-Application/ERMVATSettlementwithApply.Codeunit.al Outdated
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 6

Recommendation: Request Changes

What this PR does

The new commits fix two previous test validation problems: the BE test now declares and uses the settlement document number, and the US helper no longer filters G/L Entry by Source Code. The posting logic did not change. The main LCY fallback still matches the reported VAT settlement problem, but the NO reverse charge proportional deduction branch can still set Source Currency Amount before it overwrites the posted amount.

Status of previous suggestions
ID Title Status Author response
S1 Use the posted NO settlement amount Addressed Already fixed before this round.
S2 Update NO source amount after overwrite Not addressed The NO branch still calls CreateGenJnlLine and then assigns GenJnlLine2.Amount := VATAmountOfPropDed, leaving Source Currency Amount based on the earlier amount.
S3 Add VAT settlement posting coverage Not addressed The tests still do not cover the NO proportional deduction branch, and the helper can pass without proving that settlement source amounts were populated.
S4 Remove the extra AU BOM Addressed Already fixed before this round.
S5 Pass the BE settlement document number Addressed The BE test now declares DocNo and passes it to the verifier.
S6 Avoid the G/L Entry Source Code filter Addressed The US helper no longer filters by Source Code.
New observations (commits since round 5)

None - the new commits only address previous validation issues. The remaining blockers are carried forward from the earlier review.

Risk assessment and necessity

Risk: This is still a high-risk financial posting path because report 20 creates G/L entries and writes source currency fields. The remaining NO proportional deduction path can persist a source amount that does not match the final posted amount. No public event or API surface is changed.

Necessity: The change is needed because VAT settlement entries should keep source currency values when Additional Reporting Currency is blank. The scope is still appropriate, but the remaining posting mismatch and weak test coverage should be fixed before merge.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11186 round=6 by=alexei-dobriansky at=2026-09-10T07:11:24Z lastSha=efacbc3eff857ac5ac72e74f16a40834de996f80 reviewKey=4986e379aa031f55090cda4288aa59b61977fe598d7b1cefc3cd0e3b80248b52 suggestions=S1@a2b5b091:addressed,S2@856aab21:notaddressed,S3@4762d6d3:notaddressed,S4@61c2a50d:addressed,S5@c1bf37f9:addressed,S6@e60f04dc:addressed parentRound=5

@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟠\ High\ Severity\ —\ Agent}$

In the BE report (src/Layers/BE/BaseApp/Finance/VAT/Reporting/CalcandPostVATSettlement.Report.al), BalanceReverseChargeVAT() creates GenJnlLine2 via CreateGenJnlLine(), which correctly routes 'Source Currency Amount' through the new SourceCurrencyAmount(AmountLCY, AmountACY) fallback (so it uses LCY when GLSetup."Additional Reporting Currency" is blank). Immediately after, the code does GenJnlLine2."Source Currency Amount" += VATEntry."Non Ded. Source Curr. VAT Amt."; (around line 960), adding a VAT Entry field that is itself an additional-currency-derived amount, without going through the same blank-ACY fallback logic. This is the exact bug pattern this PR fixes elsewhere in the same file (and across AU/ES/IT/NO/W1): when no Additional Reporting Currency is configured, this line will corrupt the just-fixed LCY-based 'Source Currency Amount' by adding an ACY-only amount, defeating the fix for reverse-charge VAT settlements with non-deductible VAT. The equivalent 'VATAmountAddCurr -= VATEntry."Non Ded. Source Curr. VAT Amt.";' running total a few lines below has the same issue before it is later passed through SourceCurrencyAmount. This appears to be a call site the PR's fix missed.

Line mapping was unavailable, so this was posted as an issue comment.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.41.6

@neeleshsinghal

Copy link
Copy Markdown
Contributor Author

Good Sense Reviewer - Round 6

Recommendation: Request Changes

What this PR does
The new commits fix two previous test validation problems: the BE test now declares and uses the settlement document number, and the US helper no longer filters G/L Entry by Source Code. The posting logic did not change. The main LCY fallback still matches the reported VAT settlement problem, but the NO reverse charge proportional deduction branch can still set Source Currency Amount before it overwrites the posted amount.

Status of previous suggestions
ID Title Status Author response
S1 Use the posted NO settlement amount Addressed Already fixed before this round.
S2 Update NO source amount after overwrite Not addressed The NO branch still calls CreateGenJnlLine and then assigns GenJnlLine2.Amount := VATAmountOfPropDed, leaving Source Currency Amount based on the earlier amount.
S3 Add VAT settlement posting coverage Not addressed The tests still do not cover the NO proportional deduction branch, and the helper can pass without proving that settlement source amounts were populated.
S4 Remove the extra AU BOM Addressed Already fixed before this round.
S5 Pass the BE settlement document number Addressed The BE test now declares DocNo and passes it to the verifier.
S6 Avoid the G/L Entry Source Code filter Addressed The US helper no longer filters by Source Code.
New observations (commits since round 5)
None - the new commits only address previous validation issues. The remaining blockers are carried forward from the earlier review.

Risk assessment and necessity
Risk: This is still a high-risk financial posting path because report 20 creates G/L entries and writes source currency fields. The remaining NO proportional deduction path can persist a source amount that does not match the final posted amount. No public event or API surface is changed.

Necessity: The change is needed because VAT settlement entries should keep source currency values when Additional Reporting Currency is blank. The scope is still appropriate, but the remaining posting mismatch and weak test coverage should be fixed before merge.

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11186 round=6 by=alexei-dobriansky at=2026-09-10T07:11:24Z lastSha=efacbc3eff857ac5ac72e74f16a40834de996f80 reviewKey=4986e379aa031f55090cda4288aa59b61977fe598d7b1cefc3cd0e3b80248b52 suggestions=S1@a2b5b091:addressed,S2@856aab21:notaddressed,S3@4762d6d3:notaddressed,S4@61c2a50d:addressed,S5@c1bf37f9:addressed,S6@e60f04dc:addressed parentRound=5

No simi9lar NO related test found

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 7

Recommendation: Request Changes

What this PR does

The latest code continues the VAT settlement source-currency fix across the localized settlement reports and improves several test helpers. The runtime direction is right, but the remaining test verifier can still pass without proving the settlement entries contain the required source-currency values.

Status of previous suggestions
ID Title Status Author response
S1 Use the posted NO settlement amount Addressed The NO amount now includes the proportional deduction amount where the settlement line amount is set.
S2 Update NO source amount after overwrite Addressed The NO source amount now uses the same proportional-deduction amount path as the posted amount.
S3 Add VAT settlement posting coverage Not addressed The verifier still only checks rows where Source Currency VAT Amount is already nonzero and does not assert the Source Currency Amount on each settlement entry.
S4 Remove the extra AU BOM Addressed The duplicate BOM in the AU file was corrected.
S5 Pass the BE settlement document number Addressed The BE test now verifies using the stored document number.
S6 Avoid the G/L Entry Source Code filter Addressed The helper no longer depends on the broad source-code filter alone.
New observations (commits since round 6)

None - the remaining blocker is the still-open test coverage issue from the previous round.

Risk assessment and necessity

Risk: High. VAT settlement writes G/L entries; a weak verifier can allow missing source-currency values to reach posted settlement entries unnoticed.

Necessity: The fix is needed so settlement entries keep correct source-currency values when no additional reporting currency is configured, but the test must prove the posted values directly.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11186 round=7 by=alexei-dobriansky at=2026-09-11T11:22:20.345Z lastSha=cff158988f15e8b5a7a23cef2b2019222bb9de1d reviewKey=d5df5f4ebc6552a7b839b931d0bc0e3737c60d746a2e8b25db82ecb58520b0a4 suggestions=S1@a2b5b091:addressed,S2@856aab21:addressed,S3@4762d6d3:notaddressed,S4@61c2a50d:addressed,S5@c1bf37f9:addressed,S6@e60f04dc:addressed parentRound=6

@neeleshsinghal

Copy link
Copy Markdown
Contributor Author

Good Sense Reviewer - Round 7

Recommendation: Request Changes

What this PR does
The latest code continues the VAT settlement source-currency fix across the localized settlement reports and improves several test helpers. The runtime direction is right, but the remaining test verifier can still pass without proving the settlement entries contain the required source-currency values.

Status of previous suggestions
ID Title Status Author response
S1 Use the posted NO settlement amount Addressed The NO amount now includes the proportional deduction amount where the settlement line amount is set.
S2 Update NO source amount after overwrite Addressed The NO source amount now uses the same proportional-deduction amount path as the posted amount.
S3 Add VAT settlement posting coverage Not addressed The verifier still only checks rows where Source Currency VAT Amount is already nonzero and does not assert the Source Currency Amount on each settlement entry.
S4 Remove the extra AU BOM Addressed The duplicate BOM in the AU file was corrected.
S5 Pass the BE settlement document number Addressed The BE test now verifies using the stored document number.
S6 Avoid the G/L Entry Source Code filter Addressed The helper no longer depends on the broad source-code filter alone.
New observations (commits since round 6)
None - the remaining blocker is the still-open test coverage issue from the previous round.

Risk assessment and necessity
Risk: High. VAT settlement writes G/L entries; a weak verifier can allow missing source-currency values to reach posted settlement entries unnoticed.

Necessity: The fix is needed so settlement entries keep correct source-currency values when no additional reporting currency is configured, but the test must prove the posted values directly.

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11186 round=7 by=alexei-dobriansky at=2026-09-11T11:22:20.345Z lastSha=cff158988f15e8b5a7a23cef2b2019222bb9de1d reviewKey=d5df5f4ebc6552a7b839b931d0bc0e3737c60d746a2e8b25db82ecb58520b0a4 suggestions=S1@a2b5b091:addressed,S2@856aab21:addressed,S3@4762d6d3:notaddressed,S4@61c2a50d:addressed,S5@c1bf37f9:addressed,S6@e60f04dc:addressed parentRound=6

No NO related test found other cases covered

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

Labels

Team: Finance GitHub request for Finance area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants