Track receipt on invoice per purchase order line

[Joshua (mynjj)]

Stacked on #10526 (stack #10527).

AB#625392

[github-actions]

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

UpgradePurchaseLineReceiptOnInvoice() backfills Purchase Line."Receipt on Invoice" with DataTransfer.CopyFields(), but the new field's OnValidate is load-bearing: it calls InitQtyToReceive() for orders and updates existing matched rows through ApplyPurchaseLineReceiptSettingToMatches(). Because DataTransfer skips validation, migrated open orders that already had header-level Receipt on Invoice enabled will get the new line flag without the required derived-state updates (Qty. to Receive not reinitialized, Matched Order Line rows not refreshed). The same pattern is duplicated in the AT/BE/CH/IT/NL/NO/RU/W1 upgrade codeunits.

Knowledge:

• microsoft/knowledge/upgrade/datatransfer-skips-triggers-and-subscribers.md
• microsoft/knowledge/upgrade/datatransfer-for-bulk-init.md

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

[github-actions]

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

This PR changes Purchase Line validation from raising an error to silently clearing "Receipt on Invoice" when "Location Code" or "No." is changed to a value that is no longer eligible (directed put-away/pick location, or item with serial/lot/package tracking). The rejection reason built by IsLineReceiptOnInvoiceAllowed is computed and then discarded instead of surfaced, so a previously blocking validation now silently mutates document state. Extensions, tests, or automations that relied on Validate(...) failing will now continue with mutated state instead of an error. The same pattern is duplicated across the localized PurchaseLine.Table.al copies. Consider surfacing the reason (e.g. via a non-blocking notification) or gating the auto-reset behind an explicit, documented opt-in.

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Data\ Modeling}$

The upgrade routine copies header-level "Receipt on Invoice" = true onto every purchase order line via DataTransfer, bypassing the new line-level validation that rejects directed put-away locations, specific-tracked items, or lines that already have posted receipts. This can persist line state immediately after upgrade that the runtime would now refuse if set through PurchaseLine.Validate. Consider migrating only eligible lines, or reusing the same per-line eligibility checks as ApplyReceiptOnInvoiceToEligibleLines so the new field is never backfilled with a value the current validation logic considers illegal.

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

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Error\ Handling}$

ApplyVendorsReceiptOnInvoicePolicy prompts for confirmation before disabling an already-enabled "Receipt on Invoice" in interactive sessions, but when GuiAllowed() is false (API/job-queue contexts) it skips the confirmation entirely and silently resets the header and its eligible lines. This makes non-interactive behavior diverge from the UI path and hides a material document change from the caller. If confirmation is required in the UI, the non-interactive path should fail with a clear error instead of mutating the document silently.

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

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

[github-actions]

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

"Buy-from Vendor No." validation now calls ApplyVendorsReceiptOnInvoicePolicy(Rec) after the trigger's existing publisher calls, so it overwrites "Receipt on Invoice" after any subscriber to the existing OnAfter events has already run, with no new post-policy event to hook. Subscribers that used to set or preserve this field via the existing seams can no longer reliably do so. Consider moving the policy application earlier, or adding a new thin publisher after the policy is applied so dependent extensions retain a stable final-state hook.

Knowledge:

• microsoft/knowledge/events/publish-thin-onbefore-onafter-integration-events.md

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

[github-actions]

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

ApplyVendorsReceiptOnInvoicePolicy does a full Vendor.Get even though it only reads "Receipt on Invoice Policy". Vendor is a wide table, so this hot-path lookup (invoked on every Buy-from Vendor No. validation) should call SetLoadFields before Get to avoid loading unused columns.

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

        Vendor.SetLoadFields("Receipt on Invoice Policy");
        if not Vendor.Get(PurchaseHeader."Buy-from Vendor No.") then
            exit;

Knowledge:

• microsoft/knowledge/performance/use-setloadfields-for-partial-records.md

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

[github-actions]

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

RefreshMatchedOrderLineReceipt regresses the old batched-update strategy into one ModifyAll call per purchase line. On orders with many lines, this turns a handful of set-based updates into N separate updates against Matched Order Line. Consider restoring a batched-update strategy (e.g. grouping line SystemIds and updating matches per batch) instead of issuing one ModifyAll per line.

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

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

[github-actions]

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

UpgradePurchaseLineReceiptOnInvoice loops every qualifying Purchase Header and runs DataTransfer.CopyFields once per order, defeating the set-based benefit of DataTransfer on tenants with many eligible orders. This pattern is duplicated in the AT/BE/CH/IT/NL/NO/RU/W1 upgrade codeunits. Consider reworking into a single set-based pass (or far fewer batches) over Purchase Line.

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

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

[github-actions]

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

The new Vendor field "Receipt on Invoice Policy" is added without its own DataClassification in all 14 changed Vendor.Table.al copies (APAC, BE, CH, DACH, ES, FI, FR, GB, IT, NA, NL, NO, RU, W1). Table-scope DataClassification does not satisfy field-level classification for a new field, so this shipped field remains effectively unclassified unless an explicit value (likely CustomerContent, matching the table) is added.

Knowledge:

• microsoft/knowledge/privacy/table-level-data-classification-cascades.md

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

[github-actions]

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

Tests cover the Automatic vendor policy, but there is no test for the Manual downgrade branch in ApplyVendorsReceiptOnInvoicePolicy, which resets an already-enabled order and its lines after a Confirm() (or leaves them unchanged if the user declines). A regression in either confirm outcome would not be caught. Add handler-backed tests for both outcomes.

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

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

[github-actions]

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

UpgradePurchaseLineReceiptOnInvoice backfills the new line field with DataTransfer for existing purchase orders, but no upgrade test exercises this path. Because it only runs during upgrade and is guarded by an upgrade tag, a broken filter or missed case would silently ship to upgraded tenants unnoticed. Add an upgrade test that seeds legacy orders, runs the upgrade, and verifies both correct backfill and idempotence.

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

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

[github-actions]

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

The new Purchase Line field has its own OnValidate eligibility guard, but added tests only cover the already-posted-receipt rejection case, not the specific-tracked-item or directed put-away/pick rejection branches of PurchaseLine.Validate("Receipt on Invoice", true). Regressions in those two disallow paths could slip through undetected. Add explicit line-level tests for both.

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

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

[github-actions]

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

The same PurchaseHeader/PurchaseLine "Receipt on Invoice" field additions and the new Vendor "Receipt on Invoice Policy" field/enum are duplicated verbatim across ~14 country layers (APAC, AT, BE, CH, DACH, ES, FI, FR, GB, IS, IT, NA, NL, NO, RU, SE, W1). This is consistent with BCApps' layered-country architecture, but it multiplies the surface that must stay in sync for future changes (e.g. a fix or field-number shift in one layer must be manually replicated to all others). Consider whether the common Vendor/Purchase fields could be consolidated in W1 with country layers only adding true localization deltas, to reduce long-term maintenance risk.

Line mapping was unavailable, so this was posted as an issue comment.

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

[github-actions]

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

The new extensible enum "Receipt on Invoice Policy" is consumed through a case statement in ApplyVendorsReceiptOnInvoicePolicy. This hard-codes the policy variants into the consumer, so adding another policy requires editing this codeunit, and extension-defined values silently fall through the else branch with no behavior applied. Consider modeling the policy as enum-backed interface dispatch instead of branching on the enum in the consumer.

Knowledge:

• microsoft/knowledge/interfaces/prefer-interface-over-case-branching.md

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

[github-actions]

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

ApplyVendorsReceiptOnInvoicePolicy can raise a Confirm dialog while the caller's flow is mid-update of the purchase document; if the user leaves the dialog open, the transaction/locks are held for the wait duration. Compute the policy choice and confirm before entering the write/line-recreation path, or move the confirm ahead of any document/line modification.

Knowledge:

• microsoft/knowledge/performance/avoid-user-prompts-inside-transactions.md

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

[github-actions]

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

Both new Purchase Line update loops (ApplyReceiptOnInvoiceToLines and ApplyReceiptOnInvoiceToEligibleLines) call FindSet() and then Validate/Modify each iterated row. That takes a read lock first and upgrades it on every Modify, the deadlock-prone shape this rule warns against. Use FindSet(true) for these write loops so the read acquires the update lock up front.

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

    internal procedure ApplyReceiptOnInvoiceToLines(PurchaseHeader: Record "Purchase Header")
    var
        PurchaseLine: Record "Purchase Line";
    begin
        PurchaseLine.SetRange("Document Type", PurchaseHeader."Document Type");
        PurchaseLine.SetRange("Document No.", PurchaseHeader."No.");
        if PurchaseLine.FindSet(true) then
            repeat
                PurchaseLine.Validate("Receipt on Invoice", PurchaseHeader."Receipt on Invoice");
                PurchaseLine.Modify();
            until PurchaseLine.Next() = 0;
    end;

    local procedure ApplyReceiptOnInvoiceToEligibleLines(PurchaseHeader: Record "Purchase Header")
    var
        PurchaseLine: Record "Purchase Line";
        LineReceiptOnInvoice: Boolean;
        ErrorMessage: Text;
    begin
        PurchaseLine.SetRange("Document Type", PurchaseHeader."Document Type");
        PurchaseLine.SetRange("Document No.", PurchaseHeader."No.");
        if PurchaseLine.FindSet(true) then
            repeat
                LineReceiptOnInvoice := PurchaseHeader."Receipt on Invoice";
                if LineReceiptOnInvoice and not IsLineReceiptOnInvoiceAllowed(PurchaseLine, ErrorMessage) then
                    LineReceiptOnInvoice := false;
                PurchaseLine.Validate("Receipt on Invoice", LineReceiptOnInvoice);
                PurchaseLine.Modify();
            until PurchaseLine.Next() = 0;
    end;

Knowledge:

• microsoft/knowledge/performance/findset-true-applies-updlock-on-read.md

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

[github-actions]

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

ReceiptOnInvoiceForMatch re-fetches a full Purchase Line row even though it only reads the new Receipt on Invoice flag. This helper sits on the save path for matching groups, so loading the whole wide line record for every match adds avoidable payload. Call SetLoadFields("Receipt on Invoice") before GetBySystemId().

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

    begin
        OrderLine.SetLoadFields("Receipt on Invoice");
        OrderLine.GetBySystemId(OrderLineSystemId);
        exit(OrderLine."Receipt on Invoice");
    end;

Knowledge:

• microsoft/knowledge/performance/use-setloadfields-for-partial-records.md

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

[github-actions]

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

CheckLineReceiptOnInvoiceAllowed calls Error(ErrorMessage) after IsLineReceiptOnInvoiceAllowed builds ErrorMessage with StrSubstNo(...) from line-specific values such as PurchaseLine."Location Code", PurchaseLine."No.", and PurchaseLine."Line No.". That makes the first Error argument dynamic instead of a Label/TextConst, losing the static classified template needed for error telemetry.

Knowledge:

• microsoft/knowledge/privacy/avoid-strsubstno-prebuild-before-error.md

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

[github-actions]

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

The PR adds new Normal-classified fields without an explicit DataClassification in every touched copy of Purchase Line and Vendor: "Receipt on Invoice" in 14 src/Layers//BaseApp/Purchases/Document/PurchaseLine.Table.al files, and "Receipt on Invoice Policy" in 14 src/Layers//BaseApp/Purchases/Vendor/Vendor.Table.al files. Each new field should declare its own non-ToBeClassified DataClassification value rather than relying on table-level defaults.

Knowledge:

• microsoft/knowledge/privacy/table-level-data-classification-cascades.md

Line mapping was unavailable, so this was posted as an issue comment.

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

[github-actions]

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

ResetReceiptOnInvoiceQst is declared inside ApplyVendorsReceiptOnInvoicePolicy's local var block. BCQuality recommends moving Labels to the codeunit's top-level var block because procedure-scoped Labels are fragile in XLIFF extraction and translation review.

Knowledge:

• microsoft/knowledge/style/labels-declared-at-object-scope.md

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Error\ Handling}$

CheckLineReceiptOnInvoiceAllowed / IsLineReceiptOnInvoiceAllowed prebuild the error text with StrSubstNo across three branches and then raise Error(ErrorMessage) from the new "Receipt on Invoice" field OnValidate. This is flagged from three angles: (1) error-handling guidance prefers FieldError for field-validation failures so the platform supplies field/record context automatically instead of a hand-built Error; (2) style guidance says Error() should receive the label and its parameters directly rather than a pre-formatted Text, so translation/analyzer tooling can still see the original label identity; (3) privacy/telemetry guidance notes that prebuilding the message before raising Error loses the static label identity needed for RT0030 error classification. All three point at the same code shape and the same

Recommendation:

• raise structured errors (FieldError or Error(Label, Params...)) directly at the point of failure instead of composing Text in a helper and raising it later.

Knowledge:

• microsoft/knowledge/error-handling/fielderror-vs-testfield.md
• microsoft/knowledge/error-handling/fielderror-default-message-logic.md
• microsoft/knowledge/style/error-passes-parameters-directly-not-strsubstno.md
• microsoft/knowledge/privacy/avoid-strsubstno-prebuild-before-error.md

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