[Subcontracting] Bug 648949: Allow subcontracting direct transfers from warehouse locations - #11280
[Subcontracting] Bug 648949: Allow subcontracting direct transfers from warehouse locations#11280alexei-dobriansky wants to merge 2 commits into
Conversation
Remove the obsolete outbound warehouse guard for AB#648949 while preserving posting-mode and in-transit handling. Add straightforward Require Shipment regressions for component and WIP transfers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fb8e3994-dce0-4773-9daa-932d8bd11dc3
Good Sense Reviewer - Round 1Recommendation: Accept with SuggestionsWhat this PR doesThis change removes the subcontracting report's extra outbound-warehouse block when it creates direct transfer orders. The lower transfer code still chooses Direct Transfer Posting from the route or Inventory Setup, still keeps in-transit routes when needed, and still blocks inbound warehouse handling on the destination. The fix matches the reported regression: the old check rejected source locations that require shipment or picking before the transfer header could use the supported Shipment and Receipt flow. The added tests cover no-route header creation, component posting through a warehouse shipment, WIP-only posting, one-step direct transfer, and in-transit preservation. One warehouse variant remains worth covering directly in the subcontracting tests. Problem-solution fitFit: Strong The reported problem is that subcontracting-created direct transfers are blocked for source locations that the standard transfer flow supports. Removing only the extra report check addresses that block while leaving destination restrictions and existing direct-transfer modes in place. SuggestionsS1 (🟠 Moderate): Cover the pick-based warehouse path Risk assessment and necessityRisk: The changed report creates transfer orders that can later post inventory and WIP movement, so wrong behavior could affect subcontracting component flow. The code change is narrow: it removes a precheck, and BaseApp validation still controls posting mode, in-transit behavior, outbound warehouse requests, and inbound destination restrictions. No public API, event, schema, upgrade, or performance risk was found. Necessity: The change is needed for BC 29 because the current report blocks a supported direct-transfer warehouse process. Without it, users must change setup or use an in-transit route even when Shipment and Receipt direct transfer is the intended flow. The scope is appropriate for a bug fix and should not be applied to 28.x without the same lower-level support.
|
Predrag Maricic (PredragMaricic)
left a comment
There was a problem hiding this comment.
Review Summary
Request changes
The production change is appropriately narrow and preserves the existing posting-mode and in-transit handling, but the required regression coverage is incomplete.
Required change
Add positive end-to-end subcontracting tests for both remaining outbound warehouse configurations from AB#648949:
- Require Pick only - create and post the component transfer through an Inventory Pick.
- Directed Put-away and Pick - create/register the Warehouse Pick and post the Warehouse Shipment.
The removed guard covered both RequirePicking and RequireShipment, while the new posting tests exercise only Require Shipment. These two paths use distinct document, bin, registration, and quantity-update flows, so the Require Shipment scenario does not verify them. Please assert successful posting and the resulting transfer/component quantities for both paths before merging.
Existing CI is green, and I found no other blocking production-code issue.
Address Predrag review feedback on PR #11280 with separate Inventory Pick and Directed Put-away and Pick scenarios for AB#648949. Verify transfer posting, component quantities, and bin balances without conditional test helpers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fb8e3994-dce0-4773-9daa-932d8bd11dc3
c549b2d
|
Predrag Maricic (@PredragMaricic) addressed your review in c549b2d. Added two positive end-to-end subcontracting scenarios:
Both assert the posted transfer shipment/receipt quantities, source/destination inventory, resulting component quantities (including zero outstanding and in-transit quantities), and source bin balances. Given/When/Then stays in each test; the setup helpers are separate and branch-free. Production code is unchanged in this follow-up. Source checks passed. I did not compile or execute the new AL scenarios locally. |
Good Sense Reviewer - Round 2Recommendation: AcceptWhat this PR doesThis round adds regression coverage for the pick-based warehouse paths that were missing before. Production code is unchanged in the follow-up commit. The new tests create subcontracting component transfers, run the Inventory Pick or Warehouse Pick plus Warehouse Shipment flow, post the transfer, and assert shipment, receipt, item ledger, bin, and component quantities. That covers the same outbound warehouse paths that the removed guard used to block. Status of previous suggestions
New observations (commits since round 1)None - changes only addressed prior suggestions. Risk assessment and necessityRisk: The report creates transfer orders that later post inventory and WIP movement, so wrong behavior could affect subcontracting component flow. The code change remains narrow, and the new tests now exercise the Inventory Pick and directed warehouse pick/shipment paths in addition to the shipment-only path. Necessity: The change is still needed for BC 29 because the report blocked supported direct-transfer warehouse flows. The scope stays appropriate: it removes the obsolete guard while leaving destination warehouse restrictions, one-step direct transfer, and in-transit behavior under the lower-level transfer code.
|
What & why
Remove the obsolete outbound warehouse guard in
Subc. Create Transf. Order. BaseApp supports direct Shipment and Receipt transfers from warehouse-enabled source locations, but the report rejected them before creating the transfer.Retain the valid changes from 640958: posting-mode selection, explicit in-transit-code preservation, and the existing one-step/in-transit regression tests. Keep the equivalent 28.x guard unchanged because that release does not have the same BaseApp capability.
Replace the incorrect negative test and add end-to-end component transfer scenarios for Require Shipment, Require Pick only (Inventory Pick), and Directed Put-away and Pick (Warehouse Pick registration and Warehouse Shipment posting), plus WIP-only transfers without a route. Assert shipment/receipt quantities, component quantities, and relevant bin balances. Each scenario is readable in its test procedure; setup helpers are separate and branch-free.
Linked work
Fixes AB#648949
How I validated this
What I tested and the outcome
Source-only validation: inspected the introducing commit and BaseApp validation/posting paths; checked the final diff and the test/helper structure.
git diff --checkpassed.The added AL scenarios cover automatic shipment and receipt, component quantities and locations, and WIP ledger effects without physical inventory entries. Local compilation, publishing, and AL test execution were not performed for this change, as requested. These scenarios have not been runtime-validated.
Review follow-up: the two newly added pick-based scenarios were source-checked; local compilation and runtime execution were not performed for this update.
Risk & compatibility
No schema or dependency changes. Preserve BaseApp destination restrictions, one-step posting, and in-transit handling. This correction is for main/29 behavior and must not be backported to 28.x unchanged.