Skip to content

[Expense Agent] Add activity log retention policy - #10416

Open
Prangshuman Das (t-prda) wants to merge 1 commit into
mainfrom
feature/646820-expense-activity-retention
Open

[Expense Agent] Add activity log retention policy#10416
Prangshuman Das (t-prda) wants to merge 1 commit into
mainfrom
feature/646820-expense-activity-retention

Conversation

@t-prda

@t-prda Prangshuman Das (t-prda) commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Add Business Central retention-policy uptake for Expense Agent activity history.

  • Register Expense Activity Log Entry during per-company installation, upgrade, and company initialization.
  • Protect active-report activity with an enabled, locked Never Delete filter.
  • Make posted-report activity customer-configurable with an enabled, unlocked Never Delete default.
  • Use Occurred At as the retention date and add a source/date key for retention filtering.
  • Handle retention deletion through the existing activity-log management codeunit when the execution user has indirect delete permission.
  • Preserve the existing explicit source-document deletion cascades.

OnRefreshAllowedTables is intentionally not subscribed; lifecycle registration is sufficient for initial uptake and newly created companies.

Permissions

No permission-set or entitlement changes are included. Installation and upgrade use the existing inherently executable setup codeunits. SUPER users can use the retention framework's direct deletion path, while indirect deletion is handled through Expense Activity Log Mgt.. PR #10346 separately supplies indirect activity-table permissions to normal Expense Management and Expense Agent roles.

Validation

  • git diff --check
  • Reviewed install/upgrade tag idempotency and retention deletion safeguards
  • App compilation, publication, and runtime validation will be performed manually

Fixes AB#646820

image

Register activity history with protected active rows and configurable posted retention. Handle indirect retention deletion and add a source/date key.

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

Copilot-Session: b70db042-75b8-4a9d-ba20-b539889b7e76
@t-prda
Prangshuman Das (t-prda) requested a review from a team August 19, 2026 15:11
@t-prda
Prangshuman Das (t-prda) requested a review from a team as a code owner August 19, 2026 15:11
@github-actions github-actions Bot added AL: Apps (W1) Add-on apps for W1 Integration GitHub request for Integration area labels Aug 19, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone Aug 19, 2026
@t-prda

Copy link
Copy Markdown
Contributor Author

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

This PR registers Expense Agent activity history with the retention-policy framework. Active-report entries receive a locked Never Delete filter, while posted-report entries receive an unlocked Never Delete default based on Occurred At.

The source/date key and indirect-delete callback fit the framework design, and deletion remains bounded by the supplied RecordRef. However, valid filter-only runs are logged as errors, and the app-specific protection and deletion behavior has no automated coverage.

Suggestions

S1 - Avoid false errors for filtered runs
A valid filter-only retention run can have filters without MarkedOnly. The or condition logs an error even though the delete is bounded. Use and so only a completely unconstrained view is reported.

S2 - Add retention policy regression tests
Add tests for registration metadata, repeated registration, locked active entries, configurable posted entries, and bounded indirect deletion. Generic framework tests cannot verify these Expense Agent rules.

S3 - Restore registration during refresh
Add an OnRefreshAllowedTables subscriber with a force-update path. The upgrade-tag shortcut cannot restore the allowed-table entry if it is removed.

Risk assessment and necessity

Risk: Retention controls deletion of audit-history data. The filters and key are narrow, but false error logs can make successful runs look broken, and missing tests leave active-entry protection and posted-entry deletion unverified.

Necessity: Retention support is required so customers can manage posted activity history while active-report history remains protected. The scope is appropriate after the guard and regression coverage are completed.


[AI-PR-REVIEW] version=1 promptVersion=1 system=github pr=10416 round=1 by=t-prda at=2026-08-19T15:50:02.0758493Z lastSha=d700c0e9978d07a1185ca91186716bc359cde564 reviewKey=a60aa5584c936e4eef621305ae0312534ed84bd183035b13f80b444cd810f19c suggestions=S1@b6e3141c,S2@c2416975,S3@6c094d71

@t-prda

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I assessed all three suggestions against the approved retention contract:

  • S1 — no change. The callback requires both safeguards: a filtered RecordRef and MarkedOnly. Retention Policy Log.LogError is terminating and rolls back, so there is no fallthrough. The stricter or guard also matches current Base Application, Shopify, Performance Profiler, and Service Archive retention subscribers. Using and would allow deletion when one safeguard is absent. This was also answered on the inline thread.
  • S2 — no change in this PR. App-specific automated tests were explicitly excluded from the agreed scope; compilation/publication and runtime validation are being performed manually. Manual validation has confirmed that the policy creates the locked active Never Delete line and editable posted Never Delete line under Record Retention Policy.
  • S3 — no change. Omitting OnRefreshAllowedTables was an explicit design decision. Per-company install, tagged upgrade, and company initialization cover initial uptake and future companies, consistent with the basic Microsoft Learn extension guidance. If an administrator intentionally removes table 7100 from Allowed Tables, Refresh should not silently undo that decision.

The existing implementation therefore remains unchanged.

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 Integration GitHub request for Integration area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant