Dev/aan/remittance advice

[AndriusAndrulevicius]

What & why

Linked work

Fixes #

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Agent} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

In PopulateRemittanceAdviceEDocument, if Vendor.Get(TempRemitAdviceBuffer."Vendor No.") then; silently discards the result of Vendor.Get with an empty then-branch.

If the vendor no longer exists (deleted after the payment was posted), Vendor stays at its default/blank record and the E-Document is populated with a blank "Bill-to/Pay-to Name" instead of surfacing an error or a clear fallback — a data-integrity gap that spans error handling and record-lifecycle correctness, which no single domain leaf's worklist covered for this exact line. Replace the empty if-then with an explicit branch, e.g. only set the name when Vendor.Get succeeds, or raise/log when it fails so a missing vendor is not silently masked as a blank name on the outgoing document.

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

        EDocument."Document No." := TempRemitAdviceBuffer."Payment Document No.";
        EDocument."Bill-to/Pay-to No." := TempRemitAdviceBuffer."Vendor No.";
        if Vendor.Get(TempRemitAdviceBuffer."Vendor No.") then
            EDocument."Bill-to/Pay-to Name" := Vendor.Name;

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Error\ Handling} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

This recoverable failure tells the user to use the Cancel action on the E-Document page, but it raises a plain Error with no navigation action.

Raise an ErrorInfo with PageNo/RecordId and AddNavigationAction so the user can open the related E-Document directly instead of being left at a dead-end dialog.

Knowledge:

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

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Events} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

The new integration event fires after AddHeaderElements has already run, but its name is OnBeforeAddHeaderElements.

That reverses the publisher position encoded in the name, so subscribers cannot tell when the hook actually runs. Rename it to an OnAfter... or other position-accurate form that matches the real firing point.

Knowledge:

• microsoft/knowledge/events/name-events-by-publisher-position.md

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Interfaces} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

The persisted, extensible "PEPPOL 3.0 Purchase" enum now adds only a DefaultImplementation for "PEPPOL Remit.

Advice Info Provider", and the new remittance export assigns the setup-stored enum value to that interface. If a localization adds a format value and is later uninstalled, an unknown ordinal will not resolve through DefaultImplementation and this path can fail with a technical runtime error. Add UnknownValueImplementation for this interface.

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

    DefaultImplementation = "PEPPOL Purchase Attachment Provider" = "PEPPOL30",
                            "PEPPOL Purchase Delivery Info Provider" = "PEPPOL30",
                            "PEPPOL Purchase Document Info Provider" = "PEPPOL30",
                            "PEPPOL Purchase Line Info Provider" = "PEPPOL30",
                            "PEPPOL Purchase Monetary Info Provider" = "PEPPOL30",
                            "PEPPOL Purchase Party Info Provider" = "PEPPOL30",
                            "PEPPOL Purchase Payment Info Provider" = "PEPPOL30",
                            "PEPPOL Purchase Tax Info Provider" = "PEPPOL30",
                            "PEPPOL Remit. Advice Info Provider" = "PEPPOL30";
    UnknownValueImplementation = "PEPPOL Remit. Advice Info Provider" = "PEPPOL30";
    Extensible = true;

Knowledge:

• microsoft/knowledge/interfaces/handle-unknown-enum-ordinals-with-unknownvalueimplementation.md

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

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

The nested DetailedVendLedgEntry loop calls VendLedgEntry3.Get(...) for every row, creating an N+1 access pattern against the persistent Vendor Ledger Entry table.

Reshape this to a joined query or cache the looked-up entries by entry number so the inner table is not re-read once per outer row.

Knowledge:

• microsoft/knowledge/performance/avoid-get-inside-loop-on-large-table.md

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

The AppliedVendLedgEntry loop calls CalcFields(Amount, "Remaining Amount") on every row even though each iteration reads the same FlowFields.

Call SetAutoCalcFields(Amount, "Remaining Amount") before FindSet() and remove the per-row CalcFields call.

Suggested change

	FindAppliedEntries(PaymentVendLedgEntry, AppliedVendLedgEntry);
	if AppliedVendLedgEntry.FindSet() then
	repeat
	FindAppliedEntries(PaymentVendLedgEntry, AppliedVendLedgEntry);
	AppliedVendLedgEntry.SetAutoCalcFields(Amount, "Remaining Amount");
	if AppliedVendLedgEntry.FindSet() then
	repeat

Knowledge:

• microsoft/knowledge/performance/use-setautocalcfields-for-per-row-flowfields.md

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

[github-actions]

$\textbf{🔴\ Critical\ Severity\ —\ Testing} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

The confirm handlers always return a hardcoded reply and never verify which prompt was raised.

That lets these tests go green even if the code shows the wrong confirm dialog, so they can pass while validating the wrong UI behavior.

Knowledge:

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

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

[github-actions]

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

The negative test pins the failure with an inline message fragment instead of a shared label or assertion helper.

That makes the test brittle to wording or localization changes and duplicates error knowledge in the test body.

Knowledge:

• microsoft/knowledge/testing/asserterror-needs-expectederror-and-code.md

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Upgrade} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

The new remittance-advice report selections are seeded only through first-install/new-company paths: InitReportSelectionPurch() is called from CompanyInitialize, and CompanyInitialize explicitly skips report-selection initialization during ExecutionContext::Upgrade.

The old SetupRemittanceReports codeunit is still Subtype = Install, so existing companies upgrading to this build will not get V.Remittance / P.V.Remit. rows unless they already had them. This ships feature-required data without a version-upgrade migration; add a per-company Subtype = Upgrade step, guard it with an upgrade tag, and register that tag for new-company seeding.

Knowledge:

• microsoft/knowledge/upgrade/install-code-does-not-run-on-version-upgrade.md
• microsoft/knowledge/upgrade/upgrade-codeunit-subtype.md
• microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.md

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

[github-actions]

Copilot PR Review

Iteration 2 · 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	2	2	0	2	0
AppSource	1	1	0	0	1
Data Modeling	1	1	0	1	0
Error Handling	1	1	0	0	0
Events	1	1	0	0	0
Interfaces	1	1	0	0	1
Performance	2	2	0	1	0
Security	1	1	0	1	0
Style	2	2	0	2	0
Testing	3	3	0	2	0
Upgrade	1	1	0	0	0

Totals: 16 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.}

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Accessibility} \quad \color{gray}{\texttt{\small Iteration\ 2}}$

The Payment Journal action tooltip mixes two different contexts ("before posting a payment journal or after posting a payment"), but this page only lets the user send remittance advice from payment journal lines before posting.

Reword the tooltip so it describes the Payment Journal workflow only, and mirror the same page-specific wording in the copied UK extension action.

Suggested change

	ToolTip = 'Send the remittance advice before posting a payment journal or after posting a payment. The advice contains vendor invoice numbers, which helps vendors to perform reconciliations.';
	ToolTip = 'Send the remittance advice for the selected payment journal lines before posting the payment. The advice contains vendor invoice numbers, which helps vendors perform reconciliations.';

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Accessibility} \quad \color{gray}{\texttt{\small Iteration\ 2}}$

The Vendor Ledger Entries action tooltip also mixes pre-posting and post-posting scenarios, even though this page only sends remittance advice for posted payment entries.

Reword the tooltip so it describes the posted-payment workflow only, and mirror the same page-specific wording in the copied UK extension action.

Suggested change

	ToolTip = 'Send the remittance advice before posting a payment journal or after posting a payment. The advice contains vendor invoice numbers, which helps vendors to perform reconciliations.';
	ToolTip = 'Send the remittance advice for the selected posted payments. The advice contains vendor invoice numbers, which helps vendors perform reconciliations.';

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ AppSource} \quad \color{gray}{\texttt{\small Iteration\ 2}}$

The new reportextensions add requestpage members named ElectronicDocument and CreateEDocuments to base reports.

