Skip to content

[Subcontracting] Bug 648949: Allow subcontracting direct transfers from warehouse locations - #11280

Open
alexei-dobriansky wants to merge 2 commits into
mainfrom
fix/648949-subcontracting-warehouse-direct-transfer
Open

[Subcontracting] Bug 648949: Allow subcontracting direct transfers from warehouse locations#11280
alexei-dobriansky wants to merge 2 commits into
mainfrom
fix/648949-subcontracting-warehouse-direct-transfer

Conversation

@alexei-dobriansky

@alexei-dobriansky alexei-dobriansky commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

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

  • I read the full diff and it contains only changes I intended.
  • I built the affected app(s) locally with no new analyzer warnings.
  • I ran the change in Business Central and confirmed it behaves as expected.
  • I added or updated tests for the new behavior.

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 --check passed.

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.

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
@alexei-dobriansky
alexei-dobriansky requested a review from a team September 9, 2026 15:06
@alexei-dobriansky
alexei-dobriansky requested a review from a team as a code owner September 9, 2026 15:06
@github-actions github-actions Bot added the AL: Apps (W1) Add-on apps for W1 label Sep 9, 2026
@alexei-dobriansky alexei-dobriansky self-assigned this Sep 9, 2026
@alexei-dobriansky alexei-dobriansky added Team: SCM GitHub request for SCM area Subcontracting Subcontracting related activities Ownership: Manual Preserve the manually selected team ownership labels Sep 9, 2026
@alexei-dobriansky alexei-dobriansky changed the title Bug 648949: Allow subcontracting direct transfers from warehouse locations [Subcontracting] Bug 648949: Allow subcontracting direct transfers from warehouse locations Sep 9, 2026
@github-actions github-actions Bot added this to the Version 30.0 milestone Sep 9, 2026
@alexei-dobriansky

Copy link
Copy Markdown
Contributor Author

Good Sense Reviewer - Round 1

Recommendation: Accept with Suggestions

What this PR does

This 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 fit

Fit: 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.

Suggestions

S1 (🟠 Moderate): Cover the pick-based warehouse path
The old report check blocked both picking and shipment, but the new posting tests use a source location that only requires shipment. Please add a Require Pick or Directed Put-away and Pick subcontracting case that creates the needed pick before posting. That covers the other warehouse path this change now allows.

Risk assessment and necessity

Risk: 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.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11280 round=1 by=alexei-dobriansky at=2026-09-09T18:34:27.6115374Z lastSha=253a542a5ed276c7ffdd1f21af90c8069e688c28 reviewKey=e233a52f948d45ee0f6e52aeab822bddd6e0ed3717bcc2fa27c4934262a3181c suggestions=S1@9589202f

attilatoury
attilatoury previously approved these changes Sep 10, 2026

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.

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
@alexei-dobriansky

Copy link
Copy Markdown
Contributor Author

Predrag Maricic (@PredragMaricic) addressed your review in c549b2d.

Added two positive end-to-end subcontracting scenarios:

  • Require Pick only: create and post the component transfer through an Inventory Pick.
  • Directed Put-away and Pick: create/register the Warehouse Pick, assert the picked quantity, and post the Warehouse Shipment.

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.

@alexei-dobriansky

Copy link
Copy Markdown
Contributor Author

Good Sense Reviewer - Round 2

Recommendation: Accept

What this PR does

This 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
ID Title Status Author response
S1 Cover the pick-based warehouse path Addressed The follow-up commit adds Require Pick and Directed Put-away and Pick end-to-end scenarios.
New observations (commits since round 1)

None - changes only addressed prior suggestions.

Risk assessment and necessity

Risk: 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.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11280 round=2 by=alexei-dobriansky at=2026-09-11T12:34:55Z lastSha=c549b2ddc3fe3c2bc11b230342f60ec283c65ed1 reviewKey=e173e5bc5a4ca811ce74b6ee4cfaab9101ffe1f9e2cda94ddf064a292a08208f suggestions=S1@9589202f:addressed parentRound=1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AL: Apps (W1) Add-on apps for W1 Ownership: Manual Preserve the manually selected team ownership Subcontracting Subcontracting related activities Team: SCM GitHub request for SCM area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants