Skip to content

Fix subcontracting repricing regressions #11292

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Chethan Thopaiah (ChethanT) wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix subcontracting repricing regressions #11292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/Apps/W1/Subcontracting/App/src/Manufacturing/SubcReqWkshMakeOrd.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ codeunit 20516 "Subc. Req. Wksh. Make Ord."
local procedure OnInsertPurchOrderLineOnAfterTransferFromReqLineToPurchLine(var PurchOrderLine: Record "Purchase Line"; RequisitionLine: Record "Requisition Line")
var
SubcPriceManagement: Codeunit "Subc. Price Management";
ReqLineDatePriceListCost: Decimal;
begin
#if not CLEAN29
#pragma warning disable AL0432
Expand All @@ -43,6 +44,13 @@ codeunit 20516 "Subc. Req. Wksh. Make Ord."
if (RequisitionLine."Prod. Order No." = '') or (RequisitionLine."Operation No." = '') then
exit;

if not SubcPriceManagement.TryGetSubcPriceListCostForPurchLine(
PurchOrderLine, RequisitionLine."Order Date", ReqLineDatePriceListCost)
then
exit;
if PurchOrderLine."Direct Unit Cost" <> ReqLineDatePriceListCost then
exit;

SubcPriceManagement.GetSubcPriceForPurchLine(PurchOrderLine);
end;

Expand Down
101 changes: 79 additions & 22 deletions src/Apps/W1/Subcontracting/App/src/Purchase/SubcPriceManagement.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,10 +506,8 @@ codeunit 20508 "Subc. Price Management"
procedure GetSubcPriceForPurchLine(var PurchaseLine: Record "Purchase Line")
var
ProdOrderRoutingLine: Record "Prod. Order Routing Line";
SubcontractorPrice: Record "Subcontractor Price";
PriceListUOM: Code[10];
OrderDate: Date;
DirectCost, PriceListCost, PriceListQty, PriceListQtyPerUOM : Decimal;
DirectCost: Decimal;
begin
#if not CLEAN29
#pragma warning disable AL0432
Expand All @@ -521,36 +519,85 @@ codeunit 20508 "Subc. Price Management"
if OrderDate = 0D then
OrderDate := WorkDate();

if not TryGetSubcPriceListCostForPurchLine(PurchaseLine, OrderDate, DirectCost) then begin
GetProdOrderRtngLine(
PurchaseLine."Prod. Order No.", PurchaseLine."Routing Reference No.",
PurchaseLine."Routing No.", PurchaseLine."Operation No.", ProdOrderRoutingLine);
ProdOrderRoutingLine.TestField(Type, "Capacity Type"::"Work Center");
DirectCost := GetNonPriceListDirectCost(ProdOrderRoutingLine);
end;

PurchaseLine."Direct Unit Cost" := DirectCost;
PurchaseLine.Validate("Line Discount %");
end;

internal procedure TryGetSubcPriceListCostForPurchLine(PurchaseLine: Record "Purchase Line"; OrderDate: Date; var DirectCost: Decimal): Boolean
var
ProdOrderRoutingLine: Record "Prod. Order Routing Line";
SubcontractorPrice: Record "Subcontractor Price";
PriceListUOM: Code[10];
PriceListCost, PriceListQty, PriceListQtyPerUOM : Decimal;
begin
DirectCost := 0;
#if not CLEAN29
#pragma warning disable AL0432
if not SubcFeatureFlagHandler.IsSubcontractingEnabled() then
#pragma warning restore AL0432
exit(false);
#endif
if OrderDate = 0D then
OrderDate := WorkDate();

if not TryFindProdOrderRtngLine(
PurchaseLine."Prod. Order No.", PurchaseLine."Routing Reference No.",
PurchaseLine."Routing No.", PurchaseLine."Operation No.", ProdOrderRoutingLine)
then
exit(false);

SubcontractorPrice.SetRange("Vendor No.", PurchaseLine."Buy-from Vendor No.");
SubcontractorPrice.SetRange("Work Center No.", PurchaseLine."Work Center No.");
SubcontractorPrice.SetRange("Item No.", PurchaseLine."No.");
SubcontractorPrice.SetFilter("Variant Code", '%1|%2', PurchaseLine."Variant Code", '');
SubcontractorPrice.SetFilter("Unit of Measure Code", '%1|%2', PurchaseLine."Unit of Measure Code", '');

GetProdOrderRtngLine(PurchaseLine."Prod. Order No.", PurchaseLine."Routing Reference No.", PurchaseLine."Routing No.", PurchaseLine."Operation No.", ProdOrderRoutingLine);

SubcontractorPrice.SetFilter("Standard Task Code", '%1|%2', ProdOrderRoutingLine."Standard Task Code", '');
SubcontractorPrice.SetFilter("Currency Code", '%1|%2', PurchaseLine."Currency Code", '');
SubcontractorPrice.SetRange("Starting Date", 0D, OrderDate);
SubcontractorPrice.SetFilter("Ending Date", '>=%1|%2', OrderDate, 0D);

if SubcontractorPrice.FindLast() then begin
if SubcontractorPrice."Unit of Measure Code" = PurchaseLine."Unit of Measure Code" then
PriceListUOM := SubcontractorPrice."Unit of Measure Code";
GetUOMPrice(PurchaseLine."No.", GetQuantityBase(PurchaseLine), SubcontractorPrice, PriceListUOM, PriceListQtyPerUOM, PriceListQty);
GetPriceByUOM(SubcontractorPrice, PriceListQty, PriceListCost);
if PriceListCost <> 0 then begin
ConvertPriceToUOM(PurchaseLine."Unit of Measure Code", PurchaseLine.GetQuantityPerUOM(), PriceListUOM, PriceListQtyPerUOM, PriceListCost, DirectCost);
ConvertPriceToCurrency(PurchaseLine."Currency Code", SubcontractorPrice."Currency Code", PriceListCost, DirectCost)
end;
end else begin
GetUOMPrice(PurchaseLine."No.", PurchaseLine.GetQuantityBase(), SubcontractorPrice, PriceListUOM, PriceListQtyPerUOM, PriceListQty);
ProdOrderRoutingLine.TestField(Type, "Capacity Type"::"Work Center");
DirectCost := ProdOrderRoutingLine."Direct Unit Cost";
end;
if not SubcontractorPrice.FindLast() then
exit(false);

PurchaseLine."Direct Unit Cost" := DirectCost;
PurchaseLine.Validate("Line Discount %");
if SubcontractorPrice."Unit of Measure Code" = PurchaseLine."Unit of Measure Code" then
PriceListUOM := SubcontractorPrice."Unit of Measure Code";
GetUOMPrice(PurchaseLine."No.", GetQuantityBase(PurchaseLine), SubcontractorPrice, PriceListUOM, PriceListQtyPerUOM, PriceListQty);
GetPriceByUOM(SubcontractorPrice, PriceListQty, PriceListCost);
if PriceListCost = 0 then
exit(true);

ConvertPriceToUOM(PurchaseLine."Unit of Measure Code", PurchaseLine.GetQuantityPerUOM(), PriceListUOM, PriceListQtyPerUOM, PriceListCost, DirectCost);
ConvertPriceToCurrency(PurchaseLine."Currency Code", SubcontractorPrice."Currency Code", PriceListCost, DirectCost);
exit(true);
end;

local procedure GetNonPriceListDirectCost(ProdOrderRoutingLine: Record "Prod. Order Routing Line"): Decimal
var
GeneralLedgerSetup: Record "General Ledger Setup";
ProdOrderLine: Record "Prod. Order Line";
begin
GetLine(ProdOrderLine, ProdOrderRoutingLine);
GeneralLedgerSetup.Get();
if ProdOrderRoutingLine."Unit Cost Calculation" = ProdOrderRoutingLine."Unit Cost Calculation"::Units then
exit(
Round(
ProdOrderRoutingLine."Direct Unit Cost" * ProdOrderLine."Qty. per Unit of Measure",
GeneralLedgerSetup."Unit-Amount Rounding Precision"));

ProdOrderLine.CalcFields("Total Exp. Oper. Output (Qty.)");

@github-actions github-actions Bot Sep 9, 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.

$\textbf{🟡\ Medium\ Severity\ —\ Agent}$