Their multi-level namespace only replaces the owned-object affix; it does not satisfy AppSource member-affix rules on another publisher's object, and these added control names do not carry a reserved affix at the member-name boundary. Rename the added requestpage controls to include the app's configured affix so AppSourceCop AS0011 does not reject the extension members.

Knowledge:

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

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

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Data\ Modeling} \quad \color{gray}{\texttt{\small Iteration\ 2}}$

The new "Remit.

Advice E-Doc. Created" field stores one payment-group fact on every "Gen. Journal Line" even though the feature treats remittance advice state as group-level (same template/batch/account/document). That denormalization can leave a group with mixed flag values when lines are added, copied, or otherwise changed after export, which in turn makes per-line UI and lookup behavior inconsistent. Persist the remittance-advice link once per payment group (for example in a dedicated mapping keyed by a stable group identifier or E-Document entry) instead of duplicating it on each journal line.

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Interfaces} \quad \color{gray}{\texttt{\small Iteration\ 2}}$

"PEPPOL 3.0 Purchase" is an extensible enum stored in setup and the new remittance-advice flow converts that persisted value to "PEPPOL Remit.

Advice Info Provider", but the enum still declares only DefaultImplementation. If a previously stored extension value later becomes unknown, this interface conversion can fail with a technical runtime error instead of controlled handling. Add UnknownValueImplementation for the enum's implemented interfaces with a safe fallback/error implementation.

Knowledge:

• microsoft/knowledge/interfaces/handle-unknown-enum-ordinals-with-unknownvalueimplementation.md

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

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 2}}$

These new remittance buffer loops read only a subset of fields from wide Vendor Ledger Entry and Detailed Vendor Ledg.

Entry rows, but they never call SetLoadFields before FindSet/Get. Add SetLoadFields for the columns this export actually uses to avoid transferring full rows for every applied entry.

Knowledge:

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

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 2}}$

BuildFromJournalPayment and BuildFromPostedPayment populate remittance-advice payment data directly into a caller-supplied temporary buffer.

Because the buffer is a var out-parameter, a failure after partial population leaves vendor/payment/bank data in caller state, which is the temporary-table pattern the guidance warns against. Build into a local temporary buffer and copy it to the caller only after the build succeeds, so error paths do not leak partially populated sensitive data.

Knowledge:

• microsoft/knowledge/security/protect-sensitive-data-in-temporary-tables.md

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

[github-actions]

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

The temporary Record "Gen.

Journal Line" buffer is named ProcessedGroup here and again in the helper procedure parameters below. Prefix temporary record variables and ordinary temporary-record parameters with Temp (for example TempProcessedGroup) so call sites immediately read as in-memory buffer operations rather than persisted-record writes.

Knowledge:

• microsoft/knowledge/style/temporary-variable-temp-prefix.md

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

[github-actions]

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

The temporary Record "Remit.

Advice Buffer" parameter is named RemitAdviceBuffer in the new GetPaymentMeansInfo contract and both implementations. Rename it consistently to TempRemitAdviceBuffer so the interface advertises that callers pass an in-memory buffer, not a persisted record.

Knowledge:

• microsoft/knowledge/style/temporary-variable-temp-prefix.md

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Testing} \quad \color{gray}{\texttt{\small Iteration\ 2}}$

The codeunit calls Commit() in RunRemitAdviceJournalReport/RunRemitAdviceEntriesReport and in several test methods, but none of the [Test] methods declare [TransactionModel(TransactionModel::AutoCommit)].

Under the default test transaction model these scenarios fail on COMMIT instead of reaching their business assertions.

Knowledge:

• microsoft/knowledge/testing/transactionmodel-attribute-governs-test-transactions.md

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

[github-actions]

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

FormatImplementationReceivesJournalAndEntryRecRef dequeues a fixed prefix from LibraryVariableStorage and then clears or returns without asserting the queue is empty, so the test does not prove the format Create event fired exactly once in each phase.

Add LibraryVariableStorage.AssertEmpty() after the expected dequeues.

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

[github-actions]

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

This batch action passes the result of CurrPage.SetSelectionFilter(VendorLedgerEntry) straight into processing without checking MarkedOnly. If the user runs the action with only the cursor positioned, or uses Ctrl+A, Business Central can reduce the scope to the current row, so the action silently processes only a fraction of the visible records.

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

CurrPage.SetSelectionFilter(VendorLedgerEntry);
if not VendorLedgerEntry.MarkedOnly then
    VendorLedgerEntry.Copy(Rec);

Knowledge:

• microsoft/knowledge/ui/set-selection-filter-list-scope.md

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

[github-actions]

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

This batch action passes the result of CurrPage.SetSelectionFilter(GenJournalLine) straight into processing without checking MarkedOnly. If the user runs the action with only the cursor positioned, or uses Ctrl+A, Business Central can reduce the scope to the current row, so the action silently processes only a fraction of the visible records.

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

CurrPage.SetSelectionFilter(GenJournalLine);
if not GenJournalLine.MarkedOnly then
    GenJournalLine.Copy(Rec);

Knowledge:

• microsoft/knowledge/ui/set-selection-filter-list-scope.md

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

[github-actions]

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

This batch action passes the result of CurrPage.SetSelectionFilter(GenJournalLine) straight into processing without checking MarkedOnly. If the user runs the action with only the cursor positioned, or uses Ctrl+A, Business Central can reduce the scope to the current row, so the action silently processes only a fraction of the visible records.

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

CurrPage.SetSelectionFilter(GenJournalLine);
if not GenJournalLine.MarkedOnly then
    GenJournalLine.Copy(Rec);

Knowledge:

• microsoft/knowledge/ui/set-selection-filter-list-scope.md

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

[github-actions]

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

This batch action passes the result of CurrPage.SetSelectionFilter(VendorLedgerEntry) straight into processing without checking MarkedOnly. If the user runs the action with only the cursor positioned, or uses Ctrl+A, Business Central can reduce the scope to the current row, so the action silently processes only a fraction of the visible records.

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

CurrPage.SetSelectionFilter(VendorLedgerEntry);
if not VendorLedgerEntry.MarkedOnly then
    VendorLedgerEntry.Copy(Rec);

Knowledge:

• microsoft/knowledge/ui/set-selection-filter-list-scope.md

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

[github-actions]

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

This batch action passes the result of CurrPage.SetSelectionFilter(GenJournalLine) straight into processing without checking MarkedOnly. If the user runs the action with only the cursor positioned, or uses Ctrl+A, Business Central can reduce the scope to the current row, so the action silently processes only a fraction of the visible records.

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

CurrPage.SetSelectionFilter(GenJournalLine);
if not GenJournalLine.MarkedOnly then
    GenJournalLine.Copy(Rec);

Knowledge:

• microsoft/knowledge/ui/set-selection-filter-list-scope.md

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

[github-actions]

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

This batch action passes the result of CurrPage.SetSelectionFilter(GenJournalLine) straight into processing without checking MarkedOnly. If the user runs the action with only the cursor positioned, or uses Ctrl+A, Business Central can reduce the scope to the current row, so the action silently processes only a fraction of the visible records.

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

CurrPage.SetSelectionFilter(GenJournalLine);
if not GenJournalLine.MarkedOnly then
    GenJournalLine.Copy(Rec);

Knowledge:

• microsoft/knowledge/ui/set-selection-filter-list-scope.md

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

[github-actions]

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

This batch action passes the result of CurrPage.SetSelectionFilter(GenJournalLine) straight into processing without checking MarkedOnly. If the user runs the action with only the cursor positioned, or uses Ctrl+A, Business Central can reduce the scope to the current row, so the action silently processes only a fraction of the visible records.

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

CurrPage.SetSelectionFilter(GenJournalLine);
if not GenJournalLine.MarkedOnly then
    GenJournalLine.Copy(Rec);

Knowledge:

• microsoft/knowledge/ui/set-selection-filter-list-scope.md

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

[github-actions]

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

This batch action passes the result of CurrPage.SetSelectionFilter(VendorLedgerEntry) straight into processing without checking MarkedOnly. If the user runs the action with only the cursor positioned, or uses Ctrl+A, Business Central can reduce the scope to the current row, so the action silently processes only a fraction of the visible records.

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

CurrPage.SetSelectionFilter(VendorLedgerEntry);
if not VendorLedgerEntry.MarkedOnly then
    VendorLedgerEntry.Copy(Rec);

Knowledge:

• microsoft/knowledge/ui/set-selection-filter-list-scope.md

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

[github-actions]

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

This batch action passes the result of CurrPage.SetSelectionFilter(GenJournalLine) straight into processing without checking MarkedOnly. If the user runs the action with only the cursor positioned, or uses Ctrl+A, Business Central can reduce the scope to the current row, so the action silently processes only a fraction of the visible records.

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

CurrPage.SetSelectionFilter(GenJournalLine);
if not GenJournalLine.MarkedOnly then
    GenJournalLine.Copy(Rec);

Knowledge:

• microsoft/knowledge/ui/set-selection-filter-list-scope.md

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

[github-actions]

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

This batch action passes the result of CurrPage.SetSelectionFilter(VendorLedgerEntry) straight into processing without checking MarkedOnly. If the user runs the action with only the cursor positioned, or uses Ctrl+A, Business Central can reduce the scope to the current row, so the action silently processes only a fraction of the visible records.

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

CurrPage.SetSelectionFilter(VendorLedgerEntry);
if not VendorLedgerEntry.MarkedOnly then
    VendorLedgerEntry.Copy(Rec);

Knowledge:

• microsoft/knowledge/ui/set-selection-filter-list-scope.md

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

[github-actions]

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

This batch action passes the result of CurrPage.SetSelectionFilter(GenJournalLine) straight into processing without checking MarkedOnly. If the user runs the action with only the cursor positioned, or uses Ctrl+A, Business Central can reduce the scope to the current row, so the action silently processes only a fraction of the visible records.

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

CurrPage.SetSelectionFilter(GenJournalLine);
if not GenJournalLine.MarkedOnly then
    GenJournalLine.Copy(Rec);

Knowledge:

• microsoft/knowledge/ui/set-selection-filter-list-scope.md

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

[github-actions]

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

This batch action passes the result of CurrPage.SetSelectionFilter(VendorLedgerEntry) straight into processing without checking MarkedOnly. If the user runs the action with only the cursor positioned, or uses Ctrl+A, Business Central can reduce the scope to the current row, so the action silently processes only a fraction of the visible records.

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

CurrPage.SetSelectionFilter(VendorLedgerEntry);
if not VendorLedgerEntry.MarkedOnly then
    VendorLedgerEntry.Copy(Rec);

Knowledge:

• microsoft/knowledge/ui/set-selection-filter-list-scope.md

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

[github-actions]

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

This batch action passes the result of CurrPage.SetSelectionFilter(GenJournalLine) straight into processing without checking MarkedOnly. If the user runs the action with only the cursor positioned, or uses Ctrl+A, Business Central can reduce the scope to the current row, so the action silently processes only a fraction of the visible records.

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

CurrPage.SetSelectionFilter(GenJournalLine);
if not GenJournalLine.MarkedOnly then
    GenJournalLine.Copy(Rec);

Knowledge:

• microsoft/knowledge/ui/set-selection-filter-list-scope.md

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

[github-actions]

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

This batch action passes the result of CurrPage.SetSelectionFilter(VendorLedgerEntry) straight into processing without checking MarkedOnly. If the user runs the action with only the cursor positioned, or uses Ctrl+A, Business Central can reduce the scope to the current row, so the action silently processes only a fraction of the visible records.

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

CurrPage.SetSelectionFilter(VendorLedgerEntry);
if not VendorLedgerEntry.MarkedOnly then
    VendorLedgerEntry.Copy(Rec);

Knowledge:

• microsoft/knowledge/ui/set-selection-filter-list-scope.md

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

[github-actions]

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

"E-Document Type" is a persisted extensible enum that implements IEDocumentFinishDraft and is assigned to that interface during draft finish/undo, but it still relies only on DefaultImplementation. If an enum extension value is later removed, a stored unknown ordinal will not resolve through the interface and can fail with a technical runtime error instead of the safe fallback. Add UnknownValueImplementation for the same fallback codeunit.

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

enum 6121 "E-Document Type" implements IEDocumentFinishDraft
{
    Extensible = true;
    DefaultImplementation = IEDocumentFinishDraft = "E-Doc. Unspecified Impl.";
    UnknownValueImplementation = IEDocumentFinishDraft = "E-Doc. Unspecified Impl.";

Knowledge:

• microsoft/knowledge/interfaces/handle-unknown-enum-ordinals-with-unknownvalueimplementation.md

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

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

[github-actions]

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

The report extension passes "Vendor Ledger Entry" into export logic from OnAfterAfterGetRecord without AddLoadFields for trigger-only fields, so values the export path depends on, including "Closed by Entry No." and "Recipient Bank Account", are loaded just-in-time per row instead of being preloaded once before iteration.

Knowledge:

• microsoft/knowledge/performance/addloadfields-in-report-onpredataitem.md

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

[github-actions]

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

The report extension reads trigger-only "Gen. Journal Line" fields through downstream export code but does not add them in OnPreDataItem, so fields such as "Applies-to ID", "Applies-to Doc. No.", "Bank Payment Type", and "Recipient Bank Account" fall back to just-in-time loads during report iteration and when the record is copied into the temporary buffer.

Knowledge:

• microsoft/knowledge/performance/addloadfields-in-report-onpredataitem.md

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

[github-actions]

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

This extensible enum implements IEDocumentFinishDraft and is persisted on the E-Document table, but it still relies only on DefaultImplementation. If an extension-defined document type is stored and later removed, resolving that unknown ordinal to the interface can fail with a technical runtime error instead of a controlled fallback. Add UnknownValueImplementation alongside the existing default mapping.

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

    Extensible = true;
    DefaultImplementation = IEDocumentFinishDraft = "E-Doc. Unspecified Impl.";
    UnknownValueImplementation = IEDocumentFinishDraft = "E-Doc. Unspecified Impl.";

Knowledge:

• microsoft/knowledge/interfaces/handle-unknown-enum-ordinals-with-unknownvalueimplementation.md

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

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

[github-actions]

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

OnAfterVendLedgEntryInsert runs for every vendor-ledger insert during posting, but it always probes the E-Document table before checking the local remittance-advice flag. Most journal lines will never have a remittance-advice e-document, so adding an early GenJournalLine."Remit. Advice E-Doc. Created" guard avoids a needless database lookup on the hot path.

Suggested change

	if PreviewMode then
	exit;
	if PreviewMode then
	exit;
	if (not GenJournalLine."Remit. Advice E-Doc. Created") or IsNullGuid(GenJournalLine.SystemId) then
	exit;

Knowledge:

• microsoft/knowledge/performance/guard-event-subscribers-before-db-call.md

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

[github-actions]

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

The new remittance-advice validation path loads Vendor and checks only identification fields, but it never re-checks the vendor's blocked state before sending. That lets a payment journal line or posted payment continue through remittance-advice sending even after the vendor is blocked for Payment or All. Enforce the vendor block at this point of use (for example via the standard vendor blocked-check routine) before allowing remittance-advice generation.

Knowledge:

• microsoft/knowledge/data-modeling/check-blocked-in-referencing-code-not-in-master.md

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

[github-actions]

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

This extensible enum implements IEDocumentFinishDraft and is persisted on the E-Document table, but it still relies only on DefaultImplementation. If an enum-extension value is later removed, a stored unknown ordinal will not resolve through the interface and can fail at runtime; add UnknownValueImplementation with the same safe fallback used for DefaultImplementation.

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

