Add Recalculate to expense vat specification - #10236
Add Recalculate to expense vat specification#10236Alexander Yakunin (Alexander-Ya) wants to merge 7 commits into
Conversation
|
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' |
|
|
||
| trigger OnValidate() | ||
| begin | ||
| if not Recalculate then |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
| field(43; Recalculate; Boolean) | ||
| { | ||
| Caption = 'Recalculate'; | ||
| ToolTip = 'Specifies whether the VAT specification line should be recalculated automatically.'; | ||
| } |
There was a problem hiding this comment.
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.
| 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 |
There was a problem hiding this comment.
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:
- microsoft/knowledge/security/internal-access-is-not-a-security-boundary.md
- microsoft/knowledge/security/indirect-permissions-for-elevated-access.md
👍 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; |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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
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
…ttps://github.com/microsoft/BCApps into bugs/Add-Recalculate-to-Expense-VAT-Specification
|
|
||
| trigger OnValidate() | ||
| begin | ||
| if not Recalculate then |
There was a problem hiding this comment.
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
What & why
Linked work
Fixes #
How I validated this
What I tested and the outcome (required — be specific: scenarios, commands, screenshots for UI changes)
Risk & compatibility