Media: Record the trashing user against the History audit entry (closes #22661)#22668
Merged
Merged
Conversation
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes incorrect attribution in Media history/audit by ensuring media items’ WriterId is updated to the acting user when moved/trashed, aligning media behavior with documents and enabling correct fallback attribution in RelateOnTrashNotificationHandler.
Changes:
- Update
MediaServicemove/trash save path to setmedia.WriterId = userIdduring move operations (including trashing). - Refactor/organize media editing integration tests into a partial-class layout and add regression coverage for
WriterId+ existingAuditType.Moveattribution. - Add companion content tests to guard existing document behavior from regressions.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/Umbraco.Core/Services/MediaService.cs |
Pass userId through move-save helper and update WriterId before saving so audit/history attribution reflects the acting user. |
tests/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj |
Nests the media recycle-bin test file under the new MediaEditingServiceTests.cs via <DependentUpon>. |
tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MediaEditingServiceTests.cs |
Introduces fixture/shared helpers for media editing tests (partial-class layout). |
tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MediaEditingServiceTests.MoveToRecycleBin.cs |
Adds regression tests for WriterId update on trash and validates AuditType.Move is attributed to the deleter. |
tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentEditingServiceTests.MoveToRecycleBin.cs |
Adds companion tests ensuring content trash keeps CreatorId stable, updates WriterId, and records AuditType.Move against the acting user. |
kjac
approved these changes
May 3, 2026
kjac
left a comment
Contributor
There was a problem hiding this comment.
Works like a charm.
Good job on refactoring the MediaEditingServiceTests too 👏
This was referenced Jun 26, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
When user B trashes a media item that user A originally created/uploaded, the History panel for that item shows user A against the Media deleted entry instead of user B. Documents behave correctly (they show user B). The bug is reported on v13 in #22661 but is still present in the latest code.
Root cause
The Media deleted entry in the History panel is written by
RelateOnTrashNotificationHandler.HandleAsynconMediaMovedToRecycleBinNotification:The handler uses the back-office user when available, otherwise falls back to
item.Entity.WriterId, and this is what is being logged.For documents,
ContentService.PerformMoveContentLockedalready updatesWriterId = userIdbefore saving, so the fallback resolves to user B (the deleting user). For media,MediaService.PerformMoveMediaLockedsaved the entity without ever updatingWriterId, so the fallback resolved to user A (the original creator).Fix
src/Umbraco.Core/Services/MediaService.cs: passuserIdthrough toPerformMoveMediaLockedand setmedia.WriterId = userIdbefore saving — mirroringContentService.PerformMoveContentLockedand aligning the behaviour with documents.Testing
Automated
An integration tests has been added verifying that the writer is set correctly for media, and, for completeness, a similar one added for documents.
Manual
To verify manually: