Add Travel Request API lifecycle to Expense Agent

[Claudiu Ciumedean (claudiuciumedean)]

What & why

Extends the Expense Agent beta API so travel requests can be managed through the same scoped OData model as expenses and expense reports.

• Advertises the travelRequest capability so clients can fail closed against older Business Central versions.
• Adds owner-scoped travelRequests navigation under expenseUser, relating expenseUser.number to travelRequest.requestedBy.
• Adds approver-scoped Released travel requests under approverView.
• Exposes the detail fields used by the Travel Request UI, including type, expense category, description, currency, and expected amounts.
• Adds bound submit, approve, and reject actions with owner/approver authorization, lifecycle guards, audit fields, and rejection reasons.
• Prevents ordinary PATCH requests from changing status or transferring ownership.
• Creates one linked expense report when a travel request is approved, including approvals performed through OData or automatic approval when the agent is disabled.
• Keeps nested travel request details and travelers writable while preventing their parent key from being changed.
• Automatically creates the requested-for traveler after an OData-created travel request is inserted.

No Travel Request UI approval or rejection actions are added.

Linked work

Fixes AB#626966

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

• Compiled Expense Agent locally with the AL compiler and the Business Central service assembly probing path; compilation completed without errors. The only warning is the pre-existing obsolete VATSpecification action warning.
• Added focused AL tests for capability discovery, automatic traveler creation, owner-authorized submission, approver-authorized approval and rejection, rejection reason persistence, automatic-approval authorization, and linked expense report creation.
• Ran the BCQuality AL review against all 16 changed AL files and addressed the actionable security, permission, performance, and tooltip findings.
• Confirmed the final PR diff contains only Expense Agent .al source and test files.

Risk & compatibility

• The API remains beta. Approve and reject now require the acting approver expense-user number; submit requires the acting submitter expense-user number.
• The Spend Request table extension adds submission and approval audit fields plus an approver FlowFilter. Synchronizing the extension schema is required.
• Status and requestedBy are intentionally read-only through ordinary API PATCH operations; callers must use the lifecycle actions.
• Expense report creation is idempotent by Travel Request number and validates that the source request is an approved Travel Request.
• Automatic approval is allowed only while Expense Agent is disabled.

[github-actions]

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

The file name src/Apps/W1/ExpenseAgent/app/src/Permissions/ExpenseManagementObjects.permissionset.al does not follow the <ObjectName>.<ObjectType>.al convention from the style guidance: the type segment should use the AL object-type name PermissionSet (ExpenseManagementObjects.PermissionSet.al). Keeping the canonical type segment matters for grep/symbol tooling and for the filesystem-to-object mapping reviewers rely on.

Knowledge:

• microsoft/knowledge/style/file-name-object-type-pattern.md

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

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

[Claudiu Ciumedean (claudiuciumedean)]

Addressed the permission-set filename casing feedback in 595be6f by renaming it to ExpenseManagementObjects.PermissionSet.al.

[alexei-dobriansky]

Good Sense Reviewer - Round 1

Recommendation: Request Changes

What this PR does

The change adds travel request API lifecycle actions, owner and approver scoping, capability discovery, audit fields, and automatic expense report creation after approval. The release event is raised after the request is marked Released, so the automatic approval subscriber runs at a sensible point. However, the owner link and the remaining writable endpoint do not consistently use the same owner key, so the API contract is not complete yet.

Problem-solution fit

Fit: Partial

The requested travel request API flow is mostly present, including submit, approve, reject, approver filtering, and linked expense report creation. Two ownership paths are still wrong or incomplete, so owner-scoped navigation and the no-transfer rule are not fully enforced.

Suggestions

S1 (🔴 High): Link travel requests by employee number
Requested By stores an Employee No., but this link compares it to the Expense User No. In normal setup those values differ, so the nested travelRequests collection can be empty or point to the wrong owner. Link it to Employee No. and cover the case where the numbers differ.

S2 (🔴 High): Close the old requestedBy bypass
This guard only protects the new travelRequests page. The old spendRequests page is still published before CLEAN30 and can still PATCH requestedBy on the same table. Add the same check there or block travel request updates through that page.

Risk assessment and necessity

Risk: The regression surface is the beta Expense Agent API over travel requests, expense users, approver views, and expense report creation. Wrong owner scoping can hide valid travel requests or bind them to the wrong user, and the old endpoint can still change ownership before submit.

Necessity: The feature is needed so clients can manage travel requests and create linked expense reports through the API. The scope is reasonable, but it must use one consistent owner identity across all API paths.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11007 round=1 by=alexei-dobriansky at=2026-09-04T10:39:16Z lastSha=755a784d82fd99129a5d66a521345d88ccc36024 reviewKey=858d522179d4332d05fd662a8f241cfdc7742f1442442552163bb045b1257944 suggestions=S1@d34f62e9,S2@fc1cdb42

[Claudiu Ciumedean (claudiuciumedean)]

alexei-dobriansky Addressed both Round 1 findings in 1690791:

• S1: expenseUser.travelRequests now links Expense User."Employee No." to Spend Request."Requested By". Added an end-to-end $expand=travelRequests API test with different Expense User and Employee numbers and a second user's request to verify scoping.
• S2: the obsolete spendRequests API now rejects changes to Requested By for Travel Request records. Added an API PATCH test that verifies the request fails and ownership remains unchanged.

[alexei-dobriansky]

Good Sense Reviewer - Round 2

Recommendation: Request Changes

What this PR does

The changes pushed since round 1 replace the nested travel request link with the employee owner key, add the missing legacy owner-change guard, and add API tests for both paths. That fixes the two ownership gaps: the nested collection now uses the same key that travel requests store, and the legacy endpoint now rejects owner transfer for Travel Request records before the modify is accepted. The new tests cover different Expense User and Employee numbers plus the failed PATCH path, but one new test line does not pass the required analyzer rules yet.

Status of previous suggestions

ID	Title	Status	Author response
S1	Link travel requests by employee number	Addressed	Fixed in 1690791; coverage was added for different Expense User and Employee numbers.
S2	Close the old requestedBy bypass	Addressed	Fixed in 1690791; coverage was added for the legacy PATCH path and preserved ownership.

New observations (commits since round 1)

S3 (🔴 High): Fix the new analyzer warning
The new legacy PATCH test builds the request body with a literal format string in StrSubstNo. The AL analyzer reports AA0217 for this new line, and the required build checks fail on the new warning. Move the JSON template to a locked label before merging.

Risk assessment and necessity

Risk: The round-2 runtime change is narrow and lowers ownership risk by using the stored employee owner key and by blocking owner transfer through the legacy endpoint. The remaining risk is validation risk: the test app currently fails the required build checks because of the new analyzer warning.

Necessity: The ownership fixes are necessary for a safe travel request API lifecycle. The remaining change is mechanical, but it must be made so the PR can pass validation cleanly.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11007 round=2 by=alexei-dobriansky at=2026-09-04T14:08:41Z lastSha=169079189a1775fc30c20486e7847debcb05ce45 reviewKey=cdfba7a0a170aad480a98f49cf2ef167a9ea26415498f217ee33e958dd23c9c6 suggestions=S1@d34f62e9:addressed,S2@fc1cdb42:addressed,S3@6b2dd5e7:new parentRound=1

[Claudiu Ciumedean (claudiuciumedean)]

alexei-dobriansky Addressed S3 in 38e1de0 by moving the PATCH JSON template into the locked RequestedByRequestBodyLbl label. The updated app also compiles locally with CLEAN30, all four CI analyzers, and src/rulesets/base.ruleset.json; no new warnings are reported.

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Breaking\ Changes}$

New lifecycle-validation logic (OnModifyRecord blocking a change to "Requested By" for travel requests) was added to the already-obsolete "Spend Requests API" page (ObsoleteState = Pending, ObsoleteReason references the Travel Requests API replacement, ObsoleteTag = '30.0'), inside its #if not CLEAN30 region. Building fresh behavior onto a deprecated surface instead of confining the new contract to the supported "Travel Requests API" replacement risks entrenching the obsolete API and complicates its eventual removal.

Knowledge:

• microsoft/knowledge/breaking-changes/do-not-modify-code-already-marked-obsolete.md

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

[github-actions]

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

