Bug 633674: [Subcontracting] Subcontracting Type field and enum captions are unclear — rename to Component Supply Method

[ChethanT]

Bug Summary

The Subcontracting Type wording was unclear in Subcontracting pages and enum captions. The labels implied a subcontracting classification instead of the actual intent: how components are supplied to the subcontractor.

Root Cause

User-facing names and captions did not reflect the functional meaning of the field and enum values, which caused confusion in UI interpretation and setup decisions.

Fix

• Renamed enum object from Subcontracting Type to Component Supply Method.
• Updated enum value captions:
  • Purchase with Service -> Vendor-Supplied
  • Inventory by Vendor -> Consignment at Vendor
  • Transfer -> Transfer to Vendor
• Updated related captions/tooltips/messages in Subcontracting App to align terminology.
• Updated references in app/test AL code to the renamed enum object.

Test Added

• Codeunit: 139990 (Subc. Subcontracting UI Test)
• Test function: ComponentSupplyMethodCaptionsAreClear
• Coverage:
  • Verifies field caption is Component Supply Method on Production BOM Line, Planning Component, and Prod. Order Component.
  • Verifies enum value captions are Vendor-Supplied, Consignment at Vendor, and Transfer to Vendor.

Validation

• App compiles and publishes.
• Tests run and pass.

AB#633674

[ChethanT]

@SPinkow

[github-actions]

AL Documentation Audit

Documentation gaps were detected in the following apps:

• Subcontracting-Test: 0% documentation coverage
• Subcontracting: 0% documentation coverage

To generate documentation, run /al-docs init or /al-docs update using GitHub Copilot CLI or Claude Code.
This review is for awareness to help keep documentation in sync with code changes. It is okay to dismiss this request.

[github-actions]

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

Transfer header lookup ignores purchase order scope

The filter TransferHeader.SetRange("Subcontr. Purch. Order No.", "Purchase Header"."No.") was removed from the transfer-header lookup. The query now matches any transfer order with the same from/to locations, so components from the current purchase order can be appended to a transfer order belonging to a completely different purchase order that shares those locations.

Recommendation:

• Restore the "Subcontr. Purch. Order No." range filter so the lookup is scoped to the current purchase order only.

TransferHeader.SetRange("Transfer-from Code", CompLineLocation);
TransferHeader.SetRange("Transfer-to Code", TransferToLocationCode);
TransferHeader.SetRange("Return Order", false);
TransferHeader.SetRange("Subcontr. Purch. Order No.", "Purchase Header"."No.");
if not TransferHeader.FindFirst() then begin

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

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

[github-actions]

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

Transfer order lookup missing purchase order filter

The TransferHeader.SetRange("Subcontr. Purch. Order No.", "Purchase Header"."No.") filter was removed before the FindFirst() call. The query now matches any transfer header with the same transfer-from/transfer-to locations, regardless of which purchase order it belongs to, so a transfer order from a completely different subcontracting PO can be reused and updated.

Recommendation:

• Reinstate the SetRange("Subcontr. Purch. Order No.", ...) filter before FindFirst() to ensure only transfer orders belonging to the current purchase order are considered.

TransferHeader.SetRange("Transfer-from Code", CompLineLocation);
TransferHeader.SetRange("Transfer-to Code", TransferToLocationCode);
TransferHeader.SetRange("Return Order", false);
TransferHeader.SetRange("Subcontr. Purch. Order No.", "Purchase Header"."No.");
if not TransferHeader.FindFirst() then begin

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

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

[github-actions]

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

Transfer order lookup missing purchase-order filter

The filter TransferHeader.SetRange("Subcontr. Purch. Order No.", "Purchase Header"."No.") was removed from the TransferHeader lookup before FindFirst(). Without it, the report finds the first transfer order matching only the from/to location codes, which may belong to a completely different purchase order. Lines for the current PO will then be inserted into the wrong transfer order, causing data corruption.

Recommendation:

• Restore the purchase-order number filter to ensure only transfer orders linked to the current subcontracting purchase order are reused.

TransferHeader.SetRange("Transfer-from Code", CompLineLocation);
TransferHeader.SetRange("Transfer-to Code", TransferToLocationCode);
TransferHeader.SetRange("Return Order", false);
TransferHeader.SetRange("Subcontr. Purch. Order No.", "Purchase Header"."No.");
if not TransferHeader.FindFirst() then begin

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

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

[github-actions]

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

Public procedure renamed; breaking API change

