[Master]-ACY amount on Value Entries is recalculated from LCY when the document currency equals the Additional Reporting Currency, causing a mismatch with G/L Entries- - #10244
Conversation
…e document currency equals the Additional Reporting Currency, causing a mismatch with G/L Entries - v2 APAC fixes - Remove redundant variable clearances (OvhdCostACY, PurchVarACY) in the ShouldUseDocumentAmountForACY branch - Fix misplaced PurchVarACY assignment indentation in APAC localization - Add explanatory comment to ShouldUseDocumentAmountForACY() helper function documenting guard conditions
|
In the APAC-layer CalcPosShares, the pre-PR code assigned PurchVarACY exactly once, unconditionally, after the whole ACY block, so it was computed identically regardless of which of the three sub-branches (vendor-exchange-rate branch, Source Currency Code = '' fallback, Source Currency Code <> '' branch) executed. This PR moved that assignment inside only the newly-nested ShouldUseDocumentAmountForACY()=false branch. As a result, on the vendor-exchange-rate path and on the Source Currency Code <> '' path, PurchVarACY is never (re)assigned in this call and is left holding the earlier LCY-based value set near the top of the procedure (or a stale caller-supplied value), instead of an ACY-consistent one. The APAC file's ShouldUseDocumentAmountForACY()=true branch also omits the 'OvhdCostACY := 0; PurchVarACY := 0;' reset that the sibling W1/CH/ES/IT/RU layer files perform in the equivalent branch. Net effect: APAC postings using Additional Reporting Currency can post an incorrect (stale/LCY-derived) Purchase Variance ACY and Overhead Cost ACY on paths other than the plain CalcACYAmt recompute path. Recommend restoring an explicit PurchVarACY (and OvhdCostACY) assignment on every branch of the ACY block in this file, matching the pattern already used in the other five layer files. Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4 |
|
The new nested ACY branch added to CalcPosShares in the APAC layer is misindented: the block opened at line 3688 no longer has its body indented one level deeper, and the closing Suggested fix (apply manually — could not be anchored as a one-click suggestion): if GLSetup."Additional Reporting Currency" <> '' then begin
if ItemJnlLine."Source Currency Code" = '' then begin
if ItemJnlLine."Vendor Exchange Rate (ACY)" <> 0 then begin
DirCostACY := Round(DirCost * ItemJnlLine."Vendor Exchange Rate (ACY)");
OvhdCostACY := Round(OvhdCost * ItemJnlLine."Vendor Exchange Rate (ACY)");
ItemJnlLine."Unit Cost (ACY)" := Round(ItemJnlLine."Unit Cost" * ItemJnlLine."Vendor Exchange Rate (ACY)");
end else begin
if ShouldUseDocumentAmountForACY() then begin
if Expected then
DirCostACY := ItemJnlLine."Unit Cost (ACY)" * ItemJnlLine.Quantity + RoundingResidualAmountACY
else
DirCostACY := ItemJnlLine."Unit Cost (ACY)" * ItemJnlLine."Invoiced Quantity";
OvhdCostACY := 0;
PurchVarACY := 0;
end else begin
DirCostACY := ACYMgt.CalcACYAmt(DirCost, ItemJnlLine."Posting Date", false);
OvhdCostACY := ACYMgt.CalcACYAmt(OvhdCost, ItemJnlLine."Posting Date", false);
ItemJnlLine."Unit Cost (ACY)" :=
Round(
CurrExchRate.ExchangeAmtLCYToFCY(
ItemJnlLine."Posting Date", GLSetup."Additional Reporting Currency", ItemJnlLine."Unit Cost",
CurrExchRate.ExchangeRate(
ItemJnlLine."Posting Date", GLSetup."Additional Reporting Currency")),
Currency."Unit-Amount Rounding Precision");
PurchVarACY := ItemJnlLine."Unit Cost (ACY)" * ItemJnlLine."Invoiced Quantity" - DirCostACY - OvhdCostACY;
end;
end;
end else begin
DirCostACY := ACYMgt.CalcACYAmt(DirCost, ItemJnlLine."Posting Date", false);
OvhdCostACY := ACYMgt.CalcACYAmt(OvhdCost, ItemJnlLine."Posting Date", false);
ItemJnlLine."Unit Cost (ACY)" :=
Round(
CurrExchRate.ExchangeAmtLCYToFCY(
ItemJnlLine."Posting Date", GLSetup."Additional Reporting Currency", ItemJnlLine."Unit Cost",
CurrExchRate.ExchangeRate(
ItemJnlLine."Posting Date", GLSetup."Additional Reporting Currency")),
Currency."Unit-Amount Rounding Precision");
end;
end;Knowledge: Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4 |
…ACY/PurchVarACY resets Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PLEASE CHECVK HAVE MAULAYY CORRECTED |
… layer Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| ItemJnlLine."Posting Date", GLSetup."Additional Reporting Currency")), | ||
| Currency."Unit-Amount Rounding Precision"); | ||
| PurchVarACY := ItemJnlLine."Unit Cost (ACY)" * ItemJnlLine."Invoiced Quantity" - DirCostACY - OvhdCostACY; | ||
| end; |
There was a problem hiding this comment.
Note: underlying impact is major (financial posting correctness) but this is an unreferenced agent finding so severity is capped at minor per policy. In the APAC copy of CalcPosShares, the refactor introducing ShouldUseDocumentAmountForACY moved the PurchVarACY assignment inside only one inner branch (the ACYMgt.CalcACYAmt fallback branch and the new document-amount branch) and removed the unconditional 'PurchVarACY := ItemJnlLine."Unit Cost (ACY)" * ItemJnlLine."Invoiced Quantity" - DirCostACY - OvhdCostACY;' statement that previously ran after all branches whenever GLSetup."Additional Reporting Currency" <> ''. As a result, when ItemJnlLine."Vendor Exchange Rate (ACY)" <> 0, or when ItemJnlLine."Source Currency Code" <> '' (the outer else at line ~3713), PurchVarACY is no longer recomputed and retains its stale/incoming value instead, silently propagating an incorrect ACY purchase-variance amount into the posted item ledger/value entries. The other 5 layers (CH, ES, IT, RU, W1) do not have this vendor-exchange-rate/source-currency-code branch structure and correctly set PurchVarACY in both remaining branches, so this regression is APAC-specific. Recommend restoring the shared PurchVarACY calculation after the inner branches (or explicitly assigning it in the vendor-exchange-rate branch and the Source Currency Code <> '' branch) so it is set unconditionally whenever the ACY block runs.
Suggested fix (apply manually — could not be anchored as a one-click suggestion):
end else begin
DirCostACY := ACYMgt.CalcACYAmt(DirCost, ItemJnlLine."Posting Date", false);
OvhdCostACY := ACYMgt.CalcACYAmt(OvhdCost, ItemJnlLine."Posting Date", false);
ItemJnlLine."Unit Cost (ACY)" :=
Round(
CurrExchRate.ExchangeAmtLCYToFCY(
ItemJnlLine."Posting Date", GLSetup."Additional Reporting Currency", ItemJnlLine."Unit Cost",
CurrExchRate.ExchangeRate(
ItemJnlLine."Posting Date", GLSetup."Additional Reporting Currency")),
Currency."Unit-Amount Rounding Precision");
end;
PurchVarACY := ItemJnlLine."Unit Cost (ACY)" * ItemJnlLine."Invoiced Quantity" - DirCostACY - OvhdCostACY;
end;👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
| CurrExchRate.ExchangeRate( | ||
| ItemJnlLine."Posting Date", GLSetup."Additional Reporting Currency")), | ||
| Currency."Unit-Amount Rounding Precision"); | ||
| if ShouldUseDocumentAmountForACY() then begin |
There was a problem hiding this comment.
In the APAC layer's CalcPosShares, the new ShouldUseDocumentAmountForACY() call was nested inside the if ItemJnlLine."Source Currency Code" = '' then begin ... end branch (a structure unique to APAC that does not exist in the CH/ES/IT/RU/W1 layers). ShouldUseDocumentAmountForACY() requires ItemJnlLine."Source Currency Code" = GLSetup."Additional Reporting Currency" to be true, and this whole block is only reached when GLSetup."Additional Reporting Currency" <> ''. Since the outer branch already guarantees Source Currency Code = '', the inner condition (Source Currency Code = a non-empty ACY code) can never be true, so the new document-amount-for-ACY code path is dead/unreachable in APAC. The intended fix (bypassing the ACY exchange-rate recalculation when the source document is already denominated in the Additional Reporting Currency) silently never applies for APAC, while it works correctly in the other five layers where the call sits directly under the outer GLSetup."Additional Reporting Currency" <> '' check without the extra Source-Currency-Code wrapper.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
| exit( | ||
| (ItemJnlLine."Source Currency Code" = GLSetup."Additional Reporting Currency") and | ||
| (Item."Costing Method" <> Item."Costing Method"::Standard) and | ||
| (ItemJnlLine."Discount Amount" = 0) and | ||
| (ItemJnlLine."Indirect Cost %" = 0) and | ||
| (ItemJnlLine."Overhead Rate" = 0)); |
There was a problem hiding this comment.
The new ShouldUseDocumentAmountForACY() helper formats its multi-line exit predicate differently from the surrounding code in this object: each condition is flush with exit( instead of using the hanging indent already used by nearby helpers. Reindent the continued lines one level deeper so the boolean expression matches the established AL formatting in this file.
| exit( | |
| (ItemJnlLine."Source Currency Code" = GLSetup."Additional Reporting Currency") and | |
| (Item."Costing Method" <> Item."Costing Method"::Standard) and | |
| (ItemJnlLine."Discount Amount" = 0) and | |
| (ItemJnlLine."Indirect Cost %" = 0) and | |
| (ItemJnlLine."Overhead Rate" = 0)); | |
| exit( | |
| (ItemJnlLine."Source Currency Code" = GLSetup."Additional Reporting Currency") and | |
| (Item."Costing Method" <> Item."Costing Method"::Standard) and | |
| (ItemJnlLine."Discount Amount" = 0) and | |
| (ItemJnlLine."Indirect Cost %" = 0) and | |
| (ItemJnlLine."Overhead Rate" = 0)); |
Agent judgement — not directly backed by a BCQuality knowledge article.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
| exit( | ||
| (ItemJnlLine."Source Currency Code" = GLSetup."Additional Reporting Currency") and | ||
| (Item."Costing Method" <> Item."Costing Method"::Standard) and | ||
| (ItemJnlLine."Discount Amount" = 0) and | ||
| (ItemJnlLine."Indirect Cost %" = 0) and | ||
| (ItemJnlLine."Overhead Rate" = 0)); |
There was a problem hiding this comment.
The new ShouldUseDocumentAmountForACY() helper formats its multi-line exit predicate differently from the surrounding code in this object: each condition is flush with exit( instead of using the hanging indent already used by nearby helpers. Reindent the continued lines one level deeper so the boolean expression matches the established AL formatting in this file.
| exit( | |
| (ItemJnlLine."Source Currency Code" = GLSetup."Additional Reporting Currency") and | |
| (Item."Costing Method" <> Item."Costing Method"::Standard) and | |
| (ItemJnlLine."Discount Amount" = 0) and | |
| (ItemJnlLine."Indirect Cost %" = 0) and | |
| (ItemJnlLine."Overhead Rate" = 0)); | |
| exit( | |
| (ItemJnlLine."Source Currency Code" = GLSetup."Additional Reporting Currency") and | |
| (Item."Costing Method" <> Item."Costing Method"::Standard) and | |
| (ItemJnlLine."Discount Amount" = 0) and | |
| (ItemJnlLine."Indirect Cost %" = 0) and | |
| (ItemJnlLine."Overhead Rate" = 0)); |
Agent judgement — not directly backed by a BCQuality knowledge article.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
Address PR review Style finding: indent the multi-line exit predicate one level deeper to match established AL formatting.
Agentic PR Review - Round 1Recommendation: Request ChangesWhat this PR doesThis PR changes CalcPosShares in W1 and five country layers so Value Entry ACY amounts use the document amount when the source currency is the Additional Reporting Currency and there are no cost add-ons. The W1-style change matches the reported bug for a simple purchase invoice, but the APAC copy has an extra source-currency branch. That branch makes the new APAC document-amount path unreachable for the actual bug condition, so one changed layer still keeps the old LCY-to-ACY recalculation for the scenario this PR is meant to fix. SuggestionsS1 - APAC never reaches the new branch S2 - Add a posting regression test Risk assessment and necessityRisk: This is a financial posting path in codeunit 22. A wrong branch posts wrong ACY cost amounts and can keep inventory valuation out of balance with G/L in Additional Reporting Currency. The APAC path is especially risky because it has extra Vendor Exchange Rate and Source Currency Code handling that is not present in W1/CH/ES/IT/RU. Necessity: The bug is valid and important. The work item shows a clear repro where a EUR document in a EUR Additional Reporting Currency posts a correct G/L ACY amount but an incorrect Value Entry ACY amount. The scope is right, but the APAC logic and missing posting test must be fixed before merge.
|
alexei-dobriansky
left a comment
There was a problem hiding this comment.
Please review the suggestions
…RU layers Apply hanging indent to the multi-line exit() predicate so continued conditions sit one level deeper than exit(, matching nearby helpers and resolving the Style review findings. Formatting only; no logic change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
In the APAC-only refactor of Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4 |
…ches The APAC-only refactor removed the single unconditional PurchVarACY assignment at the end of the ARC block and only re-added it inside the inner CalcACYAmt else-branch. The Vendor Exchange Rate (ACY) <> 0 branch and the Source Currency Code <> '' branch then fell through without setting the PurchVarACY var parameter, leaving a stale value. Assign PurchVarACY explicitly on every sub-branch so the ACY purchase variance is always computed; the outer assignment stays removed to preserve the intentional PurchVarACY := 0 in the ShouldUseDocumentAmountForACY path.
| CurrExchRate.ExchangeRate( | ||
| ItemJnlLine."Posting Date", GLSetup."Additional Reporting Currency")), | ||
| Currency."Unit-Amount Rounding Precision"); | ||
| if ShouldUseDocumentAmountForACY() then begin |
There was a problem hiding this comment.
This adds an unguarded runtime branch in the shipped posting path: when source currency equals the additional reporting currency, costing is non-Standard, and discount/indirect-cost/overhead inputs are zero, posting now derives ACY amounts from document amounts instead of the prior exchange-rate calculation. That silently changes posted Unit Cost (ACY), DirCostACY, OvhdCostACY, and PurchVarACY for upgraded tenants and extensions relying on existing ledger/G/L values. Preserve legacy behavior for existing tenants or gate the new calculation behind an explicit upgrade-controlled feature flag or opt-in.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| PurchVarACY := ItemJnlLine."Unit Cost (ACY)" * ItemJnlLine."Invoiced Quantity" - DirCostACY - OvhdCostACY; | ||
| end; | ||
| if GLSetup."Additional Reporting Currency" <> '' then | ||
| if ShouldUseDocumentAmountForACY() then begin |
There was a problem hiding this comment.
This adds an unguarded runtime branch in the shipped posting path: when source currency equals the additional reporting currency, costing is non-Standard, and discount/indirect-cost/overhead inputs are zero, posting now derives ACY amounts from document amounts instead of the prior exchange-rate calculation. That silently changes posted Unit Cost (ACY), DirCostACY, OvhdCostACY, and PurchVarACY for upgraded tenants and extensions relying on existing ledger/G/L values. Preserve legacy behavior for existing tenants or gate the new calculation behind an explicit upgrade-controlled feature flag or opt-in.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| PurchVarACY := ItemJnlLine."Unit Cost (ACY)" * ItemJnlLine."Invoiced Quantity" - DirCostACY - OvhdCostACY; | ||
| end; | ||
| if GLSetup."Additional Reporting Currency" <> '' then | ||
| if ShouldUseDocumentAmountForACY() then begin |
There was a problem hiding this comment.
This adds an unguarded runtime branch in the shipped posting path: when source currency equals the additional reporting currency, costing is non-Standard, and discount/indirect-cost/overhead inputs are zero, posting now derives ACY amounts from document amounts instead of the prior exchange-rate calculation. That silently changes posted Unit Cost (ACY), DirCostACY, OvhdCostACY, and PurchVarACY for upgraded tenants and extensions relying on existing ledger/G/L values. Preserve legacy behavior for existing tenants or gate the new calculation behind an explicit upgrade-controlled feature flag or opt-in.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| PurchVarACY := ItemJnlLine."Unit Cost (ACY)" * ItemJnlLine."Invoiced Quantity" - DirCostACY - OvhdCostACY; | ||
| end; | ||
| if GLSetup."Additional Reporting Currency" <> '' then | ||
| if ShouldUseDocumentAmountForACY() then begin |
There was a problem hiding this comment.
This adds an unguarded runtime branch in the shipped posting path: when source currency equals the additional reporting currency, costing is non-Standard, and discount/indirect-cost/overhead inputs are zero, posting now derives ACY amounts from document amounts instead of the prior exchange-rate calculation. That silently changes posted Unit Cost (ACY), DirCostACY, OvhdCostACY, and PurchVarACY for upgraded tenants and extensions relying on existing ledger/G/L values. Preserve legacy behavior for existing tenants or gate the new calculation behind an explicit upgrade-controlled feature flag or opt-in.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| PurchVarACY := ItemJnlLine."Unit Cost (ACY)" * ItemJnlLine."Invoiced Quantity" - DirCostACY - OvhdCostACY; | ||
| end; | ||
| if GLSetup."Additional Reporting Currency" <> '' then | ||
| if ShouldUseDocumentAmountForACY() then begin |
There was a problem hiding this comment.
This adds an unguarded runtime branch in the shipped posting path: when source currency equals the additional reporting currency, costing is non-Standard, and discount/indirect-cost/overhead inputs are zero, posting now derives ACY amounts from document amounts instead of the prior exchange-rate calculation. That silently changes posted Unit Cost (ACY), DirCostACY, OvhdCostACY, and PurchVarACY for upgraded tenants and extensions relying on existing ledger/G/L values. Preserve legacy behavior for existing tenants or gate the new calculation behind an explicit upgrade-controlled feature flag or opt-in.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| CurrExchRate.ExchangeRate( | ||
| ItemJnlLine."Posting Date", GLSetup."Additional Reporting Currency")), | ||
| Currency."Unit-Amount Rounding Precision"); | ||
| if ShouldUseDocumentAmountForACY() then begin |
There was a problem hiding this comment.
In the APAC layer's CalcPosShares, the new ShouldUseDocumentAmountForACY() fast path is nested inside if ItemJnlLine."Source Currency Code" = '' then ... else begin ... if ShouldUseDocumentAmountForACY() then .... But ShouldUseDocumentAmountForACY() itself requires ItemJnlLine."Source Currency Code" = GLSetup."Additional Reporting Currency", and this whole outer block only runs when GLSetup."Additional Reporting Currency" <> ''. So the fast-path predicate can only be true when Source Currency Code = '' (from the enclosing branch) AND Source Currency Code = GLSetup."Additional Reporting Currency" (non-empty) simultaneously — a contradiction. The new branch is therefore dead code in APAC: it can never execute, so APAC never benefits from (or is affected by) this change, unlike the other five layers (CH, ES, IT, RU, W1) where the same helper is reachable. This is very likely an integration oversight when porting the fix to the APAC-specific branch structure and should be fixed so the new path is reachable there too, consistent with the other layers.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
AB#646085
This PR improves upon the initial fix with three refinements:
Removed redundant variable clearances —
OvhdCostACY := 0; PurchVarACY := 0;in theShouldUseDocumentAmountForACY()branch are unnecessary (already zero by construction from the guard conditions) and have been removed for cleaner code.Fixed APAC localization indentation issue — The misplaced
PurchVarACYassignment that was orphaned outside nested blocks has been moved into the correctelsebranch where it belongs, fixing a structural issue introduced during Copilot code generation.Added documentation — The
ShouldUseDocumentAmountForACY()helper now includes an explanatory comment that clarifies WHY each of the five guard conditions is necessary (Standard costing, discount, indirect cost, overhead rate, and source currency equivalence).Root cause
When a document's currency equals the Additional Reporting Currency, the ACY amount on Value Entries was being back-calculated from the LCY amount via exchange rate (ARC→LCY→ARC double conversion) instead of taken directly from the document's native ACY amount. This lossy round-trip introduced rounding differences that mismatched the "Additional-Currency Amount" on the corresponding G/L Entries.
Fix scope
CalcPosSharesprocedure using direct ACY derivation when the document currency = ARC and no cost components present.Test plan
🤖 Generated with Claude Code