Skip to content

[master] Contact No. Lookup on Sales Quote returns All Customers instead of related Contacts after Updating to v28.2 - regression.#9374

Merged
neeleshsinghal merged 5 commits into
mainfrom
bugs/Bug-642199-Contact-No.-Lookup-on-Sales-Quote-returns-All-Customers
Jul 15, 2026
Merged

[master] Contact No. Lookup on Sales Quote returns All Customers instead of related Contacts after Updating to v28.2 - regression.#9374
neeleshsinghal merged 5 commits into
mainfrom
bugs/Bug-642199-Contact-No.-Lookup-on-Sales-Quote-returns-All-Customers

Conversation

@neeleshsinghal

@neeleshsinghal neeleshsinghal commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Workitem: Bug 642199: [master] [ALL-E] Contact No. Lookup on Sales Quote returns All Customers instead of related Contacts after Updating to v28.2 - regression.

Fixes AB#642199

Issue: On a Sales Quote, the Contact No. field lookup showed all contacts (customers and vendors) instead of only the selected customer's contacts (regression in v28.2; other documents worked fine).

Cause: In SelltoContactLookup() the filter condition included ("Document Type" <> "Document Type"::Quote), which skipped the contact-relation filtering entirely for quote documents.

Solution: Removed the Quote exclusion so quotes apply the same customer-based contact filter as other documents, while the "Sell-to Customer No." <> '' guard preserves the quote-for-a-prospect flow.

@github-actions github-actions Bot added this to the Version 29.0 milestone Jul 13, 2026
@neeleshsinghal neeleshsinghal added the SCM GitHub request for SCM area label Jul 13, 2026
@neeleshsinghal neeleshsinghal marked this pull request as ready for review July 14, 2026 04:33
@neeleshsinghal neeleshsinghal requested a review from a team July 14, 2026 04:33
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Copilot PR Review

Iteration 3 · Outcome: completed

All sub-skills ran; no domain-specific knowledge violations found. The diff is a targeted, consistently-applied logic fix (removing an incorrect Document-Type=Quote exclusion from the Sell-to Contact lookup filter across all 15 country layers) backed by a new automated test.

Knowledge source: https://github.com/microsoft/BCQuality@822cae1b2771ac25f665f73369f69093bd4fd630

Orchestrator pre-filter (13 file(s) excluded)

  • layer-disabled (knowledge) : 13 file(s)

Findings produced by the Copilot CLI agent against BCQuality at 822cae1b2771ac25f665f73369f69093bd4fd630. 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 removes the Quote exclusion in Sales Header.SelltoContactLookup() across the Sales Header layer copies. That makes the Contact No. lookup on a Sales Quote use the same customer contact filtering as the other sales documents when Sell-to Customer No. is filled.

The code fix matches the root cause. The Sales Quote Contact No. page field calls Rec.SelltoContactLookup(), and the remaining Sell-to Customer No. <> '' guard keeps the blank-customer quote flow unchanged. Purchase and Service already use the same vendor/customer based contact filtering pattern.

Suggestions

S1 - Test the Contact No. lookup

The new test calls SalesQuote."Sell-to Contact".Lookup(), but the changed code runs from the Contact No. / "Sell-to Contact No." lookup. Please change the test to look up SalesQuote."Sell-to Contact No.", and use bug ID 642199 in the scenario comment. Otherwise this test can pass even if the regression returns all contacts again.

Risk assessment and necessity

Risk: The runtime change is narrow, but it affects all existing Sales Header layer copies. The main risk is test confidence: the added test exercises the Contact name lookup path, not the changed Contact No. lookup path. There is no public API or event signature change.

Necessity: The linked work item is a Bug and describes a real v28.2 regression. Without this fix, Sales Quote users can see unrelated contacts and choose the wrong contact for the selected customer. The code scope is right, but the regression test must cover the actual lookup that regressed.


[AI-PR-REVIEW] version=1 system=github pr=9374 round=1 by=alexei-dobriansky at=2026-07-14T05:46:40Z lastSha=458efada863b5c2cf6f0980e3b75e6346e562534 suggestions=S1

…Bug-642199-Contact-No.-Lookup-on-Sales-Quote-returns-All-Customers
@neeleshsinghal

Copy link
Copy Markdown
Contributor Author

@alexei-dobriansky Suggested fix done.

@github-actions

Copy link
Copy Markdown
Contributor

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

SelltoContactLookup (SalesHeader.Table.al) was changed to always filter the contact list by the customer's Company No., removing the prior '("Document Type" <> "Document Type"::Quote)' exemption — this is the intended fix for bug 642199 and is well covered by the new test.

However, the sibling OnValidate trigger for the same field still contains that exemption a few hundred lines earlier: '("Sell-to Customer No." <> '') and ("Sell-to Contact No." <> '') and ("Document Type" <> "Document Type"::Quote)' guarding the call to CheckContactRelatedToCustomerCompany. After this change, a user can no longer pick an off-company contact via the lookup on a Quote, but can still type/paste one directly into the field on a Quote without triggering the company-relation error that Orders/Invoices/Credit Memos get on the same field. The lookup and validation are now inconsistent specifically for Quotes, which is exactly the scenario this PR targets. If the intent is that Quotes should also enforce company-matching (consistent with the new lookup behavior and with Bill-to Contact No., which validates the company relation unconditionally for all document types), the same '<> "Document Type"::Quote' exemption on the Validate trigger should be reconsidered/removed as part of this fix; if it is intentionally left permissive for manual entry, that asymmetry deserves a code comment so it isn't mistaken for a leftover of the bug being fixed.

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

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

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Agentic PR Review - Round 2

Recommendation: Accept

What this PR does

The new commit updates the regression test from round 1. The test now uses SalesQuote."Sell-to Contact No.".Lookup() and the scenario comment now references bug 642199.

This addresses the previous concern. The Sales Quote Contact No. page field is bound to "Sell-to Contact No.", and its lookup calls Rec.SelltoContactLookup(), which is the changed code path. The modal page handler still verifies that the lookup is filtered by the customer contact company.

Status of previous suggestions
ID Title Status Author response
S1 Test the Contact No. lookup Addressed Stated that the suggested fix was done. Commit c22956aa changed the test to call SalesQuote."Sell-to Contact No.".Lookup() and updated the scenario to 642199.
New observations (commits since round 1)

None - changes only addressed prior suggestions.

Risk assessment and necessity

Risk: The round-2 commit only changes the test, so it adds no runtime regression surface. The test now exercises the same Sales Quote Contact No. lookup path that the bug describes.

Necessity: The linked work item is a Bug for a v28.2 regression where Sales Quote Contact No. lookup showed unrelated contacts. Keeping this regression test is necessary because the bug path is narrow and easy to miss.


[AI-PR-REVIEW] version=1 system=github pr=9374 round=2 by=alexei-dobriansky at=2026-07-14T11:04:49Z lastSha=c22956aa226539ec9606cf0ad3efe5170eba7b36 suggestions=S1:addressed parentRound=1

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a regression in v28.2 where Sales Quote “Sell-to Contact No.” lookup stopped filtering to the selected customer’s related contacts, causing the lookup to show all contacts instead.

Changes:

  • Removed the "Document Type" <> Quote exclusion in SelltoContactLookup() so Sales Quotes apply the same customer-based contact filtering as other sales documents.
  • Applied the same fix across multiple localized BaseApp layers (W1 + country layers).
  • Added a targeted test covering Sales Quote contact lookup filtering for a customer scenario.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Layers/W1/Tests/Marketing/TestContactLookup.Codeunit.al Adds a regression test for Sales Quote “Sell-to Contact No.” lookup filtering.
src/Layers/W1/BaseApp/Sales/Document/SalesHeader.Table.al Fixes SelltoContactLookup() by removing Quote exclusion from customer-based filtering.
src/Layers/SE/BaseApp/Sales/Document/SalesHeader.Table.al Same fix as W1 for SE layer.
src/Layers/RU/BaseApp/Sales/Document/SalesHeader.Table.al Same fix as W1 for RU layer.
src/Layers/NO/BaseApp/Sales/Document/SalesHeader.Table.al Same fix as W1 for NO layer.
src/Layers/NL/BaseApp/Sales/Document/SalesHeader.Table.al Same fix as W1 for NL layer.
src/Layers/NA/BaseApp/Sales/Document/SalesHeader.Table.al Same fix as W1 for NA layer.
src/Layers/IT/BaseApp/Sales/Document/SalesHeader.Table.al Same fix as W1 for IT layer.
src/Layers/GB/BaseApp/Sales/Document/SalesHeader.Table.al Same fix as W1 for GB layer.
src/Layers/FR/BaseApp/Sales/Document/SalesHeader.Table.al Same fix as W1 for FR layer.
src/Layers/FI/BaseApp/Sales/Document/SalesHeader.Table.al Same fix as W1 for FI layer.
src/Layers/ES/BaseApp/Sales/Document/SalesHeader.Table.al Same fix as W1 for ES layer.
src/Layers/CZ/BaseApp/Sales/Document/SalesHeader.Table.al Same fix as W1 for CZ layer.
src/Layers/CH/BaseApp/Sales/Document/SalesHeader.Table.al Same fix as W1 for CH layer.
src/Layers/BE/BaseApp/Sales/Document/SalesHeader.Table.al Same fix as W1 for BE layer.
src/Layers/APAC/BaseApp/Sales/Document/SalesHeader.Table.al Same fix as W1 for APAC layer.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Layers/W1/Tests/Marketing/TestContactLookup.Codeunit.al

@qasimikram qasimikram left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The production fix is focused and consistently applied across all layers, and the regression test covers the quote-specific filtering scenario.

S1: Please add [Scope('OnPrem')] to SellToContactLookupOnSalesQuoteWithCustomerTest. It references ContactListModalPageHandler, which is scoped OnPrem, and the existing tests that use this handler carry the same scope.

@neeleshsinghal neeleshsinghal added this pull request to the merge queue Jul 15, 2026
Merged via the queue into main with commit b5e2e47 Jul 15, 2026
482 of 492 checks passed
@neeleshsinghal neeleshsinghal deleted the bugs/Bug-642199-Contact-No.-Lookup-on-Sales-Quote-returns-All-Customers branch July 15, 2026 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

SCM GitHub request for SCM area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants