Bug 648535: Preserve subcontracting costs during scheduling

[Chethan Thopaiah (ChethanT)]

AB#648535

What & why

Follow-up to #10917. The original fix repriced subcontracting purchase lines after carry-out and date validation, but that could overwrite calculated or manually entered worksheet costs, fail date changes on released orders, and miss lead-time-only changes where Planned Receipt Date stays the same.

This change schedules and prices subcontracting requisition lines from the routing Ending Date before carry-out, preserves that requisition date and cost instead of replacing them with the purchase-header date, and only applies date-driven purchase-line repricing when the resulting Order Date changes on an open order. If no subcontractor price matches, the existing calculated cost is preserved.

Regression coverage

• Initial direct and worksheet-created lines use the price valid on the backward-scheduled Order Date.
• Manual worksheet costs survive carry-out.
• Lead-time-only Order Date changes reapply date-effective pricing.
• Date changes with no matching price preserve the existing cost.
• Released-order date changes remain allowed without changing financial terms.

Validation

• git diff --cached --check passed before commit.
• Local AL build/tests were not run because this machine has neither Docker nor a local AL compiler; CI is required for compile, analyzer, and runtime test validation.

[github-actions]

$\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:

• microsoft/knowledge/events/name-events-by-publisher-position.md

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

[github-actions]

$\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:

• microsoft/knowledge/performance/use-setloadfields-for-partial-records.md
• microsoft/knowledge/performance/guard-event-subscribers-before-db-call.md

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

[github-actions]

$\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:

• microsoft/knowledge/style/event-subscriber-param-names-match-publisher.md

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

[alexei-dobriansky]

Good Sense Reviewer - Round 1

Recommendation: Request Changes

What this PR does

This change keeps subcontracting requisition and purchase-line pricing aligned with the backward-scheduled order date. It also avoids repricing released orders and preserves an existing cost when no matching subcontractor price exists.

The approach is targeted, and the added tests cover creation, worksheet carry-out, date changes, no-price fallback, and released orders. However, the new event subscriber uses parameter names that do not match the publisher, so the app cannot compile until that signature is corrected.

Problem-solution fit

Fit: Strong

The reported scenario is clear: the price date must follow the final purchase-line order date rather than the header date. The diff targets that problem by pricing after the requisition line has the scheduled date and routing context, and by repricing only when date changes affect open lines.

Suggestions

S1 (🔴 High): Subscriber parameter names do not match
This event subscriber does not compile because PurchaseHeader and PurchaseLine do not match the published event parameter names. Rename them to PurchOrderHeader and PurchOrderLine so the app can build.

Risk assessment and necessity

Risk: This touches subcontractor direct unit cost calculation for purchase lines created from production routing and later schedule changes. The behavioral scope is narrow, but the current build failure means the runtime regression tests cannot protect the pricing path until the compile error is fixed.

Necessity: The change is needed because a wrong date-effective subcontractor price can persist on purchase lines. Avoiding the header-date overwrite and limiting repricing to the right open-line cases is the right scope.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11291 round=1 by=alexei-dobriansky at=2026-09-10T12:22:31.3166507Z lastSha=673cf9069a99562204f2dbd09ccb9339181f43f3 reviewKey=cc3767f928238765ccde0af66184dccf749f22464a8e51abdf7eec4786435b89 suggestions=S1@31fccce3

[Chethan Thopaiah (ChethanT)]

Closing this PR as superseded by #11292.

#11292 retains the post-scheduling pricing flow introduced by #10917 while addressing the uncovered regressions: calculated Units/Time fallback, manual worksheet cost preservation, released-order date edits, lead-time-only Order Date changes, and minimum-quantity tier handling. Equivalent changes are already synchronized to the 29.x and 29.0 backports in #11197 and #11198.

This PR takes a broader scheduling-first approach by changing requisition-line Due Date validation and suppressing the purchase-header Order Date copy. Those behavioral changes are not required for the accepted bug fix and should not be mixed into the release backports. The outstanding review findings are therefore not being addressed here.