Skip to content

[releases/29.0] Update expense VAT specification source- #10838 - #10943

Open
Alexander Yakunin (Alexander-Ya) with Copilot wants to merge 40 commits into
releases/29.0from
copilot/releases290-backport-10838
Open

[releases/29.0] Update expense VAT specification source- #10838#10943
Alexander Yakunin (Alexander-Ya) with Copilot wants to merge 40 commits into
releases/29.0from
copilot/releases290-backport-10838

Conversation

Copilot AI commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Backports #10838 (itself a backport of #10312) onto releases/29.0, enforcing source-based controls on Expense VAT Specification records and improving VAT reclaim error handling during expense report posting.

Bug AB#648678

Source-based integrity

  • Source field set to Agent when VAT specs are created via API, Manual when created via UI; field is now read-only with system metadata classification.
  • Validation logic on Expense VAT Specification skips category/subcategory checks and calculations for agent-created records.

Immutability of agent-created records

  • Modify/delete triggers block changes to VAT spec records sourced from the Expense Agent API, with clear error messages.
  • UpdateVATSpecification no longer overwrites or deletes agent-created lines.

Posting & error handling

  • More specific errors (with navigation actions) when VAT reclaim status is pending during expense report posting.
  • Rounding difference logic now handles both amount and local currency amount.
  • Expense report lines from VAT specs now copy currency code/factor and update reimbursement amounts correctly.
  • Expense Location truncated to fit field length.

Cleanup

  • Removed #if not CLEAN30 obsolete-pending blocks (VATSpecification action/actionref) as done in source PR.

PooyaKharamesh and others added 26 commits August 28, 2026 11:08
…es/29.0)

Backport the validated authorization hardening to releases/29.0. Scope and security details are restricted to the linked tracking item. Includes regression coverage for authorized and unauthorized paths.

Related work items: #648158
…ation Impl (AA0021 + missing User Permissions dependency)

Fixes two issues that make the latest BCApps-Mirror `releases/29.0` uptake fail to compile in NAV's modular System Application build. Both stem from the WI648158 authorization-hardening backport (`8fb3a6863`), which added a new `CanManageExtensions(UserSecurityIdToCheck: Guid)` overload to `codeunit 2500 "Extension Installation Impl"`.

### 1. AA0021 — variable declaration order
```
ExtensionInstallationImpl.Codeunit.al(200,9): error AA0021: Variable declarations should be ordered by type.
```
The `Codeunit` variable was declared after the ungrouped `ModuleInfo`. Reordered so sorted types (Record, Codeunit) come first:
```al
AccessControl: Record "Access Control";
UserPermissions: Codeunit "User Permissions";
CurrentModuleInfo: ModuleInfo;
ExtensionManagementAdminTok: Label 'Exten. Mgt. - Admin', Locked = true;
```

### 2. Missing module dependency (AL0791 / AL0185)
```
ExtensionInstallationImpl.Codeunit.al(10,23): error AL0791: The namespace 'User' is unknown.
ExtensionInstallationImpl.Codeunit.al(199,26): error AL0185: Codeunit 'User Permissions' is missing
```
The backport uses `Codeunit "User Permissions"` (namespace `System.Security.User`) but did not declare the dependency. Upstream compiles the System Application as one app so it never surfaced; NAV compiles Extension Management as its own module, so the dependency is required. Added the **User Permissions** module (`c56e3ef4-7ab0-4636-ae87-013a62f12213`) to `Extension Management/app.json`. (`Record "Access Control"` is a platform table and needs no dependency.)

Validated by NAV test-uptake PR !253347, which points the BCApps submodule at this branch.

Related work items: #648158
Restore the affected 29.0 application and test sources to their prior state. Scope and details remain in the linked tracking item.

Related work items: #648158
This pull request introduces several important changes to the Expense
Agent application, focusing on enforcing stricter controls over the
creation and modification of VAT specification records, improving data
integrity, and enhancing error handling and reporting. The changes
ensure that records created via the Expense Agent API are clearly
distinguished and protected from modification, and that error messages
during expense report posting are more informative and actionable.

**Key changes include:**

* Added logic to set the `Source` field to `Agent` when VAT
specifications are created via the API, and to `Manual` when created
through the UI. This distinction is enforced in both the API and page
triggers (`OnNewRecord`, `OnInsertRecord`).
[[1]](diffhunk://#diff-bb08e5737d916802a07fe0b53c9e007bba3dcd3849e39a1b1c998b9adf10c672R108-R131)
[[2]](diffhunk://#diff-9262d4cc9dead274e2f614199b52dab823f01302d8cb8886ef3d593db9924ffbR69-R74)
* Made the `Source` field non-editable and added system metadata
classification in the `Expense VAT Specification` table.
* Added validation logic throughout the `Expense VAT Specification`
table to skip certain field validations and calculations if the `Source`
is `Agent`, ensuring agent-created records are handled differently from
manual entries.
[[1]](diffhunk://#diff-975ae126b7573570554253f501e44ef04cc551aae729dffb01d849f6ef47e0e4R37-R43)
[[2]](diffhunk://#diff-975ae126b7573570554253f501e44ef04cc551aae729dffb01d849f6ef47e0e4R62-R70)
[[3]](diffhunk://#diff-975ae126b7573570554253f501e44ef04cc551aae729dffb01d849f6ef47e0e4R98)
[[4]](diffhunk://#diff-975ae126b7573570554253f501e44ef04cc551aae729dffb01d849f6ef47e0e4R159-R170)
[[5]](diffhunk://#diff-975ae126b7573570554253f501e44ef04cc551aae729dffb01d849f6ef47e0e4R182)
[[6]](diffhunk://#diff-975ae126b7573570554253f501e44ef04cc551aae729dffb01d849f6ef47e0e4R193)
[[7]](diffhunk://#diff-975ae126b7573570554253f501e44ef04cc551aae729dffb01d849f6ef47e0e4R206-R208)

* Introduced triggers to block modifications and deletions of VAT
specification records created by the Expense Agent API, with clear error
messages.
[[1]](diffhunk://#diff-975ae126b7573570554253f501e44ef04cc551aae729dffb01d849f6ef47e0e4R264-R281)
[[2]](diffhunk://#diff-242f0f782d0eaacabf61a7043b9b871c54d90db8d36abd1a0d5e8cc53d4c2036R791)
* Updated the `UpdateVATSpecification` procedure to prevent overwriting
or deleting agent-created VAT specification lines, enforcing
immutability for API-generated records.

* Improved logic for generating VAT specification lines, ensuring that
manual and agent-created lines are handled separately and that the
correct `Source` is set during creation.
[[1]](diffhunk://#diff-242f0f782d0eaacabf61a7043b9b871c54d90db8d36abd1a0d5e8cc53d4c2036L1334-R1347)
[[2]](diffhunk://#diff-242f0f782d0eaacabf61a7043b9b871c54d90db8d36abd1a0d5e8cc53d4c2036R1358-R1375)
* Ensured that the correct category and subcategory fields are validated
and set only for manual entries, not for agent-created lines.
[[1]](diffhunk://#diff-975ae126b7573570554253f501e44ef04cc551aae729dffb01d849f6ef47e0e4R37-R43)
[[2]](diffhunk://#diff-975ae126b7573570554253f501e44ef04cc551aae729dffb01d849f6ef47e0e4R182)
[[3]](diffhunk://#diff-975ae126b7573570554253f501e44ef04cc551aae729dffb01d849f6ef47e0e4R206-R208)

* Improved error reporting during expense report posting by introducing
more specific error messages and navigation actions when VAT reclaim
status is pending, making it easier for users to resolve issues.
[[1]](diffhunk://#diff-8e8ca750ce38e56ef3c6273fd62f5334be1c20ab032f988c1e7db28b32b8e642R74-R76)
[[2]](diffhunk://#diff-8e8ca750ce38e56ef3c6273fd62f5334be1c20ab032f988c1e7db28b32b8e642L247-R267)
* Added a helper procedure to generate detailed `ErrorInfo` objects for
pending VAT reclaim statuses, including navigation actions for user
convenience.

* Ensured that string values (e.g., `Expense Location`) are properly
truncated to fit field length constraints, preventing data errors.
* When creating expense report lines from VAT specs, now copies currency
code and factor from the expense, and updates reimbursement amounts for
accuracy.
[[1]](diffhunk://#diff-f396a74c30e7dd3192138ba8ce28f30e32d62301e595bf6475d527b52dae60e7R330-R331)
[[2]](diffhunk://#diff-f396a74c30e7dd3192138ba8ce28f30e32d62301e595bf6475d527b52dae60e7R345)
* Adjusted rounding difference logic to handle both amount and local
currency amount, improving accuracy in posting.
[[1]](diffhunk://#diff-8e8ca750ce38e56ef3c6273fd62f5334be1c20ab032f988c1e7db28b32b8e642L431-R448)
[[2]](diffhunk://#diff-8e8ca750ce38e56ef3c6273fd62f5334be1c20ab032f988c1e7db28b32b8e642L474-R495)

These changes collectively improve the robustness, security, and
usability of the Expense Agent's VAT specification handling and expense
report posting workflows.<!--
Thanks for contributing to BCApps!

A few things before you hit "Create pull request":
- Your PR must link to an approved issue. New here? See CONTRIBUTING.md.
- You must have built and run your change yourself. CI is a safety net,
not a substitute.
- If you used AI or an agent to write this PR, you are still the author.
Read the diff,
  build it, and try it before requesting review.

Contributing guide:
https://github.com/microsoft/BCApps/blob/main/CONTRIBUTING.md
Local dev environment:
https://github.com/microsoft/BCApps/blob/main/LOCAL_DEV_ENV.md
-->

<!-- A few sentences: what does this change do, and what problem does it
solve? -->

<!-- Required: link an approved GitHub issue using "Fixes #<number>".
Microsoft contributors: also link the ADO work item with "AB#<number>"
if you have one. -->

Fixes
[AB#626087](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/626087)

- [x] I read the full diff and it contains only changes I intended.
- [x] I built the affected app(s) locally with no new analyzer warnings.
- [x] I ran the change in Business Central and confirmed it behaves as
expected.
- [x] 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)*

<!-- Example:
- Ran the new "Post and Send" action on a sales invoice in a fresh
container; document posted and email queued (see screenshot).
- New unit tests in MyFeatureTest.Codeunit.al pass locally; full module
test suite green.
- No tests added because change is comment-only / refactor with existing
coverage. -->

<!-- Anything reviewers should watch for: breaking changes, upgrade/data
impact, permissions,
telemetry, feature flags, follow-up work. Write "None" if there's
nothing to call out. -->

---------

Co-authored-by: Claudiu Ciumedean <cciumedean@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit aebad9c)

Co-authored-by: Alexander-Ya <179119309+Alexander-Ya@users.noreply.github.com>
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 1

Recommendation: Request Changes

What this PR does

This backport adds source-based handling for expense VAT specifications, protects agent-created VAT spec lines from later edits, and updates posting so VAT spec amounts are carried in reimbursement currency. The approach is mostly aligned with the requested behavior, and the tests cover several VAT spec posting and currency paths. However, the page conflict resolution left references to visibility variables that are not declared in the reviewed branch, so the app cannot compile as submitted.

Problem-solution fit

Fit: Partial

The change covers the requested VAT source control and reimbursement-currency posting paths, but it is not deliverable while one page keeps stale visibility assignments after removing the related variable. Fixing that conflict should make the implementation fit the scenario more cleanly.

Suggestions

S1 (🔴 High): Page uses undeclared visibility variables
UpdateControls() still assigns AllowVATReclaim after the variable was removed, and this backport also assigns AgentEnabled without declaring it on the reviewed branch. The page will not compile. Remove the stale assignment and declare or remove each remaining visibility variable.

Risk assessment and necessity

Risk: This touches expense report posting, VAT reclaim approval, currency conversion, and upgrade backfill, so a wrong change can post wrong reimbursement or VAT amounts, or block report posting. The immediate risk is build failure in ExpenseReport.Page.al; no BaseApp publisher dependency is involved.

Necessity: The change is needed to protect agent-created VAT specification records and keep reimbursement-currency amounts accurate during posting. The scope is appropriate for the backport, but the page conflict must be corrected before merge.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=10943 round=1 by=alexei-dobriansky at=2026-09-02T13:22:59Z lastSha=c1a931ac710258992eb0c4f5d70953eb5d41edc8 reviewKey=0e5f6aa8da34b3523e91a1187c0d0804d0964ba251e515cef07744e1c29db94f suggestions=S1@5a041a1e

@github-actions github-actions Bot added AL: Apps (W1) Add-on apps for W1 and removed needs-approval Workflow runs require maintainer approval to start labels Sep 3, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone Sep 4, 2026
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 2

Recommendation: Accept

What this PR does

Since round 1, the attributable code change removes the stale AllowVATReclaim assignment from UpdateControls() on the expense report page. The remaining AgentEnabled visibility variable is declared and assigned from setup, so the previous page compile issue is resolved. I did not find any new issue in the round-2 change.

Status of previous suggestions
ID Title Status Author response
S1 Page uses undeclared visibility variables Addressed No reply; fixed by the latest code change.
New observations (commits since round 1)

None - the attributable change only addresses the prior compile blocker.

Risk assessment and necessity

Risk: The full PR still touches expense report posting, VAT reclaim approval, currency conversion, and upgrade backfill, so the functional area remains sensitive. The round-2 change itself is narrow and only removes a stale page assignment.

Necessity: The change is needed to make the backport deliverable after the merge conflict. With the compile blocker resolved, the implementation now fits the reported VAT source-control and reimbursement-currency scenario.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=10943 round=2 by=alexei-dobriansky at=2026-09-04T13:59:32Z lastSha=004be75f4aed4310f8a108ca19031580f344c25f reviewKey=8c28c2f572fe3332302d533e172d9014c655454a2a574fca68010584a0244789 suggestions=S1@5a041a1e:addressed parentRound=1

@Alexander-Ya Alexander Yakunin (Alexander-Ya) added the needs-approval Workflow runs require maintainer approval to start label Sep 4, 2026
@github-actions github-actions Bot removed the needs-approval Workflow runs require maintainer approval to start label Sep 4, 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 ExpenseManagement Team: Finance GitHub request for Finance area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants