Apply review suggestions

Emby.Server.Implementations/Playlists/PlaylistManager.cs

@@ -22,6 +22,7 @@ using MediaBrowser.Controller.Providers;
 using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Model.Playlists;
+using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.Logging;
 using PlaylistsNET.Content;
@@ -59,7 +60,7 @@ namespace Emby.Server.Implementations.Playlists
             _appConfig = appConfig;
         }
 
-        public Playlist GetPlaylist(Guid userId, Guid playlistId)
+        public Playlist GetPlaylistForUser(Guid playlistId, Guid userId)
         {
             return GetPlaylists(userId).Where(p => p.Id.Equals(playlistId)).FirstOrDefault();
         }
@@ -178,7 +179,7 @@ namespace Emby.Server.Implementations.Playlists
             return Playlist.GetPlaylistItems(playlistMediaType, items, user, options);
         }
 
-        public Task AddToPlaylistAsync(Guid playlistId, IReadOnlyCollection<Guid> itemIds, Guid userId)
+        public Task AddItemToPlaylistAsync(Guid playlistId, IReadOnlyCollection<Guid> itemIds, Guid userId)
         {
             var user = userId.IsEmpty() ? null : _userManager.GetUserById(userId);
 
@@ -245,7 +246,7 @@ namespace Emby.Server.Implementations.Playlists
                 RefreshPriority.High);
         }
 
-        public async Task RemoveFromPlaylistAsync(string playlistId, IEnumerable<string> entryIds)
+        public async Task RemoveItemFromPlaylistAsync(string playlistId, IEnumerable<string> entryIds)
         {
             if (_libraryManager.GetItemById(playlistId) is not Playlist playlist)
             {
@@ -550,7 +551,7 @@ namespace Emby.Server.Implementations.Playlists
 
         public async Task UpdatePlaylist(PlaylistUpdateRequest request)
         {
-            var playlist = GetPlaylist(request.UserId, request.Id);
+            var playlist = GetPlaylistForUser(request.Id, request.UserId);
 
             if (request.Ids is not null)
             {
@@ -563,7 +564,7 @@ namespace Emby.Server.Implementations.Playlists
                         EnableImages = true
                     }).ConfigureAwait(false);
 
-                playlist = GetPlaylist(request.UserId, request.Id);
+                playlist = GetPlaylistForUser(request.Id, request.UserId);
             }
 
             if (request.Name is not null)
@@ -584,24 +585,25 @@ namespace Emby.Server.Implementations.Playlists
             await UpdatePlaylistInternal(playlist).ConfigureAwait(false);
         }
 
-        public async Task AddToShares(Guid playlistId, Guid userId, PlaylistUserPermissions share)
+        public async Task AddUserToShares(PlaylistUserUpdateRequest request)
         {
-            var playlist = GetPlaylist(userId, playlistId);
+            var userId = request.UserId;
+            var playlist = GetPlaylistForUser(request.Id, userId);
             var shares = playlist.Shares.ToList();
-            var existingUserShare = shares.FirstOrDefault(s => s.UserId.Equals(share.UserId));
+            var existingUserShare = shares.FirstOrDefault(s => s.UserId.Equals(userId));
             if (existingUserShare is not null)
             {
                 shares.Remove(existingUserShare);
             }
 
-            shares.Add(share);
+            shares.Add(new PlaylistUserPermissions(userId, request.CanEdit ?? false));
             playlist.Shares = shares;
             await UpdatePlaylistInternal(playlist).ConfigureAwait(false);
         }
 
-        public async Task RemoveFromShares(Guid playlistId, Guid userId, PlaylistUserPermissions share)
+        public async Task RemoveUserFromShares(Guid playlistId, Guid userId, PlaylistUserPermissions share)
         {
-            var playlist = GetPlaylist(userId, playlistId);
+            var playlist = GetPlaylistForUser(playlistId, userId);
             var shares = playlist.Shares.ToList();
             shares.Remove(share);
             playlist.Shares = shares;

Jellyfin.Api/Controllers/PlaylistsController.cs

@@ -121,7 +121,7 @@ public class PlaylistsController : BaseJellyfinApiController
     {
         var callingUserId = User.GetUserId();
 
-        var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId);
+        var playlist = _playlistManager.GetPlaylistForUser(playlistId, callingUserId);
         if (playlist is null)
         {
             return NotFound("Playlist not found");
@@ -167,7 +167,7 @@ public class PlaylistsController : BaseJellyfinApiController
     {
         var userId = User.GetUserId();
 
-        var playlist = _playlistManager.GetPlaylist(userId, playlistId);
+        var playlist = _playlistManager.GetPlaylistForUser(playlistId, userId);
         if (playlist is null)
         {
             return NotFound("Playlist not found");
@@ -184,7 +184,7 @@ public class PlaylistsController : BaseJellyfinApiController
     /// </summary>
     /// <param name="playlistId">The playlist id.</param>
     /// <param name="userId">The user id.</param>
-    /// <param name="canEdit">Edit permission.</param>
+    /// <param name="updatePlaylistUserRequest">The <see cref="UpdatePlaylistUserDto"/>.</param>
     /// <response code="204">User's permissions modified.</response>
     /// <response code="401">Unauthorized access.</response>
     /// <response code="404">Playlist not found.</response>
@@ -195,14 +195,14 @@ public class PlaylistsController : BaseJellyfinApiController
     [HttpPost("{playlistId}/User/{userId}")]
     [ProducesResponseType(StatusCodes.Status204NoContent)]
     [ProducesResponseType(StatusCodes.Status401Unauthorized)]
-    public async Task<ActionResult> ModifyPlaylistUserPermissions(
+    public async Task<ActionResult> UpdatePlaylistUser(
         [FromRoute, Required] Guid playlistId,
         [FromRoute, Required] Guid userId,
-        [FromBody] bool canEdit)
+        [FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Allow)] UpdatePlaylistUserDto updatePlaylistUserRequest)
     {
         var callingUserId = User.GetUserId();
 
-        var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId);
+        var playlist = _playlistManager.GetPlaylistForUser(playlistId, callingUserId);
         if (playlist is null)
         {
             return NotFound("Playlist not found");
@@ -216,7 +216,12 @@ public class PlaylistsController : BaseJellyfinApiController
             return Unauthorized("Unauthorized access");
         }
 
-        await _playlistManager.AddToShares(playlistId, callingUserId, new PlaylistUserPermissions(userId, canEdit)).ConfigureAwait(false);
+        await _playlistManager.AddUserToShares(new PlaylistUserUpdateRequest
+        {
+            Id = playlistId,
+            UserId = userId,
+            CanEdit = updatePlaylistUserRequest.CanEdit
+        }).ConfigureAwait(false);
 
         return NoContent();
     }
@@ -243,7 +248,7 @@ public class PlaylistsController : BaseJellyfinApiController
     {
         var callingUserId = User.GetUserId();
 
-        var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId);
+        var playlist = _playlistManager.GetPlaylistForUser(playlistId, callingUserId);
         if (playlist is null)
         {
             return NotFound("Playlist not found");
@@ -263,7 +268,7 @@ public class PlaylistsController : BaseJellyfinApiController
             return NotFound("User permissions not found");
         }
 
-        await _playlistManager.RemoveFromShares(playlistId, callingUserId, share).ConfigureAwait(false);
+        await _playlistManager.RemoveUserFromShares(playlistId, callingUserId, share).ConfigureAwait(false);
 
         return NoContent();
     }
@@ -278,13 +283,13 @@ public class PlaylistsController : BaseJellyfinApiController
     /// <returns>An <see cref="NoContentResult"/> on success.</returns>
     [HttpPost("{playlistId}/Items")]
     [ProducesResponseType(StatusCodes.Status204NoContent)]
-    public async Task<ActionResult> AddToPlaylist(
+    public async Task<ActionResult> AddItemToPlaylist(
         [FromRoute, Required] Guid playlistId,
         [FromQuery, ModelBinder(typeof(CommaDelimitedArrayModelBinder))] Guid[] ids,
         [FromQuery] Guid? userId)
     {
         userId = RequestHelpers.GetUserId(User, userId);
-        await _playlistManager.AddToPlaylistAsync(playlistId, ids, userId.Value).ConfigureAwait(false);
+        await _playlistManager.AddItemToPlaylistAsync(playlistId, ids, userId.Value).ConfigureAwait(false);
         return NoContent();
     }
 
@@ -316,11 +321,11 @@ public class PlaylistsController : BaseJellyfinApiController
     /// <returns>An <see cref="NoContentResult"/> on success.</returns>
     [HttpDelete("{playlistId}/Items")]
     [ProducesResponseType(StatusCodes.Status204NoContent)]
-    public async Task<ActionResult> RemoveFromPlaylist(
+    public async Task<ActionResult> RemoveItemFromPlaylist(
         [FromRoute, Required] string playlistId,
         [FromQuery, ModelBinder(typeof(CommaDelimitedArrayModelBinder))] string[] entryIds)
     {
-        await _playlistManager.RemoveFromPlaylistAsync(playlistId, entryIds).ConfigureAwait(false);
+        await _playlistManager.RemoveItemFromPlaylistAsync(playlistId, entryIds).ConfigureAwait(false);
         return NoContent();
     }
 

Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistDto.cs

@@ -7,7 +7,7 @@ using MediaBrowser.Model.Entities;
 namespace Jellyfin.Api.Models.PlaylistDtos;
 
 /// <summary>
-/// Updateexisting playlist dto.
+/// Update existing playlist dto. Fields set to `null` will not be updated and keep their current values.
 /// </summary>
 public class UpdatePlaylistDto
 {

Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistUserDto.cs

@@ -0,0 +1,12 @@
+namespace Jellyfin.Api.Models.PlaylistDtos;
+
+/// <summary>
+/// Update existing playlist user dto. Fields set to `null` will not be updated and keep their current values.
+/// </summary>
+public class UpdatePlaylistUserDto
+{
+    /// <summary>
+    /// Gets or sets a value indicating whether the user can edit the playlist.
+    /// </summary>
+    public bool? CanEdit { get; set; }
+}

MediaBrowser.Controller/Playlists/IPlaylistManager.cs

@@ -14,10 +14,10 @@ namespace MediaBrowser.Controller.Playlists
         /// <summary>
         /// Gets the playlist.
         /// </summary>
-        /// <param name="userId">The user identifier.</param>
         /// <param name="playlistId">The playlist identifier.</param>
+        /// <param name="userId">The user identifier.</param>
         /// <returns>Playlist.</returns>
-        Playlist GetPlaylist(Guid userId, Guid playlistId);
+        Playlist GetPlaylistForUser(Guid playlistId, Guid userId);
 
         /// <summary>
         /// Creates the playlist.
@@ -43,20 +43,18 @@ namespace MediaBrowser.Controller.Playlists
         /// <summary>
         /// Adds a share to the playlist.
         /// </summary>
-        /// <param name="playlistId">The playlist identifier.</param>
-        /// <param name="userId">The user identifier.</param>
-        /// <param name="share">The share.</param>
+        /// <param name="request">The <see cref="PlaylistUserUpdateRequest"/>.</param>
         /// <returns>Task.</returns>
-        Task AddToShares(Guid playlistId, Guid userId, PlaylistUserPermissions share);
+        Task AddUserToShares(PlaylistUserUpdateRequest request);
 
         /// <summary>
-        /// Rremoves a share from the playlist.
+        /// Removes a share from the playlist.
         /// </summary>
         /// <param name="playlistId">The playlist identifier.</param>
         /// <param name="userId">The user identifier.</param>
         /// <param name="share">The share.</param>
         /// <returns>Task.</returns>
-        Task RemoveFromShares(Guid playlistId, Guid userId, PlaylistUserPermissions share);
+        Task RemoveUserFromShares(Guid playlistId, Guid userId, PlaylistUserPermissions share);
 
         /// <summary>
         /// Adds to playlist.
@@ -65,7 +63,7 @@ namespace MediaBrowser.Controller.Playlists
         /// <param name="itemIds">The item ids.</param>
         /// <param name="userId">The user identifier.</param>
         /// <returns>Task.</returns>
-        Task AddToPlaylistAsync(Guid playlistId, IReadOnlyCollection<Guid> itemIds, Guid userId);
+        Task AddItemToPlaylistAsync(Guid playlistId, IReadOnlyCollection<Guid> itemIds, Guid userId);
 
         /// <summary>
         /// Removes from playlist.
@@ -73,7 +71,7 @@ namespace MediaBrowser.Controller.Playlists
         /// <param name="playlistId">The playlist identifier.</param>
         /// <param name="entryIds">The entry ids.</param>
         /// <returns>Task.</returns>
-        Task RemoveFromPlaylistAsync(string playlistId, IEnumerable<string> entryIds);
+        Task RemoveItemFromPlaylistAsync(string playlistId, IEnumerable<string> entryIds);
 
         /// <summary>
         /// Gets the playlists folder.

MediaBrowser.Model/Playlists/PlaylistUpdateRequest.cs

@@ -5,7 +5,7 @@ using MediaBrowser.Model.Entities;
 namespace MediaBrowser.Model.Playlists;
 
 /// <summary>
-/// A playlist creation request.
+/// A playlist update request.
 /// </summary>
 public class PlaylistUpdateRequest
 {

MediaBrowser.Model/Playlists/PlaylistUserUpdateRequest.cs

@@ -0,0 +1,24 @@
+using System;
+
+namespace MediaBrowser.Model.Playlists;
+
+/// <summary>
+/// A playlist user update request.
+/// </summary>
+public class PlaylistUserUpdateRequest
+{
+    /// <summary>
+    /// Gets or sets the id of the playlist.
+    /// </summary>
+    public Guid Id { get; set; }
+
+    /// <summary>
+    /// Gets or sets the id of the updated user.
+    /// </summary>
+    public Guid UserId { get; set; }
+
+    /// <summary>
+    /// Gets or sets a value indicating whether the user can edit the playlist.
+    /// </summary>
+    public bool? CanEdit { get; set; }
+}