UpdateSubcontractingTypeForProdOrderComponent is a public procedure and is renamed to UpdateComponentSupplyMethodForProdOrderComponent without retaining the old name. External callers will get a compile error.

Recommendation:

• Retain the old procedure name as an [Obsolete]-attributed wrapper that delegates to the new one, and remove it in a later major release.

[Obsolete('Use UpdateComponentSupplyMethodForProdOrderComponent instead.', '26.0')]
procedure UpdateSubcontractingTypeForProdOrderComponent(var ProdOrderComponent: Record "Prod. Order Component")
begin
    UpdateComponentSupplyMethodForProdOrderComponent(ProdOrderComponent);
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\ —\ Upgrade} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Planning Component field renamed without obsolescence

Field 99001524 on Planning Component is renamed from "Subcontracting Type" to "Component Supply Method" without an obsoletion step, breaking any external AL code that references PlanningComponent."Subcontracting Type".

Recommendation:

• Obsolete the old field name before removing it, consistent with the BC extensibility breaking-change policy. Add the new field under a separate field number and deprecate the old one with ObsoleteState = Pending.

field(99001524; "Subcontracting Type"; Enum "Subcontracting Type")
{
    ObsoleteState = Pending;
    ObsoleteReason = 'Use field "Component Supply Method" instead.';
    ObsoleteTag = '26.0';
    Caption = 'Subcontracting Type';
    DataClassification = CustomerContent;
}

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

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

[github-actions]

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

Production BOM Line field renamed without obsolescence

Field 99001522 on Production BOM Line is renamed from "Subcontracting Type" to "Component Supply Method" without an obsoletion step, immediately breaking partner extensions that reference this field by the old name.

Recommendation:

• Add the new field under a new field number, keep the old field marked ObsoleteState = Pending, and remove it in a later major release.

field(99001522; "Subcontracting Type"; Enum "Subcontracting Type")
{
    ObsoleteState = Pending;
    ObsoleteReason = 'Use "Component Supply Method" instead.';
    ObsoleteTag = '26.0';
    Caption = 'Subcontracting Type';
    DataClassification = CustomerContent;
}

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

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

[github-actions]

$\textbf{🔴\ Critical\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Reservation quantity hardcoded to zero

The TransferReservations call now passes literal 0 instead of TransferLine."Quantity (Base)" as the quantity-to-transfer argument. This causes reservation entries to be transferred with zero quantity, silently corrupting reservation data for all subcontracting transfer orders.

Recommendation:

• Restore the original TransferLine."Quantity (Base)" argument to correctly represent the actual quantity being transferred in the reservation entry.

TransferLine."Quantity (Base)",

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

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

[github-actions]

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

Partial transfer reservation guard removed

The check ComponentHasExcessReservations that blocked creating a transfer order when the reserved quantity exceeds the transfer quantity has been removed. Users can now create partial subcontracting transfer orders that leave orphaned reservation entries on the production order component, leading to over-committed inventory.

Recommendation:

• Restore the excess reservation check, or replace it with equivalent logic that prevents partial transfers when outstanding reservations would exceed the transfer quantity.

if SubcontractingManagement.ComponentHasExcessReservations(ProdOrderComponent, TransferLine."Quantity (Base)") then
    Error(ExcessReservationsErr, TransferLine."Quantity (Base)", SubcontractingManagement.GetComponentReservedQtyBase(ProdOrderComponent), ProdOrderComponent."Item No.");

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

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

[github-actions]

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

Reservation quantity parameter changed to 0

The TransferReservations call changed the quantity parameter from TransferLine."Quantity (Base)" to 0. This will cause incorrect reservation transfers from production order components to transfer lines, potentially leading to reservation data corruption.

Recommendation:

• Restore the original logic to pass TransferLine."Quantity (Base)" as the 6th parameter to TransferReservations. If 0 is intentional, add comments explaining why and verify all reservation scenarios work correctly.

ReservationEntry.TransferReservations(
 ReservationEntry,
 TransferLine."Item No.",
 TransferLine."Variant Code",
 TransferLine."Transfer-from Code",
 true,
 TransferLine."Quantity (Base)",  // Restore original quantity
 TransferLine."Qty. per Unit of Measure",
 Database::"Transfer Line",
 0,
 TransferLine."Document No.",
 '',
 0,
 TransferLine."Line No.");

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}}$

Removed reservation validation allows invalid states

Removed ComponentHasExcessReservations validation that prevented creating transfer orders when reserved quantity exceeded transfer quantity. This check and the associated error message were deleted, allowing transfers to be created with invalid reservation states that could cause runtime errors later.

Recommendation:

• Restore the validation or document why it's safe to remove. If the reservation transfer logic now handles partial quantities correctly (via the 0 parameter change), add comments explaining the new behavior.

// Before calling TransferReservationEntryFromProdOrderCompToTransferOrder:
if SubcontractingManagement.ComponentHasExcessReservations(ProdOrderComponent, TransferLine."Quantity (Base)") then
    Error(ExcessReservationsErr, TransferLine."Quantity (Base)", SubcontractingManagement.GetComponentReservedQtyBase(ProdOrderComponent), ProdOrderComponent."Item No.");

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}}$

Comments still use old enum member name 'Purchase'

Scenario comments at lines 86 and 92 say "Component Supply Method = Purchase" and "Setting Component Supply Method to Purchase" — "Purchase" was the old AL member identifier, not the new name "Vendor-Supplied".

Recommendation:

• Update comments to use the new enum member identifier "Vendor-Supplied" to accurately describe what the test exercises.

Suggested change

	// [SCENARIO 624295] Setting Subcontracting Type = Purchase on a Production BOM Line with a Non-Inventory item must succeed (only Transfer is blocked).
	// [SCENARIO 624295] Setting Component Supply Method = Vendor-Supplied on a Production BOM Line with a Non-Inventory item must succeed (only Transfer to Vendor is blocked).
	...
	// [WHEN] Setting Component Supply Method to Vendor-Supplied on the BOM line

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

[github-actions]

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

Stale scenario comments reference old enum caption

Four comments at lines 198, 200, 202, and 237 still reference "Purchase with Service" — the Caption of the deleted Purchase enum value — rather than "Vendor-Supplied", which is the Caption of the replacement "Vendor-Supplied" value. This mismatches the actual code under test.

Recommendation:

• Replace all occurrences of "Purchase with Service" in these scenario comments with "Vendor-Supplied" to reflect the new enum caption.

Suggested change

	// with Subcontracting Type = "Purchase with Service" and Flushing Method = Backward,
	// with Component Supply Method = "Vendor-Supplied" and Flushing Method = Backward,
	// BOM: 1 component item (Component Supply Method = Vendor-Supplied, linked to Routing Line 100).
	// Purchase order has 2 lines: Finished Good (output) + Component (Vendor-Supplied).
	// [GIVEN] A production BOM with one component, Component Supply Method = Vendor-Supplied

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

[github-actions]

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

Test procedure name has redundant 'Component'

CheckCustCtrl_PagePlanningComponentComponentSupplyMethod contains "Component" twice consecutively, making it harder to read and search. This happened because the old suffix "SubcontractingType" was replaced with the full new concept name.

Recommendation:

• Rename to CheckCustCtrl_PagePlanningCompComponentSupplyMethod or CheckCustCtrl_PagePlanningComponentSupplyMethod to eliminate the duplication.

Suggested change

	procedure CheckCustCtrl_PagePlanningComponentSubcontractingType()
	procedure CheckCustCtrl_PagePlanningCompComponentSupplyMethod()

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

[github-actions]

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

Public procedures renamed without obsolescence

UpdateSubcontractingTypeForPlanningComponent and UpdateSubcontractingTypeForProdOrderComponent are renamed to UpdateComponentSupplyMethodForPlanningComponent and UpdateComponentSupplyMethodForProdOrderComponent. Any external extension that calls the old procedure names will break to compile with no deprecation warning.

Recommendation:

• Retain the old procedure names with ObsoleteState = Pending or Removed, delegating to the new procedures, so callers have a migration window before the old names are fully removed.

[Obsolete('Use UpdateComponentSupplyMethodForPlanningComponent instead.', '26.0')]
procedure UpdateSubcontractingTypeForPlanningComponent(var PlanningComponent: Record "Planning Component")
begin
    UpdateComponentSupplyMethodForPlanningComponent(PlanningComponent);
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}}$

Stale enum value name in scenario comment

The scenario description at line 84 was partially updated and now reads "Setting Component Supply Method = Purchase on a Production BOM Line". The word Purchase is the old AL identifier for the enum value; its current name is "Vendor-Supplied", so the comment is misleading to anyone reading the test.

Recommendation:

• Update the comment to use the new enum value name Vendor-Supplied instead of Purchase.

// [SCENARIO 624295] Setting Component Supply Method = Vendor-Supplied on a Production BOM Line with a Non-Inventory item must succeed (only Transfer to Vendor is blocked).

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

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