[master]-[ES][Cartera Bills] Applying a foreign-currency payment to a bill that leads to gain-loss entry cannot be reversed

[MSNehaNawal]

Bug 642151: [master][All-E][ES][Cartera Bills] Applying a foreign-currency payment to a bill that leads to gain-loss entry cannot be reversed

Fixes AB#642151

[github-actions]

Copilot PR Review

Iteration 3 · Outcome: completed

Knowledge source: https://github.com/microsoft/BCQuality@809af9708e265f110ced6752fd8f2e40f1efc820

Findings by domain

Findings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).

Domain	Findings	Knowledge-backed	Agent	Inline	Fallback
Other	1	1	0	1	0

Totals: 1 knowledge-backed · 0 agent findings.

Orchestrator pre-filter (2 file(s) excluded)

• layer-disabled (knowledge) : 2 file(s)

_{Findings produced by the Copilot CLI agent against BCQuality at 809af9708e265f110ced6752fd8f2e40f1efc820. Reply 👎 on any inline comment to flag false positives.}

[alexei-dobriansky]

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

This PR changes ES Gen. Jnl.-Post Line so a Cartera bill receivable G/L entry can reuse the saved entry number after detailed entries, such as realized currency gain/loss entries, have moved NextEntryNo. It also adds a test that posts an FCY Cartera invoice, applies a payment after an exchange-rate change, unapplies it, and reverses the transaction.

The main idea matches the root cause: the bill receivable entry must use the reserved G/L entry number so reversal can find the expected link. But the implementation is not complete. NeedsSaveEntryNoFix disables the old NextEntryNo decrement for every Cartera document amount, while only the DocAmountLCY branch actually consumes SaveEntryNo. The other Cartera amount branches can still leave the same entry-number mismatch.

Suggestions

S1 - Consume saved entry number in all branches
UpdateReceivableDoc can set DiscDocAmountLCY, CollDocAmountLCY, RejDocAmountLCY, or factoring amounts while DocAmountLCY stays zero. In that case NeedsSaveEntryNoFix skips the old NextEntryNo decrement, but no branch calls UpdateGLEntryNo. Consume SaveEntryNo in the first posted Cartera document entry, or keep the old decrement when no branch will use it.

S2 - Strengthen the reversal regression test
The new test is useful, but it should prove that the realized gain/loss detailed ledger entries exist and are linked before unapply and reversal. Please also cover the non-DocAmountLCY Cartera path if this helper is meant to handle posted bill group, rejection, discount, or factoring amounts. This is sensitive posting code, so the test should fail on the entry-number bug itself.

Risk assessment and necessity

Risk: This is high-risk financial posting code. A wrong SaveEntryNo decision can leave G/L entries out of sequence, break the link used by reversal/unapply, or affect posted/closed Cartera document flows. I also checked the related ES Cartera extraction PR #9002: the current extraction branch still has this logic in ES GenJnlPostLine, so this target is not obviously wrong, but merge order can require a rebase or port.

Necessity: The bug is important. The linked ADO bug is a live-site, high-risk ES Cartera scenario where an FCY payment with realized exchange gain/loss cannot be reversed. A targeted fix and a regression test are warranted, but the current implementation still leaves a reachable entry-number mismatch.

---

[AI-PR-REVIEW] version=1 system=github pr=9349 round=1 by=alexei-dobriansky at=2026-07-14T05:48:55.771Z lastSha=8be5df291828ab81d583991ac939144375f50c33 suggestions=S1,S2

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 2}}$

In the rewritten PostReceivableDocs, the var block declares GLEntry: Record "G/L Entry" after the primitive DocAmountAddCurr and GLAccNo declarations.

CodeCop AA0021 requires complex types (Record) to appear before primitive types (Decimal, Code, Boolean) in a var block.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

        GLEntry: Record "G/L Entry";
        DocAmountAddCurr: Decimal;
        GLAccNo: Code[20];
        NeedsSaveEntryNoFix: Boolean;

Knowledge:

• microsoft/knowledge/style/variable-declaration-order-by-type.md

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

[alexei-dobriansky]

Agentic PR Review - Round 2

Recommendation: Accept

What this PR does

The new commit narrows NeedsSaveEntryNoFix so the saved G/L entry number is reused only when the DocAmountLCY branch can consume it. For the other Cartera amount branches, the code now follows the same entry-number path as before the PR instead of skipping that path without using SaveEntryNo.

This addresses the round-1 correctness concern. The DocAmountLCY path still consumes SaveEntryNo through UpdateGLEntryNo, and the non-DocAmountLCY paths no longer enter the special saved-entry-number branch. The regression test was also strengthened to prove realized gain/loss detailed ledger entries exist, have non-zero LCY amounts, are linked to customer ledger entries, and that all G/L entries in the payment transaction are reversed.

Status of previous suggestions

ID	Title	Status	Author response
S1	Consume saved entry number in all branches	Addressed	Fixed in a4c6c2056a by restricting the saved-entry-number fix to the DocAmountLCY branch and keeping the old path for the other Cartera amounts.
S2	Strengthen the reversal regression test	Addressed	Fixed in a4c6c2056a by adding checks for realized gain/loss detailed ledger entries and full reversal of the payment transaction's G/L entries.

New observations (commits since round 1)

None - changes only addressed prior suggestions.

Risk assessment and necessity

Risk: This remains high-risk financial posting code because wrong G/L entry numbers can break unapply and reversal. The round-2 change is narrow: it limits the new saved-entry-number behavior to the branch that actually uses SaveEntryNo, and it leaves discount, collection, rejection, and factoring postings on the existing path.

Necessity: The linked ADO bug is an active high-risk live-site ES Cartera scenario. A targeted posting fix and a regression test are needed, and the round-2 changes now match that scope.

---

[AI-PR-REVIEW] version=1 system=github pr=9349 round=2 by=alexei-dobriansky at=2026-07-14T11:06:28.226Z lastSha=a4c6c2056a6479c1ed84139ab1bdb893650864d3 suggestions=S1:addressed,S2:addressed parentRound=1