[29.x] Backport Travel Request API lifecycle and employee/report alignment - #11651
Open
Claudiu Ciumedean (claudiuciumedean) wants to merge 8 commits into
Open
Claudiu Ciumedean (claudiuciumedean) wants to merge 8 commits into
Claudiu Ciumedean (claudiuciumedean) wants to merge 8 commits into
Conversation
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. - Adds owner-scoped `travelRequests` navigation under `expenseUser`, linked by SystemId through a GUID FlowFilter. The linked employee number is resolved internally; `travelRequest.requestedBy` remains an Employee No. - Adds approver-scoped Released travel requests under `approverView`, linked by SystemId through a GUID FlowFilter while preserving the existing approval-assignment rules. Default submitter numbers are quoted so filter operators inside identifiers remain literal. - Exposes detail type, expense category, description, currency, and expected amounts. - Normalizes the configured local currency on header/detail API writes and reads using the existing currency helper, while retaining foreign-currency and status validation. - Preserves the original Expense Management roles and D365 permission extensions. Required operations use scoped Permissions on the Travel Request codeunit and API/UI pages; the existing agent dependency set supplies only indirect request/detail writes and ledger-link deletion, without SUPER or wildcard grants. - Adds bound submit, approve, and reject actions with owner/approver authorization, status 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 automatic approval when the agent is disabled. - Keeps nested details and travelers writable while preventing their parent key from being changed. - Automatically creates the requested-for traveler after insertion. - Blocks deletion while any unposted or posted expense report header or line references the Travel Request. Indexed lookups protect line-only references and zero-net-spend history; ErrorInfo provides a Show it action to the blocking record. Financial documents are never cascade-deleted. - Emits distinct submit, approve, automatic-approve, and reject usage events with no user/request identifiers or rejection text. - Preserves supplied expected dates on POST and validates PATCH against the final effective date pair, retaining stored dates for omitted fields and rejecting invalid ranges. No Travel Request UI approval or rejection actions are added. Fixes [AB#626966](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/626966) - [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. - [ ] 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. The current app and test projects compiled successfully in Default and CLEAN25;CLEAN26;CLEAN27;CLEAN28;CLEAN29;CLEAN30 configurations with all four repository analyzers and unchanged warning sets. The published permission and filter-quoting corrections match those compiled sources. Runtime tests were not executed locally and nothing was deployed to a BC tenant. No passing end-to-end Travel Request run is claimed. Coverage includes lifecycle authorization, linked-report creation and deletion protection, permission boundaries, API payloads, owner immutability, and the expected posting-preview path. Duplicate traveler inserts were removed from the API and permission fixtures. The eleven HTTP methods in codeunit 148347 `Travel Requests API Test` and `TravelRequestCapabilityEnabledViaAPI` are excluded from BCApps CI in `Expense_Agent_Tests.DisabledTest.json`, as [requested in review](#11007 (comment)). They are retained and compiled, not deleted. The setup must be committed so the separate OData session can see it. Execution also requires an authenticated OData endpoint, a dedicated test company and compatible disabled test isolation. The dedicated codeunit sets `RequiredTestIsolation = Disabled`. **Re-enable criteria:** provision that HTTP execution environment in CI, run the scenarios with committed fixtures and cleanup between runs, then remove the twelve exclusions once they pass there. Run legacy endpoint cases only in the non-CLEAN30 configuration. No alternative automated HTTP runner has been established. These prerequisites and the removal step are also documented beside the affected AL tests. Business-logic, permission, and direct page-action tests remain enabled. This follow-up adds twelve enabled in-process regressions for app-owned role access, indirect detail/header updates, posted/line-only reference protection, GUID owner scoping, and literal wildcard/OR characters in approver-filter user numbers, and extends existing GUID approver and HTTP scenarios. The direct page-action names explicitly distinguish them from HTTP coverage. - The API remains `beta`. Approve and reject require the acting approver expense-user number; submit requires the acting submitter expense-user number. - Schema synchronization is required for the submission/approval audit fields, GUID scoping filters, and new report-reference indexes. No second persisted ownership key or ownership backfill is introduced. - The beta contract intentionally requires `requestedBy` on POST and rejects subsequent reassignment. Clients must supply the employee number at creation rather than assigning it later. Resending the unchanged owner on PATCH remains supported. - Status changes require submit/approve/reject actions. - Expense report creation is idempotent by Travel Request number and requires an approved Travel Request. - Automatic approval is allowed only while Expense Agent is disabled. - The currency fix is included. The subsequent CR pass adds explicit date/currency tooltips without changing the date-pair logic or currency normalization. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 96a6e99)
…lifecycle (#11333) Aligns the Expense Agent travel request and expense report API flow used by the web client. - Adds a filterable `isExpenseUser` field to the Employees API. - Keeps traveler writes employee-based while mapping Employee numbers to Expense Users internally. - Adds read-only `employees` navigation for a travel request's travelers, selected by the request SystemId. - Ensures travel request approval creates an Expense Report for the `Requested For` Expense User when needed and fails the transaction if required creation does not persist. Existing matching posted history permits reapproval without creating another report. - Adds owner-scoped report recreation for approved requests after draft deletion. Existing drafts and posted header/line history for the same request/user prevent recreation and provide actionable errors. - Adds the indirect permissions and nonunique lookup keys required by these operations. - Adds object-level self-modify permission to BaseApp table 6840, Spend Request, so its trusted total-update methods can use callers' existing indirect rights. No direct user grants or entitlement changes are introduced. [AB#626966](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/626966) Tracks [AB#650277](https://dev.azure.com/dynamicssmb2/Dynamics%20SMB/_workitems/edit/650277): repair and re-enable eight posted-history tests. This PR quarantines those tests; it does not resolve the fixture bug. Also tracks [AB#650245](https://dev.azure.com/dynamicssmb2/Dynamics%20SMB/_workitems/edit/650245): the three unrelated, existing Expense Management role tests are individually quarantined for later repair. The travel-request permission tests remain enabled. **Review follow-up (2026-09-16, commit 66aacb5)** - Set Internal access on the two Employee helper fields (not the tableextension object) and added a non-unique Expense User key on Employee No. - Expense Agent and its dependent test app both compiled against the changed package. Local tenant2-1 publication failed with an unsatisfied Base Application dependency; no runtime tests of this follow-up ran locally. The owner explicitly approved pushing this follow-up and using CI for runtime checks. No shared services, versions or metadata were changed. - Earlier head 81f5749 is verified in CI run 35046472281 (attempt 2): 15/15 active Expense Permissions tests and 63/63 active Spend Request tests passed in actual W1 XML results. Its ES setup failure recovered on a bot-triggered retry. These prior results do not validate the new follow-up commit. - [x] I read the full diff and it contains only changes I intended. - [x] The full changed BaseApp built successfully locally; earlier app/test builds are recorded below. - [ ] Runtime validation of the latest BaseApp permission correction is complete. - [x] I added or updated tests for the new behavior, or explained below why none are needed. **Latest BaseApp correction (2026-09-16, commit 81f5749)** - The complete BaseApp 30.0.0.0 package built successfully using official System 30.0.54683.0 compiler symbols. Verified that it contains the table's self-modify permission; compiler-generated report-layout edits were excluded from the source commit. - Publication to tenant2-1 was attempted with Synchronize but rejected by NST with AL1024: its System 29.0.54137.0 symbols do not meet this BaseApp package's minimum System 30.0.0.0 requirement. The new BaseApp permission is therefore not runtime-validated locally. - The user explicitly approved pushing the compiled correction with that limitation documented. No application/manifest versions, shared metadata, services, or other tenants were changed. - Kept the related negative-approval assertion on its stable permission error code and affected-table caption, with a transaction boundary preserving its fixture. Quarantined only ExpenseMgmtReadRetainsAppPermissions, ExpenseMgmtEditRetainsAppPermissions, and ExpenseMgmtAdminRetainsAppPermissions under bugId 650245; their implementation was left unchanged. - CI run https://github.com/microsoft/BCApps/actions/runs/35046472281 completed successfully on attempt 2; actual W1 results confirm 15/15 active permission tests and 63/63 active Spend Request tests passed. Local BaseApp runtime validation remains blocked. **Prior local runtime validation (2026-09-15)** - After merging main `b6861a2327bddf153088a404df2e78ceec935b5e`, rebuilt and published the merged application/test packages at version 30.0.0.0 to tenant2-1. Resolved the disabled-test-list conflict by retaining both branches' entries; no environment reset or shared-service changes were made. - Ran codeunit 148339, Spend Request Test, with Codeunit isolation using runner 130450: **63 active tests passed, zero failed**. Main adds exclusions for three action tests; those three were also run separately and **all passed**, giving 66 locally verified methods. Their upstream exclusions remain intact. - The eight posted-history methods are individually listed with `bugId: 650277`. Their fixture incorrectly assumes ordinary negative expense entries offset spend-request spending; production posting deliberately excludes negative entries unless they are corrections. These methods are not counted as passing. - Fixed the wildcard test's cross-test state leakage by clearing Expense Approval Setup alongside the expense users in test initialization. The wildcard and other-approver exclusion assertions remain unchanged and enabled; no production permission/filter change was required. - Corrected test enum assertions to compare formatted WebServiceActionResultCode values, preserving expected result codes while avoiding NST JIT boxing errors. Also corrected expected-error transaction boundaries, the posted report's Last Posting No. lookup, and the Requested For field-error assertion. - An earlier green CI run masked the enum-related metadata/JIT failure during test initialization through its tolerance handling. That run is not evidence of passing Expense Agent runtime tests; this follow-up was validated locally before being pushed. **Earlier local compilation checks** - Compiled the full Expense Agent app and test project in Default and CLEAN25-CLEAN30 modes using CodeCop, AppSourceCop, PerTenantExtensionCop, UICop, the CI base ruleset, and compatibility comparisons. - All four compilations completed with zero errors and zero new warnings against the existing baselines (3 existing app warnings and 9 existing test warnings per mode). - Inspected compiled API symbols: `employeeNumber` exists in both modes; the obsolete compatibility controls exist only in Default. - Verified the six new in-process contract tests and existing duplicate-error/approval/recreation action regressions are compiled in both modes and not excluded. - Added coverage for membership filtering, employee mapping/readback, blank/unlinked rejection, duplicate/open-status validation, and request-specific employee navigation. Strengthened duplicate-report error fixtures and corrected posted-report setup to validate fields before approval. - `git diff --check` passes. **Not completed:** Runtime validation of the eight quarantined posted-history methods (tracked by [AB#650277](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/650277)), authenticated OData end-to-end tests, and a complete independent PR audit. HTTP tests remain excluded until CI provisions an authenticated endpoint and an isolated test company. Earlier Default/Clean compilation checks are separate from the latest 66-test local runtime pass. Remote CI is reported separately. The beta Travelers API adds `employeeNumber`. Default builds retain `expenseUserNo` and `expenseUserName` as Pending-obsolete controls tagged 30.0; CLEAN30 omits them. Clients should migrate to the employee-based contract. Expense User numbers remain stored internally, with no data migration or new unique constraint. Employee reads retain the existing root API contract and remain subject to BC object/table permissions and security filters. The new navigation narrows existing Employee reads to a request's travelers; it does not introduce per-person read authorization or elevate Employee table access. Report recreation has a separate owner-scoped write guard. Changing the overall read authorization model is outside this PR's contract. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Prangshuman Das <prdas@microsoft.com> Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7 (cherry picked from commit 1e87bfc)
Claudiu Ciumedean (claudiuciumedean)
requested review from
a team
as code owners
September 21, 2026 10:51
Claudiu Ciumedean (claudiuciumedean)
deployed
to
triage
September 21, 2026 10:51 — with 
GitHub Actions
Active
GitHub Actions
Active
github-actions
Bot
added
AL: Apps (W1)
Use CLEAN29 and 29.0 obsoletion tags on 29 release branches, and align the related API tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Prangshuman Das (t-prda)
previously approved these changes
Sep 21, 2026
Consolidate role assertion and real posted-history fixture repairs on main; re-enable fourteen Expense Agent tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7 (cherry picked from commit d331712)
…ests Re-enable six Travel Requests API cases with fresh cross-session persistence checks. Keep HTTP integration coverage separate from in-process Spend Request tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7 (cherry picked from commit eb3bf74)
Retain NAV API enablement separately and omit added feature tags as requested. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7 (cherry picked from commit ce3e12a)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7 (cherry picked from commit 79ef58e)
RU UnitTest databases need a complete posting setup before Library - ERM Country Data updates its accounts. Reuse standard library creators without changing test coverage or production behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7 (cherry picked from commit 0ed0d44)
Contributor
Good Sense Reviewer - Round 1Recommendation: AcceptWhat this PR doesThis backport adds travel request API lifecycle support, owner and approver scoped navigation, employee-based traveler mapping, linked expense report creation and recreation rules, deletion protection for linked reports, permissions, and test fixture repairs. Problem-solution fitFit: Strong The feature increment is broad but coherent for the release branch: it aligns travel requests, travelers, employees, approval state, and report creation behind the beta API surface. SuggestionsNone. Risk assessment and necessityRisk: The API can create expense reports and change approval state, so the main risks are wrong scoping and duplicate or missing report creation. The diff adds targeted guards around ownership, status transitions, posted-history checks, permissions, and report creation idempotency. Necessity: The change is justified because clients need a consistent travel request lifecycle and linked report creation flow on this branch.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Backports both Travel Request API changes, plus the fixture repairs from #11654, together to
releases/29.x, in their original order:Linked work
Backport of #11007 and #11333. AB#626966.
Existing test-fixture follow-ups remain tracked by AB#650277 and AB#650245; this backport does not resolve them.
NAV baseline synchronization
Job 3641564 failed before compilation in GetSyncCommit because NAV releases/29.x advanced its BCApps gitlink. NAV uptake merge commit 147309f1a0573d4057e6d97a26b42b6ee604a3d4 integrates release baseline 4f6091c927a4ddead1bcd60a33068a23fe4c92b1 and retains BCApps 6464bef, which already contains the baseline's BCApps commit 3803493. The NAV merge succeeds locally, and the only difference from its release baseline is the intended BCApps gitlink. No additional application-code changes were needed. Replacement job: 3641577 (US).
Fixture-repair follow-up from #11654
Includes all five commits through 0ed0d44: permission observations are captured under the restricted role before restoring Assert access; posted-history fixtures use genuinely posted zero-amount lines; API fixtures use a non-unit normal exchange rate and fresh cross-session readback; Spend Request coverage is classified as UnitTest, with complete posting setup created before country-specific initialization. Production permissions and application behavior are not changed by this follow-up.
All AL added/removed lines match the source PR's combined changes. Release-specific permission tests and CLEAN29 guards are preserved. Role/history tests are re-enabled; all HTTP exclusions in BCApps remain, matching #11654. No NAV tests are newly excluded. The previously completed private deployment jobs predate this follow-up and are not evidence of passing runtime tests. NAV PR 254466 will validate the new uptake with its runtime-test gate; HTTP failures remain unproven until that gate passes.