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, receipt count, and policy status
• expose read-only report-scoped and Expense User role-scoped APIs, plus active/posted/manager factboxes and the ActivityLog capability
• add retention-policy eligibility, source deletion cleanup, posting reassignment, unit/API/posting tests, and persistent end-to-end scenarios

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 persisted 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

[github-actions]

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

The changed permission-set source file is named ExpenseManagementObjects.permissionset.al, but the style contract requires <ObjectName>.<ObjectType>.al with the exact AL object type segment. Keeping permissionset lowercase weakens the file-system-to-object mapping that grep, tooling, and code navigation rely on; rename it to ExpenseManagementObjects.PermissionSet.al.

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

[Prangshuman Das (t-prda)]

The filename comment refers to a pre-existing repository file, ExpenseManagementObjects.permissionset.al, that this PR only updates. Renaming that shared permission-set file would be unrelated churn, so it is intentionally left unchanged.

[github-actions]

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

Capability-specific behaviour is still selected with a case over the Expense Capability enum. In this PR, adding ActivityLog required synchronized edits to both the enum and IsEnabled, which is the variant-dispatch anti-pattern this rule calls out. Model the capability variants as enum-backed interface implementations and dispatch through an interface variable so future capabilities do not require editing the consumer codeunit.

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

[github-actions]

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

The file is named ExpenseManagementObjects.permissionset.al, but the object-type segment in <ObjectName>.<ObjectType>.al must use the AL object type name PermissionSet. Keeping permissionset lowercase breaks the file-system-to-object mapping that tooling and code search rely on; rename the file to ExpenseManagementObjects.PermissionSet.al.

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

[github-actions]

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

The new Expense Report Header table relies on table-level DataClassification while its 25 Normal fields (including "Expense User No.", "Expense User Name", and "Description") omit field-level DataClassification. AS0016 requires classification on every Normal field individually; the table-level property does not cascade, so these fields remain implicitly ToBeClassified and the sensitive user-identifying fields are under-classified for GDPR/telemetry purposes.

Knowledge:

• microsoft/knowledge/privacy/table-level-data-classification-cascades.md
• microsoft/knowledge/privacy/data-classification-required-on-pii-fields.md

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

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

[github-actions]

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

The new Posted Expense Report Header table also relies on table-level DataClassification while its 25 Normal fields omit field-level DataClassification. Sensitive fields such as "Expense User No." and "Expense User Name" remain implicitly ToBeClassified instead of being explicitly classified at field scope, as AS0016 and PII-classification guidance require.

Knowledge:

• microsoft/knowledge/privacy/table-level-data-classification-cascades.md
• microsoft/knowledge/privacy/data-classification-required-on-pii-fields.md

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

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

[Prangshuman Das (t-prda)]

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

This PR adds expense activity history with lifecycle writes, read-only APIs, factboxes, retention support, and unit/API/posting scenarios. The lifecycle calls are placed in the same transactions as the state changes, and the active/posted navigation is tested.

The current implementation does not match the linked Deliverable 644172 contract in several merge-blocking areas. It rewrites one log table during posting instead of using separate active and posted history, exposes different API field names and navigation paths than the service consumes, and always attributes posting to a BC user. The analogous BaseApp approval flow copies active Approval Entry rows into Posted Approval Entry; no new BaseApp event publisher dependency is involved here.

Suggestions

S1 - Keep posted history append-only
The deliverable requires separate active and posted log tables and says entries are never edited. This loop rewrites existing audit rows in place; copy them to a posted log as the BaseApp PostApprovalEntries pattern does, and keep the original history immutable.

S2 - Match the published activity API contract
The linked deliverable says the connector consumes exact names such as entryNo, sequence, subjectType, subjectSystemId, documentNo, amount, defaultCurrencyAmount, currencyCode, and documentKind. This page exposes different names and omits sequence and documentKind, so the service will silently drop or normalize values; align the schema and update the API assertions.

