Skip to content

[master] Overdue Balance (LCY) filters do not behave as expected if you open the Customer Card from a document or journal.#8940

Open
neeleshsinghal wants to merge 14 commits into
microsoft:mainfrom
neeleshsinghal:bugs/Bug-640988-Overdue-Balance-filters-do-not-behave-as-expected
Open

[master] Overdue Balance (LCY) filters do not behave as expected if you open the Customer Card from a document or journal.#8940
neeleshsinghal wants to merge 14 commits into
microsoft:mainfrom
neeleshsinghal:bugs/Bug-640988-Overdue-Balance-filters-do-not-behave-as-expected

Conversation

@neeleshsinghal

@neeleshsinghal neeleshsinghal commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Workitem Bug 640988: [master] [all-e]Overdue Balance (LCY) filters do not behave as expected if you open the Customer Card from a document or journal.

Fixes AB#640988

Issue: Overdue Balance (LCY) on the Customer Card shows 145 instead of 45 when the card is opened from a document or journal.

Cause: The card kept an inherited Date Filter from the document (guard if Rec.GetFilter("Date Filter") = '' skipped the default), so upperlimit("Date Filter") no longer equaled the work date and included future-dated (not-yet-due) entries.

Solution: In OnAfterGetCurrRecordFunc, unconditionally set Rec.SetRange("Date Filter", 0D, WorkDate()) so the overdue calculation is always "as of today" regardless of how the card is opened.

@github-actions github-actions Bot added From Fork Pull request is coming from a fork Linked Issue is linked to a Azure Boards work item labels Jun 30, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone Jun 30, 2026
@neeleshsinghal neeleshsinghal reopened this Jul 1, 2026
@neeleshsinghal neeleshsinghal reopened this Jul 7, 2026
@JesperSchulz JesperSchulz added the Finance GitHub request for Finance area label Jul 7, 2026
@neeleshsinghal neeleshsinghal reopened this Jul 7, 2026
@neeleshsinghal neeleshsinghal marked this pull request as ready for review July 8, 2026 05:17
@neeleshsinghal neeleshsinghal requested a review from a team July 8, 2026 05:17
@neeleshsinghal neeleshsinghal reopened this Jul 8, 2026
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Copilot PR Review

Iteration 12 · Outcome: completed

