Skip to content

Add Recalculate to expense vat specification - #10236

Open
Alexander Yakunin (Alexander-Ya) wants to merge 7 commits into
mainfrom
bugs/Add-Recalculate-to-Expense-VAT-Specification
Open

Add Recalculate to expense vat specification#10236
Alexander Yakunin (Alexander-Ya) wants to merge 7 commits into
mainfrom
bugs/Add-Recalculate-to-Expense-VAT-Specification

Conversation

@Alexander-Ya

@Alexander-Ya Alexander Yakunin (Alexander-Ya) commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

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 github-actions Bot added the AL: Apps (W1) Add-on apps for W1 label Aug 13, 2026
@Alexander-Ya Alexander Yakunin (Alexander-Ya) changed the title Bugs/add recalculate to expense vat specification Add Recalculate to expense vat specification Aug 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Could not find a linked ADO work item. Please link one by using the pattern 'AB#' followed by the relevant work item number. You may use the 'Fixes' keyword to automatically resolve the work item when the pull request is merged. E.g. 'Fixes AB#1234'

@github-actions github-actions Bot added the Integration GitHub request for Integration area label Aug 13, 2026

trigger OnValidate()
begin
if not Recalculate then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

The new if not Recalculate then exit; guards turn the table's long-standing Validate(...) behavior into an opt-in path, but Recalculate defaults false, is added to the list page as Visible = false (no user-facing way to enable it), and is set in the test only after the relevant Validate(...) calls already ran. Existing callers that validate Expense Category, Expense Subcategory, Amount, Amount (LCY), or VAT % will now silently stop deriving posting groups and VAT/base amounts unless every caller is updated to set the flag first — a breaking behavior change for current UI/API consumers of this table's validation logic.

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

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4


trigger OnValidate()
begin
if not Recalculate then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

The new if not Recalculate then exit; guards turn these OnValidate triggers into silent success paths: when recalculation is disabled, Validate(...) returns without recomputing dependent VAT fields and without raising any error or warning that validation logic was skipped. Since the flag is only set by callers after the fact (if at all), blocked recalculation is indistinguishable from successful validation. Either require callers to enable recalculation before invoking these validations, or raise an explicit error/warning when the guard blocks the trigger.

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

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4

Comment on lines +249 to +253
field(43; Recalculate; Boolean)
{
Caption = 'Recalculate';
ToolTip = 'Specifies whether the VAT specification line should be recalculated automatically.';
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

The new Normal table field Recalculate has no field-level DataClassification. Relying on the table's DataClassification = CustomerContent does not satisfy AppSourceCop AS0016's per-field classification requirement, so this field remains under-classified for privacy/audit purposes. Classify the field explicitly; SystemMetadata is appropriate for this internal recalculation control flag.

Suggested change
field(43; Recalculate; Boolean)
{
Caption = 'Recalculate';
ToolTip = 'Specifies whether the VAT specification line should be recalculated automatically.';
}
field(43; Recalculate; Boolean)
{
Caption = 'Recalculate';
DataClassification = SystemMetadata;
ToolTip = 'Specifies whether the VAT specification line should be recalculated automatically.';
}

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4


trigger OnValidate()
begin
if not Recalculate then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

The new if not Recalculate then exit; guards make VAT derivation opt-in, but Expense VAT Spec. API (page 7085) still writes Expense VAT Specification rows directly while the flag defaults false and the API neither exposes nor sets it. API callers can therefore persist arbitrary VAT %, VAT Amount, VAT Base Amount, and Amount values without the server-side recomputation/validation that previously ran, and those values later flow into expense-report VAT lines and totals. Keep this validation mandatory for direct API/table writes, or move the bypass behind a code path external callers cannot invoke directly.

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4

ExpenseVATSpecification.Validate("Expense Category", ExpenseCategoryCode);
ExpenseVATSpecification.Validate("Expense Subcategory", ExpenseSubcategoryCode);
ExpenseVATSpecification.Validate(Amount, Amount);
ExpenseVATSpecification.Recalculate := true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

In CreateExpenseVATSpecification, Recalculate := true is set only after Validate("Expense Category"), Validate("Expense Subcategory"), and Validate(Amount, Amount) have already executed, so those OnValidate triggers still exit early (Recalculate was false at call time) and never copy VAT posting setup or derive VAT amounts. The fixture now builds malformed 'Expense VAT Specification' rows and no longer exercises the category/subcategory cascading logic it is meant to set up.

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

        ExpenseVATSpecification.Recalculate := true;
        ExpenseVATSpecification.Validate("Expense Category", ExpenseCategoryCode);
        ExpenseVATSpecification.Validate("Expense Subcategory", ExpenseSubcategoryCode);
        ExpenseVATSpecification.Validate(Amount, Amount);

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4

Caption = 'Reasoning';
ToolTip = 'Specifies the AI reasoning text that explains how the VAT specification values were determined from the receipt or invoice.';
}
field(43; Recalculate; Boolean)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

The new normal table field "Recalculate" has no field-level DataClassification. The table-level DataClassification = CustomerContent does not satisfy the field requirement, so this field remains effectively unclassified for AS0016/AppSourceCop purposes.

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4

ExpenseVATSpecification.Validate("Expense Category", ExpenseCategoryCode);
ExpenseVATSpecification.Validate("Expense Subcategory", ExpenseSubcategoryCode);
ExpenseVATSpecification.Validate(Amount, Amount);
ExpenseVATSpecification.Recalculate := true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

The test helper enables Recalculate only after Validate("Expense Category"), Validate("Expense Subcategory"), and Validate(Amount). In this PR those table triggers now exit while Recalculate is false, so the fixture skips the VAT%/VAT-amount/posting-group population that the posting tests depend on. Move the flag assignment before the dependent Validate calls (or re-run those validates after enabling it) so the tests still build representative VAT-spec rows.

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

ExpenseVATSpecification.Recalculate := true;
ExpenseVATSpecification.Validate("Expense Category", ExpenseCategoryCode);
ExpenseVATSpecification.Validate("Expense Subcategory", ExpenseSubcategoryCode);
ExpenseVATSpecification.Validate(Amount, Amount);

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4

Copilot AI and others added 2 commits August 14, 2026 06:57
Co-authored-by: Alexander-Ya <179119309+Alexander-Ya@users.noreply.github.com>
Co-authored-by: Alexander-Ya <179119309+Alexander-Ya@users.noreply.github.com>
…Add-Recalculate-to-Expense-VAT-Specification
@github-actions github-actions Bot added the needs-approval Workflow runs require maintainer approval to start label Aug 14, 2026

trigger OnValidate()
begin
if not Recalculate then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Guarding the "Amount (LCY)" OnValidate trigger (and the other five VAT-spec triggers) with if not Recalculate then exit; makes their TestField prerequisite checks and validation cascades (e.g. the Validate("VAT %") chain) skippable whenever Recalculate is false. Validation errors that previously protected category, amount, VAT %, and currency consistency can no longer be relied on to fire once the flag is off, widening the surface for inconsistent VAT data to be persisted.

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4

@github-actions github-actions Bot removed the needs-approval Workflow runs require maintainer approval to start label Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AL: Apps (W1) Add-on apps for W1 Integration GitHub request for Integration area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants