Add expense activity history foundation

[Prangshuman Das (t-prda)]

Summary

• add a single append-only expense activity table whose source reference moves from the active report to the posted report while its logical subject remains stable
• record Created, Submitted, Resubmitted, Recalled, Approved, Rejected, Reopened by Approver, and Posted lifecycle events with actor, role, comments, financial snapshots, categories, and receipt count
• expose read-only report-scoped and Expense User role-scoped APIs, plus active/posted/manager factboxes and the ActivityLog capability
• add source deletion cleanup, posting reassignment, unit/API/posting tests, and a cleanup-safe activity API E2E scenario

Validation

• the AL tests passed in a manually published local environment before rebasing
• active report, posted report, submitter history, and approver history APIs were exercised against local scenarios
• unscoped activity and user history without historyActorRole were verified to fail closed
• API integration tests are listed in the disabled-test manifest because the current CI environment does not support their required API isolation

Fixes AB#644172

Related: AB#639704

Replaces #10237 to reset stale review and CI history after the design was finalized.

[github-actions]

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

The code filters the Expense record by SystemId and then calls FindFirst(). This is a direct single-record lookup, so GetBySystemId() is the cheaper and clearer access pattern than a filtered FindFirst().

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

        if Expense.GetBySystemId(ExpenseId) then begin

Knowledge:

• microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.md

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

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

[github-actions]

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

The code filters Posted Expense Report Header on its full primary key field "No." and then calls FindFirst(). Get() expresses the direct primary-key seek without a filtered-search shape.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

            if PostedExpenseReportHeader.Get(Rec."Last Posting No.") then

Knowledge:

• microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.md

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

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

[github-actions]

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

The code filters Posted Expense Report Header on its full primary key field "No." and then calls FindFirst(). Get() expresses the direct primary-key seek without a filtered-search shape.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

            if PostedExpenseReportHeader.Get(Rec."Last Posting No.") then

Knowledge:

• microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.md

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

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

[github-actions]

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

The changed file is named ExpenseManagementObjects.permissionset.al, but the style convention requires the type segment to use the AL object-type name PermissionSet in <ObjectName>.<ObjectType>.al. The lowercase permissionset suffix breaks the standard file-to-object mapping that grep, symbol search, and other tooling 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.33.4}

[github-actions]

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

ProcessExpenseReportLines iterates every expense-report line, and that per-line path calls CreateSalesDocument, which then does SalesHeader.FindFirst() and SalesLine.FindLast() to locate or extend an open invoice. Posting a report with many billable lines therefore re-scans sales tables once per line, creating the N+1 lookup shape this rule warns about. Cache the target invoice and next line number by (customer, currency, posting date) or batch/group the billable lines before entering the posting loop.

Knowledge:

• microsoft/knowledge/performance/avoid-get-inside-loop-on-large-table.md

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

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

[github-actions]

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

This permission-set file is still named ExpenseManagementObjects.permissionset.al, but the style rule requires the canonical object-type suffix PermissionSet in ..al. Keeping the lowercase suffix breaks the filesystem-to-object-type mapping that grep, explorers, and other tooling rely on; rename the file to ExpenseManagementObjects.PermissionSet.al while touching this object.

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.33.4}

[Prangshuman Das (t-prda)]

Review follow-up for the unanchored findings:

• The disabled-test observation (#issuecomment-5293183036), capability-interface observation (#issuecomment-5293504058), and caller-supplied approval identity observation (#issuecomment-5293504560) duplicate inline threads that now contain the detailed rationale. The API tests require isolation-disabled execution; the small capability dispatcher is intentional; and approval authorization is a pre-existing concern that should be handled as a separate security change.
• The Expense Ledger Entry permission (#issuecomment-5293590572), reimbursement-notification ErrorInfo (#issuecomment-5293600219), active/posted header classification (#issuecomment-5293803760 and #issuecomment-5293803956), existing Get/FindFirst patterns (#issuecomment-5293842749, #issuecomment-5293842945, and #issuecomment-5293843155), permission-set filename casing (#issuecomment-5293843744 and #issuecomment-5294432209), and sales-posting N+1 behavior (#issuecomment-5293895097) are pre-existing code unrelated to the activity-history changes in this PR. They are not being changed here to avoid unrelated scope expansion.

The actionable activity-history findings are being handled separately and remain open until the local fixes are reviewed and pushed.

[github-actions]

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

"Expense Capabilities Provider" selects capability-specific behaviour with a case over the "Expense Capability" enum. This means every new capability requires editing the central dispatcher again, which is the enum-branching anti-pattern the interfaces guidance calls out. Model the enum as one that implements an interface, move each capability's enablement logic into implementation codeunits, and dispatch through an interface variable instead of extending this case block.

Knowledge:

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

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

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

[github-actions]

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

The changed file path src/Apps/W1/ExpenseAgent/app/src/Permissions/ExpenseManagementObjects.permissionset.al does not follow the <ObjectName>.<ObjectType>.al convention from the style guidance: the object-type segment should use the canonical PermissionSet casing. Rename it to ExpenseManagementObjects.PermissionSet.al so tooling and file listings can identify the AL object type consistently.

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.33.4}

[Prangshuman Das (t-prda)]

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

This adds an append-oriented expense activity timeline, lifecycle snapshots, active-to-posted source reassignment, scoped APIs, FactBoxes, capabilities, permissions, and tests. The storage model, event ordering, deletion flow, and transaction placement are coherent, but the current implementation does not preserve agent provenance, enforce committed history reads, or keep table access behind the scoped surfaces.

Suggestions

S1 - Preserve agent creation provenance
The API-created report branch is always marked as User. Record Agent for that branch while keeping the Expense User as the actor.

S2 - Expose only committed activity history
Posting writes Posted before later posting steps can fail, but this API uses the default read isolation. A concurrent request can return an entry that is later rolled back; read history with ReadCommitted.

S3 - Record agent lifecycle provenance correctly
The internal Submit, Approve, and Reject paths serve Expense Agent API actions, but both logging helpers hard-code User. Pass the initiator from each caller and keep the supplied Expense User as the actor.

S4 - Keep activity reads code-mediated
Both end-user permission paths grant direct read access to the raw activity table. Use indirect read permissions and elevate only the scoped API and FactBox pages.

Risk assessment and necessity

Risk: This is audit and approval history. The current code can persist false initiator data, expose rolled-back or partly reassigned timelines to concurrent API reads, and let assigned roles bypass report and actor-role scopes through direct table access. The schema is additive and the lifecycle tests are broad, but these gaps affect the trust boundary of the new history.

Necessity: The feature is coherent and needed for expense traceability and approver history. The one-table model and source reassignment are appropriate, but the provenance, committed-read, and mediated-permission blockers must be fixed before the history can be trusted.

---

[AI-PR-REVIEW] version=1 promptVersion=1 system=github pr=10255 round=1 by=t-prda at=2026-08-14T15:36:06Z lastSha=3d37d18afd4e73b9b58448d1399e35db2a6a91d9 reviewKey=ccfefcb1a82ae0bd8ede40f7f0753173d0c35f9993e7528738536b95b0c3a872 suggestions=S1@96dea907,S2@287c7e2e,S3@36433d89,S4@a38182c7

[Prangshuman Das (t-prda)]

Response to Agentic PR Review - Round 1:

• S1/S3 (initiator provenance): keeping User is intentional for the current flows. The internal API actions represent explicit human submit/approve/send-back actions from the Expense Agent web experience; the Expense User is both the actor and the initiator. Agent is reserved for genuinely autonomous agent decisions/actions. For API-created reports we are starting with the initiating Expense User and can revisit autonomous creation provenance when that behavior is introduced.
• S2 (ReadCommitted): intentionally not changing. History is refreshable and the activity table is append-only except at bounded lifecycle operations. We prefer non-blocking reads over lock waits/timeouts during posting; a transient in-flight view can be corrected by refresh.
• S4 (direct table reads): intentional permission design. Human read permission is required for the FactBox without page-level elevation, while the agent data/object split follows the existing API permission model. The activity API independently rejects unscoped requests and accepts identity only from navigation filters.

The codeunit now has code-mediated tabledata User = r permission for resolving protected BC User actor names. No changes are planned for S1-S4 in this PR unless the product semantics or permission model are changed explicitly.