Media: Record the trashing user against the History audit entry (closes #22661)

src/Umbraco.Core/Services/MediaService.cs

@@ -1280,7 +1280,7 @@ private void PerformMoveLocked(IMedia media, int parentId, IMedia? parent, int u
             //media.Path = (parent == null ? "-1" : parent.Path) + "," + media.Id;
             //media.SortOrder = ((MediaRepository) repository).NextChildSortOrder(parentId);
             //media.Level += levelDelta;
-            PerformMoveMediaLocked(media, trash);
+            PerformMoveMediaLocked(media, userId, trash);
 
             // if uow is not immediate, content.Path will be updated only when the UOW commits,
             // and because we want it now, we have to calculate it by ourselves
@@ -1302,7 +1302,7 @@ private void PerformMoveLocked(IMedia media, int parentId, IMedia? parent, int u
                     // update path and level since we do not update parentId
                     descendant.Path = paths[descendant.Id] = paths[descendant.ParentId] + "," + descendant.Id;
                     descendant.Level += levelDelta;
-                    PerformMoveMediaLocked(descendant, trash);
+                    PerformMoveMediaLocked(descendant, userId, trash);
                 }
 
             }
@@ -1313,17 +1313,22 @@ private void PerformMoveLocked(IMedia media, int parentId, IMedia? parent, int u
         ///     Performs the actual save of a media item during a move operation while holding a write lock.
         /// </summary>
         /// <param name="media">The media item to save.</param>
+        /// <param name="userId">The identifier of the user performing the move.</param>
         /// <param name="trash">
         ///     If <c>true</c>, marks the item as trashed; if <c>false</c>, marks the item as not trashed;
         ///     if <c>null</c>, leaves the trashed status unchanged.
         /// </param>
-        private void PerformMoveMediaLocked(IMedia media, bool? trash)
+        private void PerformMoveMediaLocked(IMedia media, int userId, bool? trash)
         {
             if (trash.HasValue)
             {
                 ((ContentBase)media).Trashed = trash.Value;
             }
 
+            // Track who moved/trashed the item so the audit trail (and any consumer of WriterId)
+            // reflects the acting user, not the original creator. Mirrors PerformMoveContentLocked.
+            media.WriterId = userId;
+
             _mediaRepository.Save(media);
         }
 

tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.MoveToRecycleBin.cs

@@ -2,14 +2,18 @@
 using NUnit.Framework;
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Configuration.Models;
+using Umbraco.Cms.Core.Models;
+using Umbraco.Cms.Core.Models.ContentEditing;
+using Umbraco.Cms.Core.Models.Membership;
+using Umbraco.Cms.Core.Services;
 using Umbraco.Cms.Core.Services.OperationStatus;
 using Umbraco.Cms.Tests.Integration.Attributes;
 
 namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services;
 
 public partial class ContentEditingServiceTests
 {
-    public static new void ConfigureDisableUnpublishWhenReferencedTrue(IUmbracoBuilder builder)
+    public static void ConfigureDisableUnpublishWhenReferencedTrue(IUmbracoBuilder builder)
         => builder.Services.Configure<ContentSettings>(config =>
             config.DisableUnpublishWhenReferenced = true);
 
@@ -105,4 +109,77 @@ public async Task Cannot_Move_To_Recycle_Bin_If_Already_In_Recycle_Bin(bool vari
         Assert.IsNotNull(content);
         Assert.IsTrue(content.Trashed);
     }
+
+    // Companion to the media regression test for https://github.com/umbraco/Umbraco-CMS/issues/22661.
+    // ContentService already updated WriterId on trash; this guards against future regressions.
+    [Test]
+    public async Task Move_To_Recycle_Bin_Updates_WriterId_To_The_Trashing_User()
+    {
+        var creator = await CreateAdminUserAsync("creator");
+        var deleter = await CreateAdminUserAsync("deleter");
+
+        var contentType = CreateInvariantContentType();
+        var createResult = await ContentEditingService.CreateAsync(
+            new ContentCreateModel
+            {
+                ContentTypeKey = contentType.Key,
+                ParentKey = Constants.System.RootKey,
+                Variants = [new VariantModel { Name = "Document To Trash" }],
+                Properties =
+                [
+                    new PropertyValueModel { Alias = "title", Value = "The title" },
+                    new PropertyValueModel { Alias = "text", Value = "The text" }
+                ],
+            },
+            creator.Key);
+        Assert.IsTrue(createResult.Success);
+        var content = createResult.Result.Content!;
+        Assert.AreEqual(creator.Id, content.CreatorId);
+        Assert.AreEqual(creator.Id, content.WriterId);
+
+        var trashAttempt = await ContentEditingService.MoveToRecycleBinAsync(content.Key, deleter.Key);
+        Assert.IsTrue(trashAttempt.Success);
+
+        var trashedContent = await ContentEditingService.GetAsync(content.Key);
+        Assert.IsNotNull(trashedContent);
+        Assert.IsTrue(trashedContent.Trashed);
+        Assert.AreEqual(creator.Id, trashedContent.CreatorId, "CreatorId must not change on trash.");
+        Assert.AreEqual(deleter.Id, trashedContent.WriterId, "WriterId must reflect the user who trashed the item.");
+    }
+
+    [Test]
+    public async Task Move_To_Recycle_Bin_Records_AuditType_Move_Against_The_Trashing_User()
+    {
+        var auditService = GetRequiredService<IAuditService>();
+        var deleter = await CreateAdminUserAsync("deleter");
+        var content = await CreateInvariantContent();
+
+        var trashAttempt = await ContentEditingService.MoveToRecycleBinAsync(content.Key, deleter.Key);
+        Assert.IsTrue(trashAttempt.Success);
+
+        var moveEntries = await auditService.GetItemsByEntityAsync(
+            content.Id,
+            skip: 0,
+            take: 100,
+            auditTypeFilter: [AuditType.Move]);
+
+        Assert.AreEqual(1, moveEntries.Items.Count(), "Expected one AuditType.Move entry for the trash operation.");
+        Assert.AreEqual(deleter.Id, moveEntries.Items.Single().UserId);
+    }
+
+    private async Task<IUser> CreateAdminUserAsync(string identifier)
+    {
+        var adminGroup = await UserGroupService.GetAsync(Constants.Security.AdminGroupAlias);
+        var createModel = new UserCreateModel
+        {
+            UserName = $"{identifier}@example.com",
+            Email = $"{identifier}@example.com",
+            Name = identifier,
+            UserGroupKeys = new HashSet<Guid> { adminGroup!.Key },
+        };
+
+        var result = await UserService.CreateAsync(Constants.Security.SuperUserKey, createModel);
+        Assert.IsTrue(result.Success);
+        return result.Result.CreatedUser!;
+    }
 }

tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MediaEditingServiceTests.MoveToRecycleBin.cs

@@ -3,52 +3,17 @@
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Configuration.Models;
 using Umbraco.Cms.Core.Models;
-using Umbraco.Cms.Core.Models.ContentEditing;
-using Umbraco.Cms.Core.Services;
 using Umbraco.Cms.Core.Services.OperationStatus;
-using Umbraco.Cms.Tests.Common.Testing;
 using Umbraco.Cms.Tests.Integration.Attributes;
-using Umbraco.Cms.Tests.Integration.Testing;
 
 namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services;
 
-[TestFixture]
-[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)]
-internal sealed class MediaEditingServiceMoveToRecycleBinTests : UmbracoIntegrationTest
+internal sealed partial class MediaEditingServiceTests
 {
-    private IMediaTypeService MediaTypeService => GetRequiredService<IMediaTypeService>();
-
-    private IMediaEditingService MediaEditingService => GetRequiredService<IMediaEditingService>();
-
-    private IRelationService RelationService => GetRequiredService<IRelationService>();
-
     public static void ConfigureDisableDeleteWhenReferencedTrue(IUmbracoBuilder builder)
         => builder.Services.Configure<ContentSettings>(config =>
             config.DisableDeleteWhenReferenced = true);
 
-    private async Task<IMedia> CreateFolderMediaAsync(string name)
-    {
-        var folderMediaType = MediaTypeService.Get(Constants.Conventions.MediaTypes.Folder);
-        var createModel = new MediaCreateModel
-        {
-            ContentTypeKey = folderMediaType!.Key,
-            ParentKey = Constants.System.RootKey,
-            Key = Guid.NewGuid(),
-            Variants = [new VariantModel { Name = name }],
-        };
-
-        var result = await MediaEditingService.CreateAsync(createModel, Constants.Security.SuperUserKey);
-        Assert.IsTrue(result.Success);
-        return result.Result.Content!;
-    }
-
-    private void Relate(IMedia parent, IMedia child)
-    {
-        var relationType = RelationService.GetRelationTypeByAlias(Constants.Conventions.RelationTypes.RelatedMediaAlias);
-        var relation = RelationService.Relate(parent.Id, child.Id, relationType);
-        RelationService.Save(relation);
-    }
-
     [Test]
     [ConfigureBuilder(ActionName = nameof(ConfigureDisableDeleteWhenReferencedTrue))]
     public async Task Cannot_Move_To_Recycle_Bin_When_Media_Is_Referenced_And_DisableDeleteWhenReferenced_Is_True()
@@ -67,4 +32,49 @@ public async Task Cannot_Move_To_Recycle_Bin_When_Media_Is_Referenced_And_Disabl
         Assert.IsNotNull(media);
         Assert.IsFalse(media.Trashed);
     }
+
+    // Regression test for https://github.com/umbraco/Umbraco-CMS/issues/22661.
+    // The "Media deleted" entry in the History panel is written by RelateOnTrashNotificationHandler,
+    // which falls back to the entity's WriterId when no back-office user can be resolved. Before the
+    // fix, MediaService.PerformMoveMediaLocked never updated WriterId on a trash, so the fallback
+    // attributed the deletion to the original creator. The fix updates WriterId to the acting user,
+    // mirroring ContentService.PerformMoveContentLocked.
+    [Test]
+    public async Task Move_To_Recycle_Bin_Updates_WriterId_To_The_Trashing_User()
+    {
+        var creator = await CreateAdminUserAsync("creator");
+        var deleter = await CreateAdminUserAsync("deleter");
+
+        var media = await CreateFolderMediaAsync("Media To Trash", creator.Key);
+        Assert.AreEqual(creator.Id, media.CreatorId);
+        Assert.AreEqual(creator.Id, media.WriterId);
+
+        var trashAttempt = await MediaEditingService.MoveToRecycleBinAsync(media.Key, deleter.Key);
+        Assert.IsTrue(trashAttempt.Success);
+
+        var trashedMedia = await MediaEditingService.GetAsync(media.Key);
+        Assert.IsNotNull(trashedMedia);
+        Assert.IsTrue(trashedMedia.Trashed);
+        Assert.AreEqual(creator.Id, trashedMedia.CreatorId, "CreatorId must not change on trash.");
+        Assert.AreEqual(deleter.Id, trashedMedia.WriterId, "WriterId must reflect the user who trashed the item.");
+    }
+
+    [Test]
+    public async Task Move_To_Recycle_Bin_Records_AuditType_Move_Against_The_Trashing_User()
+    {
+        var deleter = await CreateAdminUserAsync("deleter");
+        var media = await CreateFolderMediaAsync("Media To Trash");
+
+        var trashAttempt = await MediaEditingService.MoveToRecycleBinAsync(media.Key, deleter.Key);
+        Assert.IsTrue(trashAttempt.Success);
+
+        var moveEntries = await AuditService.GetItemsByEntityAsync(
+            media.Id,
+            skip: 0,
+            take: 100,
+            auditTypeFilter: [AuditType.Move]);
+
+        Assert.AreEqual(1, moveEntries.Items.Count(), "Expected one AuditType.Move entry for the trash operation.");
+        Assert.AreEqual(deleter.Id, moveEntries.Items.Single().UserId);
+    }
 }

tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MediaEditingServiceTests.cs

@@ -0,0 +1,69 @@
+using NUnit.Framework;
+using Umbraco.Cms.Core;
+using Umbraco.Cms.Core.Models;
+using Umbraco.Cms.Core.Models.ContentEditing;
+using Umbraco.Cms.Core.Models.Membership;
+using Umbraco.Cms.Core.Services;
+using Umbraco.Cms.Tests.Common.Testing;
+using Umbraco.Cms.Tests.Integration.Testing;
+
+namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services;
+
+[TestFixture]
+[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)]
+internal sealed partial class MediaEditingServiceTests : UmbracoIntegrationTest
+{
+    private IMediaTypeService MediaTypeService => GetRequiredService<IMediaTypeService>();
+
+    private IMediaEditingService MediaEditingService => GetRequiredService<IMediaEditingService>();
+
+    private IRelationService RelationService => GetRequiredService<IRelationService>();
+
+    private IUserService UserService => GetRequiredService<IUserService>();
+
+    private IUserGroupService UserGroupService => GetRequiredService<IUserGroupService>();
+
+    private IAuditService AuditService => GetRequiredService<IAuditService>();
+
+    private Task<IMedia> CreateFolderMediaAsync(string name, Guid? parentKey = null)
+        => CreateFolderMediaAsync(name, Constants.Security.SuperUserKey, parentKey);
+
+    private async Task<IMedia> CreateFolderMediaAsync(string name, Guid userKey, Guid? parentKey = null)
+    {
+        var folderMediaType = MediaTypeService.Get(Constants.Conventions.MediaTypes.Folder);
+        var createModel = new MediaCreateModel
+        {
+            ContentTypeKey = folderMediaType!.Key,
+            ParentKey = parentKey ?? Constants.System.RootKey,
+            Key = Guid.NewGuid(),
+            Variants = [new VariantModel { Name = name }],
+        };
+
+        var result = await MediaEditingService.CreateAsync(createModel, userKey);
+        Assert.IsTrue(result.Success);
+        return result.Result.Content!;
+    }
+
+    private async Task<IUser> CreateAdminUserAsync(string identifier)
+    {
+        var adminGroup = await UserGroupService.GetAsync(Constants.Security.AdminGroupAlias);
+        var createModel = new UserCreateModel
+        {
+            UserName = $"{identifier}@example.com",
+            Email = $"{identifier}@example.com",
+            Name = identifier,
+            UserGroupKeys = new HashSet<Guid> { adminGroup!.Key },
+        };
+
+        var result = await UserService.CreateAsync(Constants.Security.SuperUserKey, createModel);
+        Assert.IsTrue(result.Success);
+        return result.Result.CreatedUser!;
+    }
+
+    private void Relate(IMedia parent, IMedia child)
+    {
+        var relationType = RelationService.GetRelationTypeByAlias(Constants.Conventions.RelationTypes.RelatedMediaAlias);
+        var relation = RelationService.Relate(parent.Id, child.Id, relationType);
+        RelationService.Save(relation);
+    }
+}

tests/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj

@@ -100,6 +100,9 @@
     <Compile Update="Umbraco.Infrastructure\Services\ContentEditingServiceTests.Validate.cs">
       <DependentUpon>ContentEditingServiceTests.cs</DependentUpon>
     </Compile>
+    <Compile Update="Umbraco.Infrastructure\Services\MediaEditingServiceTests.MoveToRecycleBin.cs">
+      <DependentUpon>MediaEditingServiceTests.cs</DependentUpon>
+    </Compile>
     <Compile Update="Umbraco.Infrastructure\Services\ContentPublishingServiceTests.Publish.cs">
       <DependentUpon>ContentPublishingServiceTests.cs</DependentUpon>
     </Compile>