Update expense VAT specification source

[Alexander Yakunin (Alexander-Ya)]

What & why

Linked work

Fixes AB#626087

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\ —\ Data\ Modeling}$

Expense Report Line VAT Spec. stores Document Line No. as part of its key, but the field has no TableRelation even though non-aggregate rows are supposed to point to Expense Report Line and sibling child tables enforce that link. That allows VAT-spec rows to be saved against arbitrary line numbers and weakens referential validation. Keep 0 for report-level aggregate rows, but add a conditional relation for nonzero values.

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

field(2; "Document Line No."; Integer)
{
    Caption = 'Document Line No.';
    TableRelation = if ("Document Line No." = filter(<> 0)) "Expense Report Line"."Line No." where("Document No." = field("Document No."));
    ToolTip = 'Specifies the line number of the associated expense report line.';
}

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

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

[github-actions]

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

Posted Exp. Rep. Line VAT Spec has the same issue on Expense Report Line No.: detail rows are modeled as children of Posted Expense Report Line, but the field has no TableRelation. As a result, posted VAT-spec rows can retain nonexistent line references instead of being validated against the posted-line table. Preserve 0 for report-level aggregate rows, but enforce the relation for nonzero values.

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

field(2; "Expense Report Line No."; Integer)
{
    Caption = 'Expense Report Line No.';
    TableRelation = if ("Expense Report Line No." = filter(<> 0)) "Posted Expense Report Line"."Line No." where("Document No." = field("Expense Report No."));
    ToolTip = 'Specifies the line number of the posted expense report line this VAT specification line belongs to. A value of 0 indicates that this is a report-level aggregate line representing totals for the entire report.';
}

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

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

[github-actions]

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

This actionable Show-it error uses ErrorInfo and navigation, but it only sets Message. Add a Title and DetailedMessage so the user gets clear context and guidance before following the navigation action.

Knowledge:

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

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

[github-actions]

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

Both backfill loops call header Get() inside FindSet iteration, so the upgrade performs an inner lookup for every VAT-spec row even though many rows share the same document number. Cache the current header per document number, or process the rows grouped by document, to avoid this N+1 access pattern during upgrade.

Knowledge:

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

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

[github-actions]

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

The XML summary above field "VAT Base Amount (RCY)" is a copy/paste mismatch: it says the value is a VAT amount converted to local currency, but the field stores the VAT base amount in reimbursement currency. Update the summary so generated documentation and reviewer context match the field's actual meaning.

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

        /// <summary>
        /// VAT base amount converted to reimbursement currency for accounting and reporting purposes.
        /// </summary>

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

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

[github-actions]

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

The new upgrade codeunit backfills reimbursement amounts for both open and posted VAT-spec rows, but this PR adds no test that seeds pre-upgrade data, runs the upgrade path, and verifies the recalculated RCY fields and upgrade-tag idempotency. Current tests only cover newly created reports, so a regression in the migration logic could ship unnoticed.

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

[github-actions]

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

Expense Report Line VAT Spec. is now part of the upgrade call path: OnUpgradePerCompany -> BackfillExpenseReportLineVATSpecs() -> UpdateReimbursementAmounts(...) -> UpdateReclaimAmounts(...). Inside that helper, GLSetup.Get() is unguarded. If a company is missing General Ledger Setup, this throws during upgrade and aborts the whole company upgrade instead of skipping the backfill safely.

Knowledge:

• microsoft/knowledge/upgrade/guard-database-reads.md
• microsoft/knowledge/upgrade/do-not-block-upgrade-on-data-errors.md

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

[github-actions]

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

Both backfill loops skip rows whose parent header cannot be read, but they do so silently and still let the upgrade tag be set afterward. That means orphaned VAT-spec rows stay unbackfilled with no telemetry telling support which data was skipped. The upgrade guidance is to log unexpected missing data with Session.LogMessage and continue.

Knowledge:

• microsoft/knowledge/upgrade/do-not-block-upgrade-on-data-errors.md

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

[github-actions]

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

The new per-VAT-row "Expense Category" and "Expense Subcategory" references are now used when building posting descriptions and selecting posting accounts, but posting still only checks the parent Expense Report Line for inactive masters. A VAT specification row can therefore still point to an inactive category or subcategory and be posted. Validate each referenced VAT-spec master record at posting time before using it for description or account selection.

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

[github-actions]

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

Approve All Reclaims updates each iterated VAT-spec row, but it opens the set with read-only FindSet(). Use FindSet(true) so the read takes the UpdLock the subsequent Modify(true) calls need instead of upgrading locks row-by-row inside the loop.

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

if ExpenseReportLineVATSpec.FindSet(true) then

Knowledge:

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

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

[github-actions]

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

OnOpenPage fetches an Expense Report Header row only to read "Reimbursement Currency Code". Add SetLoadFields("Reimbursement Currency Code") before Get(...) so the factbox does not materialize the rest of the header row just to decide whether RCY fields should be shown.

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

ExpenseReportHeader.SetLoadFields("Reimbursement Currency Code");
if ExpenseReportHeader.Get(Rec.GetRangeMin("Document No.")) then
    ShowRCYFields := ExpenseReportHeader."Reimbursement Currency Code" <> '';

Knowledge:

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

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

[github-actions]

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

