[Master]-Rounding Differences in ARAP with Deferrals and Non-Deductible VAT

[DeepsShukla]

Workitem Bug 642527: [Main]-Rounding Differences in ARAP with Deferrals and Non-Deductible VAT

Fixes AB#6432527

[github-actions]

Copilot PR Review

Iteration 2 · Outcome: completed

Knowledge source: https://github.com/microsoft/BCQuality@186d8a131465475c79244d994acb872cd5c0d4bf

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

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

_{Findings produced by the AL review agent v1.7.3. Reply 👎 on any inline comment to flag false positives.}

[github-actions]

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

InsertDeferralNonDeductibleVATGLEntries splits the single VATAmountRounding var parameter into PositiveVATAmountRounding/NegativeVATAmountRounding. The deprecated OnBeforeInsertDeferralNonDeductibleVATGLEntries event (still raised under #if not CLEAN29) is passed a fresh local VATAmountRounding by var, but immediately after that call (when IsHandled is still false) the local variable is unconditionally overwritten from PositiveVATAmountRounding/NegativeVATAmountRounding based on Sign (e.g. GenJnlPostLine.Codeunit.al lines 8236 and 8248/8254 in the W1 layer). Any existing subscriber to the deprecated event that adjusts VATAmountRounding without setting IsHandled (a partial-override subscriber, as opposed to a full IsHandled take-over) will have its adjustment silently discarded before the rounding calculation runs. This is a functional regression for subscribers of the old event during the CLEAN29 deprecation window, not merely a signature change. The same pattern is duplicated identically across all twelve modified GenJnlPostLine.Codeunit.al copies (APAC, BE, CH, ES, FI, FR, IT (both overloads), NA, NO, RU, W1). Because this behavior would otherwise be a major compatibility defect, it is reported here at capped minor severity per BCQuality's agent-finding rules; consider either restoring the subscriber's mutation into the calculation before it is overwritten, or documenting the behavior change in the [Obsolete] reason and release notes, and consider authoring a knowledge-file rule for 'do not let a still-raised deprecated event's var-parameter mutation be discarded during a parameter-split refactor' so it can gate future PRs.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.12.4}

[github-actions]

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

InsertDeferralNonDeductibleVATGLEntries obsoletes the local integration event OnBeforeInsertDeferralNonDeductibleVATGLEntries and adds a parallel OnBeforeInsertDeferralNonDeductibleVATGLEntries2 to carry the new PositiveVATAmountRounding/NegativeVATAmountRounding parameters. Because the publisher is a local Business/Integration event, subscribers bind parameters by name and can omit ones they do not use, so the two new parameters could have been added directly to the existing event without breaking any subscriber and without the obsoletion dance. The chosen approach also introduces a real regression during the deprecation window: the old event is now raised with a VATAmountRounding argument that is a brand-new local variable (always 0 at that point), and the value is unconditionally overwritten immediately afterward by 'VATAmountRounding := PositiveVATAmountRounding' or '... := NegativeVATAmountRounding' before it is used in the rounding calculation. Any extension still subscribed to the obsolete event that modifies VATAmountRounding through its var parameter (the pattern the old event exists to support) has its change silently discarded — the modification never reaches UnroundedVATAmount/VATAmount. This same pattern is duplicated identically in APAC, BE, CH, ES, FI, FR, IT (both overloads), NA, NO, and RU layers of GenJnlPostLine.Codeunit.al.

Recommendation:

• extend the existing local event with the two new parameters in place (per BCQuality guidance for local/internal events) instead of shipping a duplicate '...2' event, which both avoids the unnecessary obsoletion churn and removes the dropped-modification bug.

Knowledge:

• microsoft/knowledge/events/treat-local-and-internal-events-as-subscriber-contracts.md
• microsoft/knowledge/events/prefer-reusing-or-extending-existing-events.md
• microsoft/knowledge/breaking-changes/deprecate-public-members-with-the-obsolete-lifecycle.md

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.12.4}