diff --git a/src/Umbraco.Core/Services/MediaService.cs b/src/Umbraco.Core/Services/MediaService.cs index 0301c6c3ae14..0a48e99b476e 100644 --- a/src/Umbraco.Core/Services/MediaService.cs +++ b/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. /// /// The media item to save. + /// The identifier of the user performing the move. /// /// If true, marks the item as trashed; if false, marks the item as not trashed; /// if null, leaves the trashed status unchanged. /// - 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); } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.MoveToRecycleBin.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.MoveToRecycleBin.cs index 10471b7ea892..67f74879342b 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.MoveToRecycleBin.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.MoveToRecycleBin.cs @@ -2,6 +2,10 @@ 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; @@ -9,7 +13,7 @@ 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(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(); + 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 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 { adminGroup!.Key }, + }; + + var result = await UserService.CreateAsync(Constants.Security.SuperUserKey, createModel); + Assert.IsTrue(result.Success); + return result.Result.CreatedUser!; + } } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MediaEditingServiceTests.MoveToRecycleBin.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MediaEditingServiceTests.MoveToRecycleBin.cs index 2da8b61dd712..46d4ba0195ae 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MediaEditingServiceTests.MoveToRecycleBin.cs +++ b/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(); - - private IMediaEditingService MediaEditingService => GetRequiredService(); - - private IRelationService RelationService => GetRequiredService(); - public static void ConfigureDisableDeleteWhenReferencedTrue(IUmbracoBuilder builder) => builder.Services.Configure(config => config.DisableDeleteWhenReferenced = true); - private async Task 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); + } } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MediaEditingServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MediaEditingServiceTests.cs new file mode 100644 index 000000000000..79698d8f6b6c --- /dev/null +++ b/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(); + + private IMediaEditingService MediaEditingService => GetRequiredService(); + + private IRelationService RelationService => GetRequiredService(); + + private IUserService UserService => GetRequiredService(); + + private IUserGroupService UserGroupService => GetRequiredService(); + + private IAuditService AuditService => GetRequiredService(); + + private Task CreateFolderMediaAsync(string name, Guid? parentKey = null) + => CreateFolderMediaAsync(name, Constants.Security.SuperUserKey, parentKey); + + private async Task 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 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 { 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); + } +} diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj b/tests/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj index ecc972ce09e8..34867fd9f491 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj +++ b/tests/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj @@ -100,6 +100,9 @@ ContentEditingServiceTests.cs + + MediaEditingServiceTests.cs + ContentPublishingServiceTests.cs