    DefaultImplementation = IEDocumentFinishDraft = "E-Doc. Unspecified Impl.";
    UnknownValueImplementation = IEDocumentFinishDraft = "E-Doc. Unspecified Impl.";

Knowledge:

• microsoft/knowledge/interfaces/handle-unknown-enum-ordinals-with-unknownvalueimplementation.md

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

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

[github-actions]

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

The new remittance-advice tests use handler-backed UI flows, but the handlers are still driven by shared state and the tests never prove the queued expectations were fully consumed. That leaves room for extra confirms or skipped queued interactions to pass without a precise UI-contract failure. Drive each expected prompt/reply through LibraryVariableStorage inside the handlers and finish each UI test with LibraryVariableStorage.AssertEmpty().

Knowledge:

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

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

[github-actions]

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

In NA, the new Payment Journal remittance-advice action reuses Report Selection Usage::"V.Remittance", but the local Report Selection Mgt. still seeds that usage with report "Export Electronic Payments" and InsertRepSelection does not overwrite existing rows. On upgraded tenants, existing V.Remittance selections stay pointed at the old report, so the new action can send the payment-export report instead of a remittance advice. Use a dedicated usage or add an NA upgrade migration that rewrites existing V.Remittance report selections before this action ships.

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

[github-actions]

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

The new PEPPOL validation helpers surface missing GLN/VAT setup with plain Error calls even though each branch already knows the exact record the user must fix (Company Information, Customer, or Vendor). These are recoverable setup failures; raise an ErrorInfo with a Show-it navigation action to the affected record instead of a dead-end dialog.

Knowledge:

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

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

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

[github-actions]

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

This PR introduces new remittance-advice export entry points, but the success paths in EDocRemitAdviceExport never emit FeatureTelemetry. The same app already records Used and LogUsage for the existing E-Document send flow, so leaving this new flow silent means the new remittance-advice feature cannot be measured for adoption or successful use. Add a dedicated LogUptake/LogUsage pair after a successful remittance-advice export.

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

[github-actions]

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

Export Remit. Advice PEPPOL30 adds a new successful PEPPOL remittance-advice generation path, but GenerateXml completes without any FeatureTelemetry call. The PEPPOL app already logs usage for the existing invoice-generation path in PEPPOL30Impl.GetGeneralInfo, so this new exporter being silent leaves successful remittance-advice exports invisible in PEPPOL adoption telemetry.

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Web\ Services}$

Both new CheckRemittanceAdvice overloads validate company and vendor identity but never validate the payment currency. When the payment uses LCY, GenerateXml falls back to General Ledger Setup."LCY Code" for the mandatory cbc:DocumentCurrencyCode and currencyID attributes; if that setup value is blank or not a 3-letter ISO code, the export still produces an invalid remittance-advice XML. Call CheckCurrencyCode in both remittance-advice validation entry points before export.

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

procedure CheckRemittanceAdvice(GenJournalLine: Record "Gen. Journal Line")
    var
        Vendor: Record Vendor;
    begin
        this.CheckCompanyInfoForRemittanceAdvice();
        this.CheckCurrencyCode(GenJournalLine."Currency Code");
        Vendor.Get(GenJournalLine."Account No.");
        this.CheckVendorForRemittanceAdvice(Vendor);
    end;

    procedure CheckRemittanceAdvice(VendorLedgerEntry: Record "Vendor Ledger Entry")
    var
        Vendor: Record Vendor;
    begin
        this.CheckCompanyInfoForRemittanceAdvice();
        this.CheckCurrencyCode(VendorLedgerEntry."Currency Code");
        Vendor.Get(VendorLedgerEntry."Vendor No.");
        this.CheckVendorForRemittanceAdvice(Vendor);
    end;

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Web\ Services}$

The new remittance-advice exporter reuses the vendor's electronic endpoint (EndpointID/schemeID) as cac:PartyLegalEntity/cbc:CompanyID. That conflates two different identifiers: for vendors with GLN enabled, the XML writes the PEPPOL endpoint (0088/GLN) where a legal-entity registration or VAT identifier should go, which can fail PEPPOL validation or misidentify the payee. Extend the remittance-advice provider contract to return legal-entity CompanyID and scheme separately.

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