[Expense tests] Re-enable Expense Agent API tests

[Prangshuman Das (t-prda)]

Review focus: the Expense test changes and the auth-helper deletion; the enableCleanTestCodeunitExecution line. The large JSON diffs are temporary disables, undone in C.

Re-enables the Expense Agent API tests on top of the default web service key auth. No Key Vault password and no auth subscriber.

• Deletes Expense API Test Auth Helper and its usages.
• Re-applies the Expense fixes from [Expense tests] Re-enable reviewed APIs and consolidate Expense fixes #12325 (reverted in Revert Expense API test re-enablement and consolidated fixes #12514): TravelRequestApproval, Expense permission and spend-request tests, the ExpDetailTest permission set, and SpendRequestTests.
• Turns on enableCleanTestCodeunitExecution, as [Expense tests] Re-enable reviewed APIs and consolidate Expense fixes #12325 did, so RequiredTestIsolation = Disabled codeunits run in clean-tenant lanes.
• Expense disabled tests: keeps only the entries NAV master also disables (12).
• Temporarily disables the unrelated APIV1/APIV2 tests that the clean lanes start executing, the same entries [Expense tests] Re-enable reviewed APIs and consolidate Expense fixes #12325 added. They are re-enabled in the next layers.

Related: AB#646383, AB#653393

Stack (each PR shows only its own layer; merge bottom first)

1. [API tests] Default to web service key authentication #12674 A: web service key auth (base: main)
2. [Expense tests] Re-enable Expense Agent API tests #12675 B: Expense API tests + clean test codeunit execution
3. [API tests] Re-enable APIV1 and APIV2 API tests #12676 C: re-enable APIV1/APIV2 API tests
4. [API tests] Simplify API test web service key handling #12890 Review fixes for A: simplify web service key handling
5. Area fixes: [API tests] Ensure VAT posting setup in API document fixtures #12677 D1 VAT fixtures → [API tests] Ensure cancellation reason codes in API sales document tests #12678 D2 cancellation reasons → [API tests] Handle localized purchase invoice payment discounts in APIV1 tests #12679 D3 localized discounts → [API tests] Set sales quote document and posting dates in API tests #12680 D4 quote dates → [API tests] Assign a number series to API vendor payment journals #12681 D5 journal numbering → [API tests] Ignore localized fields in API document comparisons #12682 D6 field comparison → [API tests] Return shipment PDF selection and pdfDocument test isolation #12683 D7 PDF documents → [API tests] Improve APIV1 fixture and verification reliability #12684 D8 APIV1 reliability

Experiments (on D8, do not merge): #12685 E single tenant, #12686 F no clean execution. Combined validation (do not merge): #12687, same tree as D8.

Platform: main already pins BCPlatform 30.0.56112.0, which includes the SQL connection-pool fix (AB#653457).

Note for CI: AL-Go's new-warning check needs a CI/CD baseline from the base branch, and features/* bases have none, so a PR build there fails with a 404. The current CI runs were triggered while these PRs targeted main, so their results are valid for the cumulative change.

[github-actions]

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

The change enables UnlinkedExpenseUserIsHiddenFromAPI, but its negative request still uses bare asserterror without checking the expected 404 error. Any unrelated request failure, including authentication or setup failure, can therefore make the newly enabled test pass; assert the expected response error (and code where available) after the call.

Knowledge:

• microsoft/knowledge/testing/asserterror-needs-expectederror-and-code.md

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

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

[alexei-dobriansky]

Good Sense Reviewer - Round 2

Recommendation: Accept

What this PR does

The change re-enables Expense API coverage on the shared web service key authentication, enables clean execution for disabled-isolation codeunits, and restores the related Expense fixes and regression tests.

Since round 1, the rebased change adds the outbound-request policy to the Expense API test codeunits and adds permission coverage for submit and reject flows. The policy matches the other API suites, and the new tests exercise indirect Spend Request modification under the Expense Agent role. The approver-filter implementation and the staged test exclusions remain consistent with the stack.

Status of previous suggestions

ID	Title	Status	Author response
S1	Negative API test accepts unrelated failures	Disputed	The exact 404 assertion is handled in a follow-up, and this point was withdrawn for this PR.

New observations (commits since round 1)

None. The newly added request-policy and permission-test changes are consistent with the surrounding test infrastructure.

Risk assessment and necessity

Risk: The main regression surface is the repository-wide clean-codeunit setting and reliance on the shared authentication provider. The Expense API codeunits now explicitly allow their outbound requests, the indirect submit/reject paths have focused permission coverage, and the temporary APIV1/APIV2 exclusions are removed by the next stack layer. The current failed check is an artifact-service network timeout, not a code or test failure.

Necessity: The change is needed to restore Expense API coverage without a test-only credential helper and to run disabled-isolation codeunits in the intended clean lanes. The scope remains appropriate for the staged rollout.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=12675 round=2 by=alexei-dobriansky at=2026-10-14T13:15:46Z lastSha=15f12fc37b9ef10c254a23dc5c812863733b2a74 reviewKey=2b2114491984d4898b836ab7422111742bcdf7383c37571097da3515b7c27a88 suggestions=S1@77d8d452:disputed parentRound=1

[github-actions]

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

The re-enabled Expense Agent API tests no longer bind the only authentication subscriber: ExpenseAPITestAuthHelper was deleted and its BindSubscription calls were removed, while the corresponding disabled-test entries were removed. The HTTP calls, such as this Travel Requests POST, therefore run without the credentials that the deleted helper supplied and can fail with an authentication response instead of exercising the API contract. Retain an equivalent authenticated-request setup (or keep these scenarios excluded until the endpoint and test-company prerequisites are available).

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

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

[alexei-dobriansky]

Good Sense Reviewer - Round 3

Recommendation: Request Changes

What this PR does

The rebased change re-enables the Expense API tests through the shared web service key authentication, enables clean execution for disabled-isolation codeunits, and restores the related approval, permission, and spend-request fixes.

The new travel-request filter correctly serializes each assigned user as an exact value, so compressed ranges and filter operators cannot expose another user's requests. The parallel pending expense-report collection still uses the old broad filter construction, so the ownership fix is incomplete.

Status of previous suggestions

ID	Title	Status	Author response
S1	Negative API test accepts unrelated failures	Disputed	The exact 404 assertion is tracked in a follow-up, and this point was withdrawn for this PR.

New observations (commits since round 2)

S2 (🔴 High): Expense report approver filter still leaks records
Apply the same exact-value filter construction to Expense Reports API.GetSubmitterFilter and AppendDefaultSubmittersForDefaultApprover. That path still compresses setup rows, appends raw user numbers, and changes an overlong filter to *, so an approver can receive another user's pending expense reports and amounts.

Risk assessment and necessity

Risk: Both pending collections are exposed through the same approver view and use the same approval setup. The travel-request path now fails closed, but the expense-report path can still match a user between assigned rows, interpret filter characters in a user number, or broaden an overlong filter to every user. This can expose another approver's report descriptions, comments, and monetary amounts. The shared authentication dependency is present in the stacked base, and no public API or event signature changes are introduced.

Necessity: Restoring the Expense API coverage and removing the test-only credential helper are needed after the shared authentication and runtime fixes became available. The scope must also apply the same exact ownership rule to both child collections before merge.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=12675 round=3 by=alexei-dobriansky at=2026-10-14T20:09:52.4696689Z lastSha=baa93fcfc3ce0e0be53f137f2dc9260a5e6fcfc4 reviewKey=c424537864ea95ec1582e99bf66a60d2fd71185eda101544e803a7ac8780fce9 suggestions=S1@77d8d452:disputed,S2@54ddc552:new parentRound=2

[Prangshuman Das (t-prda)]

alexei-dobriansky thanks, S2 is a valid finding, but it predates this PR. Expense Reports API.GetSubmitterFilter and AppendDefaultSubmittersForDefaultApprover are unchanged from main, and no layer in this stack touches them. The range from GetSelectionFilter can cover an expense user who has no approval setup row between two of the approver's users, and the default-approver path appends raw user numbers.

This PR only aligns the travel-request path, which the re-enabled Expense tests cover. We'll fix the expense-report path in a separate PR from main, mirroring the travel-request change and its tests, so it can be backported on its own.