All 16 sub-skills executed against the PR diff (11 identical layer-copy edits to CustomerCard.Page.al's OnOpenPage/OnAfterGetCurrRecordFunc triggers, plus a new regression test in ERMCustomerStatistics.Codeunit.al). The change is a deliberate, well-tested fix (bug 640988) that moves the 'Date Filter' reset from a once-at-open, empty-only check into an unconditional per-record reset inside the GUI code path so a wider Date Filter inherited from a caller no longer leaks into the Balance Due cue/FlowField. No sub-skill found a knowledge-backed violation, and the super-skill self-review pass found no cross-cutting defect meeting the agent-finding precision bar.

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.

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

This PR moves the Customer Card default Date Filter from OnOpenPage to OnAfterGetCurrRecordFunc, and always sets it to 0D..WorkDate(). That fixes the main Balance Due (LCY) FlowField when the card is opened with an inherited document or journal date filter.

The fix is not complete for every display path. StartBackgroundCalculations() still runs before the new SetRange, so the background task receives the inherited Rec.GetView() and can calculate the payments cue from the wrong date context. The W1 test covers the top FlowField, but not that cue.

Suggestions

S1 - Reset before starting background calculations
StartBackgroundCalculations() is called before Rec.SetRange("Date Filter", 0D, WorkDate()). The background task stores Rec.GetView() and later calls Customer.CalcOverdueBalance(), so the overdue payments cue can still use the leaked filter. Move the reset before the background task, or pass a clean view to the calculation.

S2 - Test the overdue cue path
The new test asserts CustomerCard."Balance Due (LCY)", which is the card FlowField. It does not assert the asynchronous Balance Due cue that uses OverdueBalance. Add coverage for that cue or the background calculation so the FlowField/cue bug cannot return.

S3 - Keep non-GUI date filter behavior
The deleted OnOpenPage code also set a default Date Filter for non-GUI page runs. The new code only runs in the GUI OnAfterGetCurrRecordFunc path. Keep the old default for OData/API/background clients, or add a test that proves the behavior should change.

Risk assessment and necessity

Risk: This is financially sensitive UI data: users can make collection decisions from the overdue balance and payments cue. The main FlowField is improved, but the background cue still receives the old filter order in every changed BaseApp layer. No event subscribers or publisher contracts are involved; the Vendor Card analog still uses a guarded Date Filter, so it is not proof that this approach covers all equivalent paths.

Necessity: The bug is valid and important. ADO Bug 640988 describes a direct Customer Card value of 45 and a document/journal-opened value of 145 for the same customer. The scope is justified, but the fix needs to clean the date context before all overdue calculations use it.


[AI-PR-REVIEW] version=1 system=github pr=8940 round=1 by=alexei-dobriansky at=2026-07-08 lastSha=d4c04d17cb88f721e51a807e442fb5e2eab5153f suggestions=S1,S2,S3

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

@neeleshsinghal please review if the suggestions are valid

@neeleshsinghal

Copy link
Copy Markdown
Contributor Author

@alexei-dobriansky yes suggestions seems to be valid. I ll fix and commit the changes.

…Bug-640988-Overdue-Balance-filters-do-not-behave-as-expected
@neeleshsinghal

Copy link
Copy Markdown
Contributor Author

@alexei-dobriansky
Changes has been done based on the suggestions.

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Agentic PR Review - Round 2

Recommendation: Request Changes

What this PR does

The round-2 commit moves the Customer Card Date Filter reset before StartBackgroundCalculations() in the GUI path. In W1, Rec.SetRange("Date Filter", 0D, WorkDate()) now runs at src/Layers/W1/BaseApp/Sales/Customer/CustomerCard.Page.al:2385, before the background task starts at line 2389; the same ordering was applied to the other changed Customer Card pages.

It also restores the old non-GUI guarded default in OnOpenPage: W1 now sets 0D..WorkDate() only when the incoming Date Filter is blank at lines 2446-2449. The remaining gap is test coverage: the new test calls Customer.CalcOverdueBalance() directly and manually resets the record filter. It does not open the Customer Card in a leaking document or journal context, and it does not prove that the page background/cue path uses the clean view.

Status of previous suggestions
ID Title Status Author response
S1 Reset before starting background calculations Addressed The code now resets the GUI Date Filter before StartBackgroundCalculations() (src/Layers/W1/BaseApp/Sales/Customer/CustomerCard.Page.al:2385-2389).
S2 Test the overdue cue path Not addressed A new test was added, but it is record-level only (src/Layers/W1/Tests/ERM/ERMCustomerStatistics.Codeunit.al:1124-1165). It does not open TestPage "Customer Card" or exercise the background/cue path.
S3 Keep non-GUI date filter behavior Addressed The non-GUI OnOpenPage path again applies the default only when the incoming Date Filter is blank (src/Layers/W1/BaseApp/Sales/Customer/CustomerCard.Page.al:2446-2449).
New observations (commits since round 1)

None - no new issue found beyond the still-open S2 test gap.

Risk assessment and necessity

Risk: The functional ordering problem is fixed in the Customer Card page path, and the non-GUI fallback is restored. The remaining risk is regression risk: the new test would still pass if the page reset moved back after StartBackgroundCalculations(), because it does not run the page path that was broken.

Necessity: The bug is still valid and important. Users can make collection decisions from overdue balances, so this financial UI path needs a regression test that covers the leaking document or journal context and the Customer Card value shown from that path.


[AI-PR-REVIEW] version=1 system=github pr=8940 round=2 by=alexei-dobriansky at=2026-07-08 lastSha=5ec8e30087280b206e4920384e07ce3e40bedd03 suggestions=S1:addressed,S2:notaddressed,S3:addressed parentRound=1

@neeleshsinghal

Copy link
Copy Markdown
Contributor Author

@alexei-dobriansky S2 fixed.

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Agentic PR Review - Round 3

Recommendation: Request Changes

What this PR does

The round-3 commit updates the W1 test for Bug 640988. It now opens TestPage "Customer Card" with a wide leaked Date Filter and asserts the Balance Due (LCY) FlowField and Balance (LCY) values.

That improves the page FlowField coverage, but it still does not cover the overdue payments cue/background value. The cue is field("Balance Due"; OverdueBalance) on the Customer Card, and it is filled from Customer Card Calculations after StartBackgroundCalculations() passes Rec.GetView(). The new test still calls Customer.CalcOverdueBalance() directly and never asserts the Customer Card cue value, so it would not fail if the page again captured the leaked view before resetting the Date Filter.

Status of previous suggestions
ID Title Status Author response
S1 Reset before starting background calculations Addressed Still addressed. The Customer Card resets Date Filter before StartBackgroundCalculations() in the GUI path.
S2 Test the overdue cue path Not addressed A reply says S2 is fixed, but the new test asserts Customer.CalcOverdueBalance() directly and CustomerCard."Balance Due (LCY)". It does not assert CustomerCard."Balance Due" / OverdueBalance, the cue value filled by the background task.
S3 Keep non-GUI date filter behavior Addressed Still addressed. The guarded non-GUI OnOpenPage default remains in place.
New observations (commits since round 2)

None - the only remaining issue is the still-open S2 test gap.

Risk assessment and necessity

Risk: The product code change remains narrow and the ordering fix still looks correct. The remaining risk is regression risk: the new test does not prove that the asynchronous cue uses the clean Customer Card view.

Necessity: The bug is valid and important. Users can make collection decisions from the overdue value, so the regression test needs to cover the document or journal leaked-filter path and the actual Customer Card overdue cue value.


[AI-PR-REVIEW] version=1 system=github pr=8940 round=3 by=alexei-dobriansky at=2026-07-09 lastSha=17417f1cfd8bb2997665f13c2d3c214643df8800 suggestions=S1:addressed,S2:notaddressed,S3:addressed parentRound=2

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

@neeleshsinghal will approve after the buddy build goes through

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Agentic PR Review - Round 4

Recommendation: Request Changes

What this PR does

The round-4 commit updates the W1 regression test. It now checks that the Customer Card Date Filter no longer contains the leaked upper bound, still asserts Balance Due (LCY) and Balance (LCY), and then replays Customer.CalcOverdueBalance() on a separate customer record using the page filter.

This is closer to the broken path, but it still does not assert the actual Customer Card cue value. The cue is field("Balance Due"; OverdueBalance), and OverdueBalance is assigned only from the Customer Card Calculations background task result. The test verifies a manual record-level replay, not CustomerCard."Balance Due" after the background task populates the page value.

Status of previous suggestions
ID Title Status Author response
S1 Reset before starting background calculations Addressed Still addressed. The Customer Card resets Date Filter before StartBackgroundCalculations() in the GUI path, and the round-4 commit does not change that product code.
S2 Test the overdue cue path Not addressed No new reply was posted after round 3. The new commit checks the page filter and manually calls CustomerForBackgroundTask.CalcOverdueBalance(), but it still does not assert CustomerCard."Balance Due" / OverdueBalance, the cue value filled by the background task.
S3 Keep non-GUI date filter behavior Addressed Still addressed. The guarded non-GUI OnOpenPage default remains in place, and the round-4 commit does not change that product code.
New observations (commits since round 3)

None - the only remaining issue is the still-open S2 test gap.

Risk assessment and necessity

Risk: The product fix still looks narrow and correctly ordered. The remaining risk is regression risk: the test can pass while the page cue/background assignment path is still not asserted.

Necessity: The bug is valid and important. The work item shows overdue balance values changing from 45 to 145 when the Customer Card is opened from a document or journal, so the regression test should cover that context and the actual overdue cue value shown on the page.


[AI-PR-REVIEW] version=1 system=github pr=8940 round=4 by=alexei-dobriansky at=2026-07-09 lastSha=10476ea7a6aeeb5204c7048c725525604178f046 suggestions=S1:addressed,S2:notaddressed,S3:addressed parentRound=3

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Agentic PR Review - Round 5

Recommendation: Accept

What this PR does

The round-5 commit finishes the remaining test gap. The W1 regression test now opens the Customer Card with a leaked wide Date Filter, verifies the card filter was reset to 0D..WorkDate(), asserts both Balance Due (LCY) and the Balance Due cue, and keeps the Balance (LCY) assertion.

The product code still resets the GUI Date Filter before StartBackgroundCalculations() captures Rec.GetView(). Customer Card Calculations then receives the clean view for the cue, while Balance (LCY) remains unaffected by the date filter and CalcOverdueBalance() behavior is unchanged. The country-layer changes only remove the redundant second reset after the background task; each touched layer still has the reset before the task plus the guarded non-GUI default.

Status of previous suggestions
ID Title Status Author response
S1 Reset before starting background calculations Addressed Still addressed. The Date Filter reset remains before StartBackgroundCalculations() in W1 and the touched country layers.
S2 Test the overdue cue path Addressed The round-5 test now asserts CustomerCard."Balance Due".AsDecimal(), the page field backed by the background-task OverdueBalance value.
S3 Keep non-GUI date filter behavior Addressed Still addressed. The guarded non-GUI OnOpenPage default remains in W1 and the touched country layers.
New observations (commits since round 4)

None - no new issue found in commit d6faef4a. The diff only adds the missing cue assertion and removes duplicate post-background resets from APAC, BE, ES, FR, GB, IT, NA, NL, NO, and RU Customer Card pages.

Risk assessment and necessity

Risk: The remaining regression surface is low. The change touches a financial UI value, but the functional product code path is unchanged in W1 in this round, the country-layer removal is redundant because the reset already happens before background calculations, and no public API or event contract changes.

Necessity: Bug 640988 is valid; opening the Customer Card from a document or journal can show 145 instead of 45. The round-5 test now covers the actual page cue and FlowField values for the leaked filter scenario, so the previous review blocker is resolved.


[AI-PR-REVIEW] version=1 system=github pr=8940 round=5 by=alexei-dobriansky at=2026-07-09T23:08:49Z lastSha=226d13a04efeedcf27934a13b6947f73d5b52564 suggestions=S1:addressed,S2:addressed,S3:addressed parentRound=4

@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Agent} \quad \color{gray}{\texttt{\small Iteration\ 8}}$

This fix isolates the GUI path from a leaked wider 'Date Filter' by unconditionally resetting Rec.SetRange("Date Filter", 0D, WorkDate()) inside OnAfterGetCurrRecordFunc (called on every GUI record change), but the OData/API client path (OnAfterGetCurrRecordFuncOData, invoked when ClientTypeManagement.GetCurrentClientType() is ODataV4 or Api) never resets the filter on subsequent record navigations — only the one-time conditional reset in OnOpenPage's non-GUI branch applies, and only if the filter was still empty at that moment.

The page directly binds field("Balance Due (LCY)"; Rec."Balance Due (LCY)") to this FlowField, so an OData/API caller that inherits or sets a wider 'Date Filter' upper bound before/while iterating Customer Card entities (the same class of bug this PR's test 'OverdueBalanceIsolatedFromLeakedWideDateFilterOnCustomer' targets for the GUI) can still receive an inflated 'Balance Due (LCY)' value through the API surface. If material impact would otherwise be major, note it here: this should be promoted to a knowledge-backed rule and the fix extended to OnAfterGetCurrRecordFuncOData (or centralized in a single place both paths call) before being treated as fully resolved. This applies identically across all ten localized layers (APAC, BE, ES, FR, GB, IT, NA, NL, NO, RU) since the same pattern was replicated verbatim.

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

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Agent} \quad \color{gray}{\texttt{\small Iteration\ 11}}$

