[Subcontracting] Fix subcontracting transfer order issues and add field modification protection

[AleksandricMarko]

What & why

Bug Fixes

Transferred qty not counted correctly — Removed Open filter from the component FlowField so posted transfer entries are included in the calculation.
Duplicate return TO not blocked — Fixed TransferLineAlreadyExists logic so creating a second return transfer order is correctly prevented.
Missing Subc. Location Code error unclear — Replaced generic TestField error with a descriptive message naming the vendor.

Field Modification Protection

Purchase Line fields locked — Blocked changing Quantity, No., Location Code, Bin Code, Variant Code, and Unit of Measure Code on subcontracting purchase lines when transfer orders exist or stock is at the subcontractor.
Transfer Line fields locked — Blocked changing Item No., Variant Code, and Quantity on transfer lines linked to a production order.
Transfer Header fields locked — Blocked changing Transfer-from/to Code on subcontracting transfer headers with linked lines.

Code Organization

Extracted transfer procedures — Moved all transfer-related logic from Subcontracting Management into a new dedicated Subc. Transfer Management codeunit for better separation of concerns.

Linked work

Fixes AB#630809, AB#630810, AB#630811, AB#636821, AB#636822, AB#634269

[alexei-dobriansky]

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

This PR moves subcontracting transfer logic into Subc. Transfer Management, fixes transfer quantity/return-order handling, improves a missing subcontractor-location error, and adds guards around key purchase/transfer fields. The transfer fixes look directionally aligned with the described issues, and the PR does include regression tests, but the new transfer codeunits are not granted through the app object permission set and the field-protection implementation only guards validation events, not all persisted modifications. Those two gaps mean the change is not safe to merge yet.

Suggestions

S1 - Grant execute permission to new codeunits
Subc. Transfer Management and Subc. Transfer Header Ext. are new executable objects, but SubcontractObjs.PermissionSet.al was not updated. Add both codeunits to the object permission set, otherwise users assigned the subcontracting permission sets can hit permission failures when reports/pages/event subscribers execute this refactored logic.

S2 - Protect persisted field changes, not only validation
The new guards are attached only to OnBeforeValidateEvent subscribers in the purchase line, transfer line, and transfer header extension codeunits, so code that assigns the protected fields directly and calls Modify(true) can bypass them. Add an OnBeforeModify/equivalent persisted-change guard that compares the protected fields, and add regression tests that perform real Validate(...) and direct Modify(true) attempts instead of only calling the helper procedures.

Risk assessment and necessity

Risk: This is high regression surface because subcontracting purchase lines, transfer orders, reservations, WIP ledger entries, and production order components are all involved. The new refactor centralizes transfer behavior in SubcTransferManagement.Codeunit.al, while missing permissions and validation-only field protection can break normal user execution or leave integrity gaps.

Necessity: The transfer-order fixes appear necessary, and tests are present for duplicate/recreate and reservation scenarios. The scope is broad for a bug PR because it combines transfer fixes, field protection, message changes, and a refactor across multiple AB links; that can be acceptable only if the object permissions and end-to-end guard coverage are made complete. Work items were not independently verified.

---

[AI-PR-REVIEW] version=1 system=github pr=8563 round=1 by=alexei-dobriansky at=2026-06-10T15:22:47Z lastSha=be6cd11a641786718d83785fa39b241310094e9a suggestions=S1,S2

[alexei-dobriansky]

Please review the suggestions

[alexei-dobriansky]

.

[AleksandricMarko]

Please review the suggestions
S1 - fix
S2 - Protect persisted field changes, not only validation
My intention is just to block users to manually make changes.

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Upgrade} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

FlowField now includes closed ILEs

The Open = const(true) filter was removed from the "Subc. Qty. transf. to Subcontr" FlowField. This changes its semantics from "items currently at the subcontractor" to "all items ever transferred", including those already consumed or returned (whose ILEs are closed). Any caller of this field that was not updated in this PR will silently receive an inflated value.

Recommendation:

• Audit all usages of "Subc. Qty. transf. to Subcontr" across the codebase. If the intent is intentionally to count total transfers (compensating via CalcConsumedQtyAtSubcLocation), rename or add a comment to the field caption to make the new semantics explicit. Otherwise, restore the Open = const(true) filter and implement consumed-qty subtraction in the FlowField itself.

// If total-transferred semantics is intentional, rename the field to reflect it:
Caption = 'Total Qty. Transferred to Subcontractor';
// Or, to restore original open-only semantics:
field("Subc. Qty. transf. to Subcontr"; Quantity) = EXIST(..., Open = CONST(true));

Line mapping was unavailable, so this was posted as an issue comment.

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

[github-actions]

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

Flowfield now returns total-transferred, not remaining stock

Removing Open = const(true) from the "Subc. Qty. transf. to Subcontr" flowfield changes its semantics: it now sums ALL Transfer ILEs (open and closed/fully consumed) rather than only those with remaining quantity. HasStockAtSubcLocation compensates by subtracting consumed qty, so that path is correct. However, SubcCreateTransfOrder.Report.al (lines 187, 210) and SubcDispatchingList.Report.al (line 1027) use the field directly without any consumed-qty adjustment, and the field is displayed as-is on SubcProdOrderComponents.Page.al and SubcProdOrderComp.PageExt.al. After full consumption at the subcontractor, these surfaces will show the original transferred quantity instead of zero remaining, which is misleading to users.

Recommendation:

• Document the new semantics of this field clearly (total ever transferred) and audit all direct consumers of the flowfield. For display pages, consider showing net remaining stock by subtracting consumed qty, or rename the field to reflect its new meaning (e.g., 'Total Qty. Sent to Subcontractor').

// In SubcDispatchingList.Report.al line 1027, adjust the column formula:
column(RemainingQtyToTransfer;
    "Expected Qty. (Base)" - "Subc. Qty. transf. to Subcontr"
    + SubcTransferManagement.CalcConsumedQtyAtSubcLocation(Rec)
    - "Subc. Qty. in Transit (Base)")

Line mapping was unavailable, so this was posted as an issue comment.

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

[github-actions]

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

Adjust action missing InherentPermissions

AdjustProdOrderLineQuantity() modifies Prod. Order Line records from an ErrorInfo action but declares no InherentPermissions and the codeunit has no Permissions property. Users who can work with subcontracting purchase lines but lack direct modify rights on production order lines will receive a permission error when they choose the guided fix.

Recommendation:

• Add the minimal InherentPermissions attribute granting modify on Prod. Order Line to this procedure.

[InherentPermissions(PermissionObjectType::TableData, Database::"Prod. Order Line", 'm')]
internal procedure AdjustProdOrderLineQuantity(OverDeliveryErrorInfo: ErrorInfo)

Line mapping was unavailable, so this was posted as an issue comment.

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

[github-actions]

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

Whse adjust action missing InherentPermissions

The warehouse receipt AdjustProdOrderLineQuantity() also updates Prod. Order Line without declaring InherentPermissions or codeunit-level object permissions. The recovery action will fail at runtime for any user without direct production order modify rights.

Recommendation:

• Add the same minimal InherentPermissions attribute for Prod. Order Line modify to this procedure.

[InherentPermissions(PermissionObjectType::TableData, Database::"Prod. Order Line", 'm')]
internal procedure AdjustProdOrderLineQuantity(OverDeliveryErrorInfo: ErrorInfo)

Line mapping was unavailable, so this was posted as an issue comment.

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Over-delivery check uses total qty not remaining

CheckOverDeliveryQty() and AdjustProdOrderLineQuantity() compare against ProdOrderLine.Quantity instead of ProdOrderLine."Remaining Quantity". After partial output has already been posted, the check passes for quantities that still exceed what remains, and the adjustment calculates the wrong delta to add to the production order total.

Recommendation:

• Base both the pre-check and the delta calculation on ProdOrderLine."Remaining Quantity".

if PurchaseLine.Quantity > ProdOrderLine."Remaining Quantity" then begin
    NewProdOrderQty := ProdOrderLine.Quantity + (PurchaseLine.Quantity - ProdOrderLine."Remaining Quantity");
    // build ErrorInfo and Error(...)
end;

Line mapping was unavailable, so this was posted as an issue comment.

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Whse over-delivery check uses total qty

CheckOverDelivery() compares WarehouseReceiptLine.Quantity against ProdOrderLine.Quantity, and the adjustment at line 263 repeats the same incorrect comparison. On partially processed production orders this allows genuine over-deliveries to slip through undetected.

Recommendation:

• Validate against ProdOrderLine."Remaining Quantity" and compute the adjustment delta from that field.

if WarehouseReceiptLine.Quantity > ProdOrderLine."Remaining Quantity" then begin
    NewProdOrderQty := ProdOrderLine.Quantity + (WarehouseReceiptLine.Quantity - ProdOrderLine."Remaining Quantity");
    // build ErrorInfo and Error(...)
end;

Line mapping was unavailable, so this was posted as an issue comment.

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

[github-actions]

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

Open-anyway action bound to wrong codeunit

The "Open item tracking anyway" action in the warehouse receipt mismatch dialog is wired to codeunit "Subc. Purchase Line Ext" instead of the current file's handler. That handler does not have access to the stored Warehouse Receipt Line SystemId in CustomDimensions, so tracking is reopened with purchase-line semantics rather than the receipt-line quantities that triggered the error.

Recommendation:

• Bind the action to the warehouse receipt codeunit's own OpenItemTrackingWithoutAdjustment() procedure.

CannotInvoiceErrorInfo.AddAction(
    OpenItemTrackingAnywayActionLbl,
    Codeunit::"Subc. WhsePostReceipt Ext",
    'OpenItemTrackingWithoutAdjustment'
);

Line mapping was unavailable, so this was posted as an issue comment.

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