Skip to content

Bug 646576: Preserve manually changed component Flushing Method on subcontracting transfers - #10206

Merged
Chethan Thopaiah (ChethanT) merged 3 commits into
mainfrom
bugs/Subcontracting/bugs/646576-PreserveFlushingMethodOnSubcTransfer
Aug 13, 2026
Merged

Bug 646576: Preserve manually changed component Flushing Method on subcontracting transfers#10206
Chethan Thopaiah (ChethanT) merged 3 commits into
mainfrom
bugs/Subcontracting/bugs/646576-PreserveFlushingMethodOnSubcTransfer

Conversation

@ChethanT

@ChethanT Chethan Thopaiah (ChethanT) commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

When a subcontracting transfer moves a released production order component to (or back from) the subcontractor location, a manually chosen Flushing Method on the component was silently reset. Fixes AB#646576.

Root cause

Subcontracting shuttles the component by validating its Location Code. The base-app Prod. Order Component."Location Code" OnValidate calls GetUpdateFromSKU, which — when no SKU exists — copies planning parameters from the item, including Validate("Flushing Method", ...). So every automatic subcontracting location move overwrote the user's manual flushing method with the component item's flushing method. Resetting flushing on a user-initiated location change is by design, so the base app is left unchanged; the subcontracting app must preserve the value around its automatic shuttle moves.

Fix

New helper Subcontracting Management.ValidateProdOrderCompLocationPreservingFlushingMethod(var ProdOrderComponent, NewLocationCode):

  1. captures "Flushing Method",
  2. Validate("Location Code", NewLocationCode) (all existing side effects — SKU refresh, reservation checks, bin update — preserved),
  3. restores the captured value via plain assignment if it changed.

Plain assignment (not Validate) is essential: the Flushing Method OnValidate errors for Backward on a released order once consumption entries exist — precisely the state of the return leg. Plain assignment never runs OnValidate, never throws, and leaves Bin Code byte-identical (bin derivation does not depend on flushing method).

All subcontracting location shuttles now route through the helper:

  • Forward transfer report (Subc. Create Transf. Order, both the untracked and order-tracked/BindSubscription branches)
  • Return report (Subc. Create SubCReturnOrder)
  • Direct-transfer post (OnBeforeInsertDirectTransLine) and transfer receipt (OnCheckTransLine)
  • ChangeLocationOnProdOrderComponent — consignment/vendor-supplied forward move and original-location restore
  • De-subcontracting restore (Subc. Prod. Order Comp. Ext.) and return-delete revert (Subc. Transfer Management)

Intentionally excluded

  • The planning re-derivation in Subc. Prod. Order Comp. Ext. that pulls the location from the SKU ("Components at Location") for a component that was never shuttled (no Subc. Original Location Code) — that is legitimate planning-defaults behavior, not a subcontracting shuttle.
  • Planning Component equivalents — a different table with a transient/regenerated lifecycle, outside the scope of this released-prod-order-component bug.

Tests

Added to Subc. Purch. Subcont. Test (codeunit 139991):

  • SubcTransferPreservesManuallyChangedComponentFlushingMethod — forward transfer preserves a manual Backward method.
  • SubcReturnTransferPreservesManuallyChangedComponentFlushingMethod — full lifecycle (forward transfer -> partial consumption at the subcontractor -> return of remaining stock) preserves the manual method, exercising the plain-assignment restore with consumption entries present.

Full codeunit 139991 suite: 15/15 pass (13 pre-existing + 2 new), no regressions.

Both tests were confirmed to fail before the fix and pass after.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

…bcontracting transfers

Subcontracting automatically shuttles a released prod. order component between the shop floor and the subcontractor location by validating the component Location Code. That base-app validation re-reads planning parameters from the item/SKU (Prod. Order Component.GetUpdateFromSKU), which silently overwrites a manually chosen Flushing Method, so the user-selected value was lost every time a subcontracting transfer was created, posted or returned.

Add Subcontracting Management.ValidateProdOrderCompLocationPreservingFlushingMethod: it captures the flushing method, validates the location, and restores the manual value via plain assignment. Plain assignment (not Validate) is required because the Flushing Method OnValidate errors on a Backward method once consumption entries exist, which is exactly the state of the return leg. All subcontracting location shuttles are routed through the helper (forward transfer report, return report, direct-transfer post, transfer receipt, ChangeLocationOnProdOrderComponent consignment forward/restore, de-subcontracting restore, and return-delete revert). The planning re-derivation at SubcProdOrderCompExt that pulls the location from the SKU for a never-shuttled component is intentionally left untouched.

Tests: SubcTransferPreservesManuallyChangedComponentFlushingMethod (forward) and SubcReturnTransferPreservesManuallyChangedComponentFlushingMethod (full forward + partial consumption + return lifecycle).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ChethanT
Chethan Thopaiah (ChethanT) requested a review from a team August 12, 2026 14:18
@github-actions github-actions Bot added AL: Apps (W1) Add-on apps for W1 SCM GitHub request for SCM area labels Aug 12, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone Aug 12, 2026
Comment thread src/Apps/W1/Subcontracting/Test/Tests/SubcPurchSubcontTest.Codeunit.al Outdated
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

This PR routes automatic subcontracting component location moves through a helper that preserves a manually selected Flushing Method. The direction is correct for the reported bug: Location Code validation can re-read item or SKU planning parameters and overwrite the component value. However, BaseApp also recalculates Bin Code from Flushing Method, so restoring the flushing method by plain assignment can leave a persisted bin that was calculated from the temporary item or SKU flushing value.

Suggestions

S1 - Recalculate bin after restoring flushing
The helper restores Flushing Method after Validate("Location Code"), but the temporary value can already have updated Bin Code. Recalculate the bin after the restore, or make every caller do it, so the component does not keep a bin for the wrong flushing method.

S2 - Cover bin and SKU defaults
The new tests only assert Flushing Method. Add coverage with different item or SKU flushing defaults and different default bins, so the test fails if the helper preserves flushing but leaves Bin Code from the temporary default.

Risk assessment and necessity

Risk: The affected code updates released production order components during subcontracting transfer creation, return creation, direct transfer posting, receipt posting, and transfer-line deletion cleanup. If Bin Code is persisted for the wrong flushing method, later reservation, picking, or consumption flows can use the wrong warehouse bin.

Necessity: The bug is valid and important because an automatic subcontracting shuttle should not lose a user's manual component setup. The scope is right, but the helper must preserve the related bin state consistently before this is safe to merge.


[AI-PR-REVIEW] version=1 promptVersion=2 system=github pr=10206 round=1 by=alexei-dobriansky at=2026-08-12T14:55:23.4342079Z lastSha=f51c2487ecf096d6d519227edcd579d025ce0aca reviewKey=c8ab6fdf764ee4b5575ba1b508a56178a7e65ea902f6edac1a305118fcd85547 suggestions=S1@dcb1530c,S2@0eb90a25

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

Please review the suggestions

@ChethanT Chethan Thopaiah (ChethanT) added the Subcontracting Subcontracting related activities label Aug 12, 2026
@ChethanT

Copy link
Copy Markdown
Contributor Author

Validated the review feedback against the base Prod. Order Component triggers.

  • S1/S2 are valid: Location Code validation applies the item/SKU flushing method, and that temporary Flushing Method validation can recalculate Bin Code. Restoring only the flushing value can therefore leave a bin selected for the wrong method. I will centralize bin preservation/recalculation in the helper and add regression coverage with distinct Open Shop Floor and To-Production bins.
  • The inline recommendation to restore via Validate("Flushing Method", ...) is not safe for this bug. The return scenario intentionally covers posted consumption, where the base trigger rejects restoring Backward. The raw flushing assignment must remain; the related bin state will be corrected explicitly.
  • The UI-handler feedback is valid as test hardening, though not a high-severity production issue. I will make the new tests use expectation-backed handlers.

…nsfers

Address PR review feedback on #10206:

- Centralize the default-bin invariant in
  ValidateProdOrderCompLocationPreservingFlushingMethod. Preserve the old
  location, flushing method and bin; validate the location; restore the
  flushing method by direct assignment (re-validating it errors once
  consumption has been posted at the subcontractor, which the return leg must
  support). After the raw restore, realign the bin: if the component actually
  moved, recompute the default bin (now evaluated against the preserved
  flushing method); if the location did not change (the direct-transfer post
  revalidates the current location to trigger side effects), restore the
  original bin so the temporary flushing method cannot leave its bin behind.
- Remove the now-redundant caller GetDefaultBin() in Subc. Create Transf. Order
  and Subc. Create SubCReturnOrder; the helper now owns the bin invariant.
- Add regression test SubcDirectPostPreservesComponentBinForManualFlushingMethod
  which fails on f51c248 (bin left on To-Production) and passes after the fix.
- Harden the new bug tests' UI handlers with Library Variable Storage
  expectations: assert the exact confirm, transfer order page(s) and posting
  message(s) in order, and AssertEmpty to enforce the exact interaction count.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ChethanT

Copy link
Copy Markdown
Contributor Author

Review feedback addressed — pushed a2143f2f21

1. Raw "Flushing Method" restore (thread) — kept, with the real risk (bin) fixed centrally.
Re-validating the flushing method (the literal recommendation) errors on the return leg once consumption is posted at the subcontractor, so the raw assignment stays. The valid underlying concern — a raw restore leaving the "Bin Code" derived for the temporary flushing method — is fixed in ValidateProdOrderCompLocationPreservingFlushingMethod: preserve old location/flushing/bin → validate location → raw-restore flushing → if the component moved, recompute the default bin against the restored flushing method; if the location is unchanged (direct-post revalidation), restore the preserved bin. Redundant caller GetDefaultBin() removed from Subc. Create Transf. Order and Subc. Create SubCReturnOrder.

2. UI handler hardening (thread).
The new bug tests use dedicated handlers with Library - Variable Storage enqueue/dequeue expectations (exact confirm via ExpectedConfirm, posting message via ExpectedMessage, transfer-order page surfaced) and AssertEmpty() to enforce the exact interaction count. Shared empty handlers left untouched for other tests.

Tests

  • New regression SubcDirectPostPreservesComponentBinForManualFlushingMethod: red on f51c248 (bin GU00000083 To-Production vs expected GU00000082 Open Shop Floor), green after the fix.
  • Full codeunit 139991: 16 passed / 0 failed.

Both review threads replied to and resolved.

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Agentic PR Review - Round 1

Recommendation: Accept

What this PR does

This PR preserves a manually selected production order component flushing method when subcontracting moves the component location to or from a subcontractor location. The helper keeps the normal Location Code validation side effects, then restores the existing flushing method and realigns the bin so the temporary item or SKU planning value does not remain on the component.

The change is applied to the forward transfer, return transfer, direct-transfer post, receipt, de-subcontracting restore, and return-delete restore paths. The tests cover the forward move, the return after consumption has been posted, and the same-location direct-post bin case, which are the important edges for this bug.

Suggestions

No suggestions.

Risk assessment and necessity

Risk: This touches subcontracting production component location, bin, and flushing-method behavior, so the regression surface includes component consumption and transfer posting. The helper is narrow, keeps Location Code validation, and the added tests cover the main posting lifecycle and the bin side effect from direct-post revalidation.

Necessity: The bug is valid because automatic subcontracting transfers can silently lose a user-selected flushing method. Preserving the component value in these automatic shuttle paths is needed, while leaving normal user-initiated location validation behavior unchanged.

[AI-PR-REVIEW] version=1 promptVersion=2 system=github pr=10206 round=1 by=alexei-dobriansky at=2026-08-13T10:52:13Z lastSha=a9b8b8c03a39b403a592394c61ee5ca43e3cb418 reviewKey=caf67ccb16b5b16ce490d4e6647c34ea07f4aa93c164124625c5d89ada7456a8 suggestions=none

@ChethanT
Chethan Thopaiah (ChethanT) added this pull request to the merge queue Aug 13, 2026
Merged via the queue into main with commit 8bc02bb Aug 13, 2026
331 of 336 checks passed
@ChethanT
Chethan Thopaiah (ChethanT) deleted the bugs/Subcontracting/bugs/646576-PreserveFlushingMethodOnSubcTransfer branch August 13, 2026 21:39
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 SCM GitHub request for SCM area Subcontracting Subcontracting related activities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants