[Master]-Opening price list from Customer Card is extremely slow on large datasets (~220s) due to AddAllSourceType call#9265
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses a major performance regression when opening Sales Price Lines from the Customer Card on large datasets by reducing unnecessary record loading/iteration work in Price List source filtering, and it adds a regression UI test to validate that “All Customers” price lines are fully returned.
Changes:
- Optimized source-line discovery/marking in
Price List Managementby usingIsEmpty()for existence checks and limiting loaded fields during marking. - Added a UI test that creates many “All Customers” price lines and verifies they are all shown when opened from the Customer Card.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Layers/W1/BaseApp/Pricing/PriceList/PriceListManagement.Codeunit.al | Optimizes filtering/marking logic to reduce data loading and improve performance. |
| src/Layers/W1/Tests/ERM/PriceListsUI.Codeunit.al | Adds a regression UI test to ensure “All Customers” lines are shown when opened from Customer Card. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot PR ReviewIteration 3 · Outcome: completed Knowledge source: https://github.com/microsoft/BCQuality@822cae1b2771ac25f665f73369f69093bd4fd630 Findings by domainFindings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).
Totals: 0 knowledge-backed · 1 agent findings. Orchestrator pre-filter (13 file(s) excluded)
Findings produced by the Copilot CLI agent against BCQuality at |
Agentic PR Review - Round 1Recommendation: Request ChangesWhat this PR doesThis PR tries to speed up Price List Line Review opened from Customer Card by using The change preserves the exact source tuple behavior: each Relationship to the other PRs: PR 9219 was closed after round-2 Request Changes. PR 9265 is a different replacement/alternative, not the same diff. PR 9269 is still open for the same work item and is closer to the OR-filter approach from PR 9219, so it is a competing duplicate alternative. SuggestionsS1 - Still materializes all source lines S2 - Test does not prove the fix S3 - Close the duplicate alternative PR Risk assessment and necessityRisk: This is a pricing UI path, so incorrect filtering can show the wrong price list lines. This PR keeps the exact tuple filters and keeps the event contract, which is good. The remaining risk is that the performance bug is not actually fixed because the broad Necessity: ADO bug 641061 is valid and important. Opening Sales Prices from Customer Card can take about 220 seconds on large datasets, so a fix is needed. The scope should stay in this filtering path, but it must remove the expensive materialization, not only reduce loaded fields.
|
|
s1 Addressed |
Agentic PR Review - Round 2Recommendation: Request ChangesWhat this PR doesThe new commit adds a source pre-check so It does not fully fix the reported Customer Card scenario. If both customer-specific rows and Status of previous suggestions
New observations (commits since round 1)S4 - Source filter event now runs twice Risk assessment and necessityRisk: This is a pricing UI path, so incorrect filters can show wrong price list lines. The remaining performance risk is high because large Necessity: ADO bug 641061 is valid and important. The reported Customer Card Sales Prices path can take about 220 seconds on large datasets, so a fix is needed. The fix must remove the broad materialization while keeping exact source tuple semantics, and one duplicate PR should be closed.
|
AB#641061