[Backport 29.0] Bug 648535: Reprice subcontracting lines after scheduling (#10917)

[Chethan Thopaiah (ChethanT)]

Backport of #10917 to releases/29.0

Backports #10917 — Bug 648535: Reprice subcontracting lines after scheduling — to the releases/29.0 release branch.

What & why

Subcontracting purchase lines were priced before backward scheduling had finalized the purchase-line Order Date and before routing context was transferred from the requisition line, so a subcontractor price valid for the purchase-header date could be selected instead of the price valid for the final purchase-line date.

The review follow-up also:

• preserves the worksheet's calculated Units/Time cost when no subcontractor price matches;
• preserves manually overridden worksheet costs during carry-out;
• keeps date scheduling editable on released purchase orders without changing financial terms;
• reprices lead-time-only changes when the resulting Order Date crosses a price boundary.

Source

• Source PR: Bug 648535: Reprice subcontracting lines after scheduling #10917
• Source squash-merge commit: 049258b463302e36649979d241a6f122c7eb87c0
• Original backport commit: c1924162a
• Corrective follow-up commit: 327c1d82c5
• Main follow-up PR: Bug 648535: Reprice subcontracting lines after scheduling #11292
• ADO: AB#648535

Cherry-pick / conflict resolution

The product changes applied cleanly. The test file conflicted because the release branch predates the Subc. Management Library helper refactor. Only the relevant regression tests were applied, using equivalent local helper procedures already required by this backport. No unrelated main tests were introduced. The resolution is identical to the releases/29.x backport.

Validation

• Static verification: no conflict markers or duplicate procedures; git diff --check passes.
• Four corrective regressions cover no-price fallback, manual worksheet overrides, released-order date edits, and lead-time-only Order Date changes.
• Full AL compile and test execution are deferred to the AL-Go PR build because local symbol packages are unavailable and sandbox policy blocks CoreXT package-state initialization.

⚠️ Do not merge until the AL-Go PR build passes. Not auto-merged.

[Chethan Thopaiah (ChethanT)]

Applied the same corrective follow-up as #11197 in 327c1d8. It preserves calculated no-price fallback costs and manual worksheet overrides, keeps released-order scheduling editable without changing financial terms, and handles lead-time-only Order Date shifts. The four corresponding regression tests are included.

[Copilot]

🟡 Changes recommended

A division-by-zero edge case was introduced in the new non-price-list direct cost calculation (see stored review comment) that can cause runtime failures.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Backport to releases/29.0 that ensures subcontracting purchase lines are repriced using the final (possibly backward-scheduled) purchase-line Order Date, and that scheduling-related edits don’t unintentionally alter financial terms (especially on released orders).

Changes:

• Reprice subcontracting purchase lines after date scheduling/validation so date-effective vendor pricing is applied to the resulting Order Date.
• Preserve worksheet-derived or manually overridden costs during carry-out when repricing should not override them.
• Add regression tests covering backward-scheduled pricing, rescheduling repricing, released-order date edits, and lead-time-only Order Date shifts.

File summaries

File	Description
src/Apps/W1/Subcontracting/Test/Tests/SubcPricingTest.Codeunit.al	Adds regression tests and local helpers for date-effective pricing and carry-out preservation scenarios.
src/Apps/W1/Subcontracting/App/src/Purchase/SubcPurchaseLineExt.Codeunit.al	Adds subscribers to trigger subcontracting repricing on Planned Receipt Date / Order Date changes, with a guard for non-open headers.
src/Apps/W1/Subcontracting/App/src/Purchase/SubcPriceManagement.Codeunit.al	Refactors purchase-line pricing to support “no price match” fallback and exposes a helper to compute price-list cost without mutating the line.
src/Apps/W1/Subcontracting/App/src/Manufacturing/SubcReqWkshMakeOrd.Codeunit.al	Reapplies subcontractor pricing after requisition→purchase transfer when the transferred value reflects the pre-scheduling price.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[alexei-dobriansky]

Good Sense Reviewer - Round 1

Recommendation: Request Changes

What this PR does

The change backports the subcontracting repricing fix to the 29.0 branch. It reprices subcontracting purchase lines after requisition fields are transferred and after schedule-related date changes, so the final purchase-line Order Date drives date-effective subcontractor pricing. It also tries to preserve worksheet and manual costs and keeps released-order date edits from changing financial terms.

The BaseApp event publisher for OnInsertPurchOrderLineOnAfterTransferFromReqLineToPurchLine exists in the target branch and fires after TransferFromReqLineToPurchLine but before the purchase line is inserted, so the new subscriber has the manufacturing fields it needs. The Purchase Line date flow assigns Order Date directly while validating Planned Receipt Date, so the separate Planned Receipt Date and Order Date subscribers are justified. The remaining issue is the new fallback used when no price list entry matches.

Problem-solution fit

Fit: Partial

The date-based pricing issue is covered for creation and rescheduling, and the tests exercise those paths. The no-price fallback is part of the correction, but one new formula is not safe when expected output quantity is 0, so the solution is not complete yet.

Suggestions

S1 (🔴 High): Guard zero expected output before dividing
When no price matches and the routing cost is time-based, this code divides by ProdOrderLine."Total Exp. Oper. Output (Qty.)" with no zero check. A zero expected output quantity will stop subcontracting repricing with a division-by-zero error. Please handle 0 before the division, for example by keeping the current cost or returning a safe fallback.

Risk assessment and necessity

Risk: Moderate to high because Direct Unit Cost affects subcontracting purchase costs and the new handlers run during purchase-line creation and date validation. The change is scoped to Subcontracting and guarded by feature and open-header checks, but the unguarded no-price fallback can turn a scheduling or carry-out flow into a runtime error. The target-branch BaseApp publisher and purchase-line date flow were verified; the current PR build has not provided a successful validation pass.

Necessity: Justified. Without the change, a subcontracting purchase line can keep a price that is valid for a different date than its final Order Date. The backport scope matches the bug, but the fallback path should be made safe before merge.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11198 round=1 by=alexei-dobriansky at=2026-09-10T00:18:57.9626275Z lastSha=327c1d82c581a4dd8cbb00bb27db37b21a5eafa0 reviewKey=90fbd8771802006d8815ef36608ca189d223215b0a9322c5a74ab877dd509949 suggestions=S1@006633ee

[Chethan Thopaiah (ChethanT)]

Applied the accepted #11292 review follow-up in eae59488df: zero-output fallback guard, pre-lookup applicability guards, partial routing-line loading, and direct no-price fallback coverage for Units, Time, and zero expected output. The suggested two-Get routing lookup was intentionally not adopted because the existing Released|Finished FindFirst is one bounded clustered-key query.

[Copilot]

🟡 Changes recommended

A confirmed pricing-path edge case can incorrectly treat “no minimum-quantity price match” as a successful lookup and set Direct Unit Cost to 0, which needs correction before merge.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/Apps/W1/Subcontracting/Test/Tests/SubcPricingTest.Codeunit.al:1063

• PurchaseLine.Find() here is relying on whatever filters/key state happens to be on the variable after releasing the header. That’s brittle and can become flaky if additional purchase lines exist in the same test run. Re-fetch the exact line by primary key instead.

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

A new test uses PurchaseLine.Find() without deterministic filters/key after release, which can make the test flaky or target the wrong record.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/Apps/W1/Subcontracting/Test/Tests/SubcPricingTest.Codeunit.al:1120

• PurchaseLine.Find(); after releasing the header is not deterministic unless filters/key are set; it can move the record pointer to an unrelated line. To reliably reload the same line after ReleasePurchaseDocument, re-GET by primary key.
  src/Apps/W1/Subcontracting/App/src/Manufacturing/SubcReqWkshMakeOrd.Codeunit.al:52
• In OnInsertPurchOrderLineOnAfterTransferFromReqLineToPurchLine, the variable name ReqLineDatePriceListCost is misleading: TryGetSubcPriceListCostForPurchLine returns the Direct Unit Cost (after UOM/currency conversion), not the raw price list cost. This makes the intent of the equality check harder to understand and increases the risk of misuse later.

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[alexei-dobriansky]

Good Sense Reviewer - Round 2

Recommendation: Request Changes

What this PR does

The new commits address the previous zero-output fallback issue and add coverage for no-price fallbacks, minimum-quantity tiers, released-line date edits, and lead-time-only rescheduling. The original date-based repricing path is still mostly in place: the creation subscriber compares the transferred cost with the requisition-date price before repricing with the final purchase-line Order Date, and the date subscribers reprice open subcontracting lines when Order Date changes.

The zero-output division is now guarded, but the new minimum-quantity false return leaves a carry-out path where final-date repricing can be skipped. If the requisition-date price record has no applicable minimum-quantity tier, the creation subscriber exits before checking the final purchase-line Order Date.

Status of previous suggestions

ID	Title	Status	Author response
S1	Guard zero expected output before dividing	Addressed	A zero-output guard and regression test were added.

New observations (commits since round 1)

S2 (🔴 High): Carry-out can skip final-date pricing after a quantity miss
The new false return from TryGetSubcPriceListCostForPurchLine makes the creation subscriber exit when the requisition-date price has no applicable minimum-quantity tier. That happens before GetSubcPriceForPurchLine can reprice with the final purchase-line Order Date. In that case, a valid final-date price can be missed and the purchase line can keep 0 or another worksheet cost.

Risk assessment and necessity

Risk: High because the affected value is Direct Unit Cost on subcontracting purchase lines, and the missed branch can persist a wrong purchase cost. The change is still scoped to Subcontracting and the target-branch publisher and date flow were verified, but the minimum-quantity/date-effective combination remains inside the main carry-out path. The remote build was still running when this review was rendered; git diff --check passed locally.

Necessity: The change is justified because subcontracting pricing must use the final purchase-line Order Date after backward scheduling. The scope is right for the reported bug, but the carry-out guard should still allow final-date repricing when the requisition-date lookup fails only because no minimum-quantity tier applies.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11198 round=2 by=alexei-dobriansky at=2026-09-10T12:32:05.6985192Z lastSha=12de38cc875495f0c04220bb392ed09b6e42ee75 reviewKey=077396b0c86fd8e545011da5a2193931d8d10a866368479cd531cd0d5d7e456d suggestions=S1@006633ee:addressed,S2@d13a1cdd:new parentRound=1

[Chethan Thopaiah (ChethanT)]

Synchronized the latest #11292 review fixes in 88998c83f0. This includes requisition-context automatic-cost detection, final purchase-line date pricing, minimum-quantity fallback handling, strict missing-routing diagnostics, and release-compatible regression tests.

[Copilot]

🟡 Changes recommended

There is a confirmed divide-by-zero risk in the updated minimum-amount adjustment logic when repricing with zero quantity (see stored comment).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

[alexei-dobriansky]

Good Sense Reviewer - Round 3

Recommendation: Request Changes

What this PR does

The latest commit addresses the previous carry-out pricing gap by comparing the requisition line with the automatic subcontracting cost, then repricing the purchase line with the final line Order Date when the cost was not manually overridden. It also keeps the fallback path for no applicable price and adds more regression coverage for final-date pricing and minimum-quantity fallback.

The pricing flow now matches the reported scenario: automatic worksheet costs can be replaced by the price valid on the final purchase-line date, while manual worksheet overrides are preserved. The remaining blocker is not the pricing logic itself; the new test code does not compile on the target branch because a library variable was not backported.

Status of previous suggestions

ID	Title	Status	Author response
S1	Guard zero expected output before dividing	Addressed	The zero-output guard and regression test are still present.
S2	Carry-out can skip final-date pricing after a quantity miss	Addressed	The creation path now uses automatic-cost detection and still calls final-date purchase-line pricing when the requisition-date lookup has no applicable tier.

New observations (commits since round 2)

S3 (🔴 High): Declare LibraryRandom for new tests
The new tests use LibraryRandom, but this test codeunit does not declare it on the target branch. The W1 compile fails with AL0118 for this symbol. Add LibraryRandom: Codeunit "Library - Random"; to the global var block, or replace the new random calls.

Risk assessment and necessity

Risk: High until the compile error is fixed, because the Subcontracting test app cannot build. The functional change still touches Direct Unit Cost on subcontracting purchase lines, so the added regressions need to compile and run before merge.

Necessity: The change is justified because subcontracting pricing must use the final purchase-line Order Date after scheduling. The latest product code is targeted and matches the needed behavior, but the backport is not merge-ready while its tests reference an undeclared library.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11198 round=3 by=alexei-dobriansky at=2026-09-11T09:35:48Z lastSha=88998c83f040e805b6bed627f8407d96bf25fff6 reviewKey=880c570da34275300e5c86317a7b9a9a089456204926e03a8954e2b6f6250227 suggestions=S1@006633ee:addressed,S2@d13a1cdd:addressed,S3@0f191555:new parentRound=2