Adding TravelRequest required another case Capability of branch in IsEnabled. That continues the enum-plus-dispatcher pattern, so every new capability still edits the consumer instead of letting the enum select behaviour through an interface-backed implementation. Consider modelling capabilities with an interface and enum-backed implementations so new variants do not keep growing this case block.

Knowledge:

• microsoft/knowledge/interfaces/prefer-interface-over-case-branching.md

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

[github-actions]

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

The Travel Requests API rebuilds the approver-derived Requested For filter inside ProcessApproverFilter, which is called from OnFindRecord. Each record fetch can therefore repeat Expense User and Expense Approval Setup reads and, for the default approver path, a full Expense User FindSet loop. Cache the derived filter once per page or request, or precompute it before iteration, so listing or expanding travel requests does not redo the same database work on every OnFindRecord call.

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

[github-actions]

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

The PR adds approver-facing pending-travel-request API wiring (Approver View API -> Travel Requests API via Approver Expense User Filter), but the tests only exercise TravelRequestApproval.ApplyApproverFilter directly. Add an integration test that queries the published API surface (for example the approver view expansion or the travelRequests collection with the approver filter) so broken SubPageLink/OnFindRecord plumbing cannot ship unnoticed.

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

[github-actions]

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

Travel Requests API now rejects direct changes to status and requestedBy in OnModifyRecord, but this PR adds no negative test for that page; the only owner-change API test still targets the legacy Spend Requests API. Add a PATCH-style test against Travel Requests API that proves both lifecycle guards fail end to end.

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

[github-actions]

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

CreateTravelRequestLine hand-rolls a Spend Request Detail with Init/field assignment/Insert(true) even though this suite already uses LibraryExpense.CreateSpendRequestDetail elsewhere. That bypasses the library's fixture defaults and validation path, so these new line-type tests can start failing in setup when Spend Request Detail gains new required fields instead of failing on the behavior under test.

Knowledge:

• microsoft/knowledge/testing/use-library-codeunits-for-test-fixtures.md

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

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

[alexei-dobriansky]

Good Sense Reviewer - Round 3

Recommendation: Request Changes

What this PR does

The changes pushed since round 2 move the PATCH JSON template into a locked label, keep the approval helper internal, and limit the approval-check bypass to a local expense report record. The previous ownership and literal-format issues are fixed in shape. However, the new label is still declared outside the #if not CLEAN30 test block that uses it, so the clean analyzer build still fails on the changed test code.

Status of previous suggestions

ID	Title	Status	Author response
S1	Link travel requests by employee number	Addressed	Fixed in 1690791; coverage was added for different Expense User and Employee numbers.
S2	Close the old requestedBy bypass	Addressed	Fixed in 1690791; coverage was added for the legacy PATCH path and preserved ownership.
S3	Fix the new analyzer warning	Addressed	Fixed in 38e1de0; the JSON body now uses a locked label.

New observations (commits since round 2)

S4 (🔴 High): Guard the legacy-only test labels
The CLEAN30 build excludes LegacySpendRequestsAPIRejectsTravelRequestOwnerChange, but the labels used only by that test still stay in the global var block. The analyzer reports AA0137 on the new RequestedByRequestBodyLbl line, so the required clean build still fails. Move the legacy-only labels into the same #if not CLEAN30 scope, or guard their declarations too.

Risk assessment and necessity

Risk: The runtime changes remain narrow and lower ownership risk for the beta travel request API. The remaining risk is validation risk: clean builds fail on legacy-only test variables, so the PR cannot validate cleanly yet.

Necessity: The feature is needed so clients can manage trip-scoped travel requests and create linked expense reports through the API. The scope is reasonable, and the remaining fix is mechanical.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11007 round=3 by=alexei-dobriansky at=2026-09-05T13:32:04Z lastSha=38e1de062c6ca0f280b6bd4b6f17e7740f8fef45 reviewKey=d80e82277abf7413ab4cc2879e24686ac6e88b5e6255034d78bf72699ef2b0f9 suggestions=S1@d34f62e9:addressed,S2@fc1cdb42:addressed,S3@6b2dd5e7:addressed,S4@70cf969d:new parentRound=2