S3 - Expose approver history at the required path
The connector contract calls expenseUsers({me})/approverView/activityHistory, but this change only adds activityHistory directly under the expense user and requires a role filter. Add the collection under Approver View API so the existing connector path works.

S4 - Record automatic posting as System
This call always records Posted as a BC user, but the deliverable defines automatic batch posting as System with no actor. Pass the posting provenance into the writer so interactive posts stay User and background or batch posts are not attributed to a person.

Risk assessment and necessity

Risk: This is compliance-facing audit data and a service integration contract. The schema and path mismatches can make the connector silently lose fields or fail to reach approver history, while rewriting and misattributing rows weakens the audit trail. The core writer and posting tests are useful, but the API tests that would catch contract drift are disabled in the current CI runner.

Necessity: The Slice 634361 capability is important and the activity-history foundation is justified. The scope is appropriate for a feature increment, but the AL storage and API surfaces must match the already implemented service contract before merge.

---

[AI-PR-REVIEW] version=1 promptVersion=1 system=github pr=10237 round=1 by=t-prda at=2026-08-13T17:58:36.7864434Z lastSha=01047709b70702b94e78ebd9be88bc3a4c79c49f reviewKey=118f8783fd26bccc4714e42ce99a79cef7f4b17d0a39478fc172a5f83b89b91d suggestions=S1@2c9292d9,S2@1ca91f49,S3@f51e22cc,S4@bf4a358d

[Prangshuman Das (t-prda)]

Response to Agentic PR Review - Round 1

The review used the previous Deliverable 644172 description, which still documented the rejected two-table prototype and stale connector commit. The deliverable is now corrected to the final reviewed design: https://dynamicssmb2.visualstudio.com/Dynamics%20SMB/_workitems/edit/644172/

Suggestion	Status	Resolution
S1 - separate posted table	Disputed	Final design uses one table. Posting changes only the source owner pointer; stable subject identity and all event/audit content remain unchanged.
S2 - old API field contract	Disputed	The current service PR maps the published AL names (entryNumber, subjectId, documentNumber, BC amount fields, actor table/id, category JSON) in the BC connector. TypeSpec/Functions remain ERP-agnostic.
S3 - old approver navigation	Disputed	The current connector uses expenseUsers({id})/expenseActivityLogEntries?$filter=historyActorRole eq 'Approver'. BC selects matching subjects and returns every row in those timelines.
S4 - automatic posting as System	Disputed	The implemented posting path runs in an authenticated BC session and records that BC User. No unattended/system posting entry point is introduced by this slice; future explicit provenance remains separate work.

No code change is required for these four suggestions. The stale source-of-truth text was the defect and has been replaced with the implemented contract and acceptance criteria.

[github-actions]

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

The agent-facing ExpenseReportsAPI actions ApprovedExpenseReport/RejectedExpenseReport/RejectAndReopenExpenseReport accept an arbitrary ApproverExpenseUserNo parameter from the caller and pass it straight to ExpenseReportApprovalMgmt.Approve/Reject(ExpenseReportHeader, ApproverExpenseUserNo). That overload only calls CheckApproverPermissions(ExpenseUser), which validates the named Expense User has Can Approve set and a non-blank User Id For Approvals — it never confirms this approver is actually the one authorized for THIS report (the interactive-user path additionally checks Expense Approval Setup/Default Approver linkage via CheckApproverPermissions(ExpenseReportHeader)). Any caller of this API can therefore supply any enabled approver's Expense User No. to approve or reject any pending expense report, bypassing the approval hierarchy enforced for interactive users.

Recommendation:

