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

[Franco111000]

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 Nikola Jovicic (@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).

[alexei-dobriansky]

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

This PR removes the SufficientEntriesFound early exit from the customer, vendor, and employee Apply-to-Oldest preparation loops in codeunit 12 across the layer copies. That restores the sign decision to use the full sum of the new entry plus all eligible old entries, instead of a partial sum that can select same-sign entries after mixed-sign balances.

The changed customer and vendor logic matches the root cause in GitHub issue #9529. The new tests cover the reported customer case, the vendor mirror, and a mixed-entry customer case. The employee procedure has the same changed decision and the same event-adjusted sum path, but there is no matching employee regression test in this PR.

Suggestions

S1 - Add employee regression coverage
PrepareTempEmplLedgEntry changes the same Apply-to-Oldest sign decision as the customer and vendor paths. ERMApplyUnapplyEmployee.Codeunit.al already has Apply-to-Oldest coverage, so add the same mixed-sign regression test for employee ledger entries. This is finance posting logic, so the employee path should not rely only on customer and vendor tests.

Risk assessment and necessity

Risk: The regression surface is finance posting for Apply-to-Oldest on accounts with mixed-sign open entries. The change is narrow and does not change public signatures or events, but it changes how much data the weighing loop reads and restores the pre-28.2 full scan. The customer and vendor tests reduce risk for the reported paths; the employee path remains untested.

Necessity: The linked GitHub bug is detailed and important: the current behavior can silently apply documents to same-sign entries and damage remaining amounts and due-date tracking. There is no linked ADO AB# work item in the PR body, so I derived bug mode from GitHub issue #9529 and the diff. The scope is right, but the employee change needs direct coverage before merge.

---

[AI-PR-REVIEW] version=1 system=github pr=9881 round=1 by=alexei-dobriansky at=2026-08-03T10:03:14.844Z lastSha=0e45364cc4894c21a3e4de331c0e0dbfb301046c suggestions=S1

[Franco111000]

Thanks for the review! S1 addressed in 8012bb6: added ApplyToOldestPaymentDoesNotApplyToOtherPayments to codeunit 134114 "ERM Apply Unapply Employee", next to the existing Apply-to-Oldest employee test and mirrored into the CZ copy of that codeunit. Same mixed-sign shape as the customer and vendor tests: three reimbursement payments, a larger expense posted last but back-dated before them (with a setup assert proving it is still open), then a new payment that must settle the expense only, with the excess reducing the oldest payment and the other payments untouched. Same even-integer amount scheme, so all expected values are whole numbers.

[alexei-dobriansky]

Agentic PR Review - Round 2

Recommendation: Accept

What this PR does

The new commit adds the missing employee Apply-to-Oldest mixed-sign regression test in the W1 and CZ employee test codeunits. The test mirrors the customer and vendor regression shape from round 1: older payments, a larger back-dated expense, and a new payment that must close only the expense and leave the other payments open.

This addresses the round-1 gap. The test exercises the employee path that changed in PrepareTempEmplLedgEntry, uses whole expected amounts, checks the setup state before the final posting, and verifies both the closed opposite-sign entry and the untouched same-sign entries after posting.

Status of previous suggestions

ID	Title	Status	Author response
S1	Add employee regression coverage	Addressed	Author said commit 8012bb6 adds the mirrored W1 and CZ employee regression tests.

New observations (commits since round 1)

None - changes only addressed the previous suggestion.

Risk assessment and necessity

Risk: The new commit only adds tests, so it does not expand runtime behavior. The remaining PR risk is still the finance posting Apply-to-Oldest change reviewed in round 1, now covered for customer, vendor, and employee paths.

Necessity: The linked GitHub issue describes an important data-integrity bug. There is still no linked ADO AB# work item in the PR body, so I could not read an ADO Bug or Slice; I reviewed from GitHub issue #9529 and the diff. The added employee test is necessary and closes the only round-1 blocker.

---

[AI-PR-REVIEW] version=1 system=github pr=9881 round=2 by=alexei-dobriansky at=2026-08-03T17:11:12.306Z lastSha=8012bb6fe48ab31ed8a9dac28fd3e15ac8c93342 suggestions=S1:addressed parentRound=1

[dcenic]

This will reintroduce the severe performance issue that is affecting customers who have many (tens of thousands of) open ledger entries for one or very few vendors, and use 'Apply to Oldest'.
We should try to keep the performance optimization and address the bug that it introduced.

[dcenic]

Franco111000 I opened an ICM for this regression, and I am working on a fix that will fix it and also keep the performance optimization of 'Apply-to-Oldest'.

[dcenic]

Franco111000 I made a new pull request #10227 in which I:

• copied your new regression tests from this PR
• made sure to fix it while keeping the performance optimization (take sign into consideration when looping and summing)
• added one more test to make sure the performance optimization is still applied.

[Franco111000]

Superseded by #10227 (merged as efa0b87), which fixes the regression while keeping the Apply-to-Oldest optimization and carries these regression tests forward. Closing as committed. Thanks dcenic for the quick turnaround and the collaboration on this one.