From 85ff1ec1c4e8834ac62304260a9da6cbd5354c31 Mon Sep 17 00:00:00 2001 From: Onat Buyukakkus <55088871+onbuyuka@users.noreply.github.com> Date: Wed, 3 Jun 2026 16:08:50 +0200 Subject: [PATCH] [Retention Policy] Keep record marks for indirect-permission subset deletes 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) --- .../ApplyRetentionPolicyImpl.Codeunit.al | 15 ++++++++++----- .../RetenPolDeleteImpl.Codeunit.al | 8 ++++++-- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/System Application/App/Retention Policy/src/Apply Retention Policy/ApplyRetentionPolicyImpl.Codeunit.al b/src/System Application/App/Retention Policy/src/Apply Retention Policy/ApplyRetentionPolicyImpl.Codeunit.al index b191b1baf6..4539ac3961 100644 --- a/src/System Application/App/Retention Policy/src/Apply Retention Policy/ApplyRetentionPolicyImpl.Codeunit.al +++ b/src/System Application/App/Retention Policy/src/Apply Retention Policy/ApplyRetentionPolicyImpl.Codeunit.al @@ -243,13 +243,18 @@ codeunit 3904 "Apply Retention Policy Impl." RetenPolDeleting.DeleteRecords(RecordRef, TempRetenPolDeletingParam); if not TempRetenPolDeletingParam."Skip Event Indirect Perm. Req." then begin - ApplyRetentionPolicyFacade.OnApplyRetentionPolicyIndirectPermissionRequired(RecordRefDuplicate, Handled); + // Indirect-permission path: RecordRef is still open and retains its record marks. + // RecordRef.Duplicate() copies filters and the MarkedOnly flag but NOT the marks, so the + // subscriber must receive the original marked RecordRef - otherwise subset (marked) deletes + // would delete nothing. Count the remaining marked records, then close the original ref. + ApplyRetentionPolicyFacade.OnApplyRetentionPolicyIndirectPermissionRequired(RecordRef, Handled); if not Handled then - RetentionPolicyLog.LogError(LogCategory(), StrSubstNo(IndirectPermissionsRequiredErr, RecordRefDuplicate.Number, RecordRefDuplicate.Caption)); + RetentionPolicyLog.LogError(LogCategory(), StrSubstNo(IndirectPermissionsRequiredErr, RecordRef.Number, RecordRef.Caption)); Handled := false; - end; - - RecordCountAfter := Count(RecordRefDuplicate); + RecordCountAfter := Count(RecordRef); + RecordRef.Close(); + end else + RecordCountAfter := Count(RecordRefDuplicate); NumberOfRecordsDeleted := RecordCountBefore - RecordCountAfter; TotalNumberOfRecordsDeleted += NumberOfRecordsDeleted; diff --git a/src/System Application/App/Retention Policy/src/Apply Retention Policy/RetenPolDeleteImpl.Codeunit.al b/src/System Application/App/Retention Policy/src/Apply Retention Policy/RetenPolDeleteImpl.Codeunit.al index 8681eb43a8..522be7866e 100644 --- a/src/System Application/App/Retention Policy/src/Apply Retention Policy/RetenPolDeleteImpl.Codeunit.al +++ b/src/System Application/App/Retention Policy/src/Apply Retention Policy/RetenPolDeleteImpl.Codeunit.al @@ -41,10 +41,14 @@ codeunit 3916 "Reten. Pol. Delete. Impl." implements "Reten. Pol. Deleting" LimitRecordsToBeDeleted(RecordRef, RecordReferenceIndirectPermission, RetenPolDeletingParam."Skip Event Rec. Limit Exceeded", RetenPolDeletingParam."Max. Number of Rec. To Delete", RetenPolDeletingParam."Total Max. Nr. of Rec. to Del."); end; - if not RetenPolDeletingParam."Indirect Permission Required" then + if not RetenPolDeletingParam."Indirect Permission Required" then begin RecordReferenceIndirectPermission.DeleteAll(RecordRef, true); + // Direct-permission path: close here. For the indirect-permission path the caller needs + // the still-open RecordRef (with its record marks intact) to raise + // OnApplyRetentionPolicyIndirectPermissionRequired, and closes it afterwards. + RecordRef.Close(); + end; RetenPolDeletingParam."Skip Event Indirect Perm. Req." := not RetenPolDeletingParam."Indirect Permission Required"; - RecordRef.Close(); end; local procedure LimitRecordsToBeDeleted(var RecordRef: RecordRef; RecordReferenceIndirectPermission: Interface "Record Reference"; var SkipOnApplyRetentionPolicyRecordLimitExceeded: Boolean; MaxNumberOfRecordsToDelete: Integer; TotalMaxNumberOfRecordsToDelete: Integer)