The PR fixes a real bug where a wide 'Date Filter' leaked onto the Customer record (e.g.

from a document/journal caller) corrupted 'Balance Due (LCY)' (CalcFormula uses upperlimit("Date Filter")) and the background-task 'Balance Due' cue. The fix only resets Rec.SetRange("Date Filter", 0D, WorkDate()) inside OnAfterGetCurrRecordFunc, which runs solely on the GuiAllowed()=true branch of trigger OnAfterGetCurrRecord. The other two branches of that same trigger are untouched: OnAfterGetCurrRecordFuncOData() (ClientType OData/Api) calls CustomerMgt.CalculateStatistic(Rec, ...) directly against Rec without resetting the filter, and the plain non-GUI/non-OData branch calls StartBackgroundCalculations() directly, which captures Rec.GetView() as-is. Both branches can still be reached with the same leaked wide Date Filter (e.g. a web-service/OData client or a background job that opens the page with a pre-set filter) and will reproduce the exact miscalculation this PR sets out to fix, just outside the interactive path the new test exercises. Note: if this can be confirmed as commonly reachable in production (e.g. OData API consumers of the Customer/Customer Card entity), the real severity is closer to major/blocker than the capped 'minor' shown here — recommend adding the same Date Filter reset (or a shared helper) to OnAfterGetCurrRecordFuncOData and to the background-task branch, and extending the new test to cover both. This same gap is replicated identically across all 11 changed layer files (APAC, BE, ES, FR, GB, IT, NA, NL, NO, RU, W1), since each applies the same partial fix.

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

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

@neeleshsinghal

Copy link
Copy Markdown
Contributor Author

@neeleshsinghal will approve after the buddy build goes through

@alexei-dobriansky Sure.

@neeleshsinghal

Copy link
Copy Markdown
Contributor Author

@alexei-dobriansky Can you please approve this PR?

@neeleshsinghal neeleshsinghal enabled auto-merge July 14, 2026 21:10
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 Linked Issue is linked to a Azure Boards work item

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants