Skip to content

[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

Open
neeleshsinghal wants to merge 8 commits into
mainfrom
bugs/Bug-646085-ACY-amount-on-Value-Entries-is-recalculated-from-LCY-v2
Open

[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
neeleshsinghal wants to merge 8 commits into
mainfrom
bugs/Bug-646085-ACY-amount-on-Value-Entries-is-recalculated-from-LCY-v2

Conversation

@neeleshsinghal

@neeleshsinghal neeleshsinghal commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

AB#646085

This PR improves upon the initial fix with three refinements:

  1. Removed redundant variable clearancesOvhdCostACY := 0; PurchVarACY := 0; in the ShouldUseDocumentAmountForACY() branch are unnecessary (already zero by construction from the guard conditions) and have been removed for cleaner code.

  2. Fixed APAC localization indentation issue — The misplaced PurchVarACY assignment that was orphaned outside nested blocks has been moved into the correct else branch where it belongs, fixing a structural issue introduced during Copilot code generation.

  3. 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

  • W1 (Worldwide): Complete fix in CalcPosShares procedure using direct ACY derivation when the document currency = ARC and no cost components present.
  • APAC (Asia-Pacific): Same logic, with additional vendor exchange rate path and now-corrected indentation/documentation.

Test plan

  1. Compile and publish the modified BaseApp (both W1 and APAC versions).
  2. Run costing/inventory tests, especially those covering:
    • Purchases/sales posted in the Additional Reporting Currency
    • Value Entry vs. G/L Entry reconciliation for ACY amounts
    • Standard, Average, and FIFO costing methods
    • Scenarios with and without discounts, overhead, indirect costs

🤖 Generated with Claude Code

…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
@neeleshsinghal
neeleshsinghal requested a review from a team August 14, 2026 04:49
@github-actions github-actions Bot added the SCM GitHub request for SCM area label Aug 14, 2026
@neeleshsinghal neeleshsinghal changed the title [Master]-ACY amount on Value Entries is recalculated from LCY — v2 APAC fixes [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- #10231 Aug 14, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone Aug 14, 2026
Comment thread src/Layers/W1/BaseApp/Inventory/Posting/ItemJnlPostLine.Codeunit.al
@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Error\ Handling}$

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

@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Style}$

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 end/end else lines between 3692 and 3721 do not line up with the if/begin they close. This makes the control flow (three nested if/else levels controlling DirCostACY/OvhdCostACY/PurchVarACY) hard to audit visually and is inconsistent with the clean, consistently-indented equivalent block in the W1/CH/ES/IT/RU layer files. Reindent so each block body sits one level below its opener and each end aligns with the line that opened its corresponding begin.

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>
@neeleshsinghal

Copy link
Copy Markdown
Contributor Author

🟡 Medium Severity — Style

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 end/end else lines between 3692 and 3721 do not line up with the if/begin they close. This makes the control flow (three nested if/else levels controlling DirCostACY/OvhdCostACY/PurchVarACY) hard to audit visually and is inconsistent with the clean, consistently-indented equivalent block in the W1/CH/ES/IT/RU layer files. Reindent so each block body sits one level below its opener and each end aligns with the line that opened its corresponding begin.

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

PLEASE CHECVK HAVE MAULAYY CORRECTED

… layer

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@neeleshsinghal neeleshsinghal changed the title [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- #10231 [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- Aug 14, 2026
ItemJnlLine."Posting Date", GLSetup."Additional Reporting Currency")),
Currency."Unit-Amount Rounding Precision");
PurchVarACY := ItemJnlLine."Unit Cost (ACY)" * ItemJnlLine."Invoiced Quantity" - DirCostACY - OvhdCostACY;
end;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Error\ Handling}$

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Agent}$

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

Comment thread src/Layers/APAC/BaseApp/Inventory/Posting/ItemJnlPostLine.Codeunit.al Outdated
Comment thread src/Layers/CH/BaseApp/Inventory/Posting/ItemJnlPostLine.Codeunit.al Outdated
Comment thread src/Layers/ES/BaseApp/Inventory/Posting/ItemJnlPostLine.Codeunit.al Outdated
Comment on lines +6087 to +6092
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));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Style}$

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.

Suggested change
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

Comment thread src/Layers/RU/BaseApp/Inventory/Posting/ItemJnlPostLine.Codeunit.al Outdated
Comment on lines +6056 to +6061
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));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Style}$

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.

Suggested change
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.
Comment thread src/Layers/W1/BaseApp/Inventory/Posting/ItemJnlPostLine.Codeunit.al
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

This 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.

Suggestions

S1 - APAC never reaches the new branch
In src/Layers/APAC/BaseApp/Inventory/Posting/ItemJnlPostLine.Codeunit.al, ShouldUseDocumentAmountForACY() is called only inside if ItemJnlLine."Source Currency Code" = ''. The helper requires the source currency to equal the non-empty Additional Reporting Currency, so this branch cannot run. Move the document-amount check outside that Source Currency Code = '' branch, or handle the Source Currency Code = Additional Reporting Currency branch before the APAC fallback conversion.

S2 - Add a posting regression test
This change affects posted Value Entry ACY amounts and G/L reconciliation. Add a test for a purchase posting where the document currency equals the Additional Reporting Currency and the currency factor differs from the posting-date exchange rate. The test should assert the Value Entry ACY amount matches the source document amount and reconciles with the G/L Entry Additional-Currency Amount.

Risk assessment and necessity

Risk: 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.


[AI-PR-REVIEW] version=1 promptVersion=2 system=github pr=10244 round=1 by=alexei-dobriansky at=2026-08-14T08:44:46Z lastSha=643aadbae5f972eb3559b711d1ecbe4264062837 reviewKey=na suggestions=S1@5ef8edb5,S2@e2dfea90

@alexei-dobriansky alexei-dobriansky left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Error\ Handling}$

In the APAC-only refactor of CalcPosShares, PurchVarACY is no longer assigned on every GLSetup."Additional Reporting Currency" <> '' sub-branch. Previously it was computed once, unconditionally, after all APAC-specific branching (Vendor Exchange Rate (ACY) <> 0 branch, Source Currency Code = '' / CalcACYAmt branch, and Source Currency Code <> '' branch). After this change, the assignment only happens inside the new inner else (the CalcACYAmt branch) added under ShouldUseDocumentAmountForACY(). The Vendor Exchange Rate (ACY) <> 0 branch (~line 3688-3691) and the outer Source Currency Code <> '' branch (~line 3713-3723) now fall through without ever setting PurchVarACY, so the caller's var parameter keeps whatever stale/uninitialized value it had, silently producing an incorrect ACY purchase-variance amount for those posting paths. Only CH/ES/IT/RU/W1 are unaffected because they never had these extra APAC-specific sub-branches. Restore an explicit PurchVarACY assignment (or zeroing, consistent with the new ShouldUseDocumentAmountForACY() branch) on every sub-branch, or move the calculation back outside the branching so it applies uniformly.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes}$

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes}$

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes}$

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes}$

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes}$

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Performance}$

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

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

Labels

SCM GitHub request for SCM area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants