Bug 640958: Guided error for subcontracting direct transfer from whse-handling location

[alexei-dobriansky]

Fixes AB#640958

SubcCreateTransfOrder.InsertTransferHeader unconditionally validated "Direct Transfer" := true when no in-transit route existed; the OnValidate calls VerifyNoOutboundWhseHandlingOnLocation which TestFields Require Pick/Require Shipment = false, throwing a raw error for warehouse-handling source locations. Add CheckDirectTransferAllowed that raises a guided error (set up an in-transit route or use Direct Transfer posting) when the source location requires a pick or shipment and posting is not Direct Transfer. Adds a negative test.

[alexei-dobriansky]

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

This PR adds a pre-check before the subcontracting report falls back to a direct transfer when no in-transit route is found. The bug scenario is valid and clear: a source location that requires pick or shipment currently gets a raw Location TestField error instead of guidance to set up an in-transit route or use Direct Transfer posting.

The check is in the right area of InsertTransferHeader, and the default Shipment and Receipt case is covered by the new test. However, the check reads TransferHeader."Direct Transfer Posting" before Validate("Direct Transfer", true) calculates the effective posting type from Inventory Setup. That means one valid configuration named in the new error message can be blocked incorrectly.

Suggestions

S1 - Effective posting type is checked too early
The new check reads TransferHeader."Direct Transfer Posting" before Validate("Direct Transfer", true) fills it from Inventory Setup. With no transfer route, the field is blank, so a company that already uses Direct Transfer posting is still blocked. Calculate the effective posting type before raising the guided error.

S2 - Regression tests miss Direct Transfer setup
Add a test where Inventory Setup has Direct Transfer Posting = Direct Transfer and the source location requires shipment or pick. The report should create the transfer order instead of raising the guided error. This would catch the early-check regression.

Risk assessment and necessity

Risk: The change is narrow, but it sits in transfer order creation for subcontracting components and WIP. BaseApp verification shows Transfer Header.Validate("Direct Transfer", true) calculates Direct Transfer Posting from the transfer route or Inventory Setup, and the no-route case leaves the field blank until that validation runs. An incorrect pre-check can block valid direct-transfer setups.

Necessity: The bug is important because users with warehouse-handling source locations otherwise get a raw Location field error and no guidance. The scope is right for a bug fix, but it must preserve the documented workaround of setting Direct Transfer Posting to Direct Transfer.

---

[AI-PR-REVIEW] version=1 system=github pr=9085 round=1 by=alexei-dobriansky at=2026-07-06T09:02:07Z lastSha=2664e07997376b164e34b0cddfd0d1481034c1a8 suggestions=S1,S2

[alexei-dobriansky]

Addressed S1 and S2: the guard now computes the effective Direct Transfer Posting type (mirroring TransferHeader.GetDirectTransferPosting — route, then Inventory Setup) instead of reading the still-blank field, so a company using Direct Transfer posting is no longer wrongly blocked. Added a positive test (Inventory Setup = Direct Transfer -> transfer created) and pinned the Inventory Setup posting type in the negative test so both rely on explicit data. Pushed in 997782d.

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

CheckDirectTransferAllowed calls Location.Get(TransferFromLocation) but only reads the boolean fields "Require Pick" and "Require Shipment" afterward.

Location is a wide master table, so this Get materializes every column when two would do. This procedure runs inside the component/WIP transfer loops (HandleComponentsForPurchLine, HandleWIPTransferForPurchLine), so the extra column transfer repeats per unique transfer-from location. Add SetLoadFields before the Get.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

        Location.SetLoadFields("Require Pick", "Require Shipment");
        if not Location.Get(TransferFromLocation) then
            exit;

Knowledge:

• microsoft/knowledge/performance/use-setloadfields-for-partial-records.md

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

InsertTransferHeader already fetches the full Transfer Route record for (TransferFromLocation, TransferToLocationCode) at the outer 'if not TransferRoute.Get(...)' guard.

When that Get fails or the route has no In-Transit Code, CheckDirectTransferAllowed is called, which internally re-fetches the same Transfer Route row (with SetLoadFields) inside GetEffectiveDirectTransferPosting purely to read "Direct Transfer"/"Direct Transfer Posting". This is a second round-trip for a key already queried moments earlier, and InsertTransferHeader runs inside the component/WIP-transfer loops so the duplicate lookup repeats per distinct location pair. Consider passing the already-loaded TransferRoute record (or its two relevant fields) into CheckDirectTransferAllowed instead of re-querying.

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

[github-actions]

Copilot PR Review

Iteration 1 · Outcome: completed

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

Findings by domain

Findings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).

Domain	Findings	Knowledge-backed	Agent	Inline	Fallback
Performance	2	2	0	2	0

Totals: 2 knowledge-backed · 0 agent findings.

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.}