[Master] Expense Report posting FCY rounding mismatch "Expense Report Posting Test" fail in Spain only (Source Currency Amount 649.97 vs 650 - #9886
Conversation
…Bug-644891-Main-ExpenseAgentExpenseAgentESExpenseReportpostingounding
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Agentic PR Review - Round 1Recommendation: Accept with SuggestionsWhat this PR doesThis PR fixes the ES employee payables G/L entry for FCY employee and expense postings. When the journal source currency matches the journal currency and the LCY amount matches the payable amount by absolute value, it keeps the original FCY amount instead of converting LCY back to FCY. The change is narrow and keeps the old SuggestionsS1 - Make the new employee test deterministic Risk assessment and necessityRisk: This is financial posting logic in the ES layer, so a wrong value can affect G/L Entry source currency reporting for employee payables. The code change is limited to Necessity: The change is needed because the active bug keeps four ES Expense Report FCY tests disabled. Re-enabling those tests is the right scope for this fix, and the added employee payables test gives a smaller regression path once it is made deterministic.
|
|
I have fix the Suggestion. Please check |
Agentic PR Review - Round 2Recommendation: AcceptWhat this PR doesSince round 1, this PR makes the new ES employee FCY regression test deterministic. It now uses a fixed amount of 650 with the 7 FCY = 1 LCY exchange rate, and it asserts the signed This addresses the previous test concern. The test now always exercises the LCY-to-FCY rounding mismatch that the posting fix is meant to avoid, and it no longer hides a wrong sign with Status of previous suggestions
New observations (commits since round 1)None - changes only addressed prior suggestions. Risk assessment and necessityRisk: The round-2 change only updates test code. The product-code risk remains limited to ES employee payables posting when source currency equals the journal currency and the payable LCY amount matches the journal LCY amount by absolute value. Necessity: The deterministic test is needed because this is sensitive financial posting logic. The PR also re-enables the disabled ES Expense Report FCY tests, so the original failing scenario is covered again.
|
There was a problem hiding this comment.
Pull request overview
Fixes an ES-layer posting issue where employee payables balancing G/L entries could get a rounded/reconverted Source Currency Amount (FCY) instead of preserving the original FCY amount from the journal line, which caused Spain-only expense report posting test failures.
Changes:
- Adjusts ES
Gen. Jnl.-Post Lineemployee payables G/L entry creation to preserve the original FCY amount when the LCY amounts match by magnitude. - Adds an ES regression test covering FCY employee payment posting and validating the payables G/L entry Source Currency Amount.
- Re-enables previously disabled Expense Agent “Expense Report Posting Test” cases by removing them from the disabled-tests list.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Layers/ES/Tests/ERM/ERMSourceCurrency.Codeunit.al | Adds a regression test for FCY employee payment payables G/L entry Source Currency Amount correctness. |
| src/Layers/ES/BaseApp/Finance/GeneralLedger/Posting/GenJnlPostLine.Codeunit.al | Updates employee payables balancing G/L entry SCY amount derivation to avoid LCY→FCY reconversion rounding mismatch. |
| src/DisabledTests/Expense_Agent_Tests/Expense_Agent_Tests.DisabledTest.json | Removes disabled entries for Expense Report Posting tests so they run again. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Agentic PR Review - Round 3Recommendation: AcceptWhat this PR doesSince round 2, this PR adds a The core fix — using Status of previous suggestions
New observations (commits since round 2)None — the single commit only adds Risk assessment and necessityRisk: The round-3 change is test-only. No product logic was modified. The Necessity: The
|
0585bdc
|
Pull request was closed
Anders (AndersLarsenMicrosoft)
added
ExpenseManagement
and removed
ExpenseManagement
labels
Agentic PR Review - Round 4Recommendation: AcceptWhat this PR doesSince round 3, the PR head changed by merging current The bug evidence describes an ES FCY rounding failure where expense report posting wrote 649.97 / -669.98 instead of 650 / -670. The current diff still keeps the existing LCY-to-FCY calculation for other employee posting cases, re-enables the four disabled expense report tests, and adds a deterministic employee FCY payment regression test in the ES ERM source currency test app. I did not find a new correctness issue in the current net diff. Status of previous suggestions
New observations (commits since round 3)None - the current update is a merge from Risk assessment and necessityRisk: This is ES financial posting logic. A wrong value can affect G/L Entry source currency reporting for employee payables. The code change remains narrow, does not change public signatures or events, and falls back to the existing calculation when the exact source-currency and LCY-amount conditions are not met. Necessity: The change is needed to fix and re-enable the ES expense report FCY tests that were disabled for this bug. The deterministic employee posting test covers the smaller root cause directly, and the main merge keeps the PR aligned with the current split test app structure.
|
|
Bug 644891: [Master][ALL-E] [Expense Agent] Expense Agent] [ES] Expense Report posting FCY rounding mismatch "Expense Report Posting Test" fail in Spain only (Source Currency Amount 649.97 vs 650
Fixes AB#644891
Issue:- Expense Agent] [ES] Expense Report posting FCY rounding mismatch "Expense Report Posting Test" fail in Spain only (Source Currency Amount 649.97 vs 650
Cause :- The employee payables balancing G/L entry is created in PostDtldEmplLedgEntries (ES layer GenJnlPostLine, line ~5967) like this:
And CalcAmountSrcCurr (line ~3258) computes:
So for the employee entry, ES unconditionally derives Source Currency Amount from the LCY amount (PayableAccAmtLCY) — it never reads GenJournalLine."Source Currency Amount". (Note: the vendor path does have a System-Created branch that uses the journal line's value, but the employee path does not.)
Solution:- : the employee payable balancing entry has PayableAccAmtLCY as positive (+650), while the journal line's "Amount (LCY)" is negative (-650). My old condition "Amount (LCY)" = PayableAccAmtLCY was therefore never true, so the code always fell back to CalcAmountSrcCurr (the LCY→FCY reconversion that yields 649.97).
The updated block in PostDtldEmplLedgEntries:
if (GenJnlLine."Source Currency Code" <> '') and (GenJnlLine."Source Currency Amount" <> 0) and (GenJnlLine."Amount (LCY)" <> 0) and (Abs(GenJnlLine."Amount (LCY)") = Abs(PayableAccAmtLCY)) then begin
AmountSrcCurr := Abs(GenJnlLine."Source Currency Amount");
if PayableAccAmtLCY < 0 then
AmountSrcCurr := -AmountSrcCurr;
end else
AmountSrcCurr := CalcAmountSrcCurr(GenJnlLine, PayableAccAmtLCY);