Restore the full-sum sign decision in Apply-to-Oldest application - #9881
Open
Franco111000 wants to merge 1 commit into
Open
Restore the full-sum sign decision in Apply-to-Oldest application#9881Franco111000 wants to merge 1 commit into
Franco111000 wants to merge 1 commit into
Conversation
…oft#9529) The 28.2 early exit in the weighing loops of codeunit 12 stopped the sum as soon as it offset the new document, but the subsequent SetRange(Positive, RemainingAmount < 0) decision needs the net of all open entries: it selects the ledger side that application may consume uncapped. With open opposite-sign entries exceeding a positive new document, the truncated sum flipped the decision and the new document consumed open same-sign entries uncapped. Removes the early exit in PrepareTempCustLedgEntry, PrepareTempVendLedgEntry and PrepareTempEmplLedgEntry across all layer copies, restoring the pre-28.2 weighing semantics, and adds regression tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Since 28.2, posting a document for a customer or vendor with Application Method = "Apply to Oldest" can apply the new document to open entries of the same sign, closing them and accumulating their amounts onto the new document (issue #9529). A posted invoice can end up with a Remaining Amount larger than its own Amount, and per-entry due-date tracking is destroyed. No error is raised.
Root cause. In codeunit 12, the weighing loops in
PrepareTempCustLedgEntry,PrepareTempVendLedgEntryandPrepareTempEmplLedgEntryfeed the sign decisionSetRange(Positive, RemainingAmount < 0). That decision needs the net of the new document plus all open entries: it selects the ledger side that the application loop may consume uncapped (FindAmtForApplnapplies the full remaining amount of each entry when the Positive filter is set, and caps withABSMinonce the filter is cleared). 28.2 added aSufficientEntriesFoundearly exit that stops the loop as soon as the running sum offsets the new document. Because the buffer key iteratesPositive = falseentries first, a positive new document (customer invoice or refund, vendor payment or refund) whose opposite-sign open total exceeds it trips the exit inside the negative block, before the positive entries pull the sum back. The truncated sum flips the decision, and the new document consumes old same-sign entries uncapped.Fix. Remove the early exit in all three procedures, restoring the pre-28.2 weighing semantics. The decision is a property of the whole open-entry set, so any early exit leaves it on a partial sum in some conditions: a payment discount can exceed a partially credited invoice's remaining amount, the stored
Positiveflag can disagree with the current remaining sign on overshot entries (which this very defect produces), and the employee loop raisesOnPrepareTempEmplLedgEntryOnBeforeUpdateRemainingAmount, which lets subscribers adjust the summed amount. The decision must see the full sum.Why not the opposite-sign filter suggested in the issue.
SetRange(Positive, NewCVLedgEntryBuf."Remaining Amount" < 0)fixes the reported case but breaks the invariant that the selected side must be fully absorbable: with an open invoice of 1,000, an open credit memo of 5,000 and a new invoice of 2,000, it consumes the credit memo uncapped and leaves the new invoice permanently open at a remaining amount of -2,000. The full-sum rule picks the side that the netting can exhaust without overshoot.Origin. The early exit entered through the NAV sync published in this repo as 30ce9d7 (
features/BCAppsMigration, "Sync application code from NAV repo (#109)") and reached main in the layer import 748fdaa (#8848). It shipped with one companion test,ApplyToOldestPartialPaymentAppliesToOldestFirst, which covers only the single-sign case, where the exit is harmless. This PR restores the three procedure bodies to their state at30ce9d7f71^byte for byte, in all 11 layer copies (W1, APAC, BE, CH, ES, FI, FR, IT, NA, NO, RU).Out of scope, called out for transparency. There is a rarer pre-existing variant on <= 28.1: when the account's full net balance opposes the new document, the full-sum decision also selects same-sign entries. That behavior is unchanged here (it is the overshoot-protection trade-off described above and predates 28.2). Tests 3 and 4 of the reporter's attached repro app target that variant and still fail after this fix, by design. The payment-discount and stale-
Positiveedges above are likewise pre-existing and out of scope.Credit to @nikolajovicic for the precise root-cause analysis and the attached repro app; direction agreed in the issue thread.
Linked work
Fixes #9529
How I validated this
What I tested and the outcome
30ce9d7f71^, verified by direct comparison against that commit.ApplyCustLedgEntry,FindAmtForAppln,FindNextOldCustLedgEntryToApply) with concrete numbers for the repro scenario, the netting scenario and the vendor mirror, and locked in as tests:ApplyToOldestInvoiceDoesNotApplyToOtherInvoices(codeunit 134006, W1 + DACH copy): the reported defect. Under the current code the three older invoices are closed by the new invoice and the new invoice stays open above its own amount; with the fix the credit memo is closed, the new invoice is closed at 0, and only the oldest invoice absorbs the credit memo excess.ApplyToOldestPaymentNettingAgainstMixedEntries(codeunit 134006, W1 + DACH copy): pins the mixed-sign netting end state, which is identical before and after this change.ApplyToOldestPaymentDoesNotApplyToOtherPayments(codeunit 134004, W1 + CH + IT copies): the vendor mirror of the defect (vendor payments are the positive direction there).ApplyToOldestPartialPaymentAppliesToOldestFirst(single-sign) takes the same decision path before and after this change and stays green.Risk & compatibility
CalcFieldsper qualifying open entry during Apply-to-Oldest application on mixed-sign accounts. The weighing loop is entered whenever at least one open entry has the same sign as the new document; it is skipped when every open entry is opposite-sign. A correct optimization would need a decision that does not depend on the truncated sum; I am happy to explore that separately if there is interest.