From 540def7f1759befea2603ad4eb6ae4db28af9a2a Mon Sep 17 00:00:00 2001 From: Timothy Date: Tue, 7 Jul 2026 09:22:50 +0200 Subject: [PATCH] fix: address #93 adversarial review findings Backend: 401 documented on all mutating settings/resolution routes; resolution delete distinguishes 404 (unknown) from 422 (not custom); XMLTV enum bridging via exhaustive switch instead of int casts; field-level Arg.Is assertions incl. non-null watermark/filler flow. Frontend: resolution add/delete failures surfaced inline (were silent); partial saves merge succeeded groups via allSettled; empty numeric fields invalid + tunerCount min 1; HLS Direct select shows out-of-list wire values; media-source rows show derived last-scan. ErsatzTV.Tests 495; web suite 145. Co-Authored-By: Claude Fable 5 --- .../Commands/DeleteCustomResolutionHandler.cs | 18 +- .../OpenApiErrorResponseContractTests.cs | 10 + .../Controllers/ResolutionControllerTests.cs | 19 +- .../Controllers/SettingsControllerTests.cs | 91 +++++++- .../Requests/UpdateXmltvSettingsRequest.cs | 20 +- .../Controllers/Api/ResolutionController.cs | 3 + .../Controllers/Api/SettingsController.cs | 27 ++- ErsatzTV/wwwroot/openapi/v1.json | 200 +++++++++++++++++ web/src/App.test.tsx | 178 +++++++++++++++ web/src/screens/SettingsScreen.tsx | 203 ++++++++++++++---- 10 files changed, 717 insertions(+), 52 deletions(-) diff --git a/ErsatzTV.Application/Resolutions/Commands/DeleteCustomResolutionHandler.cs b/ErsatzTV.Application/Resolutions/Commands/DeleteCustomResolutionHandler.cs index 6b8ce65a1..2eab33e36 100644 --- a/ErsatzTV.Application/Resolutions/Commands/DeleteCustomResolutionHandler.cs +++ b/ErsatzTV.Application/Resolutions/Commands/DeleteCustomResolutionHandler.cs @@ -1,6 +1,7 @@ using Dapper; using ErsatzTV.Core; using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Errors; using ErsatzTV.Infrastructure.Data; using ErsatzTV.Infrastructure.Extensions; using Microsoft.EntityFrameworkCore; @@ -18,23 +19,28 @@ public class DeleteCustomResolutionHandler : IRequestHandler maybeResolution = await dbContext.Resolutions + Option maybeAnyResolution = await dbContext.Resolutions .AsNoTracking() - .SelectOneAsync(p => p.Id, p => p.Id == request.ResolutionId && p.IsCustom == true, cancellationToken); + .SelectOneAsync(p => p.Id, p => p.Id == request.ResolutionId, cancellationToken); - foreach (Resolution resolution in maybeResolution) + foreach (Resolution existingResolution in maybeAnyResolution) { + if (!existingResolution.IsCustom) + { + return BaseError.New($"Resolution {request.ResolutionId} is not a custom resolution."); + } + // reset any ffmpeg profiles using this resolution to 1920x1080 await dbContext.Connection.ExecuteAsync( @"UPDATE FFmpegProfile SET ResolutionId = 3 WHERE ResolutionId = @ResolutionId", new { request.ResolutionId }); - dbContext.Resolutions.Remove(resolution); + dbContext.Resolutions.Remove(existingResolution); await dbContext.SaveChangesAsync(cancellationToken); } - return maybeResolution.IsNone - ? BaseError.New($"Resolution {request.ResolutionId} does not exist.") + return maybeAnyResolution.IsNone + ? new NotFoundError($"Resolution {request.ResolutionId} does not exist.") : Option.None; } } diff --git a/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs b/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs index 29055f2c5..39c6f80f7 100644 --- a/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs +++ b/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs @@ -174,14 +174,24 @@ public class OpenApiErrorResponseContractTests [TestCase("/api/ffmpeg/profiles/{id}", "delete", "404")] [TestCase("/api/ffmpeg/profiles/{id}", "delete", "401")] [TestCase("/api/ffmpeg/profiles/{id}", "delete", "422")] + [TestCase("/api/settings/ffmpeg", "put", "401")] [TestCase("/api/settings/ffmpeg", "put", "422")] + [TestCase("/api/settings/playout", "put", "401")] [TestCase("/api/settings/playout", "put", "422")] + [TestCase("/api/settings/xmltv", "put", "401")] [TestCase("/api/settings/xmltv", "put", "422")] + [TestCase("/api/settings/scanner", "put", "401")] [TestCase("/api/settings/scanner", "put", "422")] + [TestCase("/api/settings/logging", "put", "401")] [TestCase("/api/settings/logging", "put", "422")] + [TestCase("/api/settings/ui", "put", "401")] [TestCase("/api/settings/ui", "put", "422")] + [TestCase("/api/settings/hdhr", "put", "401")] [TestCase("/api/settings/hdhr", "put", "422")] + [TestCase("/api/settings/resolutions", "post", "401")] [TestCase("/api/settings/resolutions", "post", "422")] + [TestCase("/api/settings/resolutions/{id}", "delete", "401")] + [TestCase("/api/settings/resolutions/{id}", "delete", "404")] [TestCase("/api/settings/resolutions/{id}", "delete", "422")] public void Static_OpenApi_Should_Document_ProblemDetails_For_Api_Error_Responses( string path, diff --git a/ErsatzTV.Tests/Controllers/ResolutionControllerTests.cs b/ErsatzTV.Tests/Controllers/ResolutionControllerTests.cs index ddb1bde5a..446844c8e 100644 --- a/ErsatzTV.Tests/Controllers/ResolutionControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/ResolutionControllerTests.cs @@ -3,6 +3,7 @@ using ErsatzTV.Controllers.Api; using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Core.Api.Settings; +using ErsatzTV.Core.Errors; using LanguageExt; using MediatR; using Microsoft.AspNetCore.Mvc; @@ -87,17 +88,25 @@ public class ResolutionControllerTests result.ShouldBeOfType(); } - // The handler (DeleteCustomResolutionHandler) reports a missing/non-custom resolution as a plain - // BaseError, not a NotFoundError, so ApiResults.ToErrorResult maps it to 422 rather than 404 - - // this test documents that observed behavior rather than the ideal REST status code. [Test] - public async Task Delete_Should_Return_422_For_Unknown_Or_NonCustom_Resolution() + public async Task Delete_Should_Return_404_For_Unknown_Resolution() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(BaseError.New("Resolution 42 does not exist."))); + .Returns(Option.Some(new NotFoundError("Resolution 42 does not exist."))); IActionResult result = await _controller.Delete(42, CancellationToken.None); + result.ShouldBeOfType(); + } + + [Test] + public async Task Delete_Should_Return_422_For_NonCustom_Resolution() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(BaseError.New("Resolution 3 is not a custom resolution."))); + + IActionResult result = await _controller.Delete(3, CancellationToken.None); + result.ShouldBeOfType(); } } diff --git a/ErsatzTV.Tests/Controllers/SettingsControllerTests.cs b/ErsatzTV.Tests/Controllers/SettingsControllerTests.cs index 687c7af5a..c04bf335e 100644 --- a/ErsatzTV.Tests/Controllers/SettingsControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/SettingsControllerTests.cs @@ -85,7 +85,43 @@ public class SettingsControllerTests var ok = result.ShouldBeOfType(); ok.Value.ShouldBeOfType().HlsDirectOutputFormat.ShouldBe(OutputFormatKind.Hls); - await _mediator.Received(1).Send(Arg.Any(), Arg.Any()); + await _mediator.Received(1).Send( + Arg.Is( + c => c.Settings.FFmpegPath == "/usr/bin/ffmpeg" && + c.Settings.FFprobePath == "/usr/bin/ffprobe" && + c.Settings.DefaultFFmpegProfileId == 1 && + c.Settings.PreferredAudioLanguageCode == "eng" && + c.Settings.UseEmbeddedSubtitles && + !c.Settings.ExtractEmbeddedSubtitles && + !c.Settings.ProbeForInterlacedFrames && + !c.Settings.SaveReports && + c.Settings.GlobalWatermarkId == null && + c.Settings.GlobalFallbackFillerId == null && + c.Settings.HlsSegmenterIdleTimeout == 60 && + c.Settings.WorkAheadSegmenterLimit == 1 && + c.Settings.InitialSegmentCount == 1 && + c.Settings.HlsDirectOutputFormat == OutputFormatKind.MpegTs && + c.Settings.DefaultMpegTsScript == "Default"), + Arg.Any()); + } + + [Test] + public async Task UpdateFfmpeg_Should_Flow_NonNull_Watermark_And_Filler_Ids_Through() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right(Unit.Default)); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new FFmpegSettingsViewModel { FFmpegPath = "/usr/bin/ffmpeg", FFprobePath = "/usr/bin/ffprobe" }); + + UpdateFFmpegSettingsRequest request = MakeFfmpegRequest() with { GlobalWatermarkId = 7, GlobalFallbackFillerId = 9 }; + + IActionResult result = await _controller.UpdateFfmpeg(request, CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.Received(1).Send( + Arg.Is( + c => c.Settings.GlobalWatermarkId == 7 && c.Settings.GlobalFallbackFillerId == 9), + Arg.Any()); } [Test] @@ -141,6 +177,12 @@ public class SettingsControllerTests CancellationToken.None); result.ShouldBeOfType().Value.ShouldBe(new PlayoutSettingsResponseModel(5, false, 30)); + await _mediator.Received(1).Send( + Arg.Is( + c => c.PlayoutSettings.DaysToBuild == 5 && + c.PlayoutSettings.SkipMissingItems == false && + c.PlayoutSettings.ScriptedScheduleTimeoutSeconds == 30), + Arg.Any()); } [Test] @@ -233,6 +275,9 @@ public class SettingsControllerTests await _controller.UpdateScanner(new UpdateScannerSettingsRequest(12), CancellationToken.None); result.ShouldBeOfType().Value.ShouldBe(new ScannerSettingsResponseModel(12)); + await _mediator.Received(1).Send( + Arg.Is(c => c.LibraryRefreshInterval == 12), + Arg.Any()); } [Test] @@ -270,6 +315,44 @@ public class SettingsControllerTests result.ShouldBeOfType(); } + [Test] + public async Task UpdateLogging_Should_Map_Request_To_Command_And_Return_Refreshed_Settings() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right(Unit.Default)); + var refreshed = new LoggingSettingsViewModel + { + DefaultMinimumLogLevel = LogEventLevel.Debug, + ScanningMinimumLogLevel = LogEventLevel.Debug, + SchedulingMinimumLogLevel = LogEventLevel.Debug, + SearchingMinimumLogLevel = LogEventLevel.Debug, + StreamingMinimumLogLevel = LogEventLevel.Debug, + HttpMinimumLogLevel = LogEventLevel.Debug + }; + _mediator.Send(Arg.Any(), Arg.Any()).Returns(refreshed); + + var request = new UpdateLoggingSettingsRequest( + LogEventLevel.Warning, + LogEventLevel.Error, + LogEventLevel.Fatal, + LogEventLevel.Verbose, + LogEventLevel.Debug, + LogEventLevel.Information); + + IActionResult result = await _controller.UpdateLogging(request, CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.Received(1).Send( + Arg.Is( + c => c.LoggingSettings.DefaultMinimumLogLevel == LogEventLevel.Warning && + c.LoggingSettings.ScanningMinimumLogLevel == LogEventLevel.Error && + c.LoggingSettings.SchedulingMinimumLogLevel == LogEventLevel.Fatal && + c.LoggingSettings.SearchingMinimumLogLevel == LogEventLevel.Verbose && + c.LoggingSettings.StreamingMinimumLogLevel == LogEventLevel.Debug && + c.LoggingSettings.HttpMinimumLogLevel == LogEventLevel.Information), + Arg.Any()); + } + [Test] public async Task GetUi_Should_Map_Vm_To_Response_Model() { @@ -293,6 +376,9 @@ public class SettingsControllerTests await _controller.UpdateUi(new UpdateUiSettingsRequest(false, "fr"), CancellationToken.None); result.ShouldBeOfType().Value.ShouldBe(new UiSettingsResponseModel(false, "fr")); + await _mediator.Received(1).Send( + Arg.Is(c => c.UiSettings.IsDarkMode == false && c.UiSettings.Language == "fr"), + Arg.Any()); } [Test] @@ -331,6 +417,9 @@ public class SettingsControllerTests IActionResult result = await _controller.UpdateHdhr(new UpdateHdhrSettingsRequest(4), CancellationToken.None); result.ShouldBeOfType().Value.ShouldBe(new HdhrSettingsResponseModel(4, uuid)); + await _mediator.Received(1).Send( + Arg.Is(c => c.TunerCount == 4), + Arg.Any()); } [Test] diff --git a/ErsatzTV/Controllers/Api/Requests/UpdateXmltvSettingsRequest.cs b/ErsatzTV/Controllers/Api/Requests/UpdateXmltvSettingsRequest.cs index 31e57b59d..bbdcc4c6e 100644 --- a/ErsatzTV/Controllers/Api/Requests/UpdateXmltvSettingsRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/UpdateXmltvSettingsRequest.cs @@ -11,7 +11,23 @@ public record UpdateXmltvSettingsRequest(int DaysToBuild, ApiXmltvTimeZone TimeZ new XmltvSettingsViewModel { DaysToBuild = DaysToBuild, - TimeZone = (XmltvTimeZone)(int)TimeZone, - BlockBehavior = (XmltvBlockBehavior)(int)BlockBehavior + TimeZone = ToVmTimeZone(TimeZone), + BlockBehavior = ToVmBlockBehavior(BlockBehavior) }); + + private static XmltvTimeZone ToVmTimeZone(ApiXmltvTimeZone timeZone) => + timeZone switch + { + ApiXmltvTimeZone.Local => XmltvTimeZone.Local, + ApiXmltvTimeZone.Utc => XmltvTimeZone.Utc, + _ => throw new ArgumentOutOfRangeException(nameof(timeZone), timeZone, null) + }; + + private static XmltvBlockBehavior ToVmBlockBehavior(ApiXmltvBlockBehavior blockBehavior) => + blockBehavior switch + { + ApiXmltvBlockBehavior.SplitTimeEvenly => XmltvBlockBehavior.SplitTimeEvenly, + ApiXmltvBlockBehavior.UseActualTimes => XmltvBlockBehavior.UseActualTimes, + _ => throw new ArgumentOutOfRangeException(nameof(blockBehavior), blockBehavior, null) + }; } diff --git a/ErsatzTV/Controllers/Api/ResolutionController.cs b/ErsatzTV/Controllers/Api/ResolutionController.cs index 52151922e..0e148497b 100644 --- a/ErsatzTV/Controllers/Api/ResolutionController.cs +++ b/ErsatzTV/Controllers/Api/ResolutionController.cs @@ -35,6 +35,7 @@ public class ResolutionController(IMediator mediator) : ControllerBase [Tags("Settings")] [EndpointSummary("Create a custom resolution")] [ProducesResponseType(typeof(ResolutionResponseModel), StatusCodes.Status201Created)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status401Unauthorized)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task Create( [Required] [FromBody] @@ -61,6 +62,8 @@ public class ResolutionController(IMediator mediator) : ControllerBase [Tags("Settings")] [EndpointSummary("Delete a custom resolution")] [ProducesResponseType(StatusCodes.Status204NoContent)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status401Unauthorized)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task Delete(int id, CancellationToken cancellationToken) { diff --git a/ErsatzTV/Controllers/Api/SettingsController.cs b/ErsatzTV/Controllers/Api/SettingsController.cs index e36ca48e4..7a1d2ec57 100644 --- a/ErsatzTV/Controllers/Api/SettingsController.cs +++ b/ErsatzTV/Controllers/Api/SettingsController.cs @@ -11,6 +11,8 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using ApiXmltvBlockBehavior = ErsatzTV.Core.Api.Settings.XmltvBlockBehavior; using ApiXmltvTimeZone = ErsatzTV.Core.Api.Settings.XmltvTimeZone; +using VmXmltvBlockBehavior = ErsatzTV.Application.Configuration.XmltvBlockBehavior; +using VmXmltvTimeZone = ErsatzTV.Application.Configuration.XmltvTimeZone; namespace ErsatzTV.Controllers.Api; @@ -34,6 +36,7 @@ public class SettingsController(IMediator mediator) : ControllerBase [Tags("Settings")] [EndpointSummary("Update global FFmpeg settings")] [ProducesResponseType(typeof(FFmpegSettingsResponseModel), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status401Unauthorized)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task UpdateFfmpeg( [Required] [FromBody] @@ -66,6 +69,7 @@ public class SettingsController(IMediator mediator) : ControllerBase [Tags("Settings")] [EndpointSummary("Update global playout settings")] [ProducesResponseType(typeof(PlayoutSettingsResponseModel), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status401Unauthorized)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task UpdatePlayout( [Required] [FromBody] @@ -98,6 +102,7 @@ public class SettingsController(IMediator mediator) : ControllerBase [Tags("Settings")] [EndpointSummary("Update global XMLTV settings")] [ProducesResponseType(typeof(XmltvSettingsResponseModel), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status401Unauthorized)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task UpdateXmltv( [Required] [FromBody] @@ -130,6 +135,7 @@ public class SettingsController(IMediator mediator) : ControllerBase [Tags("Settings")] [EndpointSummary("Update library scan cadence")] [ProducesResponseType(typeof(ScannerSettingsResponseModel), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status401Unauthorized)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task UpdateScanner( [Required] [FromBody] @@ -162,6 +168,7 @@ public class SettingsController(IMediator mediator) : ControllerBase [Tags("Settings")] [EndpointSummary("Update per-area minimum log levels")] [ProducesResponseType(typeof(LoggingSettingsResponseModel), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status401Unauthorized)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task UpdateLogging( [Required] [FromBody] @@ -194,6 +201,7 @@ public class SettingsController(IMediator mediator) : ControllerBase [Tags("Settings")] [EndpointSummary("Update UI preferences")] [ProducesResponseType(typeof(UiSettingsResponseModel), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status401Unauthorized)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task UpdateUi( [Required] [FromBody] @@ -223,6 +231,7 @@ public class SettingsController(IMediator mediator) : ControllerBase [Tags("Settings")] [EndpointSummary("Update HDHomeRun emulation settings")] [ProducesResponseType(typeof(HdhrSettingsResponseModel), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status401Unauthorized)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task UpdateHdhr( [Required] [FromBody] @@ -264,7 +273,23 @@ public class SettingsController(IMediator mediator) : ControllerBase new(vm.DaysToBuild, vm.SkipMissingItems, vm.ScriptedScheduleTimeoutSeconds); private static XmltvSettingsResponseModel ProjectToResponseModel(XmltvSettingsViewModel vm) => - new(vm.DaysToBuild, (ApiXmltvTimeZone)(int)vm.TimeZone, (ApiXmltvBlockBehavior)(int)vm.BlockBehavior); + new(vm.DaysToBuild, ToApiTimeZone(vm.TimeZone), ToApiBlockBehavior(vm.BlockBehavior)); + + private static ApiXmltvTimeZone ToApiTimeZone(VmXmltvTimeZone timeZone) => + timeZone switch + { + VmXmltvTimeZone.Local => ApiXmltvTimeZone.Local, + VmXmltvTimeZone.Utc => ApiXmltvTimeZone.Utc, + _ => throw new ArgumentOutOfRangeException(nameof(timeZone), timeZone, null) + }; + + private static ApiXmltvBlockBehavior ToApiBlockBehavior(VmXmltvBlockBehavior blockBehavior) => + blockBehavior switch + { + VmXmltvBlockBehavior.SplitTimeEvenly => ApiXmltvBlockBehavior.SplitTimeEvenly, + VmXmltvBlockBehavior.UseActualTimes => ApiXmltvBlockBehavior.UseActualTimes, + _ => throw new ArgumentOutOfRangeException(nameof(blockBehavior), blockBehavior, null) + }; private static LoggingSettingsResponseModel ProjectToResponseModel(LoggingSettingsViewModel vm) => new( diff --git a/ErsatzTV/wwwroot/openapi/v1.json b/ErsatzTV/wwwroot/openapi/v1.json index 92b194f3c..2b7057dc0 100644 --- a/ErsatzTV/wwwroot/openapi/v1.json +++ b/ErsatzTV/wwwroot/openapi/v1.json @@ -3295,6 +3295,26 @@ } } }, + "401": { + "description": "Unauthorized", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -3340,6 +3360,46 @@ "204": { "description": "No Content" }, + "401": { + "description": "Unauthorized", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "404": { + "description": "Not Found", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -4219,6 +4279,26 @@ } } }, + "401": { + "description": "Unauthorized", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -4324,6 +4404,26 @@ } } }, + "401": { + "description": "Unauthorized", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -4429,6 +4529,26 @@ } } }, + "401": { + "description": "Unauthorized", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -4534,6 +4654,26 @@ } } }, + "401": { + "description": "Unauthorized", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -4639,6 +4779,26 @@ } } }, + "401": { + "description": "Unauthorized", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -4744,6 +4904,26 @@ } } }, + "401": { + "description": "Unauthorized", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -4849,6 +5029,26 @@ } } }, + "401": { + "description": "Unauthorized", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { diff --git a/web/src/App.test.tsx b/web/src/App.test.tsx index 7dc099aca..25bc17eb7 100644 --- a/web/src/App.test.tsx +++ b/web/src/App.test.tsx @@ -2760,6 +2760,172 @@ describe('Settings screen (#93)', () => { await screen.findByText('Interface preferences. These apply to the legacy web UI; ChicoryTV theming lives in the theme switcher.') ).toBeInTheDocument(); }); + + it('saves exactly the dirty groups when multiple groups are edited', async () => { + mockDashboardApi(); + await openSettings(); + + fireEvent.change(screen.getByPlaceholderText('en-US'), { target: { value: 'de-DE' } }); + + fireEvent.click(screen.getByRole('button', { name: /Playout/ })); + await screen.findByText('Defaults for how far ahead playouts are built and how gaps are handled.'); + fireEvent.change(screen.getByDisplayValue('2'), { target: { value: '5' } }); + + await screen.findByText('2 unsaved changes'); + fireEvent.click(screen.getByRole('button', { name: /Save changes/ })); + + expect(await screen.findByText('Settings saved')).toBeInTheDocument(); + expect(requestBodyFor('/api/settings/ui')).toMatchObject({ language: 'de-DE' }); + expect(requestBodyFor('/api/settings/playout')).toMatchObject({ daysToBuild: 5 }); + + const putCallCount = (path: string) => + vi.mocked(window.fetch).mock.calls.filter(([input, init]) => input.toString() === path && init?.method === 'PUT') + .length; + + expect(putCallCount('/api/settings/ui')).toBe(1); + expect(putCallCount('/api/settings/playout')).toBe(1); + expect(putCallCount('/api/settings/ffmpeg')).toBe(0); + expect(putCallCount('/api/settings/xmltv')).toBe(0); + expect(putCallCount('/api/settings/scanner')).toBe(0); + expect(putCallCount('/api/settings/logging')).toBe(0); + expect(putCallCount('/api/settings/hdhr')).toBe(0); + }); + + it('on partial save failure, keeps only the failed group dirty and surfaces its error', async () => { + mockDashboardApi({ + settingsMutationFailures: { '/api/settings/playout': { detail: 'Playout save failed', status: 500 } } + }); + await openSettings(); + + fireEvent.change(screen.getByPlaceholderText('en-US'), { target: { value: 'de-DE' } }); + + fireEvent.click(screen.getByRole('button', { name: /Playout/ })); + await screen.findByText('Defaults for how far ahead playouts are built and how gaps are handled.'); + fireEvent.change(screen.getByDisplayValue('2'), { target: { value: '5' } }); + + await screen.findByText('2 unsaved changes'); + fireEvent.click(screen.getByRole('button', { name: /Save changes/ })); + + expect(await screen.findByText('1 unsaved change')).toBeInTheDocument(); + expect(screen.getByText(/Playout save failed/)).toBeInTheDocument(); + expect(screen.queryByText('Settings saved')).not.toBeInTheDocument(); + + // The succeeded (ui) group's edit stuck even though the draft stayed on the Playout pane. + fireEvent.click(screen.getByRole('button', { name: /General/ })); + expect(await screen.findByDisplayValue('de-DE')).toBeInTheDocument(); + }); + + it('surfaces an inline error (no unhandled rejection) when adding a duplicate resolution fails', async () => { + mockDashboardApi({ resolutionCreateFailure: { detail: 'Resolution already exists', status: 422 } }); + await openSettings(); + fireEvent.click(await screen.findByRole('button', { name: /Streaming/ })); + await screen.findByText('Custom resolutions'); + + fireEvent.change(screen.getByPlaceholderText('width'), { target: { value: '2560' } }); + fireEvent.change(screen.getByPlaceholderText('height'), { target: { value: '1080' } }); + fireEvent.click(screen.getByRole('button', { name: 'Add resolution' })); + + expect(await screen.findByText('Resolution already exists')).toBeInTheDocument(); + }); + + it('surfaces a resolution-delete failure inside the still-open confirm dialog', async () => { + mockDashboardApi({ resolutionDeleteFailure: { detail: 'Resolution is in use', status: 409 } }); + await openSettings(); + fireEvent.click(await screen.findByRole('button', { name: /Streaming/ })); + await screen.findByText('1920 × 1080'); + + fireEvent.change(screen.getByPlaceholderText('width'), { target: { value: '2560' } }); + fireEvent.change(screen.getByPlaceholderText('height'), { target: { value: '1080' } }); + fireEvent.click(screen.getByRole('button', { name: 'Add resolution' })); + await screen.findByText('2560 × 1080'); + + const deleteButtons = screen.getAllByRole('button', { name: /Delete \d+×\d+/ }); + fireEvent.click(deleteButtons[deleteButtons.length - 1]); + + const dialog = await screen.findByRole('dialog'); + fireEvent.click(within(dialog).getByRole('button', { name: 'Delete' })); + + expect(await within(dialog).findByText('Resolution is in use')).toBeInTheDocument(); + // The dialog stays open with the resolution still present - a clean draft would + // otherwise show nothing at all, since saveError only renders inside the save bar. + expect(screen.getByText('2560 × 1080')).toBeInTheDocument(); + }); + + it('disables Save and shows an inline error when a numeric field is cleared', async () => { + mockDashboardApi(); + await openSettings(); + + fireEvent.click(screen.getByRole('button', { name: /Playout/ })); + await screen.findByText('Defaults for how far ahead playouts are built and how gaps are handled.'); + fireEvent.change(screen.getByDisplayValue('2'), { target: { value: '' } }); + + await screen.findByText('1 unsaved change'); + expect(screen.getByText('Must be a whole number ≥ 0')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /Save changes/ })).toBeDisabled(); + }); + + it('treats a tunerCount of 0 as invalid (backend rejects 0)', async () => { + mockDashboardApi(); + await openSettings(); + + fireEvent.click(screen.getByRole('button', { name: /^System/ })); + await screen.findByText('HDHomeRun emulation, connected media sources and server info.'); + fireEvent.change(screen.getByDisplayValue('2'), { target: { value: '0' } }); + + await screen.findByText('1 unsaved change'); + expect(screen.getByText('Must be a whole number ≥ 1')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /Save changes/ })).toBeDisabled(); + }); + + it('shows the raw wire value as an extra HLS Direct option when it is outside the known set', async () => { + mockDashboardApi({ ffmpegSettings: defaultFfmpegSettings({ hlsDirectOutputFormat: 'Hls' }) }); + await openSettings(); + fireEvent.click(await screen.findByRole('button', { name: /Streaming/ })); + + expect(await screen.findByDisplayValue('Hls')).toBeInTheDocument(); + }); + + it('sends a non-null globalWatermarkId through in the ffmpeg save PUT body', async () => { + mockDashboardApi({ watermarks: [{ id: 5, name: 'Bug' }] }); + await openSettings(); + fireEvent.click(await screen.findByRole('button', { name: /Streaming/ })); + await screen.findByText('Global defaults'); + + const watermarkSelect = screen + .getAllByRole('combobox') + .find((select) => within(select).queryByText('Bug')) as HTMLSelectElement; + + fireEvent.change(watermarkSelect, { target: { value: '5' } }); + + await screen.findByText('1 unsaved change'); + fireEvent.click(screen.getByRole('button', { name: /Save changes/ })); + + expect(await screen.findByText('Settings saved')).toBeInTheDocument(); + expect(requestBodyFor('/api/settings/ffmpeg')).toMatchObject({ globalWatermarkId: 5 }); + }); + + it('renders the most recent last-scan time per media source', async () => { + mockDashboardApi({ + mediaSources: [ + mediaSource({ + id: 30, + kind: 'Local', + libraries: [ + library({ id: 31, lastScan: '2026-07-01T00:00:00Z' }), + library({ id: 32, lastScan: '2026-07-05T14:30:00Z' }) + ], + name: 'Local' + }), + mediaSource({ id: 40, kind: 'Jellyfin', libraries: [], name: 'Jellyfin Server' }) + ] + }); + await openSettings(); + fireEvent.click(await screen.findByRole('button', { name: /^System/ })); + + expect(await screen.findByText('Local')).toBeInTheDocument(); + expect(await screen.findByText('Jellyfin Server')).toBeInTheDocument(); + expect(screen.getByText(/Last scan/)).toBeInTheDocument(); + }); }); function jsonResponse(body: unknown, status = 200): Response { @@ -3178,6 +3344,8 @@ function mockDashboardApi({ uiSettings = defaultUiSettings(), hdhrSettings = defaultHdhrSettings(), resolutions = defaultResolutions(), + resolutionCreateFailure = null, + resolutionDeleteFailure = null, settingsMutationFailures = {}, settingsGetFailuresBeforeSuccess = {} }: { @@ -3235,6 +3403,8 @@ function mockDashboardApi({ uiSettings?: Record; hdhrSettings?: Record; resolutions?: Array>; + resolutionCreateFailure?: { detail?: string; status?: number; title?: string } | null; + resolutionDeleteFailure?: { detail?: string; status?: number; title?: string } | null; settingsMutationFailures?: Record; // path -> number of GET failures (500) before the endpoint recovers; use a large // number for a persistently-failing endpoint. @@ -3581,6 +3751,10 @@ function mockDashboardApi({ if (path === '/api/settings/resolutions') { if ((init?.method ?? 'GET') === 'POST') { + if (resolutionCreateFailure) { + return Promise.resolve(jsonResponse(resolutionCreateFailure, resolutionCreateFailure.status ?? 422)); + } + const body = JSON.parse(init?.body as string) as { height: number; width: number }; const created = { height: body.height, @@ -3596,6 +3770,10 @@ function mockDashboardApi({ } if (path.match(/^\/api\/settings\/resolutions\/\d+$/)) { + if (resolutionDeleteFailure) { + return Promise.resolve(jsonResponse(resolutionDeleteFailure, resolutionDeleteFailure.status ?? 422)); + } + const id = Number(path.split('/').at(-1)); currentResolutions = currentResolutions.filter((resolution) => Number(resolution.id) !== id); return Promise.resolve(new Response(null, { status: 204 })); diff --git a/web/src/screens/SettingsScreen.tsx b/web/src/screens/SettingsScreen.tsx index af132722a..fb2f9f43c 100644 --- a/web/src/screens/SettingsScreen.tsx +++ b/web/src/screens/SettingsScreen.tsx @@ -38,6 +38,7 @@ import { type HdhrSettings, type LoggingSettings, type LogEventLevel, + type MediaSource, type OutputFormatKind, type PlayoutSettings, type Resolution, @@ -99,6 +100,18 @@ const HLS_DIRECT_FORMAT_OPTIONS: Array<{ label: string; value: OutputFormatKind { label: 'MKV', value: 'Mkv' } ]; +// The server can hold an OutputFormatKind outside the curated option list above (e.g. +// Hls). If the current value isn't one of the known options, append it as an extra +// option labeled with the raw wire value - so the select's displayed value never lies +// about what the draft actually holds. +function hlsDirectFormatOptions(current: OutputFormatKind): Array<{ label: string; value: OutputFormatKind }> { + if (HLS_DIRECT_FORMAT_OPTIONS.some((option) => option.value === current)) { + return HLS_DIRECT_FORMAT_OPTIONS; + } + + return [...HLS_DIRECT_FORMAT_OPTIONS, { label: current, value: current }]; +} + interface Draft { ffmpeg: FfmpegSettings; hdhr: HdhrSettings; @@ -129,10 +142,43 @@ function countChangedFields(a: Record, b: Record a[key] !== b[key]).length; } -function isValidNonNegativeInt(value: number): boolean { - return Number.isInteger(value) && value >= 0; +function isValidInt(value: number, min = 0): boolean { + return Number.isInteger(value) && value >= min; } +// Most recent lastScan timestamp across a media source's libraries, or null when none +// of its libraries have ever been scanned. ISO 8601 timestamps sort correctly as plain +// strings, so no Date parsing is needed to find the max. +function mostRecentLastScan(source: MediaSource): string | null { + const scans = source.libraries.map((library) => library.lastScan).filter((scan): scan is string => Boolean(scan)); + + if (scans.length === 0) { + return null; + } + + return scans.reduce((latest, scan) => (scan > latest ? scan : latest)); +} + +function formatLastScan(value: string): string { + const parsed = new Date(value); + + if (Number.isNaN(parsed.getTime())) { + return `Last scan ${value}`; + } + + return `Last scan ${parsed.toLocaleString([], { day: 'numeric', hour: '2-digit', minute: '2-digit', month: 'short' })}`; +} + +const GROUP_LABELS: Record = { + ffmpeg: 'Streaming', + hdhr: 'System', + logging: 'Logging', + playout: 'Playout', + scanner: 'Scanner', + ui: 'General', + xmltv: 'Guide (XMLTV)' +}; + async function saveGroup(key: GroupKey, draft: Draft): Promise> { switch (key) { case 'ui': @@ -229,24 +275,32 @@ function PathInput({ onChange, value }: { onChange: (value: string) => void; val function NumInput({ invalid = false, + min = 0, onChange, unit, value }: { invalid?: boolean; + min?: number; onChange: (value: number) => void; unit?: string; value: number; }) { return ( onChange(Number(event.target.value))} + error={invalid ? `Must be a whole number ≥ ${min}` : null} + onChange={(event) => { + const raw = event.target.value; + // An empty field is NOT the same as 0 - Number('') coerces to 0, which would + // make a cleared field silently pass validation. Route it to NaN instead so + // isValidInt (Number.isInteger) correctly flags it as invalid. + onChange(raw === '' ? Number.NaN : Number(raw)); + }} size="sm" style={{ fontFamily: 'var(--font-mono)', width: 120 }} trailing={unit ? {unit} : undefined} type="number" - value={String(value)} + value={Number.isNaN(value) ? '' : String(value)} /> ); } @@ -319,6 +373,7 @@ function StreamingPane({ const [resWidth, setResWidth] = useState(''); const [resHeight, setResHeight] = useState(''); const [adding, setAdding] = useState(false); + const [addError, setAddError] = useState(null); const submitResolution = async () => { const width = Number(resWidth); @@ -329,11 +384,14 @@ function StreamingPane({ } setAdding(true); + setAddError(null); try { await onAddResolution(width, height); setResWidth(''); setResHeight(''); + } catch (error) { + setAddError(messageFromSettingsError(error, 'Unable to add resolution')); } finally { setAdding(false); } @@ -452,7 +510,7 @@ function StreamingPane({ setResWidth(event.target.value)} + onChange={(event) => { + setResWidth(event.target.value); + setAddError(null); + }} placeholder="width" size="sm" style={{ fontFamily: 'var(--font-mono)', width: 96 }} @@ -495,7 +556,10 @@ function StreamingPane({ /> × setResHeight(event.target.value)} + onChange={(event) => { + setResHeight(event.target.value); + setAddError(null); + }} placeholder="height" size="sm" style={{ fontFamily: 'var(--font-mono)', width: 96 }} @@ -503,6 +567,11 @@ function StreamingPane({ value={resHeight} /> + {addError && ( + + {addError} + + )}