• verify the supplied ApproverExpenseUserNo is linked to the specific report (via Expense Approval Setup or the report's Approver Expense User ID/Default Approver) before applying the action, mirroring the check used in the interactive overload.

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

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

[github-actions]

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

Adding the new ActivityLog capability requires another branch in "Expense Capabilities Provider".IsEnabled's case over the "Expense Capability" enum, so capability-specific behavior still lives in manual dispatch code instead of enum-backed interface dispatch. That contradicts the article's best practice and means every future capability addition still has to edit this provider rather than plugging in a new implementation.

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

[github-actions]

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

ProcessExpenseReportLines calls CreateSalesDocument for every billable expense line, and that helper re-queries Sales Header with FindFirst() and Sales Line with FindLast() on every iteration. Posting a report with many billable lines therefore creates an N+1 lookup pattern against those larger tables; cache the open invoice and next line number by billing tuple, or pre-group the lines, so the loop does not keep searching the same sales documents row by row.

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

[github-actions]

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

ShowPostedConfirmationMessage filters Posted Expense Report Header on its full primary key (No.) and then calls FindFirst(). Use Get(Rec."Last Posting No.") here so the code performs a direct key lookup instead of a filtered search.

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

            if PostedExpenseReportHeader.Get(Rec."Last Posting No.") then
                if InstructionMgt.ShowConfirm(StrSubstNo(OpenPostedExpenseReportQst, PostedExpenseReportHeader."No."),
                     InstructionMgt.ShowPostedConfirmationMessageCode())
                then
                    InstructionMgt.ShowPostedDocument(PostedExpenseReportHeader, Page::"Posted Expense Report");

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

[github-actions]

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

ShowPostedConfirmationMessage filters Posted Expense Report Header on its full primary key (No.) and then calls FindFirst(). Use Get(Rec."Last Posting No.") here so the code performs a direct key lookup instead of a filtered search.

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

            if PostedExpenseReportHeader.Get(Rec."Last Posting No.") then
                if InstructionMgt.ShowConfirm(StrSubstNo(OpenPostedExpenseReportQst, PostedExpenseReportHeader."No."),
                     InstructionMgt.ShowPostedConfirmationMessageCode())
                then
                    InstructionMgt.ShowPostedDocument(PostedExpenseReportHeader, Page::"Posted Expense Report");

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

[github-actions]

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

Expense Report Header still relies on the table-level DataClassification = CustomerContent for many Normal fields. Fields such as Expense User No., Expense User Name, Description, Amount (LCY), Reimbursement Currency Code, Submission DateTime, Posting No., and Spend Request No. have no field-level DataClassification, so they remain effectively ToBeClassified/unreviewed despite storing employee identifiers, customer content, and account data.

Knowledge:

• microsoft/knowledge/privacy/table-level-data-classification-cascades.md
• microsoft/knowledge/privacy/data-classification-required-on-pii-fields.md

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

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

[github-actions]

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

Posted Expense Report Header has the same under-classification pattern: many Normal stored fields rely on the table-level classification and omit field-level DataClassification. That leaves fields like Expense User No., Expense User Name, Description, Amount (LCY), Submission DateTime, Spend Request No., and Spend Request Close unclassified even though they store personal, customer, and financial data that is exposed through the posted-report APIs.

Knowledge:

• microsoft/knowledge/privacy/data-classification-required-on-pii-fields.md
• microsoft/knowledge/privacy/table-level-data-classification-cascades.md

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

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

[github-actions]

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

The new activity log is erased from the source report's OnDelete trigger, and the agent-facing Expense Reports API still does not disable DELETE. That lets the agent remove a report and wipe its submission/rejection/approval history in the same operation, which weakens the audit trail this PR introduces. Preserve activity entries independently of document deletion, or explicitly block deletes once history exists.

_{Agent judgement — not directly backed by a BCQuality knowledge article.}

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

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

[github-actions]

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

The new integration event is named OnCheckExpenseReportPostRestrictions, which does not encode whether the hook fires before or after the restriction check. Rename it to a position-encoding publisher name such as OnBeforeCheckExpenseReportPostRestrictions and update the call site so subscribers can infer firing order from the event name alone.

Knowledge:

• microsoft/knowledge/events/name-events-by-publisher-position.md

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

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

[github-actions]

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

Called from the per-line posting loop in ProcessExpenseReportLines, CreateSalesDocument re-queries Sales Header and then Sales Line for every billable expense line via FindSalesDocumentToAppend() and GetNextSalesLineNo(). That creates an N+1 access pattern as report size grows; cache the target invoice/next line number per distinct customer-currency-posting-date key, or reshape the logic so repeated lines do not perform fresh inner-table lookups.

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

[github-actions]

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

The service-enabled approval actions accept caller-supplied submitter/approver numbers and forward them into privileged internal routines that trust those values to set status, approver identity, and audit data. Because Access = Internal / internalsVisibleTo is not an authorization boundary, any companion caller that can reach this path can impersonate arbitrary submitters or approvers. Bind the business actor inside the privileged path (for example from the authenticated principal or explicit access-control rules) instead of trusting request parameters.

Knowledge:

• microsoft/knowledge/security/internal-access-is-not-a-security-boundary.md

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

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

[github-actions]

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

The agent permission set grants direct modify rights on system-of-record tables such as Expense Ledger Entry and the posted expense report tables even though the changed surfaces consume them through posting code and read-only APIs. Per the guidance, those writes should be indirect so they stay code-mediated; as shipped, the agent principal can mutate posted or ledger data through any executable object it can reach.

Knowledge:

• microsoft/knowledge/security/indirect-permissions-for-elevated-access.md

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

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

[github-actions]

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

AddExpenseToReport declares ExpenseNotFoundErr inside the procedure-local var block. That label will compile, but procedure-scoped labels are fragile in XLIFF extraction and should be moved to the page's top-level var block so translation keys stay stable and reviewable.

Knowledge:

• microsoft/knowledge/style/labels-declared-at-object-scope.md

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

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

[github-actions]

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

Populate takes ExpenseCapabilitiesBuffer as a temporary record parameter, but the name does not start with Temp. That makes Insert()/DeleteAll() calls read like database operations even though this buffer is in-memory only; rename it to TempExpenseCapabilitiesBuffer across the signature and its uses.

Knowledge:

• microsoft/knowledge/style/temporary-variable-temp-prefix.md

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

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

[github-actions]

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

EntryRecIDLbl contains two placeholders (%1 %2) but no Comment. From the label text alone, translators cannot infer that %1 is the table caption and %2 is the document number, so the placeholder meanings need to be documented explicitly.

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

        EntryRecIDLbl: Label '%1 %2', Locked = true, Comment = '%1 = Table Caption, %2 = Document No.';

Knowledge:

• microsoft/knowledge/style/label-comment-explains-placeholders.md

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

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

[github-actions]

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

This file is named ExpenseManagementObjects.permissionset.al, but the AL file naming convention is ..al with the type segment spelled PermissionSet. Rename it to ExpenseManagementObjects.PermissionSet.al so file listings and tooling can map the file to its object type without parsing the source.

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

[Prangshuman Das (t-prda)]

Response to latest automated review comments

I checked the latest top-level findings against origin/main...HEAD. They do not identify changes introduced by this activity-history PR:

• ExpenseManagementObjects.permissionset.al filename, EntryRecIDLbl, capability Populate parameter naming, and ExpenseNotFoundErr placement are pre-existing source/style items.
• Direct modify permissions on existing ledger/posted tables predate this PR. The new activity table permission was specifically changed to indirect r, with reads mediated by the API/factbox.
• Caller-supplied submitter/approver parameters and the existing approval authorization model predate this PR; this change adds transactional audit writes to those flows but does not create the service actions or their authorization contract.
• CreateSalesDocument N+1 behavior and OnCheckExpenseReportPostRestrictions event naming are pre-existing posting implementation concerns outside the activity-log paths changed here.
• Field-level classifications on the existing active/posted report header tables are pre-existing and are not added or changed by this PR.
• Explicit document deletion intentionally cascades activity to avoid orphaned rows; the posted Never Delete template governs time-based retention policy, not explicit deletion of the owning document.

The activity-log-specific review findings (object IDs, partial records, committed API reads, mediated permissions, query keys, scenario cleanup, API mapping, and test assertions) have been addressed in the branch. No unrelated refactor is being added for the comments above.