OnOpenPage fetches a Posted Expense Report Header row only to read "Reimbursement Currency Code". Add SetLoadFields("Reimbursement Currency Code") before Get(...) so the page does not load the rest of the posted-header row unnecessarily.

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

PostedExpenseReportHeader.SetLoadFields("Reimbursement Currency Code");
if PostedExpenseReportHeader.Get(Rec.GetRangeMin("Expense Report No.")) then
    ShowRCYFields := PostedExpenseReportHeader."Reimbursement Currency Code" <> '';

Knowledge:

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

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

[github-actions]

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

The new ErrorInfo for pending VAT reclaim rows populates ErrorInfo.Message with dynamic category/subcategory values but does not set ErrorInfo.DataClassification or ErrorType. Because ErrorInfo.Message is emitted to telemetry, classify the message explicitly and choose the intended client-facing error type instead of relying on defaults.

Suggested change

	if ExpenseReportLineVATSpec."Expense Subcategory" = '' then
	PendingVATSpecErrorInfo.Message := StrSubstNo(NotApprovedForVATReclaimCategoryErr, ExpenseReportLineVATSpec."Expense Category")
	else
	PendingVATSpecErrorInfo.Message := StrSubstNo(NotApprovedForVATReclaimErr, ExpenseReportLineVATSpec."Expense Category", ExpenseReportLineVATSpec."Expense Subcategory");
	PendingVATSpecErrorInfo.RecordId := ExpenseReportLineVATSpec.RecordId;
	PendingVATSpecErrorInfo.FieldNo := ExpenseReportLineVATSpec.FieldNo("Reclaim Status");
	PendingVATSpecErrorInfo.PageNo := Page::"Expense Report Line VAT Spec.";
	PendingVATSpecErrorInfo.AddNavigationAction(ShowItLbl);
	if ExpenseReportLineVATSpec."Expense Subcategory" = '' then
	PendingVATSpecErrorInfo.Message := StrSubstNo(NotApprovedForVATReclaimCategoryErr, ExpenseReportLineVATSpec."Expense Category")
	else
	PendingVATSpecErrorInfo.Message := StrSubstNo(NotApprovedForVATReclaimErr, ExpenseReportLineVATSpec."Expense Category", ExpenseReportLineVATSpec."Expense Subcategory");
	PendingVATSpecErrorInfo.DataClassification := DataClassification::SystemMetadata;
	PendingVATSpecErrorInfo.ErrorType := ErrorType::Client;
	PendingVATSpecErrorInfo.RecordId := ExpenseReportLineVATSpec.RecordId;
	PendingVATSpecErrorInfo.FieldNo := ExpenseReportLineVATSpec.FieldNo("Reclaim Status");
	PendingVATSpecErrorInfo.PageNo := Page::"Expense Report Line VAT Spec.";
	PendingVATSpecErrorInfo.AddNavigationAction(ShowItLbl);

Knowledge:

• microsoft/knowledge/privacy/errorinfo-telemetry-classification-and-errortype.md

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

[github-actions]

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

The PR adds a new agent-only insert path on Expense VAT Spec. API and stamps new rows as Source::Agent, but no test covers either the reject path for non-agent callers or the success path that verifies the Source value is set correctly. Add an API/page test for both cases so regressions in caller validation or source initialization are caught.

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

[github-actions]

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

The new Expenses/Confirm handlers hardcode expectations in globals and assert inline inside the handler bodies instead of driving the interaction through enqueue/dequeue expectations with an end-of-test emptiness check. That contradicts the UI-handler guidance and makes the dialog sequence less robust to diagnose if an extra or misordered interaction is introduced later.

Knowledge:

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

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

[github-actions]

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

The new upgrade code backfills the four newly added RCY fields by iterating every existing VAT-spec row with FindSet(true) ... Modify(false) in both tables. That is the exact full-table loop/modify pattern the upgrade guidance warns against for new fields on existing tables. Use DataTransfer for the set-based portions of this migration and keep row-by-row logic only for the cases that genuinely require per-row calculation.

Knowledge:

• microsoft/knowledge/upgrade/datatransfer-for-bulk-init.md

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

[github-actions]

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

Both backfill loops skip rows whose header record is missing and then still allow the upgrade tag to be set for the company. That leaves the new RCY fields at their datatype defaults for those rows with no telemetry telling the partner what was skipped and no later retry once the data is repaired. Per the upgrade guidance, missing related data should be logged before continuing so the tenant upgrades without silently stranding migrated rows.

Knowledge:

• microsoft/knowledge/upgrade/do-not-block-upgrade-on-data-errors.md

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

[github-actions]

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

This API page now adds explicit POST logic for agent-authored VAT specification rows, but it still leaves PATCH and DELETE enabled by default. Because the table only blocks modify/delete when Source = Agent, an authenticated caller can still update or delete existing non-agent VAT specification rows through this endpoint. Make the surface explicitly create-only by setting ModifyAllowed = false and DeleteAllowed = false on the page.

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

    DelayedInsert = true;
    EntityName = 'expenseVATSpecification';
    EntitySetName = 'expenseVATSpecifications';
    PageType = API;
    ODataKeyFields = SystemId;
    SourceTable = "Expense VAT Specification";
    AboutText = 'Provides access to data from the Expense VAT Specification table';
    ModifyAllowed = false;
    DeleteAllowed = false;
    AutoSplitKey = true;

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

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