Skip to content

Restore the full-sum sign decision in Apply-to-Oldest application - #9881

Open
Franco111000 wants to merge 1 commit into
microsoft:mainfrom
Franco111000:fix-9529-apply-to-oldest-partial-sum
Open

Restore the full-sum sign decision in Apply-to-Oldest application#9881
Franco111000 wants to merge 1 commit into
microsoft:mainfrom
Franco111000:fix-9529-apply-to-oldest-partial-sum

Conversation

@Franco111000

@Franco111000 Franco111000 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

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, PrepareTempVendLedgEntry and PrepareTempEmplLedgEntry feed the sign decision SetRange(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 (FindAmtForAppln applies the full remaining amount of each entry when the Positive filter is set, and caps with ABSMin once the filter is cleared). 28.2 added a SufficientEntriesFound early exit that stops the loop as soon as the running sum offsets the new document. Because the buffer key iterates Positive = false entries 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 Positive flag can disagree with the current remaining sign on overshot entries (which this very defect produces), and the employee loop raises OnPrepareTempEmplLedgEntryOnBeforeUpdateRemainingAmount, 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 at 30ce9d7f71^ 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-Positive edges 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

  • I read the full diff and it contains only changes I intended.
  • I built the affected app(s) locally with no new analyzer warnings.
  • I ran the change in Business Central and confirmed it behaves as expected.
  • I added or updated tests for the new behavior, or explained below why none are needed.

What I tested and the outcome

  • The change is the exact inverse of the hunks that introduced the early exit: all 33 edited regions (3 procedures x 11 layers) are byte-identical to their state at 30ce9d7f71^, verified by direct comparison against that commit.
  • This repo's CI compiles the W1 layer only, so the 10 country copies are outside the build. To rule out drift, the loop regions were hash-compared across all 11 copies before the edit (identical) and after the edit (identical again). The W1 compile is covered by the PR build.
  • I have no local BC environment for this repo, so the runtime behavior was verified by tracing the application flow (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).
  • The existing ApplyToOldestPartialPaymentAppliesToOldestFirst (single-sign) takes the same decision path before and after this change and stays green.

Risk & compatibility

  • Behavior changes only for Apply-to-Oldest application decisions on accounts holding open entries of both signs, reached from journal posting and from the Apply Posted Entries flows. Single-sign situations produce the same filter as before.
  • Performance: this restores the pre-28.2 cost of one Remaining Amount CalcFields per 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.
  • The regression ships in 28.2 and later. Should this be cherry-picked to the 28.x release branches, as was done for Fix agent setup field lookup casing #8941 ([28.0] Fix agent setup field lookup casing (backport #8941) #9328 - [28.x] Fix agent setup field lookup casing (backport #8941) #9332)?
  • Area: Finance (matching the issue; I cannot set labels).

…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.
@Franco111000
Franco111000 requested a review from a team July 31, 2026 16:23
@github-actions github-actions Bot added From Fork Pull request is coming from a fork Finance GitHub request for Finance area needs-approval Workflow runs require maintainer approval to start labels Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Finance GitHub request for Finance area From Fork Pull request is coming from a fork needs-approval Workflow runs require maintainer approval to start

Projects

None yet

1 participant