[master] Overdue Balance (LCY) filters do not behave as expected if you open the Customer Card from a document or journal.#8940
Conversation
…Bug-640988-Overdue-Balance-filters-do-not-behave-as-expected
Copilot PR ReviewIteration 12 · Outcome: completed
Knowledge source: https://github.com/microsoft/BCQuality@186d8a131465475c79244d994acb872cd5c0d4bf Orchestrator pre-filter (2 file(s) excluded)
Findings produced by the AL review agent v1.7.3. Reply 👎 on any inline comment to flag false positives. |
Agentic PR Review - Round 1Recommendation: Request ChangesWhat this PR doesThis PR moves the Customer Card default The fix is not complete for every display path. SuggestionsS1 - Reset before starting background calculations S2 - Test the overdue cue path S3 - Keep non-GUI date filter behavior Risk assessment and necessityRisk: 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 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.
|
|
@neeleshsinghal please review if the suggestions are valid |
|
…Bug-640988-Overdue-Balance-filters-do-not-behave-as-expected
|
@alexei-dobriansky |
Agentic PR Review - Round 2Recommendation: Request ChangesWhat this PR doesThe round-2 commit moves the Customer Card It also restores the old non-GUI guarded default in Status of previous suggestions
New observations (commits since round 1)None - no new issue found beyond the still-open S2 test gap. Risk assessment and necessityRisk: 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 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.
|
|
@alexei-dobriansky S2 fixed. |
Agentic PR Review - Round 3Recommendation: Request ChangesWhat this PR doesThe round-3 commit updates the W1 test for Bug 640988. It now opens That improves the page FlowField coverage, but it still does not cover the overdue payments cue/background value. The cue is Status of previous suggestions
New observations (commits since round 2)None - the only remaining issue is the still-open S2 test gap. Risk assessment and necessityRisk: 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.
|
|
@neeleshsinghal will approve after the buddy build goes through |
Agentic PR Review - Round 4Recommendation: Request ChangesWhat this PR doesThe round-4 commit updates the W1 regression test. It now checks that the Customer Card This is closer to the broken path, but it still does not assert the actual Customer Card cue value. The cue is Status of previous suggestions
New observations (commits since round 3)None - the only remaining issue is the still-open S2 test gap. Risk assessment and necessityRisk: 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.
|
…Bug-640988-Overdue-Balance-filters-do-not-behave-as-expected
Agentic PR Review - Round 5Recommendation: AcceptWhat this PR doesThe round-5 commit finishes the remaining test gap. The W1 regression test now opens the Customer Card with a leaked wide The product code still resets the GUI Status of previous suggestions
New observations (commits since round 4)None - no new issue found in commit Risk assessment and necessityRisk: 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.
|
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 |
…lters-do-not-behave-as-expected
…lters-do-not-behave-as-expected
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 |
@alexei-dobriansky Sure. |
|
@alexei-dobriansky Can you please approve this PR? |
…lters-do-not-behave-as-expected
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.