Extract NL customization to object extensions (TrMode) by Alexander-Ya · Pull Request #8933 · microsoft/BCApps

Alexander-Ya · 2026-06-30T13:39:15Z

What & why

Linked work

Fixes AB#640601

How I validated this

I read the full diff and it contains only changes I intended.
I built the affected app(s) locally with no new analyzer warnings.
I ran the change in Business Central and confirmed it behaves as expected.
I added or updated tests for the new behavior, or explained below why none are needed.

What I tested and the outcome (required — be specific: scenarios, commands, screenshots for UI changes)

Risk & compatibility

Alexander-Ya · 2026-06-30T13:39:57Z

Fixes AB#640601

github-actions · 2026-06-30T13:54:54Z

Copilot PR Review

Iteration 17 · Outcome: completed

Knowledge source: https://github.com/microsoft/BCQuality@186d8a131465475c79244d994acb872cd5c0d4bf

Findings by domain

Findings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).

Domain	Findings	Knowledge-backed	Agent	Inline	Fallback
Accessibility	1	1	0	0	0
Breaking Changes	2	2	0	2	0

Totals: 3 knowledge-backed · 0 agent findings.

Orchestrator pre-filter (2 file(s) excluded)

layer-disabled (knowledge) : 2 file(s)

_{Findings produced by the AL review agent v1.7.3. Reply 👎 on any inline comment to flag false positives.}

The model-reported anchor for a single-line suggestion can be off by more than 8 lines (e.g. PR #8933 VendorNL Confirm() was off by 10, label rename off by 9). Widen the search window to 40 lines either side so the correct target within the same procedure is considered, while the 0.5 similarity floor and 0.1 ambiguity margin keep an unrelated look-alike from winning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Widening the search window exposed a precision problem: a label-rename suggestion whose added Comment text echoes the field captions at the Error()/Confirm() call site would re-anchor onto the call site (PR #8933 GenJournalLineNL/GeneralLedgerSetupNL 'Text1000000/1' findings), which is worse than the original mis-anchor. Raise the similarity floor to 0.6 and use a bounded window (20). Genuine edit targets - an edited statement or a renamed declaration - score ~0.75-0.99 and re-anchor confidently (the PR #8933 Confirm() findings and the PartnerTypeMismatchMsg->Qst rename now land on the right line). Lower- confidence look-alikes stay below the floor and are suppressed, so the caller posts a manual snippet instead of a wrong auto-applicable anchor. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…Extract-NL-customization-to-object-extensions-v2

alexei-dobriansky · 2026-07-15T17:06:33Z

Agentic PR Review - Round 5

Recommendation: Request Changes

What this PR does

This PR extracts NL BaseApp Transaction Mode customization into extension objects for AB#640601. Since round 4, the author-owned AL changes are test-only: one commit fixes setup in an NL purchase document amount test, and one commit disables a W1 service-order warehouse shipment test.

The NL test setup change looks like a real test fix: it enables Check Doc. Total Amounts and fills the document amount fields before posting. It does not weaken Transaction Mode coverage, but it also does not address the open Transaction Mode scoping issue. The disabled W1 test is not justified and removes active coverage.

Status of previous suggestions

ID	Title	Status	Author response / evidence
S1	Keep the G/L Account field id stable	Addressed	Still addressed. `G/L Account`.`Omit Default Descr. in Jnl.` remains field 11400 in `src/Layers/NL/BaseApp/Finance/GeneralLedger/Account/GLAccount.Table.al`.
S2	Update the matching role center tooltip	Addressed	Still addressed for the copies where the caption was expanded to `General Business Posting Groups`.
S3	Add coverage for Transaction Mode wiring	Not addressed	The only NL test change is in `ERMPurchaseDocAmounts.Codeunit.al`, which covers purchase document amount setup/correction, not Transaction Mode copying or NL-company scoping.
S4	Keep abbreviated tooltip text where the caption is abbreviated	Not addressed	APAC, AU, IT, NA, NL, NO, and RU still show caption `Gen. Business Posting Groups` with tooltip `Open the General Business Posting Groups page.`
S5	Scope all Transaction Mode effects	Not addressed	No author reply was found after round 4, and the new commits do not touch the unguarded Transaction Mode subscribers in `SalesHeaderNL.Codeunit.al`, `PurchaseHeaderNL.Codeunit.al`, `CustomerNL.Codeunit.al`, or `GenJournalLineNL.Codeunit.al`.

New observations (commits since round 4)

S6 - Do not disable unrelated W1 tests
Skip one test comments out [Test] on PullServiceLineNoShipmentUsingUseFilter in src/Layers/W1/Tests/SCM-Service/ServiceOrderRelease.Codeunit.al. This removes a warehouse shipment regression test instead of fixing it. Re-enable it, or replace it with an active equivalent test.

Risk assessment and necessity

Risk: The main product risk is still that non-NL companies can receive NL Transaction Mode behavior through unguarded subscribers that copy or enforce Transaction Mode outside the guarded field OnValidate triggers. The new skipped W1 service-order test adds a separate test-quality risk because it reduces coverage for warehouse shipment filtering in a shared W1 area.

Necessity: The extraction remains necessary for AB#640601 because it removes NL full-object customization and lowers future merge cost. The round-5 test changes are not enough to make the PR safe: the Transaction Mode scope must cover all paths, and unrelated tests should not be disabled to get the branch green.

[AI-PR-REVIEW] version=1 system=github pr=8933 round=5 by=alexei-dobriansky at=2026-07-15T17:03:19Z lastSha=7d59f77109426874a158f3c53db67f16c2f3a498 suggestions=S1:addressed,S2:addressed,S3:notaddressed,S4:notaddressed,S5:notaddressed,S6:new parentRound=4

github-actions · 2026-07-16T16:51:20Z

$\textbf{🟡\ Medium\ Severity\ —\ Testing} \quad \color{gray}{\texttt{\small Iteration\ 15}}$

The [Test] attribute on procedure PullServiceLineNoShipmentUsingUseFilter was commented out (changed to '// [Test]'), which silently removes this test case from the test run instead of deleting it or marking it Obsolete with a documented reason.

The test body, assertions, and helper calls remain fully intact, indicating this was very likely a leftover debugging change rather than an intentional removal. If genuinely intentional, impact would be major (a real regression test for warehouse-shipment source filtering with no matching orders would stop running in CI without any record of why); capped to minor per agent-finding severity rules. Recommend restoring the [Test] attribute, or if the test is no longer valid, deleting it or using [Test, Disabled] equivalent tracked in an issue with a comment explaining the reason.

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

    [Test]

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

…Extract-NL-customization-to-object-extensions-v2

github-actions · 2026-07-17T09:45:38Z

+    Access = Internal;
+
+    [EventSubscriber(ObjectType::Page, Page::"Purch. Invoice Subform", 'OnAfterNoOnAfterValidate', '', false, false)]
+    local procedure OnAfterNoOnAfterValidate(var PurchaseLine: Record "Purchase Line"; xPurchaseLine: Record "Purchase Line")


$\textbf{🔴\ Critical\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 16}}$

The new EventSubscriber OnAfterNoOnAfterValidate on Page::"Purch.

Invoice Subform" declares its second parameter as xPurchaseLine: Record "Purchase Line" (no var), but the publisher (PurchInvoiceSubform.Page.al line 1593) declares it as var xPurchaseLine: Record "Purchase Line". AL requires a subscriber to reproduce the publisher's parameter list, including var/byref modifiers, verbatim. This var mismatch will fail to compile/bind. Compare with the sibling PurchCrMemoSubformNL.Codeunit.al, which correctly copies both var modifiers from its own publisher.

Suggested change

local procedure OnAfterNoOnAfterValidate(var PurchaseLine: Record "Purchase Line"; xPurchaseLine: Record "Purchase Line")

local procedure OnAfterNoOnAfterValidate(var PurchaseLine: Record "Purchase Line"; var xPurchaseLine: Record "Purchase Line")

Knowledge:

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-07-17T09:45:39Z

+    Access = Internal;
+
+    [EventSubscriber(ObjectType::Codeunit, Codeunit::"Sales-Post Prepayments", 'OnBeforePostCustomerEntry', '', false, false)]
+    local procedure OnBeforePostCustomerEntry(var GenJnlLine: Record "Gen. Journal Line"; SalesHeader: Record "Sales Header")


$\textbf{🔴\ Critical\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 16}}$

The new EventSubscriber OnBeforePostCustomerEntry on Codeunit::"Sales-Post Prepayments" is declared with only two parameters, `(var GenJnlLine: Record "Gen.

Journal Line"; SalesHeader: Record "Sales Header"), but the publisher (SalesPostPrepayments.Codeunit.al line 2431) declares six parameters: (var GenJnlLine: Record "Gen. Journal Line"; TotalPrepmtInvLineBuffer: Record "Prepayment Inv. Line Buffer"; TotalPrepmtInvLineBufferLCY: Record "Prepayment Inv. Line Buffer"; CommitIsSuppressed: Boolean; SalesHeader: Record "Sales Header"; DocumentType: Option Invoice,"Credit Memo")`. A subscriber cannot cherry-pick a subset of the publisher's parameters; the full parameter list, in order, must be reproduced. As written this subscriber will not bind to the publisher and the extracted NL Transaction Mode Code/Bank Account Code logic that used to run inline in the old NL override of Sales-Post Prepayments will silently stop firing.

Knowledge:

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-07-17T09:45:41Z

@@ -7581,7 +7582,7 @@ table 36 "Sales Header"
        if ReplaceVATDate then
            "VAT Reporting Date" := VATDateReq;

-        if ReplaceDocDate and ("Document Date" <> PostingDateReq) then begin
+        if ReplacePostingDate and ReplaceDocDate and ("Document Date" <> PostingDateReq) then begin


$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 16}}$

SalesHeader.Table.al's BatchConfirmUpdatePostingDate procedure was changed to if ReplacePostingDate and ReplaceDocDate and (...), but the procedure already exits immediately at its top with if not ReplacePostingDate then exit;.

By the time execution reaches this line, ReplacePostingDate is always true, so the added conjunct is dead/redundant code. The same change is duplicated identically across every per-country-layer copy of SalesHeader.Table.al touched by this PR (W1, NO, and others), suggesting an unrelated, mechanically-applied edit rather than an intentional fix. Recommend removing the redundant ReplacePostingDate and or clarifying the actual intent if this guard is meant to protect against a future refactor of the early exit.

Suggested change

if ReplacePostingDate and ReplaceDocDate and ("Document Date" <> PostingDateReq) then begin

if ReplaceDocDate and ("Document Date" <> PostingDateReq) then begin

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

…Extract-NL-customization-to-object-extensions-v2

github-actions · 2026-07-18T10:46:10Z

+        }
+    }
+
+    procedure UpdateElecTaxDeclCategoryCode()


$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes} \quad \color{gray}{\texttt{\small Iteration\ 17}}$

VATStatementLineNL.TableExt widens UpdateElecTaxDeclCategoryCode from a local procedure in the original full-object NL override to a public tableextension procedure, but it is only invoked from the companion VATStatementLineNL.Codeunit.al within the same app.

Publishing it needlessly turns an internal helper into an external contract that must now be preserved across future refactors.

Suggested change

procedure UpdateElecTaxDeclCategoryCode()

internal procedure UpdateElecTaxDeclCategoryCode()

Knowledge:

microsoft/knowledge/breaking-changes/choose-access-modifiers-deliberately.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

github-actions · 2026-07-18T10:46:12Z

+        UpdateBankAccountsQst: Label 'Do you want to update the bank accounts for this customer to reflect the new value of %1?', Comment = '%1 = Field Caption';
+        PartnerTypeMismatchQst: Label 'The Partner Type does not match the Partner Type defined in Transaction Mode. Do you still want to change the Partner Type?';
+
+    [Scope('OnPrem')]


$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes} \quad \color{gray}{\texttt{\small Iteration\ 17}}$

CustomerNL.TableExt widens UpdateCustomerBankAccounts from a local procedure in the original full-object NL override to a published, non-local procedure marked only with [Scope('OnPrem')], even though every call site remains inside the same tableextension.

That turns an implementation detail into a supported API surface for no functional reason, freezing a contract that was never meant to be external.

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

[Scope('OnPrem')] local procedure UpdateCustomerBankAccounts(UseFieldCaption: Text[250])

Knowledge:

microsoft/knowledge/breaking-changes/choose-access-modifiers-deliberately.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

…Extract-NL-customization-to-object-extensions-v2

github-actions · 2026-07-20T11:47:55Z

+    {
+        addafter("Industrial Classification")
+        {
+            field("Fiscal Entity No."; Rec."Fiscal Entity No.")


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

The new namespaced NL layer repeatedly adds unaffixed members to standard objects, for example field(11400; "Cash Journal"; ...) in src/Layers/NL/BaseApp/Local/Foundation/AuditCodes/SourceCodeSetupNL.TableExt.al:14, field("Fiscal Entity No."; ...) in src/Layers/NL/BaseApp/Local/Foundation/Company/CompanyInformationNL.PageExt.al:13, and action("Post Code Updates") in src/Layers/NL/BaseApp/Local/RoleCenters/AdministratorMainRCNL.PageExt.al:15. A two-level namespace only replaces the owned object affix; fields, controls, and actions added to base objects still need the registered member affix.

Knowledge:

microsoft/knowledge/appsource/two-level-namespace-replaces-object-affix-not-extension-member-affix.md

microsoft/knowledge/appsource/object-affixes-prevent-collisions.md

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

github-actions · 2026-07-20T11:47:56Z

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

The public IntegrationEvent OnBeforeUpdateVendorBankAccounts, moved from the base Vendor codeunit into the new VendorNL.TableExt.al, changed signature from (var IsHandled: Boolean; var VendorBankAccount: Record "Vendor Bank Account") to (var Vendor: Record Vendor; var IsHandled: Boolean; var VendorBankAccount: Record "Vendor Bank Account"). Any existing subscriber compiled against the old signature will fail to bind. Restore the original event signature and introduce a separate event if subscribers also need the Vendor record. Note: the underlying impact is a real breaking change to a public extensibility point, but this is emitted as an advisory agent finding capped at minor severity because no loaded knowledge file currently covers event-publisher signature changes directly — it should be promoted to a knowledge-backed rule.

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

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

github-actions · 2026-07-20T11:47:58Z

+            end;
+        }
+        field(11000002; "Currency Euro"; Code[10])
+        {


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

The Currency Euro OnValidate path dead-ends with a plain Error even though the code already knows the immediate

Recommendation:

when Local Currency is Euro, Currency Euro must be cleared. Raise an ErrorInfo with a Fix-it action so the user can apply the correction in one step instead of dismissing a blocking dialog.

Knowledge:

microsoft/knowledge/error-handling/prefer-errorinfo-for-actionable-errors.md

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

github-actions · 2026-07-20T11:47:59Z

+    }
+
+    var
+        TransactionModeAccountTypeErrorQst: Label '%1 can only be filled in when %2 %3 is equal to Customer or Vendor.', Comment = '%1 - Field Caption of the field causing the error, %2 - Field Caption of the Account Type field, %3 - Field Caption of the Bal. Account Type field';


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

The validation error text on GenJournalLineNL.TableExt.al says Transaction Mode Code is allowed only for Customer or Vendor, but the surrounding TableRelation and lookup logic also accept Employee. Update the message so users who hit this error are told the real set of supported account types.

Suggested change

TransactionModeAccountTypeErrorQst: Label '%1 can only be filled in when %2 %3 is equal to Customer or Vendor.', Comment = '%1 - Field Caption of the field causing the error, %2 - Field Caption of the Account Type field, %3 - Field Caption of the Bal. Account Type field';

TransactionModeAccountTypeErrorQst: Label '%1 can only be filled in when %2 %3 is equal to Customer, Vendor, or Employee.', Comment = '%1 - Field Caption of the field causing the error, %2 - Field Caption of the Account Type field, %3 - Field Caption of the Bal. Account Type field';

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

github-actions · 2026-07-20T11:48:01Z

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

The touched G/L Account field Omit Default Descr. in Jnl. is a Normal stored field but still has no DataClassification property, both before and after this PR's relocation of the field within the object. AS0016 requires field-level classification on every Normal field; leaving this field unclassified keeps its privacy posture undefined.

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.12.4}

github-actions · 2026-07-20T11:48:04Z

+
+tableextension 11382 "G/L Entry NL" extends "G/L Entry"
+{
+    fields


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

src/Layers/NL/BaseApp/Local/Finance/GeneralLedger/Ledger/GLEntryNL.TableExt.al reintroduces field 11301 Open (moved verbatim, including its InitValue = true, from the base G/L Entry table which had the same field removed in this diff) as a new tableextension field, but the NL upgrade work added in this PR only registers upgrade tags for Local SEPA Instr. Priority and Elec. Tax Declaration Setup. Moving a field's ownership from a base table to a table extension does not automatically carry over existing row values; without a tagged upgrade step, pre-upgrade G/L Entry rows will read back the Boolean default false instead of their real historical Open value.

Knowledge:

microsoft/knowledge/upgrade/initvalue-does-not-update-existing-rows.md

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

Extract NL customization to object extensions (TrMode)

a84e4c6

Alexander-Ya requested review from a team June 30, 2026 13:39

github-actions Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread src/Layers/NL/BaseApp/Local/Finance/GeneralLedger/Journal/GenJournalLineNL.TableExt.al

github-actions Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread src/Layers/NL/BaseApp/Local/Finance/GeneralLedger/Setup/GeneralLedgerSetupNL.TableExt.al Outdated

github-actions Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread src/Layers/NL/BaseApp/Local/Foundation/Address/PostCodeRange.Table.al

github-actions Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread src/Layers/NL/BaseApp/Local/Purchases/Vendor/VendorNL.TableExt.al

github-actions Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread src/Layers/NL/BaseApp/Local/Purchases/Vendor/VendorNL.TableExt.al

github-actions Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread src/Layers/NL/BaseApp/Local/Sales/Customer/CustomerNL.TableExt.al

aholstrup1 mentioned this pull request Jun 30, 2026

Trigger full PR build on changes under src/Layers #8939

Merged

4 tasks

Alexander-Ya added 3 commits June 30, 2026 18:59

Update solution

3d45ece

Update solution

17505f8

Remove delocalized objects

3cf3e95

github-actions Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread src/Layers/NL/BaseApp/Local/HumanResources/Employee/EmployeeCardNL.PageExt.al

github-actions Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread src/Layers/NL/BaseApp/Local/Finance/VAT/Registration/VATRegNoFormatNL.Codeunit.al

github-actions Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread src/Layers/NL/BaseApp/Local/Foundation/Company/COmpanyInformationNL.Codeunit.al

github-actions Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread src/Layers/NL/BaseApp/Local/Finance/GeneralLedger/Ledger/GLEntryNL.TableExt.al

Alexander-Ya added 4 commits June 30, 2026 22:38

Add missing changes

884ee8d

Merge branch 'main' of https://github.com/microsoft/BCApps into bugs/…

58cc153

…Extract-NL-customization-to-object-extensions-v2

Update 2 Role Centers

a2c7897

Merge branch 'main' of https://github.com/microsoft/BCApps into bugs/…

ca18779

…Extract-NL-customization-to-object-extensions-v2

This comment was marked as resolved.

Sign in to view

github-actions Bot reviewed Jul 1, 2026

View reviewed changes

Comment thread src/Layers/NL/BaseApp/Local/Sales/Customer/CustomerNL.TableExt.al

github-actions Bot reviewed Jul 1, 2026

View reviewed changes

Comment thread src/Layers/NL/BaseApp/Local/Sales/Customer/CustomerNL.TableExt.al

Add missing changes

b85a07a

github-actions Bot added this to the Version 29.0 milestone Jul 1, 2026

Skip one test

cc4d2c7

github-actions Bot reviewed Jul 14, 2026

View reviewed changes

Comment thread src/Layers/NL/BaseApp/Local/Purchases/Payables/VendorLedgerEntriesNL.PageExt.al

github-actions Bot reviewed Jul 14, 2026

View reviewed changes

Comment thread src/Layers/NL/BaseApp/Local/Purchases/Payables/VendorLedgerEntryNL.TableExt.al Outdated

github-actions Bot reviewed Jul 14, 2026

View reviewed changes

Comment thread src/Layers/NL/BaseApp/Local/Sales/Receivables/CustLedgerEntryNL.TableExt.al Outdated

github-actions Bot reviewed Jul 14, 2026

View reviewed changes

Comment thread src/Layers/W1/Tests/SCM-Service/ServiceOrderRelease.Codeunit.al

Merge branch 'main' of https://github.com/microsoft/BCApps into bugs/…

7d59f77

…Extract-NL-customization-to-object-extensions-v2

Sync changes

5b9206b

github-actions Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread src/Layers/NL/BaseApp/Local/Purchases/Document/PurchaseHeaderNL.Codeunit.al

github-actions Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread src/Layers/NL/BaseApp/Local/Foundation/Company/CompanyInformationNL.TableExt.al Outdated

github-actions Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread src/Layers/W1/Tests/SCM-Service/ServiceOrderRelease.Codeunit.al

Fix code review comments

6a30730

Alexander-Ya added 2 commits July 17, 2026 10:56

Merge branch 'main' of https://github.com/microsoft/BCApps into bugs/…

4950c03

…Extract-NL-customization-to-object-extensions-v2

Revert some changes to skip AL0836 errors

0f239ca

github-actions Bot reviewed Jul 17, 2026

View reviewed changes

Merge branch 'main' of https://github.com/microsoft/BCApps into bugs/…

bb7d296

…Extract-NL-customization-to-object-extensions-v2

github-actions Bot reviewed Jul 18, 2026

View reviewed changes

Alexander-Ya added 3 commits July 20, 2026 11:09

Merge branch 'main' of https://github.com/microsoft/BCApps into bugs/…

d7e89c0

…Extract-NL-customization-to-object-extensions-v2

Fix NL tests and remove delocalized objects

06ca2fb

Sync changes and resolve merge issues

3dfb410

github-actions Bot reviewed Jul 20, 2026

View reviewed changes

	local procedure OnAfterNoOnAfterValidate(var PurchaseLine: Record "Purchase Line"; xPurchaseLine: Record "Purchase Line")
	local procedure OnAfterNoOnAfterValidate(var PurchaseLine: Record "Purchase Line"; var xPurchaseLine: Record "Purchase Line")

	if ReplacePostingDate and ReplaceDocDate and ("Document Date" <> PostingDateReq) then begin
	if ReplaceDocDate and ("Document Date" <> PostingDateReq) then begin

	procedure UpdateElecTaxDeclCategoryCode()
	internal procedure UpdateElecTaxDeclCategoryCode()

	TransactionModeAccountTypeErrorQst: Label '%1 can only be filled in when %2 %3 is equal to Customer or Vendor.', Comment = '%1 - Field Caption of the field causing the error, %2 - Field Caption of the Account Type field, %3 - Field Caption of the Bal. Account Type field';
	TransactionModeAccountTypeErrorQst: Label '%1 can only be filled in when %2 %3 is equal to Customer, Vendor, or Employee.', Comment = '%1 - Field Caption of the field causing the error, %2 - Field Caption of the Account Type field, %3 - Field Caption of the Bal. Account Type field';

Uh oh!

Conversation

Alexander-Ya commented Jun 30, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What & why

Linked work

How I validated this

Risk & compatibility

Uh oh!

Alexander-Ya commented Jun 30, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Copilot PR Review

Findings by domain

Orchestrator pre-filter (2 file(s) excluded)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexei-dobriansky commented Jul 15, 2026

Agentic PR Review - Round 5

Recommendation: Request Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 16, 2026

The [Test] attribute on procedure PullServiceLineNoShipmentUsingUseFilter was commented out (changed to '// [Test]'), which silently removes this test case from the test run instead of deleting it or marking it Obsolete with a documented reason.

Uh oh!

github-actions Bot Jul 17, 2026

Choose a reason for hiding this comment

The new EventSubscriber OnAfterNoOnAfterValidate on Page::"Purch.

Uh oh!

github-actions Bot Jul 17, 2026

Choose a reason for hiding this comment

The new EventSubscriber OnBeforePostCustomerEntry on Codeunit::"Sales-Post Prepayments" is declared with only two parameters, `(var GenJnlLine: Record "Gen.

Uh oh!

github-actions Bot Jul 17, 2026

Choose a reason for hiding this comment

SalesHeader.Table.al's BatchConfirmUpdatePostingDate procedure was changed to if ReplacePostingDate and ReplaceDocDate and (...), but the procedure already exits immediately at its top with if not ReplacePostingDate then exit;.

Uh oh!

github-actions Bot Jul 18, 2026

Choose a reason for hiding this comment

VATStatementLineNL.TableExt widens UpdateElecTaxDeclCategoryCode from a local procedure in the original full-object NL override to a public tableextension procedure, but it is only invoked from the companion VATStatementLineNL.Codeunit.al within the same app.

Uh oh!

github-actions Bot Jul 18, 2026

Choose a reason for hiding this comment

CustomerNL.TableExt widens UpdateCustomerBankAccounts from a local procedure in the original full-object NL override to a published, non-local procedure marked only with [Scope('OnPrem')], even though every call site remains inside the same tableextension.

Uh oh!

github-actions Bot Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 20, 2026

Uh oh!

github-actions Bot Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 20, 2026

Uh oh!

github-actions Bot Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

Alexander-Ya commented Jun 30, 2026 •

edited by github-actions Bot

Loading

github-actions Bot commented Jun 30, 2026 •

edited

Loading

SalesHeader.Table.al's BatchConfirmUpdatePostingDate procedure was changed to `if ReplacePostingDate and ReplaceDocDate and (...)`, but the procedure already exits immediately at its top with `if not ReplacePostingDate then exit;`.