Skip to content

[Retention Policy] Keep record marks for indirect-permission subset deletes#8469

Draft
onbuyuka wants to merge 1 commit into
mainfrom
bugs/626315-retention-policy-marks
Draft

[Retention Policy] Keep record marks for indirect-permission subset deletes#8469
onbuyuka wants to merge 1 commit into
mainfrom
bugs/626315-retention-policy-marks

Conversation

@onbuyuka
Copy link
Copy Markdown
Contributor

@onbuyuka onbuyuka commented Jun 3, 2026

Summary

Retention-policy subset deletion (specific marked records — i.e. a policy that is not "apply to all records") deleted nothing for tables that require indirect delete permission (Email Logs, Retention Policy Logs, Performance Profiler, Shopify Logs). IcM 21000000010094.

Root cause

In "Apply Retention Policy Impl.".DeleteExpiredRecords:

RecordRefDuplicate := RecordRef.Duplicate();        // (1) copies filters + MarkedOnly flag, NOT the marks
...
RetenPolDeleting.DeleteRecords(RecordRef, ...);      // (2) for indirect tables this Close()s RecordRef
...
OnApplyRetentionPolicyIndirectPermissionRequired(RecordRefDuplicate, Handled);  // (3) markless duplicate

RecordRef.Duplicate() copies the filters and the MarkedOnly flag but not the individual record marks. So the subscriber received a marked-only view with zero marks, and its DeleteAll(true) deleted nothing (the subscribers gate on MarkedOnly()/filters). Meanwhile "Reten. Pol. Delete. Impl.".DeleteRecords had already Close()d the original RecordRef — which still held the marks — before the event ran.

Fix

  • RetenPolDeleteImpl.DeleteRecordsClose() the RecordRef only on the direct-permission path (where this codeunit does the DeleteAll itself). On the indirect path leave it open so the marks survive for the caller.
  • ApplyRetentionPolicyImpl.DeleteExpiredRecords — pass the original marked RecordRef to the event, compute RecordCountAfter from it (remaining marked records), then Close() it. The direct path is unchanged (still counts via the duplicate; the duplicate is still used for telemetry caption/name/number in both paths).

Counting note

On the indirect path, RecordCountAfter now counts the original ref after the subscriber's DeleteAll, i.e. the marked records still remaining (0 when all were deleted) → NumberOfRecordsDeleted = RecordCountBefore. The previous code counted the markless duplicate (always 0), which combined with deleting nothing under-deleted while still logging RecordCountBefore deleted — a misleading count this fix also corrects.

Subscriber safety

All four subscribers of OnApplyRetentionPolicyIndirectPermissionRequired use the same template (MarkedOnly()/GetFilters() check → RecRef.DeleteAll(true) → set Handled) and none of them close the ref, so passing the original ref is safe for every consumer.

Provenance / status

The previously-attempted ADO PR 243754 (submodule-pointer bump) was abandoned; this implements the fix directly in BCApps. Posting as draft.

Verification ⚠️

  • Statically traced end-to-end across DeleteExpiredRecords, DeleteRecords, all four subscribers, and the Reten. Pol. Filtering mark logic.
  • Not runtime-verified — AL does not run locally here, and this is sensitive data-deletion code. Please validate on an NST before merging. Recommended regression test: register a test table requiring indirect delete permission, configure a subset (not "apply to all") policy, mark a multi-record expired set, run apply, and assert (a) all marked records are deleted and (b) the logged deleted-count matches.

Linked work item: AB#626315

🤖 Generated with Claude Code

…eletes

Retention policy subset deletion (specific marked records, e.g. an Email Log
or Shopify Log retention policy that is not "apply to all") deleted nothing
for tables that require indirect delete permission.

Cause: in "Apply Retention Policy Impl.".DeleteExpiredRecords the code took
RecordRefDuplicate := RecordRef.Duplicate() and raised
OnApplyRetentionPolicyIndirectPermissionRequired with that duplicate.
RecordRef.Duplicate() copies the filters and the MarkedOnly flag but NOT the
individual record marks, so the subscriber received a marked-only view with
zero marks and its DeleteAll(true) deleted nothing. Worse, "Reten. Pol.
Delete. Impl.".DeleteRecords had already Close()d the original RecordRef
(which still held the marks) before the event ran.

Fix:
- DeleteRecords: only Close() the RecordRef on the direct-permission path
  (where it does the DeleteAll itself). For the indirect path leave it open
  so the marks survive for the caller.
- DeleteExpiredRecords: pass the original marked RecordRef to the event,
  compute RecordCountAfter from it (remaining marked records), then Close()
  it. The direct path is unchanged (still counts via the duplicate, which is
  also used for telemetry/caption in both paths).

All four subscribers (Email Logs, Retention Policy Logs, Performance
Profiler, Shopify Logs) rely on MarkedOnly()/filters + DeleteAll(true) and
none close the ref, so passing the original ref is safe.

Fixes AB#626315

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant