[Master]-[Subcontracting] Purchase Return Order cannot be posted for subcontracting, while Corrective Credit Memo works well

[Fixes4BC (neeleshsinghal)]

Fixes AB#647380

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes}$

MfgPurchaseDocumentMgt.Codeunit.al's OnIsSubcontractingCreditMemo event subscriber changed its condition from an exact 'Document Type = Credit Memo' check to PurchaseLine.IsCreditDocType(), which also matches 'Return Order'. This is an intentional fix (validated by the new PostPurchaseReturnOrderForSubcontractingWhenLastOperationIsNotSubcontracted test) enabling posting of subcontracting purchase return orders, but it silently widens what the event named 'OnIsSubcontractingCreditMemo' now considers true (Return Orders too), which may surprise other subscribers/callers relying on the event's literal name/semantics. Consider whether the event name/documentation should be updated to reflect it now also covers Return Orders, or whether Return Order handling should be a distinct, separately named condition.

_{Agent judgement — not directly backed by a BCQuality knowledge article.}

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

[github-actions]

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

The new test PostPurchaseReturnOrderForSubcontractingWhenLastOperationIsNotSubcontracted() calls CreateSubcontractingSetup(...), but that local procedure is defined only inside #if not CLEAN29 ... #endif blocks (e.g. src/Layers/W1/Tests/SCM-Manufacturing/SCMManufacturing70.Codeunit.al lines 3960-3976; equivalently in the BE and IT copies). The new test itself is NOT wrapped in the same #if not CLEAN29 guard, so in a CLEAN29 compilation configuration the call resolves to an undefined procedure and the codeunit fails to compile. This affects all three duplicated copies (W1, BE, IT) identically.

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

#if not CLEAN29
    [Test]
    [HandlerFunctions('PostedPurchaseDocumentLinesPageHandler')]
    procedure PostPurchaseReturnOrderForSubcontractingWhenLastOperationIsNotSubcontracted()
    ...
    end;
#endif

_{Agent judgement — not directly backed by a BCQuality knowledge article.}

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

[github-actions]

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

The new test enqueues an expectation for the PostedPurchaseDocumentLinesPageHandler modal page handler but never calls LibraryVariableStorage.AssertEmpty() to confirm the queued value was consumed. This leaves the UI-handler interaction only partially verified across all three duplicated copies (W1, BE, IT).

Knowledge:

• microsoft/knowledge/testing/ui-handlers-in-tests.md

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

[Predrag Maricic (PredragMaricic)]

S1 - Blocking: The new PostPurchaseReturnOrderForSubcontractingWhenLastOperationIsNotSubcontracted test is compiled unconditionally in the W1, BE, and IT copies, but it calls CreateSubcontractingSetup, which exists only inside #if not CLEAN29. Under a CLEAN29 test build, the helper is removed and all three codeunits contain an unresolved procedure call. Guard the new test with the same preprocessor condition, or refactor its setup to use APIs available in CLEAN29.

The production change itself matches bug 647380: Purchase Line.IsCreditDocType() includes both Return Order and Credit Memo, so extending the subcontracting quantity-validation exemption to return orders is appropriate. The missing LibraryVariableStorage.AssertEmpty() is also worth addressing as noted in the existing inline feedback.

[alexei-dobriansky]

Good Sense Reviewer - Round 1

Recommendation: Request Changes

What this PR does

This changes subcontracting purchase line logic so credit document types, including purchase return orders, skip the quantity balance check that already does not fit subcontracting credit flows. The test sets up a routing where the subcontracting operation is followed by a normal operation, posts the subcontracting purchase order, then reverses it through a purchase return order.

The production condition is targeted: IsCreditDocType() covers return orders and credit memos, and the Purchase Line call site uses this result only to skip ValidateQuantityInvIsBalanced() during Qty. to Invoice validation. However, the added regression test is not buildable under clean symbols because it calls helpers that are guarded out.

Problem-solution fit

Fit: Strong

The bug is a concrete posting failure for a subcontracting return order while the corrective credit memo flow works. The production fix matches that difference by treating both credit document types the same in the subcontracting quantity-validation exemption, and the test mirrors the reported routing shape.

Suggestions

S1 (🔴 High): Test does not compile in clean builds
The new test calls CreateSubcontractingSetup and CalculateSubcontractOrder, but those helpers are removed by clean preprocessor guards. Wrap each added test in the same guard as the helpers it calls, or change the setup to APIs that exist in clean builds. The clean build currently fails with AL0118.

S2 (🟠 Moderate): Handler queue is not verified
The test enqueues the posted invoice number for the page handler but does not assert that the queue is empty after posting. Add LibraryVariableStorage.AssertEmpty() after posting in each copy so the test fails if the handler is not called or leaves data behind.

Risk assessment and necessity

Risk: The production change touches purchase return and credit memo quantity validation for subcontracting lines, so wrong behavior can block or affect reverse document posting. The code path is narrow, but it is still a financial and inventory posting path; the current clean-build failure is immediate.

Necessity: The scenario is important because users need the return order reversal path, not only the corrective credit memo workaround. The production change is small and justified, but the PR should not merge until all added test copies compile.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11009 round=1 by=alexei-dobriansky at=2026-09-03T18:02:28Z lastSha=84f6a355113f40073f27bf40e08fb6b8b93405fe reviewKey=4f26e3b8e983724f208f10925498057f0d509e1c013a6be4a9d3f9b6cb7f5216 suggestions=S1@b45b8e29,S2@617f48c8