diff --git a/ErsatzTV.Application/MediaCollections/Commands/RenamePlaylistGroupHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/RenamePlaylistGroupHandler.cs index f58686b15..a3b914015 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/RenamePlaylistGroupHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/RenamePlaylistGroupHandler.cs @@ -34,6 +34,7 @@ public class RenamePlaylistGroupHandler(IDbContextFactory dbContextFa RenamePlaylistGroup request, CancellationToken cancellationToken) => PlaylistGroupMustExist(dbContext, request, cancellationToken) + .BindT(PlaylistGroupMustNotBeSystem) .BindT(playlistGroup => ValidateName(request).Map(_ => playlistGroup)); private static Task> PlaylistGroupMustExist( @@ -46,6 +47,13 @@ public class RenamePlaylistGroupHandler(IDbContextFactory dbContextFa .Map(o => o.ToValidation( new NotFoundError($"PlaylistGroup {request.PlaylistGroupId} does not exist."))); + // Plain BaseError (NOT NotFoundError) so it maps to 422, mirroring DeletePlaylistGroupHandler's + // system-group guard. A missing group still surfaces as NotFoundError (404) from PlaylistGroupMustExist. + private static Validation PlaylistGroupMustNotBeSystem(PlaylistGroup playlistGroup) => + playlistGroup.IsSystem + ? BaseError.New("Cannot rename system playlist group") + : playlistGroup; + private static Validation ValidateName(RenamePlaylistGroup request) => request.NotEmpty(x => x.Name) .Bind(_ => request.NotLongerThan(50)(x => x.Name)); diff --git a/ErsatzTV.Tests/Application/MediaCollections/PlaylistGroupHandlerTests.cs b/ErsatzTV.Tests/Application/MediaCollections/PlaylistGroupHandlerTests.cs new file mode 100644 index 000000000..d32acf53a --- /dev/null +++ b/ErsatzTV.Tests/Application/MediaCollections/PlaylistGroupHandlerTests.cs @@ -0,0 +1,75 @@ +using ErsatzTV.Application.MediaCollections; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Errors; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using LanguageExt; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Application.MediaCollections; + +[TestFixture] +public class PlaylistGroupHandlerTests : MediaCollectionHandlerTestBase +{ + [Test] + public async Task Rename_Should_Return_Error_When_Group_Missing() + { + var handler = new RenamePlaylistGroupHandler(Db.Factory); + + Either result = + await handler.Handle(new RenamePlaylistGroup(999, "Updated"), CancellationToken.None); + + // The handler rejects a missing group; the API surfaces 404 for this case via the + // controller's existence pre-check (LanguageExtensions.Apply collapses the handler's + // NotFoundError to a plain BaseError, so the type is not asserted here). + result.IsLeft.ShouldBeTrue(); + } + + [Test] + public async Task Rename_Should_Return_BaseError_When_Group_Is_System() + { + await SeedPlaylistGroup(5, "System", isSystem: true); + + var handler = new RenamePlaylistGroupHandler(Db.Factory); + + Either result = + await handler.Handle(new RenamePlaylistGroup(5, "Renamed"), CancellationToken.None); + + // Plain BaseError (not NotFoundError) so ApiResults maps it to 422, not 404. + BaseError error = LeftOf(result); + error.ShouldNotBeOfType(); + error.Value.ShouldBe("Cannot rename system playlist group"); + } + + [Test] + public async Task Rename_Should_Succeed_For_Non_System_Group() + { + await SeedPlaylistGroup(6, "Kids", isSystem: false); + + var handler = new RenamePlaylistGroupHandler(Db.Factory); + + Either result = + await handler.Handle(new RenamePlaylistGroup(6, "Family"), CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + result.Match(Left: _ => "", Right: vm => vm.Name).ShouldBe("Family"); + } + + private async Task SeedPlaylistGroup(int id, string name, bool isSystem) + { + await using TvContext context = Db.CreateContext(); + context.PlaylistGroups.Add(new PlaylistGroup + { + Id = id, + Name = name, + IsSystem = isSystem, + Playlists = [] + }); + await context.SaveChangesAsync(); + } + + private static BaseError LeftOf(Either either) => + either.Match(Left: e => e, Right: _ => throw new AssertionException("Expected a Left result")); +} diff --git a/ErsatzTV.Tests/Controllers/PlaylistControllerTests.cs b/ErsatzTV.Tests/Controllers/PlaylistControllerTests.cs index 4a854b249..d75763304 100644 --- a/ErsatzTV.Tests/Controllers/PlaylistControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/PlaylistControllerTests.cs @@ -7,7 +7,6 @@ using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Core.Api.MediaCollections; using ErsatzTV.Core.Domain; -using ErsatzTV.Core.Errors; using LanguageExt; using MediatR; using Microsoft.AspNetCore.Mvc; @@ -95,6 +94,8 @@ public class PlaylistControllerTests [Test] public async Task UpdateGroup_Should_Return_200_And_Map_Request() { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List { new(4, "Kids", 1, false) }); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Right(new PlaylistGroupViewModel(4, "New Name", 1, false))); @@ -111,10 +112,10 @@ public class PlaylistControllerTests } [Test] - public async Task UpdateGroup_Should_Return_404_On_NotFoundError() + public async Task UpdateGroup_Should_Return_404_When_Missing() { - _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Left(new NotFoundError("missing"))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); IActionResult result = await _controller.UpdateGroup( 4, @@ -122,11 +123,29 @@ public class PlaylistControllerTests CancellationToken.None); result.ShouldBeOfType(); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task UpdateGroup_Should_Return_422_On_System_Group_And_Not_Rename() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List { new(4, "System", 0, true) }); + + IActionResult result = await _controller.UpdateGroup( + 4, + new UpdatePlaylistGroupRequest("New Name"), + CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); } [Test] public async Task UpdateGroup_Should_Return_422_On_Validation_Error() { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List { new(4, "Kids", 1, false) }); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Left(BaseError.New("too long"))); @@ -368,6 +387,23 @@ public class PlaylistControllerTests result.ShouldBeOfType(); } + [Test] + public async Task Update_Should_Return_422_On_System_Playlist_And_Not_Replace_Items() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(new PlaylistViewModel(4, 1, "System", true))); + + IActionResult result = await _controller.Update( + 4, + new ReplacePlaylistRequest( + "Renamed", + new List()), + CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + [Test] public async Task Delete_Should_Return_204_On_Success() { @@ -452,6 +488,24 @@ public class PlaylistControllerTests result.ShouldBeOfType(); } + [Test] + public async Task Preview_Should_Return_422_When_Item_Missing_Required_Id() + { + IActionResult result = await _controller.Preview( + new ReplacePlaylistRequest( + "Draft", + new List + { + // Collection-type item with a null collectionId is invalid. + new(0, CollectionType.Collection, null, null, null, null, PlaybackOrder.Chronological, null, true, + false) + }), + CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + private static void ShouldHaveActionRoute(string actionName, string httpMethod, string route) { MethodInfo action = typeof(PlaylistController).GetMethod(actionName) diff --git a/ErsatzTV/Controllers/Api/PlaylistController.cs b/ErsatzTV/Controllers/Api/PlaylistController.cs index 6c8961907..8876105cb 100644 --- a/ErsatzTV/Controllers/Api/PlaylistController.cs +++ b/ErsatzTV/Controllers/Api/PlaylistController.cs @@ -55,6 +55,27 @@ public class PlaylistController(IMediator mediator) : ControllerBase [Required] [FromBody] UpdatePlaylistGroupRequest request, CancellationToken cancellationToken) { + // Existence pre-check (404) mirrors DeleteGroup. Required controller-side because the + // handler's NotFoundError is collapsed to a plain BaseError by LanguageExtensions.Apply + // (error.Join()), so a missing group would otherwise map to 422 instead of 404. + List groups = await mediator.Send(new GetAllPlaylistGroups(), cancellationToken); + Option maybeGroup = groups.Find(g => g.Id == id); + if (maybeGroup.IsNone) + { + return ApiResults.NotFoundProblem(); + } + + // System (generated) groups must not be renamed. The RenamePlaylistGroupHandler also + // enforces this (defense-in-depth for the Blazor path); catching it here keeps the API's + // 422 independent of the handler's error-collapsing. + foreach (PlaylistGroupViewModel group in maybeGroup) + { + if (group.IsSystem) + { + return BaseError.New("Cannot rename system playlist group").ToErrorResult(); + } + } + Either result = await mediator.Send(request.ToCommand(id), cancellationToken); return result.Match( @@ -162,6 +183,17 @@ public class PlaylistController(IMediator mediator) : ControllerBase return ApiResults.NotFoundProblem(); } + // System (generated) playlists must not be renamed or have their items replaced. + // Mirrors the DeletePlaylist system-guard (422); done controller-side so item + // replacement never reaches ReplacePlaylistItemsHandler for a generated playlist. + foreach (PlaylistViewModel playlist in maybePlaylist) + { + if (playlist.IsSystem) + { + return BaseError.New("Cannot modify system (generated) playlist").ToErrorResult(); + } + } + Either> result = await mediator.Send(request.ToCommand(id), cancellationToken); return result.Match( @@ -201,6 +233,18 @@ public class PlaylistController(IMediator mediator) : ControllerBase [Required] [FromBody] ReplacePlaylistRequest request, CancellationToken cancellationToken) { + // The preview path does not run the handler's CollectionTypesMustBeValid check + // (PreviewPlaylistPlayoutHandler is shared with Blazor and left unchanged), so + // validate the draft at the controller boundary to avoid a 500 in the playout + // builder on an item missing the id required for its collection type. + List invalidItems = request.ItemsMissingRequiredId(); + if (invalidItems.Count > 0) + { + return BaseError + .New($"Invalid playlist item(s) at index(es): {string.Join(", ", invalidItems)}") + .ToErrorResult(); + } + Either> result = await mediator.Send(new PreviewPlaylistPlayout(request.ToReplaceCommand()), cancellationToken); return result.Match( diff --git a/ErsatzTV/Controllers/Api/Requests/ReplacePlaylistRequest.cs b/ErsatzTV/Controllers/Api/Requests/ReplacePlaylistRequest.cs index 434a58b28..2e1ad4ef4 100644 --- a/ErsatzTV/Controllers/Api/Requests/ReplacePlaylistRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/ReplacePlaylistRequest.cs @@ -30,6 +30,28 @@ public record PlaylistItemRequest( Count, PlayAll, IncludeInProgramGuide); + + // Mirrors ReplacePlaylistItemsHandler.CollectionTypeMustBeValid: the id required for the + // item's collection type must be present. Kept in sync with that handler so the PUT and + // preview endpoints enforce the same rule (the handler operates on the command type in the + // Application layer and cannot reference this request DTO, hence the parallel logic). + public bool HasRequiredId() => + CollectionType switch + { + CollectionType.Collection => CollectionId is not null, + CollectionType.MultiCollection => MultiCollectionId is not null, + CollectionType.SmartCollection => SmartCollectionId is not null, + CollectionType.TelevisionShow + or CollectionType.TelevisionSeason + or CollectionType.Artist + or CollectionType.Movie + or CollectionType.Episode + or CollectionType.MusicVideo + or CollectionType.OtherVideo + or CollectionType.Song + or CollectionType.Image => MediaItemId is not null, + _ => false + }; } public record ReplacePlaylistRequest(string? Name, List? Items) @@ -41,6 +63,22 @@ public record ReplacePlaylistRequest(string? Name, List? It public ReplacePlaylistItems ToReplaceCommand() => new(0, Name ?? string.Empty, BuildItems()); + // Array indexes of draft items missing the id required for their collection type. + public List ItemsMissingRequiredId() + { + List items = Items ?? new List(); + var invalid = new List(); + for (int i = 0; i < items.Count; i++) + { + if (!items[i].HasRequiredId()) + { + invalid.Add(i); + } + } + + return invalid; + } + private List BuildItems() => (Items ?? new List()) .Select((item, index) => item.ToReplaceCommand(index))