Bug 646576: Preserve manually changed component Flushing Method on subcontracting transfers - #10206
Conversation
…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>
Agentic PR Review - Round 1Recommendation: Request ChangesWhat this PR doesThis 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. SuggestionsS1 - Recalculate bin after restoring flushing S2 - Cover bin and SKU defaults Risk assessment and necessityRisk: 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.
|
alexei-dobriansky
left a comment
There was a problem hiding this comment.
Please review the suggestions
|
Validated the review feedback against the base
|
…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>
Review feedback addressed — pushed
|
Agentic PR Review - Round 1Recommendation: AcceptWhat this PR doesThis 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. SuggestionsNo suggestions. Risk assessment and necessityRisk: 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.
|
Summary
When a subcontracting transfer moves a released production order component to (or back from) the subcontractor location, a manually chosen
Flushing Methodon the component was silently reset. Fixes AB#646576.Root cause
Subcontracting shuttles the component by validating its
Location Code. The base-appProd. Order Component."Location Code"OnValidate callsGetUpdateFromSKU, which — when no SKU exists — copies planning parameters from the item, includingValidate("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):"Flushing Method",Validate("Location Code", NewLocationCode)(all existing side effects — SKU refresh, reservation checks, bin update — preserved),Plain assignment (not
Validate) is essential: theFlushing MethodOnValidate errors forBackwardon a released order once consumption entries exist — precisely the state of the return leg. Plain assignment never runs OnValidate, never throws, and leavesBin Codebyte-identical (bin derivation does not depend on flushing method).All subcontracting location shuttles now route through the helper:
Subc. Create Transf. Order, both the untracked and order-tracked/BindSubscriptionbranches)Subc. Create SubCReturnOrder)OnBeforeInsertDirectTransLine) and transfer receipt (OnCheckTransLine)ChangeLocationOnProdOrderComponent— consignment/vendor-supplied forward move and original-location restoreSubc. Prod. Order Comp. Ext.) and return-delete revert (Subc. Transfer Management)Intentionally excluded
Subc. Prod. Order Comp. Ext.that pulls the location from the SKU ("Components at Location") for a component that was never shuttled (noSubc. Original Location Code) — that is legitimate planning-defaults behavior, not a subcontracting shuttle.Planning Componentequivalents — 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 manualBackwardmethod.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