New procedure GetNonPriceListDirectCost (SubcPriceManagement.Codeunit.al) divides by ProdOrderLine."Total Exp. Oper. Output (Qty.)" whenever the routing operation's Unit Cost Calculation is not Units, with no guard against that field being zero. "Total Exp. Oper. Output (Qty.)" is a FlowField reflecting output reported so far, which is legitimately zero for a production order/operation that has not yet had output posted — exactly the state a fresh subcontracting purchase line without a matching price-list entry can be in. This introduces an unhandled division-by-zero runtime error on the pricing path used by GetSubcPriceForPurchLine. Note this is especially notable because the identical formula is already guarded elsewhere in the same app: both SubcPurchaseOrderCreator.Codeunit.al (around the RequisitionLine.Quantity <> 0 check before the Direct Unit Cost calculation) and SubcCalculateSubcontracts.Report.al wrap the same '(Expected Operation Cost Amt. - Expected Capacity Ovhd. Cost) / Total Exp. Oper. Output (Qty.)' expression in a zero-quantity guard that falls back to a Direct Unit Cost of 0, but the new GetNonPriceListDirectCost path added by this PR omits that guard. If this were not already mirrored by an established guarded pattern elsewhere in the same feature, its impact would be major (an unhandled runtime error blocking pricing/carry-out); it should be promoted to a knowledge-backed rule and guarded to match the sibling call sites before it can gate.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.41.6

exit(
Round(
(ProdOrderRoutingLine."Expected Operation Cost Amt." - ProdOrderRoutingLine."Expected Capacity Ovhd. Cost") /
ProdOrderLine."Total Exp. Oper. Output (Qty.)",
GeneralLedgerSetup."Unit-Amount Rounding Precision"));
end;

local procedure GetProdOrderRtngLine(ProdOrderNo: Code[20]; RtngRefNo: Integer; RoutingNo: Code[20]; OperationNo: Code[10]; var ProdOrderRoutingLine: Record "Prod. Order Routing Line")
Expand All @@ -564,6 +611,16 @@ codeunit 20508 "Subc. Price Management"
ProdOrderRoutingLine.FindFirst();
end;

local procedure TryFindProdOrderRtngLine(ProdOrderNo: Code[20]; RtngRefNo: Integer; RoutingNo: Code[20]; OperationNo: Code[10]; var ProdOrderRoutingLine: Record "Prod. Order Routing Line"): Boolean
begin
ProdOrderRoutingLine.SetFilter(Status, '%1|%2', ProdOrderRoutingLine.Status::Released, ProdOrderRoutingLine.Status::Finished);

@github-actions github-actions Bot Sep 9, 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.

$\textbf{🟠\ High\ Severity\ —\ Performance}$

TryFindProdOrderRtngLine sets every component of the Prod. Order Routing Line primary key (Status, Prod. Order No., Routing Reference No., Routing No., Operation No.) via SetFilter/SetRange and then calls FindFirst, even though this is a full-primary-key lookup. This is called on the pricing hot path (every GetSubcPriceForPurchLine invocation) and should use a direct Get instead of an index-walking Find.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

        if ProdOrderRoutingLine.Get(ProdOrderRoutingLine.Status::Released, ProdOrderNo, RtngRefNo, RoutingNo, OperationNo) then
            exit(true);
        exit(ProdOrderRoutingLine.Get(ProdOrderRoutingLine.Status::Finished, ProdOrderNo, RtngRefNo, RoutingNo, OperationNo));

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.41.6

ProdOrderRoutingLine.SetRange("Prod. Order No.", ProdOrderNo);
ProdOrderRoutingLine.SetRange("Routing Reference No.", RtngRefNo);
ProdOrderRoutingLine.SetRange("Routing No.", RoutingNo);
ProdOrderRoutingLine.SetRange("Operation No.", OperationNo);
exit(ProdOrderRoutingLine.FindFirst());
end;

local procedure SetSubcontractorPriceForPriceCalculation(var SubcontractorPrice: Record "Subcontractor Price"; VendorNo: Code[20]; ItemNo: Code[20]; VariantCode: Code[10]; StandardTaskCode: Code[10]; WorkCenterNo: Code[20]; UoM: Code[10]; StartingDate: Date)
var
Vendor: Record Vendor;
Expand Down
34 changes: 31 additions & 3 deletions src/Apps/W1/Subcontracting/App/src/Purchase/SubcPurchaseLineExt.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,14 @@ codeunit 20534 "Subc. Purchase Line Ext"
if GetExecutionContext() = ExecutionContext::Upgrade then
exit;

if Rec."Planned Receipt Date" = xRec."Planned Receipt Date" then
// Gate on the resulting Order Date rather than Planned Receipt Date so lead-time-only
// reschedules (for example changing Lead Time Calculation on an open line with nonblank
// Requested and Planned Receipt Dates) still trigger date-effective repricing when the
// planned-date validation reassigns Order Date without changing Planned Receipt Date.
if Rec."Order Date" = xRec."Order Date" then
exit;

GetSubcontractingPrice(Rec);
RepriceSubcPurchLineOnScheduleChange(Rec);
end;

[EventSubscriber(ObjectType::Table, Database::"Purchase Line", OnAfterValidateEvent, "Order Date", false, false)]
Expand All @@ -124,7 +128,7 @@ codeunit 20534 "Subc. Purchase Line Ext"
if Rec."Order Date" = xRec."Order Date" then
exit;

GetSubcontractingPrice(Rec);
RepriceSubcPurchLineOnScheduleChange(Rec);
end;

[EventSubscriber(ObjectType::Table, Database::"Purchase Line", OnAfterValidateEvent, Quantity, false, false)]
Expand Down Expand Up @@ -354,6 +358,30 @@ codeunit 20534 "Subc. Purchase Line Ext"
SubcPriceManagement.GetSubcPriceForPurchLine(PurchaseLine);
end;

local procedure RepriceSubcPurchLineOnScheduleChange(var PurchaseLine: Record "Purchase Line")
var
PurchaseHeader: Record "Purchase Header";
begin
if PurchaseLine."Prod. Order No." = '' then
exit;

// Preserve released-order scheduling: repricing a subcontracting line after release
// would call Validate("Line Discount %") through GetSubcPriceForPurchLine, which in
// turn calls TestStatusOpen on the released header and fails the date edit. The base
// test suite explicitly permits Planned Receipt Date and Order Date edits on released
// purchase order lines (see ERMSalesPurchStatusError CanChangeOrderDateOnReleasedPurchOrderLine
// and CanChangePlannedReceiptDateOnReleasedPurchOrderLine). Bypass repricing entirely
// once the header is no longer Open so scheduling still works without silently
// changing released financial terms.
PurchaseHeader.SetLoadFields(Status);

@github-actions github-actions Bot Sep 9, 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.

$\textbf{🟠\ High\ Severity\ —\ Performance}$

RepriceSubcPurchLineOnScheduleChange performs PurchaseHeader.Get before checking the same cheap applicability guards that GetSubcontractingPrice already applies (Type = Item, No. <> '', Operation No. <> ''). Only 'Prod. Order No.' is checked before the Get, so every production-linked subcontracting purchase line pays an unnecessary database lookup on every schedule change (Planned Receipt Date / Order Date validation), even when the line will ultimately be excluded from repricing by those other guards.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

        if (PurchaseLine.Type <> PurchaseLine.Type::Item) or (PurchaseLine."No." = '') or (PurchaseLine."Operation No." = '') then
            exit;

        PurchaseHeader.SetLoadFields(Status);
        if not PurchaseHeader.Get(PurchaseLine."Document Type", PurchaseLine."Document No.") then
            exit;
        if PurchaseHeader.Status <> PurchaseHeader.Status::Open then
            exit;

        GetSubcontractingPrice(PurchaseLine);

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.41.6

if not PurchaseHeader.Get(PurchaseLine."Document Type", PurchaseLine."Document No.") then
exit;
if PurchaseHeader.Status <> PurchaseHeader.Status::Open then
exit;

GetSubcontractingPrice(PurchaseLine);
end;

[EventSubscriber(ObjectType::Table, Database::"Purchase Line", OnBeforeOpenItemTrackingLines, '', false, false)]
local procedure OpenProdOrderLineItemTrackingOnBeforeOpenItemTrackingLines(PurchaseLine: Record "Purchase Line"; var IsHandled: Boolean)
begin
Expand Down
Loading
Loading