From 8cf77424f6c432386fb06017eccfa0e9f880a3ba Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Tue, 2 Apr 2024 08:08:36 +0200 Subject: [PATCH] Apply review suggestions --- .../Playlists/PlaylistManager.cs | 24 +++++++------- .../Controllers/PlaylistsController.cs | 31 +++++++++++-------- .../Models/PlaylistDtos/UpdatePlaylistDto.cs | 2 +- .../PlaylistDtos/UpdatePlaylistUserDto.cs | 12 +++++++ .../Playlists/IPlaylistManager.cs | 18 +++++------ .../Playlists/PlaylistUpdateRequest.cs | 2 +- .../Playlists/PlaylistUserUpdateRequest.cs | 24 ++++++++++++++ 7 files changed, 77 insertions(+), 36 deletions(-) create mode 100644 Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistUserDto.cs create mode 100644 MediaBrowser.Model/Playlists/PlaylistUserUpdateRequest.cs diff --git a/Emby.Server.Implementations/Playlists/PlaylistManager.cs b/Emby.Server.Implementations/Playlists/PlaylistManager.cs index 517ec68dc..7a6cf9eff 100644 --- a/Emby.Server.Implementations/Playlists/PlaylistManager.cs +++ b/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 itemIds, Guid userId) + public Task AddItemToPlaylistAsync(Guid playlistId, IReadOnlyCollection 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 entryIds) + public async Task RemoveItemFromPlaylistAsync(string playlistId, IEnumerable 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; diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index de4c542d9..12186e02e 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/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 /// /// The playlist id. /// The user id. - /// Edit permission. + /// The . /// User's permissions modified. /// Unauthorized access. /// Playlist not found. @@ -195,14 +195,14 @@ public class PlaylistsController : BaseJellyfinApiController [HttpPost("{playlistId}/User/{userId}")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status401Unauthorized)] - public async Task ModifyPlaylistUserPermissions( + public async Task 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 /// An on success. [HttpPost("{playlistId}/Items")] [ProducesResponseType(StatusCodes.Status204NoContent)] - public async Task AddToPlaylist( + public async Task 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 /// An on success. [HttpDelete("{playlistId}/Items")] [ProducesResponseType(StatusCodes.Status204NoContent)] - public async Task RemoveFromPlaylist( + public async Task 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(); } diff --git a/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistDto.cs b/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistDto.cs index 93e544eed..0e109db3e 100644 --- a/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistDto.cs +++ b/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistDto.cs @@ -7,7 +7,7 @@ using MediaBrowser.Model.Entities; namespace Jellyfin.Api.Models.PlaylistDtos; /// -/// Updateexisting playlist dto. +/// Update existing playlist dto. Fields set to `null` will not be updated and keep their current values. /// public class UpdatePlaylistDto { diff --git a/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistUserDto.cs b/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistUserDto.cs new file mode 100644 index 000000000..60467b5e7 --- /dev/null +++ b/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistUserDto.cs @@ -0,0 +1,12 @@ +namespace Jellyfin.Api.Models.PlaylistDtos; + +/// +/// Update existing playlist user dto. Fields set to `null` will not be updated and keep their current values. +/// +public class UpdatePlaylistUserDto +{ + /// + /// Gets or sets a value indicating whether the user can edit the playlist. + /// + public bool? CanEdit { get; set; } +} diff --git a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs index 464620427..821b901a0 100644 --- a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs +++ b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs @@ -14,10 +14,10 @@ namespace MediaBrowser.Controller.Playlists /// /// Gets the playlist. /// - /// The user identifier. /// The playlist identifier. + /// The user identifier. /// Playlist. - Playlist GetPlaylist(Guid userId, Guid playlistId); + Playlist GetPlaylistForUser(Guid playlistId, Guid userId); /// /// Creates the playlist. @@ -43,20 +43,18 @@ namespace MediaBrowser.Controller.Playlists /// /// Adds a share to the playlist. /// - /// The playlist identifier. - /// The user identifier. - /// The share. + /// The . /// Task. - Task AddToShares(Guid playlistId, Guid userId, PlaylistUserPermissions share); + Task AddUserToShares(PlaylistUserUpdateRequest request); /// - /// Rremoves a share from the playlist. + /// Removes a share from the playlist. /// /// The playlist identifier. /// The user identifier. /// The share. /// Task. - Task RemoveFromShares(Guid playlistId, Guid userId, PlaylistUserPermissions share); + Task RemoveUserFromShares(Guid playlistId, Guid userId, PlaylistUserPermissions share); /// /// Adds to playlist. @@ -65,7 +63,7 @@ namespace MediaBrowser.Controller.Playlists /// The item ids. /// The user identifier. /// Task. - Task AddToPlaylistAsync(Guid playlistId, IReadOnlyCollection itemIds, Guid userId); + Task AddItemToPlaylistAsync(Guid playlistId, IReadOnlyCollection itemIds, Guid userId); /// /// Removes from playlist. @@ -73,7 +71,7 @@ namespace MediaBrowser.Controller.Playlists /// The playlist identifier. /// The entry ids. /// Task. - Task RemoveFromPlaylistAsync(string playlistId, IEnumerable entryIds); + Task RemoveItemFromPlaylistAsync(string playlistId, IEnumerable entryIds); /// /// Gets the playlists folder. diff --git a/MediaBrowser.Model/Playlists/PlaylistUpdateRequest.cs b/MediaBrowser.Model/Playlists/PlaylistUpdateRequest.cs index f574e679c..db290bbdb 100644 --- a/MediaBrowser.Model/Playlists/PlaylistUpdateRequest.cs +++ b/MediaBrowser.Model/Playlists/PlaylistUpdateRequest.cs @@ -5,7 +5,7 @@ using MediaBrowser.Model.Entities; namespace MediaBrowser.Model.Playlists; /// -/// A playlist creation request. +/// A playlist update request. /// public class PlaylistUpdateRequest { diff --git a/MediaBrowser.Model/Playlists/PlaylistUserUpdateRequest.cs b/MediaBrowser.Model/Playlists/PlaylistUserUpdateRequest.cs new file mode 100644 index 000000000..1840efdf3 --- /dev/null +++ b/MediaBrowser.Model/Playlists/PlaylistUserUpdateRequest.cs @@ -0,0 +1,24 @@ +using System; + +namespace MediaBrowser.Model.Playlists; + +/// +/// A playlist user update request. +/// +public class PlaylistUserUpdateRequest +{ + /// + /// Gets or sets the id of the playlist. + /// + public Guid Id { get; set; } + + /// + /// Gets or sets the id of the updated user. + /// + public Guid UserId { get; set; } + + /// + /// Gets or sets a value indicating whether the user can edit the playlist. + /// + public bool? CanEdit { get; set; } +}