Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,23 @@ codeunit 20516 "Subc. Req. Wksh. Make Ord."
HandleSubcontractingAfterPurchOrderLineInsert(PurchOrderLine, NextLineNo, RequisitionLine);
end;

[EventSubscriber(ObjectType::Codeunit, Codeunit::"Req. Wksh.-Make Order", OnInsertPurchOrderLineOnAfterTransferFromReqLineToPurchLine, '', false, false)]
local procedure OnInsertPurchOrderLineOnAfterTransferFromReqLineToPurchLine(var PurchOrderLine: Record "Purchase Line"; RequisitionLine: Record "Requisition Line")
var
SubcPriceManagement: Codeunit "Subc. Price Management";
[EventSubscriber(ObjectType::Codeunit, Codeunit::"Req. Wksh.-Make Order", OnBeforeCopyOrderDateFromPurchHeader, '', false, false)]
local procedure OnBeforeCopyOrderDateFromPurchHeader(var RequisitionLine: Record "Requisition Line"; PurchaseHeader: Record "Purchase Header"; PurchaseLine: Record "Purchase Line"; var IsHandled: Boolean)

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\ —\ Style}$

The new OnBeforeCopyOrderDateFromPurchHeader subscriber renames the publisher's parameters PurchOrderHeader and PurchOrderLine to PurchaseHeader and PurchaseLine. In AL, an EventSubscriber binds to its publisher by parameter name, not position, so renaming a kept parameter is not a style choice -- the compiler validates the match at build time and this mismatch breaks the build. Copy the publisher's parameter names verbatim (or omit unused ones) instead of renaming them.

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

    local procedure OnBeforeCopyOrderDateFromPurchHeader(var RequisitionLine: Record "Requisition Line"; PurchOrderHeader: Record "Purchase Header"; PurchOrderLine: Record "Purchase Line"; var IsHandled: Boolean)
    begin
#if not CLEAN29
#pragma warning disable AL0432
        if not SubcFeatureFlagHandler.IsSubcontractingEnabled() then
#pragma warning restore AL0432
            exit;
#endif
        if PurchOrderHeader."Document Type" <> PurchOrderHeader."Document Type"::Order then
            exit;
        if PurchOrderLine.Type <> PurchOrderLine.Type::Item then
            exit;
        if (RequisitionLine."Prod. Order No." = '') or (RequisitionLine."Operation No." = '') then
            exit;

        IsHandled := true;
    end;

Knowledge:

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

begin
#if not CLEAN29
#pragma warning disable AL0432
if not SubcFeatureFlagHandler.IsSubcontractingEnabled() then
#pragma warning restore AL0432
exit;
#endif
if PurchaseHeader."Document Type" <> PurchaseHeader."Document Type"::Order then
exit;
if PurchaseLine.Type <> PurchaseLine.Type::Item then
exit;
if (RequisitionLine."Prod. Order No." = '') or (RequisitionLine."Operation No." = '') then
exit;

SubcPriceManagement.GetSubcPriceForPurchLine(PurchOrderLine);
IsHandled := true;
end;

[EventSubscriber(ObjectType::Codeunit, Codeunit::"Req. Wksh.-Make Order", OnInsertPurchOrderLineOnAfterCheckInsertFinalizePurchaseOrderHeader, '', false, false)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,6 @@ report 20505 "Subc. Calculate Subcontracts"
ReqLine."Qty. Rounding Precision (Base)" := ProdOrderLine."Qty. Rounding Precision (Base)";
ReqLine."Prod. Order No." := ProdOrderLine."Prod. Order No.";
ReqLine."Prod. Order Line No." := ProdOrderLine."Line No.";
ReqLine."Due Date" := ProdOrderRoutingLine."Ending Date";
ReqLine."Requester ID" := CopyStr(UserId(), 1, 50);
ReqLine."Location Code" := ProdOrderLine."Location Code";
ReqLine."Bin Code" := ProdOrderLine."Bin Code";
Expand All @@ -208,6 +207,7 @@ report 20505 "Subc. Calculate Subcontracts"
ReqLine."Description 2" := ProdOrderRoutingLine."Description 2";
SetVendorItemNo();
OnAfterTransferProdOrderRoutingLine(ReqLine, ProdOrderRoutingLine);
ReqLine.Validate("Due Date", ProdOrderRoutingLine."Ending Date");

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\ —\ Events}$

OnAfterTransferProdOrderRoutingLine now fires before ReqLine.Validate("Due Date", ProdOrderRoutingLine."Ending Date") (the direct field assignment was removed and replaced with a Validate call placed after the event). The publisher no longer exposes a true 'after transfer' state at the point subscribers observe it: any subscriber that reads or adjusts Due Date at this hook can be overwritten by the later Validate, and subscribers see an incompletely-populated requisition line. Move the Due Date validation before the event, or rename/reposition the event so its firing position matches its documented semantics.

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

        ReqLine.Validate("Due Date", ProdOrderRoutingLine."Ending Date");
        OnAfterTransferProdOrderRoutingLine(ReqLine, ProdOrderRoutingLine);

Knowledge:

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

// If purchase order already exist we will change this if possible
PurchLine.Reset();
PurchLine.SetCurrentKey("Prod. Order No.", "Prod. Order Line No.", "Routing No.", "Operation No.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,16 @@ codeunit 20508 "Subc. Price Management"
end;

procedure GetSubcPriceForPurchLine(var PurchaseLine: Record "Purchase Line")
begin
ApplySubcPriceForPurchLine(PurchaseLine, true);
end;

internal procedure RepriceSubcPurchLineForDateChange(var PurchaseLine: Record "Purchase Line")
begin
ApplySubcPriceForPurchLine(PurchaseLine, false);
end;

local procedure ApplySubcPriceForPurchLine(var PurchaseLine: Record "Purchase Line"; UseRoutingCostFallback: Boolean)
var
ProdOrderRoutingLine: Record "Prod. Order Routing Line";
SubcontractorPrice: Record "Subcontractor Price";
Expand Down Expand Up @@ -544,6 +554,8 @@ codeunit 20508 "Subc. Price Management"
ConvertPriceToCurrency(PurchaseLine."Currency Code", SubcontractorPrice."Currency Code", PriceListCost, DirectCost)
end;
end else begin
if not UseRoutingCostFallback then
exit;
GetUOMPrice(PurchaseLine."No.", PurchaseLine.GetQuantityBase(), SubcontractorPrice, PriceListUOM, PriceListQtyPerUOM, PriceListQty);
ProdOrderRoutingLine.TestField(Type, "Capacity Type"::"Work Center");
DirectCost := ProdOrderRoutingLine."Direct Unit Cost";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,10 @@ codeunit 20534 "Subc. Purchase Line Ext"
if GetExecutionContext() = ExecutionContext::Upgrade then
exit;

if Rec."Planned Receipt Date" = xRec."Planned Receipt Date" then
if Rec."Order Date" = xRec."Order Date" then
exit;

GetSubcontractingPrice(Rec);
RepriceSubcontractingLineAfterDateChange(Rec);
end;

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

GetSubcontractingPrice(Rec);
RepriceSubcontractingLineAfterDateChange(Rec);
end;

[EventSubscriber(ObjectType::Table, Database::"Purchase Line", OnAfterValidateEvent, Quantity, false, false)]
Expand Down Expand Up @@ -350,10 +350,34 @@ codeunit 20534 "Subc. Purchase Line Ext"
var
SubcPriceManagement: Codeunit "Subc. Price Management";
begin
if (PurchaseLine.Type = PurchaseLine.Type::Item) and (PurchaseLine."No." <> '') and (PurchaseLine."Prod. Order No." <> '') and (PurchaseLine."Operation No." <> '') then
if IsSubcontractingPurchaseLine(PurchaseLine) then
SubcPriceManagement.GetSubcPriceForPurchLine(PurchaseLine);
end;

local procedure RepriceSubcontractingLineAfterDateChange(var PurchaseLine: Record "Purchase Line")
var
PurchaseHeader: Record "Purchase Header";
SubcPriceManagement: Codeunit "Subc. Price Management";
begin
if not IsSubcontractingPurchaseLine(PurchaseLine) then
exit;

PurchaseHeader := PurchaseLine.GetPurchHeader();

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\ —\ Performance}$

The new RepriceSubcontractingLineAfterDateChange path calls PurchaseLine.GetPurchHeader() to fetch the full Purchase Header just to inspect Status. This runs from frequently-fired Purchase Line OnAfterValidateEvent subscribers (Order Date / Planned Receipt Date changes), so it needlessly materializes a wide header row on every qualifying date change instead of loading only the Status field.

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

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

Knowledge:

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

if PurchaseHeader.Status <> PurchaseHeader.Status::Open then
exit;

SubcPriceManagement.RepriceSubcPurchLineForDateChange(PurchaseLine);
end;

local procedure IsSubcontractingPurchaseLine(PurchaseLine: Record "Purchase Line"): Boolean
begin
exit(
(PurchaseLine.Type = PurchaseLine.Type::Item) and
(PurchaseLine."No." <> '') and
(PurchaseLine."Prod. Order No." <> '') and
(PurchaseLine."Operation No." <> ''));
end;

[EventSubscriber(ObjectType::Table, Database::"Purchase Line", OnBeforeOpenItemTrackingLines, '', false, false)]
local procedure OpenProdOrderLineItemTrackingOnBeforeOpenItemTrackingLines(PurchaseLine: Record "Purchase Line"; var IsHandled: Boolean)
begin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,6 @@ codeunit 20557 "Subc. Purchase Order Creator"
RequisitionLine."Qty. Rounding Precision (Base)" := ProdOrderLine."Qty. Rounding Precision (Base)";
RequisitionLine."Prod. Order No." := ProdOrderLine."Prod. Order No.";
RequisitionLine."Prod. Order Line No." := ProdOrderLine."Line No.";
RequisitionLine."Due Date" := ProdOrderRoutingLine."Ending Date";
RequisitionLine."Requester ID" := CopyStr(UserId(), 1, MaxStrLen(RequisitionLine."Requester ID"));

RequisitionLine."Location Code" := ProdOrderLine."Location Code";
Expand All @@ -718,6 +717,7 @@ codeunit 20557 "Subc. Purchase Order Creator"
RequisitionLine.Description := ProdOrderRoutingLine.Description;
RequisitionLine."Description 2" := ProdOrderRoutingLine."Description 2";
RequisitionLine.Validate("Subc. Standard Task Code", ProdOrderRoutingLine."Standard Task Code");
RequisitionLine.Validate("Due Date", ProdOrderRoutingLine."Ending Date");
SetVendorItemNo(RequisitionLine);

if PurchLineExists(PurchaseLine, ProdOrderLine, ProdOrderRoutingLine) then begin
Expand Down
128 changes: 128 additions & 0 deletions src/Apps/W1/Subcontracting/Test/Tests/SubcPricingTest.Codeunit.al
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ codeunit 139982 "Subc. Pricing Test"
CreateDateEffectiveSubcontractingScenario(Item, ProductionOrder, ProdOrderRoutingLine, EarlierPrice, LaterPrice);
SubcontractingMgmtLibrary.CreateReqWkshTemplateAndName(ReqWkshTemplate, RequisitionWkshName);
SubcontractingMgmtLibrary.CalculateSubcontractsAndFindReqLine(RequisitionWkshName, ProductionOrder."No.", RequisitionLine);
Assert.IsTrue(RequisitionLine."Order Date" < WorkDate(), 'The worksheet line order date must be scheduled before WorkDate.');
Assert.AreEqual(EarlierPrice, RequisitionLine."Direct Unit Cost", 'The worksheet line must use the price valid on its scheduled order date.');

// [WHEN] The worksheet action is carried out for the subcontracting operation
SubcontractingMgmtLibrary.CarryOutSubcontractingAction(RequisitionLine);
Expand All @@ -136,6 +138,39 @@ codeunit 139982 "Subc. Pricing Test"
Assert.AreEqual(EarlierPrice, PurchaseLine."Direct Unit Cost", 'The purchase line must use the subcontractor price valid on its order date.');
end;

[Test]
procedure ReqWkshCreatedSubcPurchLinePreservesManualCost()
var
Item: Record Item;
ProductionOrder: Record "Production Order";
ProdOrderRoutingLine: Record "Prod. Order Routing Line";
PurchaseLine: Record "Purchase Line";
ReqWkshTemplate: Record "Req. Wksh. Template";
RequisitionLine: Record "Requisition Line";
RequisitionWkshName: Record "Requisition Wksh. Name";
EarlierPrice: Decimal;
LaterPrice: Decimal;
ManualCost: Decimal;
begin
// [SCENARIO 648535] Carrying out a subcontracting worksheet preserves a manually entered cost
Initialize();

// [GIVEN] A backward-scheduled subcontracting worksheet line with a manually changed cost
CreateDateEffectiveSubcontractingScenario(Item, ProductionOrder, ProdOrderRoutingLine, EarlierPrice, LaterPrice);
SubcontractingMgmtLibrary.CreateReqWkshTemplateAndName(ReqWkshTemplate, RequisitionWkshName);
SubcontractingMgmtLibrary.CalculateSubcontractsAndFindReqLine(RequisitionWkshName, ProductionOrder."No.", RequisitionLine);
ManualCost := EarlierPrice + LibraryRandom.RandDecInRange(10, 100, 2);
RequisitionLine.Validate("Direct Unit Cost", ManualCost);
RequisitionLine.Modify(true);

// [WHEN] The worksheet action is carried out
SubcontractingMgmtLibrary.CarryOutSubcontractingAction(RequisitionLine);

// [THEN] The purchase line keeps the manually entered worksheet cost
SubcontractingMgmtLibrary.FindSubcPurchLineForProdOrder(PurchaseLine, Item."No.", ProductionOrder."No.");
Assert.AreEqual(ManualCost, PurchaseLine."Direct Unit Cost", 'The purchase line must preserve the manually entered worksheet cost.');
end;

[Test]
procedure ExpectedReceiptDateChangeRepricesBackwardScheduledSubcPurchLine()
var
Expand Down Expand Up @@ -223,6 +258,99 @@ codeunit 139982 "Subc. Pricing Test"
Assert.AreEqual(LaterPrice, PurchaseLine."Direct Unit Cost", 'The purchase line must use the subcontractor price valid on its changed order date.');
end;

[Test]
procedure LeadTimeChangeRepricesSubcPurchLineWhenOrderDateChanges()
var
Item: Record Item;
ProductionOrder: Record "Production Order";
ProdOrderRoutingLine: Record "Prod. Order Routing Line";
PurchaseLine: Record "Purchase Line";
NewLeadTimeCalculation: DateFormula;
EarlierPrice: Decimal;
LaterPrice: Decimal;
begin
// [SCENARIO 648535] Lead-time-only rescheduling reapplies the price when the resulting order date changes
Initialize();

// [GIVEN] A backward-scheduled subcontracting purchase line with a requested receipt date
CreateDateEffectiveSubcontractingScenario(Item, ProductionOrder, ProdOrderRoutingLine, EarlierPrice, LaterPrice);
CreateSubcontractingPurchaseLine(PurchaseLine, ProdOrderRoutingLine, Item."No.", ProductionOrder."No.");
PurchaseLine.TestField("Requested Receipt Date");
Assert.AreEqual(EarlierPrice, PurchaseLine."Direct Unit Cost", 'The purchase line must initially use the earlier subcontractor price.');

// [WHEN] Lead Time Calculation is cleared without changing Planned Receipt Date
Clear(NewLeadTimeCalculation);
PurchaseLine.Validate("Lead Time Calculation", NewLeadTimeCalculation);

// [THEN] The changed Order Date selects the later date-effective price
Assert.IsTrue(PurchaseLine."Order Date" >= WorkDate(), 'The recalculated purchase line order date must be on or after WorkDate.');
Assert.AreEqual(LaterPrice, PurchaseLine."Direct Unit Cost", 'The purchase line must use the price valid on the recalculated order date.');
end;

[Test]
procedure DateChangeWithoutMatchingPricePreservesSubcPurchLineCost()
var
Item: Record Item;
ProductionOrder: Record "Production Order";
ProdOrderRoutingLine: Record "Prod. Order Routing Line";
PurchaseLine: Record "Purchase Line";
SubcontractorPrice: Record "Subcontractor Price";
EarlierPrice: Decimal;
LaterPrice: Decimal;
OriginalDirectUnitCost: Decimal;
begin
// [SCENARIO 648535] Rescheduling without a matching subcontractor price preserves the calculated purchase-line cost
Initialize();

// [GIVEN] A subcontracting purchase line whose subcontractor prices are no longer applicable
CreateDateEffectiveSubcontractingScenario(Item, ProductionOrder, ProdOrderRoutingLine, EarlierPrice, LaterPrice);
CreateSubcontractingPurchaseLine(PurchaseLine, ProdOrderRoutingLine, Item."No.", ProductionOrder."No.");
OriginalDirectUnitCost := PurchaseLine."Direct Unit Cost";
SubcontractorPrice.SetRange("Item No.", Item."No.");
SubcontractorPrice.DeleteAll(true);

// [WHEN] Planned Receipt Date is changed
PurchaseLine.Validate("Planned Receipt Date", CalcDate('<20D>', WorkDate()));

// [THEN] The existing calculated cost is preserved
Assert.AreEqual(OriginalDirectUnitCost, PurchaseLine."Direct Unit Cost", 'Rescheduling without a matching price must preserve the existing cost.');
end;

[Test]
procedure ReleasedSubcPurchLineDateChangesDoNotReprice()
var
Item: Record Item;
ProductionOrder: Record "Production Order";
ProdOrderRoutingLine: Record "Prod. Order Routing Line";
PurchaseHeader: Record "Purchase Header";
PurchaseLine: Record "Purchase Line";
NewOrderDate: Date;
NewPlannedReceiptDate: Date;
EarlierPrice: Decimal;
LaterPrice: Decimal;
ReleasedDirectUnitCost: Decimal;
begin
// [SCENARIO 648535] Scheduling dates remain editable on a released subcontracting order without changing financial terms
Initialize();

// [GIVEN] A released subcontracting purchase order using the earlier date-effective price
CreateDateEffectiveSubcontractingScenario(Item, ProductionOrder, ProdOrderRoutingLine, EarlierPrice, LaterPrice);
CreateSubcontractingPurchaseLine(PurchaseLine, ProdOrderRoutingLine, Item."No.", ProductionOrder."No.");
PurchaseHeader.Get(PurchaseLine."Document Type", PurchaseLine."Document No.");
LibraryPurchase.ReleasePurchaseDocument(PurchaseHeader);
ReleasedDirectUnitCost := PurchaseLine."Direct Unit Cost";
NewPlannedReceiptDate := CalcDate('<20D>', WorkDate());
NewOrderDate := CalcDate('<10D>', WorkDate());

// [WHEN] Planned Receipt Date and Order Date are changed
PurchaseLine.Validate("Planned Receipt Date", NewPlannedReceiptDate);
PurchaseLine.Validate("Order Date", NewOrderDate);

// [THEN] The dates change without repricing the released line
Assert.AreEqual(NewOrderDate, PurchaseLine."Order Date", 'The released purchase line order date must remain editable.');
Assert.AreEqual(ReleasedDirectUnitCost, PurchaseLine."Direct Unit Cost", 'Scheduling a released purchase line must not change its direct unit cost.');
end;

[Test]
[Scope('OnPrem')]
procedure DeleteWorkCenterWithPricesDeletesRelatedPrices()
Expand Down
Loading