Skip to content

Repair and re-enable Expense Agent permission and posted-history tests - #11654

Merged
Prangshuman Das (t-prda) merged 18 commits into
mainfrom
fix/expense-test-repairs-650245
Oct 1, 2026
Merged

Prangshuman Das (t-prda) merged 18 commits into
mainfrom
fix/expense-test-repairs-650245

Conversation

@t-prda

@t-prda Prangshuman Das (t-prda) commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

What and why

Consolidate the Expense Agent test repairs on current main, following merged #11333.

  • Fix the three Expense Mgmt. Read/Edit/Admin tests by collecting all nine permission observations under the exact restricted role, restoring test permissions, and only then calling Assert. No production permissions change.
  • Replace the invalid ordinary positive/negative posted-history fixture with a normally posted zero-amount line. This preserves genuine posted header/line history and zero recorded spend for all eight deletion, report-recreation, and reapproval cases. Keep identity, ownership, audit, duplicate-report and zero-spend assertions.
  • Re-enable Approve/Submit/Reject Travel Request page-action tests. Their WebServiceActionResultCode assertion compilation fix is already on main.
  • Classify codeunit 148339 as UnitTest, as requested; it exercises AL in-process, including direct page procedures. The separate HTTP API codeunit remains IntegrationTest.
  • Repair the foreign-currency fixture in six AB#650247 API tests (the normal rate, not the adjustment rate, must be non-unit); refresh cross-session reads and verify persisted identities, dates and ownership without removing negative operations.
  • Remove exactly 14 role/history/action exclusions from the BCApps Expense Agent disabled-test manifest. No exclusions remain for codeunits 148338 or 148339 in this branch.
  • Preserve ALL HTTP API exclusions in BCApps, including the six AB#650247 cases: BCApps still needs the separate API-authentication prerequisite. The fixture corrections are retained, but these tests must not be counted as executed in BCApps. NAV has working API authentication and enables those six tests separately.

Related: AB#650245, AB#650277, AB#625894. NAV test-enablement/uptake also tracks AB#650246 and AB#650247.

This supersedes the overlapping repair work in drafts #11451 and #11452. Their other already-upstream changes are not replayed. The older drafts remain open until their owners reconcile/close them after this work is merged.

Validation

  • Static JSON/exclusion checks and git diff --check passed; read-only review found no significant issue.
  • Latest BCApps main was fetched and verified included: 0c994435e3ee038dbaf66822f8a2791b1aa3f43a.
  • Expected complete BCApps Default suites on this source: 20 tests in codeunit 148338 and 74 in codeunit 148339, including all 14 re-enabled methods. HTTP API tests remain excluded in BCApps by explicit user decision; their runtime evidence must come from the manually queued NAV buddy build instead.
  • Local W1 tenant2-1 resolution initially timed out because MSSQLSERVER was stopped. After the user requested local repair, that stopped dependency was started and both relevant databases are ONLINE. Tenant2-1 remains Failed; its Base/System/Application/Expense/Test apps are uninstalled and Cleaned. Normal tenant synchronization reports the tenant is not mounted. Shared NST is still 29.0.54137.0, incompatible with a current full BaseApp30 build.
  • No local publication or tests have run. No running service was restarted, no tenant1-1 data/configuration was changed, and no version metadata, reset, or reprovisioning was performed. Further local repair awaits explicit approval for an isolated BC30 environment or a shared-environment upgrade.
  • API authentication uses the existing helper unchanged. PR Add opt-in API test authentication providers #10085 contains a separate shared authentication-provider change; it is not silently imported here. Until that dependency lands, BCApps API exclusions remain identical to main. NAV buddy results must establish whether the historical request-disappearance symptom is resolved.
  • CI compile, publication, and actual per-method XML/log execution verification are pending. A green aggregate check alone is not sufficient: earlier CI could report success after a codeunit runtime-compilation failure.
  • Private NAV uptake draft: NAV #254455. It removes 21 NAV method exclusions, including the six API cases, and restores the Expense Agent Tests app to Get-AppsToInstall by removing the blanket runner filter introduced in f09e1491f2adacca7a851a12587966daf91d993d.
  • The user will queue its buddy build manually. NAV remains draft until BCApps merges, the pointer is updated to an actual main commit, and buddy logs prove app installation and per-method execution. Static runner selection is not runtime installation evidence.

Whole-app NAV test exclusion and installation/execution validation: AB#650370. The scoped bugs 650245, 650246, 650247, 650277 and 650370 are Active while verification is pending.

CI iteration: 79ef58 (superseded by RU fixture correction)

Actual W1 Default JUnit artifact 10646077590 from run35596801127 records 148338 20/20 passed and 148339 74/74 passed, zero errors/skips, including all14 re-enabled methods. DE Default JUnit also records20/20+74/74.

RU Default job106353051317 failed: 14833820/20passed, but all74 Spend Request tests failed during Initialize because PrepareNormalGenPostingSetup could not find a fully populated General Posting Setup in the UnitTest database. This is fixture initialization, not74 different product failures.

Commit0ed0d44f92 creates a complete posting setup using existing Library - ERM helpers before the country-specific setup. New-head validation is pending; the prior-head W1/DE passes are not a claim that this new commit has passed. No tests were disabled for this failure. Local and NAV buddy execution caveats above remain.

CI iteration: 0ed0d44 (superseded by CH VAT fixture correction)

Actual JUnit from run35616107333 attempt2 verifies W1 and RU each passed all20 permission tests and74 Spend Request tests, zero failures/errors/skips. W1 artifact10658609682; RU artifact10658544297. This confirms the RU posting-setup correction.

CH Default job106447478820 failed: permissions20/20passed; all74 Spend Request cases stopped in country VAT initialization because the selected Normal VAT template referenced a nonexistent Sales VAT Account. Artifact10659053532 and the job stack identify Library - ERM Country Data.CreateVATPostingSetup.

Commit bd80ea3 initializes the VAT template before country setup, creating replacement Sales/Purchase VAT accounts only when the referenced account is missing. Valid account references and all assertions remain unchanged; no test is disabled. Fresh validation of W1, RU and CH on this new head is pending. Older-head pass evidence is not a claim that the new head passed.

Prior-head test execution: bd80ea3

Downloaded actual JUnit artifacts from run35644720549 attempt2 and verified every required re-enabled method, not just the job conclusion:

Country CU148338 Expense Permissions CU148339 Spend Request Artifact
W1 20/20 passed 74/74 passed 10668715386
RU 20/20 passed 74/74 passed 10667802737
CH 20/20 passed 74/74 passed 10668516148

All three have zero failures, errors and skipped cases in these suites. All14 re-enabled role/posted-history/page-action cases executed. This verifies both country fixture corrections and placement in the Default/UnitTest lane. It does not establish fully test-owned VAT setup: the current helper repairs the selected existing VAT template when necessary.

Run35644720549 attempt2 completed with160 successful jobs,2 skipped and2 failed (BE Uncategorized plus the aggregate status check). The status-check log names ONLY the BE platform/indexes/platform.json download/setup failure; no Expense test failure remains in the verified W1/RU/CH suites. The overall CI gate is therefore still red. No rerun was initiated by this agent. HTTP API tests remain intentionally excluded in BCApps; current NAV buddy and local runtime validation are not claimed. The user-supplied older NAV buddy3641608 did install Expense Agent and its test app and pass20 permission tests, but all74 Spend Request cases hit the pre-fix RU initialization failure; its queue timestamp preceded the RU correction.

Current fixture repair: 2d45e6f

The supplied NAV buddy3641753 AU log passed all20 permission tests but failed all74 Spend Request tests in UpdateZeroVATPercentInVATPostingSetup: the demo-data helper assumed an existing blank-business-group VAT row. Earlier BCApps AU also passed94/94 on bd80ea3; that baseline difference does not prove the fixtures are independent.

Replace broad demo-data normalization rather than adding another country-specific fallback:

  • Create fresh Expense Posting Groups and their G/L accounts, fresh Employee Posting Groups with valid expense accounts, and fresh payment methods. Bind the report fixtures to these records instead of reusing arbitrary posting masters.
  • Remove the unrelated country VAT/general/purchase setup mutations and the earlier RU/CH template repairs. These Spend Request lifecycle fixtures use no-VAT expense accounts; retain the explicit country helper for Journal Templ. Name Mandatory=false.
  • Restore General Ledger, Human Resources, Source Code and Expense Agent singleton setup between tests with Library - Setup Storage. Retain existing scoped Expense cleanup; do not delete unrelated VAT or ledger masters.
  • Preserve all74 test methods, assertions, scenario/feature tags and handlers. No exclusions or production behavior changed.

Validation: static call-site checks and git diff --check passed. Fresh local resolution now reports tenant2-1 Operational, but a dedicated tenant2-1 AL MCP download returned404 for System30.0.0.0 (No published package matches the provided arguments). Application symbols downloaded to the worktree/tenant cache; required symbols remain incomplete. No local compile/publication/test was performed, and no shared symbols, metadata changes, reset, service restart or tenant1-1 modification was used. Fresh CI and manually queued NAV buddy execution are pending; all bd80ea3 pass evidence above is prior-head only.

Pipeline audit: both NAV and BCApps AU UnitTest setup use an empty company, not Contoso generation. BCApps can retain the Disabled runner across ordinary app passes; the exact origin of the differing AU VAT data is unproven. Open PR #10085 has overlapping clean-tenant/split-isolation remediation. No shared-runner changes are imported into this fixture PR.

Warning-gate correction: 67d0e3c

Run35714820873 on2d45e6f finished failed. RU Default and SE/IT Clean logs identify two new AA0198 warnings: the new global LibraryHumanResource shadowed two existing local declarations in SpendRequestTest. RU stopped at the compilation/new-warning gate before publication and tests; this was introduced by the fixture change, not infrastructure.

Commit67d0e3c removes exactly those two redundant local declarations and reuses the global instance. All74 test bodies/assertions are unchanged; exact two-line diff and diff checks passed. Fresh CI compile/publication and actual W1/RU/CH/AU94-test execution remain pending. Monitoring resumes every30minutes; no buddy build or CI rerun is automatically queued, and local tenant-specific System30 symbols remain unavailable.

Payment-method fixture correction: 87357b7

The67d0e3c app builds passed for W1/RU/CH/AU, clearing AA0198. Actual AT Default JUnit artifact10705006253 from run35730199203 records CU14833820/20passed and CU14833955/74passed,19failed. All19 failures are the same fixture defect: Expense Payment Method enforces uniqueness by nonblank Reimbursement Type, so creating a new Employee Paid method for each report collides with the prior method. This is not a product assertion failure.

Commit87357b7 clears only Employee Paid/Company Paid payment-method fixtures in Initialize, after Expense transaction cleanup and before the initialized guard. The six factories use the existing find-or-create library helper: the first call creates this test's method, and further calls reuse it without violating uniqueness. Posting-account ownership and all74 scenarios/assertions/tags/handlers remain unchanged. Static checks verify every-test cleanup ordering, all six callers, preserved assertions and no HR-library shadowing; runtime verification on this new head is pending.

The earlier run also has an independent LegacyTestsBucket1 container-setup failure downloading platform/indexes/platform.json (HTTPS connection timeout). No automatic retry or shared-runner/environment change is included. Local tenant-specific System30 symbols remain unavailable; latest NAV buddy execution remains unverified.

Verified current-head runtime: 87357b7

Downloaded actual JUnit from run35751572706 attempt1 and checked complete suite counts, zero failures/errors/skips and every required re-enabled method:

Country CU148338 Permissions CU148339 Spend Request Artifact
W1 20/20 passed 74/74 passed 10714234115
RU 20/20 passed 74/74 passed 10714194977
CH 20/20 passed 74/74 passed 10715030599
AU 20/20 passed 74/74 passed 10714392965
AT 20/20 passed 74/74 passed 10713924969

All14 re-enabled methods executed in each country. This verifies the latest payment-method cleanup/reuse correction, including the AT regression. All five country app builds passed. W1 job106867710487 additionally records publication, synchronization and installation of both Expense Agent (Preview) and Expense Agent Tests30.0.2147483647.85851, followed by successful execution of both codeunits.

The overall workflow is not green: it is still running, with US LegacyTestsBucket2 job106867713372 failed because its self-hosted runner lost communication with GitHub. That infrastructure failure is separate from the verified target suites; no automatic rerun was requested.

NAV draft254455 remote commit e939957a13d896b18ae1bf053a14ab6949a052fd already consumes this exact87357b7 commit. Fresh NAV buddy verification remains necessary, especially Codeunit isolation and the six NAV-enabled HTTP API tests that remain excluded in BCApps. The supplied older NAV3642277 W1 log references67d0e3c and reproduces the now-corrected19 payment-method failures; it does not test87357b7. No current NAV/local runtime pass is claimed.

Final BCApps CI gate

Run35751572706 subsequently completed SUCCESS on attempt2 for the same87357b7 head:162 successful jobs,2 skipped,0 failed. The earlier US legacy-runner communication failure is cleared. The agent did not request this retry. Five-country actual20+74 execution evidence above remains valid for this exact source commit. NAV validation remains separate.

NAV HTTP API follow-up: af10e62 (validation pending)

The supplied NAV3642786 NL IntegrationTests log (build30.0.54998.2786) executes all15 CU148347 cases:7pass,8fail. These HTTP API cases remain intentionally excluded in BCApps, so the prior87357b7 unit-test/green-CI evidence does not validate them.

Library - Graph Mgt. appends successful responses to the supplied ResponseText and raises failed requests before populating that output. The failures expose two test-harness defects: stale concatenated responses explain the missing traveler expansion and two foreign-currency assertions; parsing empty response JSON explains four negative-case failures. This test-only commit clears all32 HTTP response buffers and checks failed requests through the helper's raised error, preserving expected HTTP400/ALDialog, domain/owner/request-number details, the server BadRequest read-only-status guard, and persistence checks. All15 methods, existing tags and conditional compilation remain unchanged.

The eighth failure is still under investigation: the dates case fails the first local GetBySystemId immediately after successful POST, before PATCH. The test now explicitly compares the returned id with the client-supplied id before the existing persistence check, so a future failure can distinguish rewritten/ignored identity from record disappearance. The supplied-GUID requirement is NOT removed and no production workaround is applied.

Static diff/buffer/case checks pass. A fresh dedicated tenant2-1 symbol download retrieved5of6 requested dependencies but again failed404 for System30.0.0.0; no local compile, publish or tests ran. Fresh CI compilation is pending and cannot substitute for NAV API execution while BCApps exclusions remain. Another NAV buddy build has not been queued automatically; the prior queue authorization was used for2773217. Both drafts remain in-progress, not fully validated.

Current follow-up head: ff267e3. The returned-identity diagnostic uses the existing AL Evaluate(Guid, JsonValue.AsText()) pattern. This supersedes af10e62; its identity investigation and runtime-validation caveats still apply. No additional NAV build has been queued.

ff267 CI gate completed

Run35869253473 attempt1 completed SUCCESS on ff267e3:162 successful jobs,2 skipped,0 failed. Actual W1 and RU build logs confirm Expense Agent Tests compiled, with unchanged warning baselines (40304 W1;42672 RU). CU148347 HTTP API cases remain excluded in BCApps: this verifies compilation and the CI gate, NOT the eight NAV API failures or the unresolved date POST identity. Local runtime validation remains pending; no NAV rerun was initiated by this monitor.

Locally verified API persistence fix: 29d7bb5

The remaining two API failures were reproduced outside the test runner: user-scoped owner/date-only POST returned201 but no stored request (blank number; subsequent GET returned0). Adding purpose caused insertion, but the tests were NOT weakened with such a payload workaround.

Fix page7134 instead: copy supplied date values onto Rec to register a real record change while retaining deferred paired validation; clear the route-prefilled owner on new records and default it at insertion only when requestedBy was omitted. Explicit supplied owners still undergo scope validation. Requested identity and all existing date/ownership/persistence assertions are retained.

Local validation after the explicitly authorized BC30 W1 reset (agents enabled, two tenants), exclusively on tenant2-1:

  • Downloaded fresh tenant-specific symbols; built and published both Expense Agent (Preview) and Expense Agent Tests30.0.0.0 through a dedicated AL MCP host.
  • CU148347: all15 HTTP API tests PASSED,0failures/0skips, runner130451 (required isolation Disabled), CRONUS W1, password-only test administrator. Includes complete date-only PATCH sequence and both original missing-record cases. Evidence:api-local-insert-fix.xml.
  • Empty Company regression: CU14833820/20 andCU14833974/74 PASSED,0failures/0skips, runner130450 Codeunit isolation. Evidence:local-unit-regression-20260924.xml. The helper's 'Disabled' setup log describes code-coverage tracking, not isolation.
  • Additional direct HTTP checks passed: supplied-id/date-only POST persists; omitted owner defaults from route and persists; explicit blank owner and unscoped missing owner return400 without inserting.

Local runtime builds use CodeCop/UICop with default severities; partner AppSourceCop defaults incorrectly reject first-party namespaces/IDs and the full repository ruleset promotes pre-existing obsolete/style warnings. No source warnings were suppressed or manifests altered; CI remains the full first-party compilation gate. Fresh current-head CI and NAV buddy results are still pending.

BCApps HTTP API exclusions remain unchanged pending the separate authentication prerequisite; NAV keeps these APIs enabled. The Windows-linked local account was unsuitable for the existing Basic-auth helper, so a password-only test account was used on tenant2-1. No production authentication behavior was changed. Canonical reset-tool compatibility fixes remain local and are not included in this PR.

Final merged-source local verification: aaa5a76

Merged current BCApps main (a18b1b3) normally to resolve the sole namespace-import conflict, retaining both imports and all upstream changes. Main contributes six additional Spend Request tests, bringing CU148339 to80; no scenarios were removed.

Rebuilt and published Expense Agent and its tests from the merged source. The seed Base Application lacked the newly inherited Purpose-validation behavior, so the matching unmodified Base Application30.0.0.0 was built from the same source and published only to tenant2-1 (verified tenant scope). No BaseApp source workaround or version/metadata override was used.

Final persisted local results on tenant2-1: 15/15 HTTP API +20/20 Expense Permissions +80/80 Spend Request =115/115 passed, zero failures/skips. HTTP runner130451 in CRONUS W1; unit runner130450 in Empty Company. Reports:api-local-final-main.xml andlocal-unit-final-main.xml. The original date/owner POST payloads and persistence assertions pass without adding purpose. Source worktree is clean; compiler-generated report-layout edits were not committed.

This supersedes the earlier unresolved-identity caveat. New-head CI and NAV buddy results remain pending; BCApps API exclusions and NAV API enablement policy are unchanged.

W1 Bucket5 fixture follow-up: 40a64b5

The supplied NAV3644685 log reports4,814passed/2failed: CU139194.CDSConnectionWizardCheckModifyCDSConnectionURL rejects a positive fixture outside dynamics.com; CU139148.NotExpiredToken fails its expiry assertion. These are separate from the repaired Expense APIs.

Test-only corrections: use valid dynamics.com hosts for positive HTTPS normalization cases, preserve explicit-port/path expectations and HTTP rejection, and exercise SaaS validation with environment restoration. JWT fixtures now use the public Unix Timestamp helper rather than manual timezone/epoch arithmetic, with a one-hour past/future margin instead of one second. Production URL/security and JWT-expiry implementations are unchanged; no further tests are disabled.

Both test apps compile locally through AL MCP with tenant2-1 symbols. The corrected CDS case passes locally. Full local validation is incomplete: the CRM suite has pre-existing local AutoRollback/EmailLogging and on-premises-mode setup failures; the latter reproduces independently of the modified case. Tests-Misc server publication rejects its existing MockAzureKeyVaultSecretProvider reference despite successful local compilation. The missing standard MockTest add-in was restored, but no service restart or validation bypass was performed. Token runtime and full-suite verification must come from the buddy build or follow-up local runtime maintenance. No full-pass claim is made for these two codeunits.

The prior115 local Expense API/permission/Spend passes remain evidence for unchanged Expense code, not proof of these new fixture repairs. Existing NAV retention/VIES quarantines and differing BCApps/NAV API policies remain unchanged.

Assisted-setup fixture follow-up: c52631a

After ADO2787333 failed, the matching WorkIQ email for NAV3644810 identified two RU Bucket5 failures in CU139196: RunAssistedSetupFromNormalSetupRecordMissing and RunAssistedSetupFromNormalSetupRecordExists. Both used TEST as a positive Server Address and hit the dynamics.com host guard.

Only those two fixture URLs become https://test.dynamics.com, with corresponding comments updated. The existing wizard-propagation assertions, handlers, test list and production validation remain unchanged; no additional exclusions were added.

The CRM test app compiled and published on tenant2-1. Local runtime verification is blocked before these assertions: unchanged Initialize calls DisableEncryption, which is unsupported in this agent-enabled environment (47 reported,4 passed,43 failed in setup). No encryption/credential/service changes or assertion bypass were performed. NAV buddy execution is the remaining validation route; these two cases are not yet claimed passing.

Scope correction: 5de4cb7

At the user's explicit request, this BCApps PR is limited to Expense Agent and Spend Request changes. Our earlier changes to CDSConnectionWizardTests, CDSConnectionSetupTest and UTREST have been restored exactly to their pre-repair aaa5 contents. The final PR diff contains only four Expense Agent AL files and its disabled-test manifest; the historical non-Expense fixture proposals above are superseded and NOT included.

NAV handles unrelated failures through exact-method temporary exclusions instead: the four confirmed Bucket5 methods (one CDS wizard URL case, two CDS assisted-setup cases and UT REST.NotExpiredToken), alongside the previously authorized four Retention Policy and ten VIES methods. These are quarantines, not fixes or passing tests; bug filing remains deferred. All other existing exclusions are preserved.

BCApps keeps the Expense non-API re-enables and its HTTP API exclusions. NAV retains the Expense app re-enable and its21 method re-enables, including the six HTTP API cases. The prior local15API+20permissions+80Spend passes apply to unchanged Expense code; fresh CI/buddy results remain pending. Future repairs in this BCApps PR must not modify unrelated tests.

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
@github-actions github-actions Bot added AL: Apps (W1) Add-on apps for W1 Team: Finance GitHub request for Finance area labels Sep 21, 2026
@github-actions github-actions Bot modified the milestone: Version 30.0 Sep 21, 2026
…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
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
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7
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
…ests

Repair missing account references on the Normal VAT template used by country initialization. Preserve valid accounts and all test assertions; avoid inheriting dangling references from prior fixtures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7
Create fresh expense and employee posting groups and payment methods. Restore singleton setup between tests and remove broad country demo-data normalization without changing the 74 scenarios or assertions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7
Reuse the shared LibraryHumanResource instance in both employee mapping tests to eliminate the two new AA0198 warnings. Test bodies and assertions are unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7
Clear Employee Paid and Company Paid payment methods after expense transaction cleanup, then use the library find-or-create helper within each test. This preserves reimbursement-type uniqueness without inheriting prior test data or weakening assertions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7
Clear HTTP response buffers before calls and assert failed requests through the helper-raised error, retaining status, domain and persistence checks. Strengthen date POST identity diagnostics without weakening the missing-record assertion. API exclusions and production behavior remain unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7
Match existing response ID parsing through Evaluate and AsText rather than an unavailable JsonValue conversion method.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7
Keep supplied date values on the source record while deferring paired validation. Default the route owner at insertion only when omitted, so owner-only POSTs produce a real record change while explicit owner validation remains intact.

Validated all 15 HTTP API tests on tenant2-1 without changing their payloads or persistence assertions, plus 20 permission and 74 Spend Request unit tests and direct owner-scope edge checks.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7
Preserve upstream approval-audit coverage and existing fixture setup. Rebuilt and published matching Base, Expense and test apps on tenant2-1; all 15 HTTP API, 20 permission and 80 Spend Request tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7
Use allowed Dataverse hosts for positive URL-normalization cases and explicitly exercise SaaS validation. Build JWT expiry fixtures with the canonical Unix Timestamp helper and a one-hour margin, preserving expiry and transport-security assertions.

Both test apps compile locally. The target CDS case passes; full local validation is limited by unrelated setup failures and server-side MockTest alias resolution. Buddy execution is required.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7
Replace the TEST placeholder in the two positive assisted-setup entry scenarios with an allowed HTTPS Dataverse address. Preserve both existing assertions, handlers and production URL validation. Compilation and publishing passed locally; runtime verification remains for NAV because local shared initialization rejects disabling encryption.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7
Restore the unrelated CRM setup, CRM wizard and UT REST test files exactly to their pre-repair contents, as requested. Keep Expense Agent and Spend Request fixes unchanged; unrelated failures are handled by method-scoped NAV exclusions instead.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7
Comment thread src/Apps/W1/ExpenseAgent/test/src/SpendRequestTest.Codeunit.al
@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟠\ High\ Severity\ —\ Web\ Services}$

This API page is keyed by SystemId, but the Travel Request Details navigation links the child through the renamable business field "No.". Define a GUID child foreign key related to the parent's SystemId and link the part with Field(SystemId), so navigation and deep-insert references remain stable.

Knowledge:

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

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.46.6

@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟠\ High\ Severity\ —\ Web\ Services}$

This API page is keyed by SystemId, but the Travelers navigation links the child through the renamable business field "No.". Define a GUID child foreign key related to the parent's SystemId and link the part with Field(SystemId), so navigation and deep-insert references remain stable.

Knowledge:

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

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.46.6

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 1

Recommendation: Accept

What this PR does

This change repairs Expense Agent test fixtures, re-enables the covered permission and posted-history tests, and fixes Travel Request API posts that returned success without persisting the request.

The API now distinguishes an omitted owner from an explicitly supplied owner and defaults the route owner only during insertion. Supplied dates mark the source record as changed while the existing paired-date validation still runs before persistence. This addresses the persistence cause without weakening owner, status, or date validation.

Problem-solution fit

Fit: Strong

The production change directly addresses the owner-only and date-only persistence failures. The updated permission, API, posting-history, and isolated fixture coverage exercises the affected paths and preserves the existing negative cases.

Suggestions

None.

Risk assessment and necessity

Risk: The production impact is limited to the insert and modify lifecycle of page 7134. Explicit and omitted owners, invalid owner changes, date ordering, partial date updates, and persistence are covered. No public schema or signature changes are introduced. The latest CI matrix was still running at analysis time.

Necessity: The change is required because valid owner-only and date-only POST requests could return success without leaving a stored request. Re-enabling the repaired tests also restores regression coverage for permissions and posted-history behavior.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11654 round=1 by=alexei-dobriansky at=2026-09-28T13:14:14Z lastSha=5de4cb76e597180a8566fb58cb284ae5d05568f8 reviewKey=af9d41d4b96752404d79b1b149be3f171eb527da61ff78f91929820b35363a42 suggestions=

Combine upstream admin-only setup permissions with permission observations captured before restoring the test harness.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 2

Recommendation: Accept

What this PR does

The PR repairs and re-enables Expense Agent permission, history, action, and API tests. The new change distinguishes setup-maintenance permissions from document-edit permissions and verifies Expense User, Expense Team, and Expense Approval Setup access under the restricted role.

Status of previous suggestions

None in round 1.

New observations

None. The added assertions align read/edit/admin expectations with the current permission-set design and keep all observations inside the restricted-role scope.

Risk assessment and necessity

Risk: Low. The latest change is test-only and strengthens permission coverage; production permission sets and business logic are unchanged.

Necessity: Edit users must be able to edit expense documents without gaining administrative setup access, while administrators retain setup maintenance permissions.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11654 round=2 by=alexei-dobriansky at=2026-09-30T13:36:14.755Z lastSha=6813951e54bf504804ae797b4c24e38ebf841b62 reviewKey=866fcf362bb938621b1dc2e15b8d1daa54bb1e6cb042dfbbbf16714212d615fe suggestions= parentRound=1

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 988fb581-008c-4e0d-99c1-285cd45d49e7
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 3

Recommendation: Accept

What this PR does

The latest change gives the E2E activity-log approver unlimited approval, so the 100 LCY report can complete its approval flow instead of exceeding the default zero approval limit. The field validation is applied after approval rights are enabled and is persisted before the approval mapping is created, matching established approval test fixtures.

Status of previous suggestions

None in round 2.

New observations (commits since round 2)

None. The test-only fixture correction lets the scenario reach its activity-history assertions without weakening them or changing production behavior.

Risk assessment and necessity

Risk: Low. The change only adjusts test setup. Approval-limit routing is outside this activity-log scenario and remains covered by dedicated approval tests.

Necessity: The report amount is 100 LCY while a new approver has a zero approval limit by default. Unlimited approval is required for this fixture to complete the intended approval round trip.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11654 round=3 by=alexei-dobriansky at=2026-10-01T01:08:21.089Z lastSha=2380c0bf2ed305059d6876ec04d4fafd890bcfcb reviewKey=7f4d51db39b0a8e5f342ac98175f07b379eb312b58853809a07a3efd7bdefcc1 suggestions= parentRound=2

@t-prda
Prangshuman Das (t-prda) added this pull request to the merge queue Oct 1, 2026
Merged via the queue into main with commit 1714549 Oct 1, 2026
172 checks passed
@t-prda
Prangshuman Das (t-prda) deleted the fix/expense-test-repairs-650245 branch October 1, 2026 15:21
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.

5 participants