[main] Hidden breaking change and instability on sales/purchase line insert

[JakovljevicDusan]

What & why

ISSUE
The existing check was introduced in v28 to prevent the insertion of sales and purchase document lines when the corresponding header does not exist at the time the record is actually inserted. The check is intentionally bypassed in specific scenarios where this behavior is allowed by design, such as when the corresponding "Sales Header" is created immediately afterward.
However, the check has proven to be unreliable in several scenarios and can raise false-positive errors for operations that are valid and expected to succeed.

SOLUTION
Changes in "Sales Line":

• Rename variable HasSalesHeader -> SuppressSalesHeaderExistsVerification, to make it clear it is not about (stale) cache state, but switch whether to skip verification.
  • Caveat is that OnBeforeGetSalesHeader() publisher retains parameter named HasSalesHeader, to avoid breaking changes.
  • Add procedure SetSuppressSalesHeaderExistsVerification(), so using SetSalesHeader()/ClearSalesHeader() is not mandatory to change this switch.
  • Default value of SuppressSalesHeaderExistsVerification switch is false. State of the SuppressSalesHeaderExistsVerification can be changed via: SetSalesHeader(), ClearSalesHeader(), SetSuppressSalesHeaderExistsVerification(), OnBeforeGetSalesHeader() event.
• Add explicit "if SalesHeader.Get()" check in VerifySalesHeaderExists() procedure, do not rely on flaky (for this purpose) GetSalesHeader().
• Skips SalesHeaderExistsVerification for temporay "Sales Line".

Symmetrical changes in "Purchase Line".

One of the scenarios when Base app suppresses this check using SetSalesHeader() is when recurring sales lines are added during validation of the "Sell-to Customer No." in the "Sales Header".

Linked work

Fixes AB#643231
Fixes #9566

[github-actions]

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

SuppressPurchaseHeaderExistsVerification is set once by SetPurchHeader/SetSuppressPurchaseHeaderExistsVerification, but GetPurchHeader no longer resets it to false when a subsequent lookup fails to find the header (the removed 'HasPurchHeader := false' in the else branch was not replaced). Once a caller has called SetPurchHeader (or explicitly suppressed the check), the flag stays true for the lifetime of the record variable even if the referenced Purchase Header is later deleted, so VerifyPurchaseHeaderExists() on a later Insert of the same Purchase Line instance will silently skip the 'cannot insert line without header' guard instead of re-validating. Under the prior implementation this path was covered because GetPurchHeader kept the flag in sync with the last known lookup result. This same pattern is replicated identically in every localized copy of Purchase Line (APAC, BE, CH, DACH, ES, FI, GB, IT, NA, NL, NO, RU, SE, W1). Reset the flag when the header lookup fails so the guard cannot be bypassed by a stale flag.

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

            end else begin
                Clear(PurchHeader);
                SuppressPurchaseHeaderExistsVerification := false;
            end;

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

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

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

[github-actions]

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

SuppressSalesHeaderExistsVerification is set once by SetSalesHeader/SetSuppressSalesHeaderExistsVerification, but GetSalesHeader no longer resets it to false when a subsequent lookup fails to find the header (the removed 'HasSalesHeader := false' in the else branch was not replaced). Once a caller has called SetSalesHeader (or explicitly suppressed the check), the flag stays true for the lifetime of the record variable even if the referenced Sales Header is later deleted, so VerifySalesHeaderExists() on a later Insert of the same Sales Line instance will silently skip the 'cannot insert line without header' guard instead of re-validating. Under the prior implementation this path was covered because GetSalesHeader kept the flag in sync with the last known lookup result. This same pattern is replicated identically in every localized copy of Sales Line (APAC, BE, CH, DACH, ES, FI, GB, IT, NA, NL, NO, RU, SE, W1). Reset the flag when the header lookup fails so the guard cannot be bypassed by a stale flag.

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

            end else begin
                Clear(SalesHeader);
                SuppressSalesHeaderExistsVerification := false;
            end;

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

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

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

[github-actions]

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

The new Rec.IsTemporary() early exit means PurchaseLine.Insert(true) no longer raises CannotInsertPurchLineWithoutHeaderErr for temporary lines unless a caller explicitly re-enables verification. That widens the suppression contract beyond SetSuppressPurchaseHeaderExistsVerification(...), and the added SCMOrdersVI coverage only exercises non-temporary flows. Either keep temporary lines on the old error path or add a focused temp-line test that proves orphan temporary purchase lines are safe.

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

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

[github-actions]

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

The new Rec.IsTemporary() early exit means SalesLine.Insert(true) no longer raises CannotInsertSalesLineWithoutHeaderErr for temporary lines unless a caller explicitly re-enables verification. That broadens the suppression behavior beyond SetSuppressSalesHeaderExistsVerification(...), and the new SCMOrdersVI tests cover only non-temporary scenarios. Either preserve the previous error behavior for temporary sales lines or add a targeted temp-line test that demonstrates the new gap is intentional and safe.

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

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

[github-actions]

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

All 14 changed Purchase Line variants add VerifyPurchaseHeaderExists() with PurchaseHeaderToVerify.Get("Document Type", "Document No.") used only as an existence check. That new hot-path lookup materializes the full Purchase Header row on every insert even though the code never reads any non-key fields. Set primary-key-only load fields before Get (for example SetLoadFields("Document Type", "No.")) so the verification does not pull the whole header payload.

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

        PurchaseHeaderToVerify.SetLoadFields("Document Type", "No.");
        if not PurchaseHeaderToVerify.Get("Document Type", "Document No.") then
            Error(CannotInsertPurchLineWithoutHeaderErr);

Knowledge:

• microsoft/knowledge/performance/use-setloadfields-for-partial-records.md
• microsoft/knowledge/performance/load-only-primary-key-fields-for-reference-work.md

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

[github-actions]

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

All 12 changed Sales Line variants add VerifySalesHeaderExists() with SalesHeaderToVerify.Get("Document Type", "Document No.") used only as an existence check. That new hot-path lookup materializes the full Sales Header row on every insert even though the code never reads any non-key fields. Set primary-key-only load fields before Get (for example SetLoadFields("Document Type", "No.")) so the verification does not pull the whole header payload.

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

        SalesHeaderToVerify.SetLoadFields("Document Type", "No.");
        if not SalesHeaderToVerify.Get("Document Type", "Document No.") then
            Error(CannotInsertSalesLineWithoutHeaderErr);

Knowledge:

• microsoft/knowledge/performance/use-setloadfields-for-partial-records.md
• microsoft/knowledge/performance/load-only-primary-key-fields-for-reference-work.md

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

[github-actions]

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

The updated W1 tests cover the implicit suppress path via SetSalesHeader/SetPurchHeader and the post-header InitType path, but they still leave the two newly introduced behaviors untested: the explicit SetSuppressSalesHeaderExistsVerification/SetSuppressPurchaseHeaderExistsVerification APIs and the Rec.IsTemporary() exemption inside VerifySalesHeaderExists/VerifyPurchaseHeaderExists. Add dedicated positive/negative cases for toggling the setter and for inserting temporary Sales/Purchase lines so regressions in either new branch fail a test.

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

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

[github-actions]

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

In all changed Purchase Line copies, SetPurchHeader() now turns suppression on, but GetPurchHeader() only clears PurchHeader on a lookup miss and never clears the suppression flag. If the same record variable is later reused for a different Document Type/No. after that miss, OnInsert will skip VerifyPurchaseHeaderExists() and can insert a line whose header does not exist. Clear the suppression flag on lookup failure, or scope suppression to the cached header keys only.

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

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

[github-actions]

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

In all changed Sales Line copies, SetSalesHeader() now turns suppression on, but GetSalesHeader() only clears SalesHeader on a lookup miss and never clears the suppression flag. If the same record variable is later reused for a different Document Type/No. after that miss, OnInsert will skip VerifySalesHeaderExists() and can insert a line whose header does not exist. Clear the suppression flag on lookup failure, or scope suppression to the cached header keys only.

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

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

[github-actions]

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

The shipped IntegrationEvent OnBeforeGetPurchHeader keeps the same signature, but its last var Boolean parameter (still named HasPurchHeader) no longer means "the purchase header was retrieved": in every changed Purchase Line copy it is now backed by the global SuppressPurchaseHeaderExistsVerification flag, and the updated tests rely on that flag skipping the new OnInsert header-existence check. Existing subscribers (including third-party AppSource extensions) that already set HasPurchHeader := true under the old meaning will now also silently suppress insert-time validation for the line, with no signature change to warn them. Preserve the old event contract and introduce a new opt-in event/parameter or setter for suppression instead; if this behavior must ship, document it explicitly as a breaking change before merge.

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

[github-actions]

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

The shipped IntegrationEvent OnBeforeGetSalesHeader likewise keeps the same signature while repurposing its existing HasSalesHeader var Boolean parameter to control SuppressSalesHeaderExistsVerification and skip the new OnInsert header-existence check; the updated tests assert that new behavior. Subscribers (including third-party AppSource extensions) written against the old "header was retrieved" meaning can now silently disable validation without any signature change warning them. Keep the old event contract and add a new opt-in event/parameter or setter for suppression instead; otherwise publish this as an explicit breaking change before merge.

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

[github-actions]

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

This PR adds VerifyPurchaseHeaderExists() to all 14 changed Purchase Line table variants, and every non-temporary insert that does not first call SetPurchHeader now executes Purchase Header.Get("Document Type", "Document No.") from OnInsert. In bulk line-creation paths that use fresh record variables, that turns header verification into one extra primary-key lookup per inserted line for the same document. Reuse the already-loaded global PurchHeader when its keys already match, or cache a verified header per document and fall back to Get only when no cached header is available.

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

[github-actions]

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

This PR adds VerifySalesHeaderExists() to all 12 changed Sales Line table variants, and every non-temporary insert that does not first call SetSalesHeader now executes Sales Header.Get("Document Type", "Document No.") from OnInsert. In bulk line-creation paths that use fresh record variables, that turns header verification into one extra primary-key lookup per inserted line for the same document. Reuse the already-loaded global SalesHeader when its keys already match, or cache a verified header per document and fall back to Get only when no cached header is available.

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

[github-actions]

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

The new SetSuppress*HeaderExistsVerification XML line ends with trailing whitespace in every changed PurchaseLine/SalesLine layer copy (git diff --check reports 26 occurrences). Remove the extra space so the duplicated files stay formatter-clean and do not add avoidable style-only diff noise.

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