From afbd50e3f05fcf27050fdf525c019dfc38f76916 Mon Sep 17 00:00:00 2001 From: Timothy Date: Mon, 20 Jul 2026 19:39:37 +0200 Subject: [PATCH 01/13] =?UTF-8?q?docs(67):=20design=20spec=20=E2=80=94=20u?= =?UTF-8?q?nified=20logo/bug=20via=20shared=20ChannelLogo=20watermark=20pr?= =?UTF-8?q?eset?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recon showed ChannelWatermarkImageSource.ChannelLogo already resolves the channel's logo artwork at render time, and prod already runs that pattern by hand (43 channels -> one 'Channel Bug' preset). So #67 is productizing a proven mechanism, not building one: seed the preset idempotently in DbInitializer (adopting any existing same-name row untouched), default the SPA creation paths to it, and add a shared that renders the resolved geometry in both the channel editor and WatermarksScreen. No schema change, no WatermarkSelector change. One additive DTO field (WatermarkResponseModel.imageSource) under the frozen-additive /api/v1. Refs #67 --- .../2026-07-20-unified-logo-bug-design.md | 133 ++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-20-unified-logo-bug-design.md diff --git a/docs/superpowers/specs/2026-07-20-unified-logo-bug-design.md b/docs/superpowers/specs/2026-07-20-unified-logo-bug-design.md new file mode 100644 index 000000000..312c8d28d --- /dev/null +++ b/docs/superpowers/specs/2026-07-20-unified-logo-bug-design.md @@ -0,0 +1,133 @@ +# Unified logo / on-screen bug — design (ersatztv#67) + +Date: 2026-07-20 +Issue: [#67](http://192.168.1.95:3000/timothy/ersatztv/issues/67) (child of the #62 "Create Channel +backend prerequisites" epic) +Status: design approved, ready for implementation planning + +## 1. What the issue asked for vs. what already exists + +#67 asks that a single uploaded image drive **both** the listing logo and the on-screen bug +(watermark) by default, with the option to set them separately, plus live preview for both uses. + +Recon showed most of that capability is already built: + +| #67 requirement | State before this work | +|---|---| +| One image drives logo + bug | **Exists.** `ChannelWatermarkImageSource.ChannelLogo` (`ErsatzTV.Core/Domain/ChannelWatermark.cs:48`) resolves the channel's `ArtworkKind.Logo` artwork at render time, at all three watermark precedence levels (`ErsatzTV.Core/FFmpeg/WatermarkSelector.cs:216-236,265-285,314-334,384-392`). Exposed as `WatermarkFullResponseModel.ImageSource`, selectable in `web/src/screens/WatermarksScreen.tsx:30-33`. | +| Override either independently | **Exists.** `ImageSource = Custom` + a watermark-target upload. | +| ...by **default** | **Missing.** New channels are created with `watermarkId: null` (`web/src/screens/ChannelsScreen.tsx:377`); nothing is logo-driven unless a watermark is hand-built and hand-assigned. | +| Live preview for both uses | **Missing.** `WatermarksScreen.tsx:534-539` renders only a raw `` of the watermark file. Nothing shows the resolved geometry (location / size / margins / opacity), which is the part users actually get wrong. | + +**Field validation.** The production instance (jazz, 192.168.1.29) already runs this pattern by hand: +all 43 channels carry `ArtworkKind.Logo` artwork and point at a single shared watermark named +`Channel Bug` with `ImageSource = ChannelLogo` and `Image = NULL`. This design therefore does not +invent a mechanism — it productizes the one the operator already proved, so that a new channel gets +it without hand-wiring, and so the geometry is visible before saving. + +Observed prod geometry, adopted as the shipped default: + +``` +Name="Channel Bug" Mode=Permanent(1) ImageSource=ChannelLogo(1) Image=NULL +Location=TopLeft(3) Size=Scaled(0) WidthPercent=5.0 +HorizontalMarginPercent=1.0 VerticalMarginPercent=1.0 +FrequencyMinutes=0 DurationSeconds=0 Opacity=80 +PlaceWithinSourceContent=0 ZIndex=0 +``` + +## 2. Structural constraints that shaped the design + +1. **A watermark is a shared, named entity**, not per-channel state: unique `Name` + (`ErsatzTV.Infrastructure/Data/Configurations/ChannelWatermarkConfiguration.cs:11-18`), referenced + by playout items, schedule items, block items and decos (`ChannelWatermark.cs:24-31`), with + `Channel.WatermarkId` pointing at one (`Channel.cs:21-22`). Per-channel bug *settings* have + nowhere to live without either new `Channel` columns or one watermark row per channel. +2. **`ImageSource = ChannelLogo` resolves per-channel at render time.** One shared preset therefore + already makes every channel use *its own* logo. This is why no schema change is needed. +3. **Logo is not a `Channel` field** — it is an `Artwork` row (`ArtworkKind.Logo = 2`) whose `Path` + holds an image-cache hash or an absolute external URL, served at `/iptv/logos/{hash}` + (`ErsatzTV/Controllers/IptvController.cs:282-293`). +4. **`/api/v1` is frozen-additive** (`docs/decisions.md`, #286): fields may be added, never removed + or reshaped. + +### Alternatives rejected + +- **Per-channel bug columns on `Channel`** — full fidelity to "separably overridable", but requires a + dual-provider migration (`scripts/add-migration.sh`) and changes to the watermark precedence chain. + Rejected as disproportionate: the shared preset already delivers the user-visible behavior. +- **Auto-create a watermark per channel** — gives per-channel geometry with no migration, but + proliferates rows under a unique-name index and fills the shared Watermarks screen with one row per + channel. + +Accepted trade-off: every channel on the shared preset shares one geometry. Per-channel tweaks mean +creating a second preset — which the existing Watermarks screen already supports. + +## 3. Backend + +`DbInitializer.SeedChannelBugWatermark(context, cancellationToken)`, invoked from `Initialize` +alongside the existing `SeedChannelTemplates` call (`ErsatzTV.Infrastructure/Data/DbInitializer.cs:134`), +following that method's idempotent name-guard pattern (`:140-186`): + +- If a `ChannelWatermark` named `Channel Bug` already exists → **return without touching it.** An + existing hand-made row (as on prod) is adopted, never duplicated and never overwritten. +- Otherwise insert one row with the geometry in §1. + +No schema change. No `WatermarkSelector` change. No change to how watermarks render. + +**Additive DTO change**: `WatermarkResponseModel` (the `GET /api/v1/watermarks` picker list) gains +`imageSource`, so the SPA can identify logo-driven presets generically instead of matching a +user-editable name. Legal under frozen-additive `/api/v1`; also useful to MCP/machine clients. + +## 4. Frontend + +- **``** — new shared presentational component: a 16:9 frame with the image absolutely + positioned from `location` (9 values, `ErsatzTV.FFmpeg/State/WatermarkState.cs`), `size`, + `widthPercent`, horizontal/vertical margin percents and `opacity`. No data fetching. +- **Where the default is applied — at creation, not by inferring "newness" in the editor.** Quick-add + creates the channel through the API and *then* navigates to `/app/edit-channel/{id}` + (`docs/decisions.md:445-456`), so the editor only ever loads an already-persisted row and cannot + reliably tell "new" from "existing". The default is therefore applied by the SPA's creation paths — + `ChannelsScreen.tsx:377` (quick-add, currently hardcoding `watermarkId: null`) and the + `ChannelBuilder` lineup flow — which set `watermarkId` to the seeded preset at create time. +- **`ChannelEditScreen`** (Branding tab, `:55`): a **"Use logo as on-screen bug"** toggle that purely + reflects and edits the stored `watermarkId` — ticked when it points at a `ChannelLogo`-sourced + preset, unticked when `null`. Toggling writes the preset id or `null`. Because it is a pure + reflection of stored state, loading and re-saving an existing channel never changes its watermark. + A `` beneath it is fed by the draft logo (local object URL or + `draft.logo.urlWithContentType`), so the bug is visible before saving. +- **Batch auto-tune (`POST /api/v1/channels/auto-tune`) is left alone** — it is a server-side create, + and the rejected "server-side default on create" option is what would have changed it. +- **`WatermarksScreen`**: the raw `` at `:534-539` is replaced by ``. + +Existing logo-upload and external-URL-precedence behavior (`docs/decisions.md:457-466`) is unchanged. + +## 5. Testing + +- **NUnit**, mirroring `ErsatzTV.Tests/Infrastructure/DbInitializerChannelTemplateTests.cs`: + seeds the preset when absent; **adopts an existing same-name row without duplicating or mutating + it** (the production case — this test is what protects the operator's tuned geometry); is + idempotent across repeated `Initialize` calls. +- **Vitest**: `BugPreview` geometry mapping across all 9 locations, plus opacity/size/margins; the + toggle's `watermarkId` wiring, including that loading and re-saving an existing channel without + touching the toggle leaves `watermarkId` unchanged, and that the creation paths stamp the preset. + +## 6. Docs to update in the same PR + +Per `CLAUDE.md`'s docs-update rule: + +- `docs/api-conventions.md` checklist + regenerate `v1.json` and `endpoint-index.md` via + `./scripts/update-openapi.sh`, then `npm run generate:api` (the `imageSource` addition). +- `docs/channels.md` — watermark section: the seeded `Channel Bug` preset and the logo-drives-bug default. +- `docs/decisions.md` — the shared-preset-over-migration decision and its accepted trade-off. +- `docs/spa-conventions.md` — only if `` establishes a new shared-component convention. + +## 7. Out of scope + +- Any dual-provider migration. +- The dead generated-initials FFmpeg fallback: `WatermarkSelector.cs:230-236` runs `File.Exists` + against a URL produced by `ChannelLogoGenerator.GenerateChannelLogoUrl`, which hardcodes localhost + (`ChannelLogoGenerator.cs:93-94`, issue #1). Deliberately deferred in `docs/decisions.md:1617-1623`. +- The initials shown at `/app/channels` on production: **not a defect.** That is fixed on `main` by + `1aa1f0cf` ("fix(464): render real channel logos in guide grid + channels list", 2026-07-19 13:28), + which postdates the deployed image (built 2026-07-18 22:38, i.e. tag `v26.10.0`). It resolves on + the pending v26.11.0 deploy — the same deploy #350 and #474 are waiting on. -- 2.47.3 From 9ff5580d498547668031849f3993fd630b4ed29e Mon Sep 17 00:00:00 2001 From: Timothy Date: Mon, 20 Jul 2026 19:48:55 +0200 Subject: [PATCH 02/13] docs(67): implementation plan + spec correction (ChannelBuilder inherits from template, excluded) Self-review against the spec caught that the spec named both SPA creation paths, but ChannelBuilder does not hardcode a watermark -- it inherits watermarkId from the selected ChannelTemplate. Defaulting it would mean mutating the seeded Standard/Music videos template rows, which the seed's name-guard deliberately skips on existing installs, so it would no-op on prod while changing fresh installs. Excluded in both documents with the reason recorded. Refs #67 --- .../plans/2026-07-20-unified-logo-bug.md | 945 ++++++++++++++++++ .../2026-07-20-unified-logo-bug-design.md | 12 +- 2 files changed, 954 insertions(+), 3 deletions(-) create mode 100644 docs/superpowers/plans/2026-07-20-unified-logo-bug.md diff --git a/docs/superpowers/plans/2026-07-20-unified-logo-bug.md b/docs/superpowers/plans/2026-07-20-unified-logo-bug.md new file mode 100644 index 000000000..054cf3d3b --- /dev/null +++ b/docs/superpowers/plans/2026-07-20-unified-logo-bug.md @@ -0,0 +1,945 @@ +# Unified logo / on-screen bug (ersatztv#67) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** A single uploaded channel logo drives both the guide listing and the on-screen bug by default, with the geometry visible before saving. + +**Architecture:** No schema change. `ChannelWatermarkImageSource.ChannelLogo` already resolves each channel's own logo artwork at render time, so we seed ONE shared `Channel Bug` watermark preset idempotently in `DbInitializer`, point the SPA's channel-creation paths at it, and add a shared `` component that renders the resolved geometry in both the channel editor and the watermarks screen. One additive DTO field (`WatermarkResponseModel.imageSource`) lets the SPA find logo-driven presets without matching a user-editable name. + +**Tech Stack:** C# / .NET 10, EF Core (dual-provider, but **no migration here**), MediatR CQRS, NUnit + Shouldly + `InMemoryTvContext`; React 19 + TypeScript + Vite, vitest + @testing-library/react. + +**Spec:** `docs/superpowers/specs/2026-07-20-unified-logo-bug-design.md` + +## Global Constraints + +- **Worktree:** `/Users/timothy/.claude/worktrees/etv67`, branch `feat/67-unified-logo-bug`, branched off `origin/main`. Never commit in `/Users/timothy/ersatztv` (shared mutable tree). +- **Test framework is NUnit + Shouldly + NSubstitute.** Never xUnit. +- **`/api/v1` is frozen-additive** (`docs/decisions.md`, #286): add fields, never remove or reshape. +- **No database migration.** If you find yourself running `scripts/add-migration.sh`, stop — the design explicitly rejected that path. +- **BOM gate (#311, fix-as-you-touch).** Two files this plan modifies currently carry a UTF-8 BOM and MUST be written back without one: `ErsatzTV.Infrastructure/Data/DbInitializer.cs` and `ErsatzTV.Application/Watermarks/Mapper.cs`. Check before every push: + ```bash + for f in $(git diff --name-only origin/main...HEAD -- '*.cs'); do head -c3 "$f" | xxd -p | grep -q "^efbbbf" && echo "BOM: $f"; done + ``` +- **Never set `ETV_UPDATE_GOLDENS`.** +- **Exact seed values** (adopted verbatim from the operator's proven production row — do not "improve" them): + `Name="Channel Bug"`, `Mode=ChannelWatermarkMode.Permanent`, `ImageSource=ChannelWatermarkImageSource.ChannelLogo`, `Image=null`, `Location=WatermarkLocation.TopLeft`, `Size=WatermarkSize.Scaled`, `WidthPercent=5.0`, `HorizontalMarginPercent=1.0`, `VerticalMarginPercent=1.0`, `FrequencyMinutes=0`, `DurationSeconds=0`, `Opacity=80`, `PlaceWithinSourceContent=false`, `ZIndex=0`. +- **SPA field names differ from the C# ones** — the wire DTO uses `width`, `horizontalMargin`, `verticalMargin` (NOT `widthPercent`/`*Percent`). Use the wire names in all TypeScript. + +**Verification gate (run before every commit):** +```bash +# backend, from the worktree root +dotnet build ErsatzTV.sln +dotnet test ErsatzTV.Tests/ErsatzTV.Tests.csproj +# frontend, from web/ +npm test && npm run lint && npm run typecheck && npm run build +``` + +--- + +### Task 1: Seed the shared `Channel Bug` watermark preset + +**Files:** +- Modify: `ErsatzTV.Infrastructure/Data/DbInitializer.cs:134` (add the call) and append a new private method after `SeedChannelTemplates` (which ends at `:186`) +- Test: `ErsatzTV.Tests/Infrastructure/DbInitializerChannelBugWatermarkTests.cs` (create) + +**Interfaces:** +- Consumes: `TvContext.ChannelWatermarks` (`ErsatzTV.Infrastructure/Data/TvContext.cs:44`), `ChannelWatermark` (`ErsatzTV.Core/Domain/ChannelWatermark.cs:6-36`), enums `ChannelWatermarkMode` / `ChannelWatermarkImageSource` (same file, `:38-52`), `WatermarkLocation` / `WatermarkSize` (`ErsatzTV.FFmpeg/State/WatermarkState.cs`). +- Produces: a `ChannelWatermark` row named exactly `Channel Bug` with `ImageSource = ChannelLogo`. Tasks 5 and 6 locate it by `imageSource`, not by name. + +- [ ] **Step 1: Write the failing tests** + +Create `ErsatzTV.Tests/Infrastructure/DbInitializerChannelBugWatermarkTests.cs`: + +```csharp +using ErsatzTV.Core.Domain; +using ErsatzTV.FFmpeg.State; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Infrastructure; + +[TestFixture] +public class DbInitializerChannelBugWatermarkTests +{ + private InMemoryTvContext _db = null!; + + [SetUp] + public async Task SetUp() => _db = await InMemoryTvContext.CreateAsync(); + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + [Test] + public async Task Initialize_Should_Seed_Channel_Bug_Watermark() + { + await using TvContext context = _db.CreateContext(); + + await DbInitializer.Initialize(context, CancellationToken.None); + + ChannelWatermark watermark = context.ChannelWatermarks.Single(w => w.Name == "Channel Bug"); + watermark.Mode.ShouldBe(ChannelWatermarkMode.Permanent); + watermark.ImageSource.ShouldBe(ChannelWatermarkImageSource.ChannelLogo); + watermark.Image.ShouldBeNull(); + watermark.Location.ShouldBe(WatermarkLocation.TopLeft); + watermark.Size.ShouldBe(WatermarkSize.Scaled); + watermark.WidthPercent.ShouldBe(5.0); + watermark.HorizontalMarginPercent.ShouldBe(1.0); + watermark.VerticalMarginPercent.ShouldBe(1.0); + watermark.Opacity.ShouldBe(80); + watermark.ZIndex.ShouldBe(0); + watermark.PlaceWithinSourceContent.ShouldBeFalse(); + } + + // The production instance already has a hand-made "Channel Bug" row with tuned geometry. + // Seeding must adopt it untouched, never overwrite it and never duplicate it. + [Test] + public async Task Initialize_Should_Adopt_Existing_Channel_Bug_Watermark_Untouched() + { + await using TvContext context = _db.CreateContext(); + await context.ChannelWatermarks.AddAsync( + new ChannelWatermark + { + Name = "Channel Bug", + Mode = ChannelWatermarkMode.Intermittent, + ImageSource = ChannelWatermarkImageSource.ChannelLogo, + Location = WatermarkLocation.BottomRight, + Size = WatermarkSize.Scaled, + WidthPercent = 12, + HorizontalMarginPercent = 3, + VerticalMarginPercent = 4, + Opacity = 55, + FrequencyMinutes = 10, + DurationSeconds = 20 + }); + await context.SaveChangesAsync(); + + await DbInitializer.Initialize(context, CancellationToken.None); + + ChannelWatermark watermark = context.ChannelWatermarks.Single(w => w.Name == "Channel Bug"); + watermark.Mode.ShouldBe(ChannelWatermarkMode.Intermittent); + watermark.Location.ShouldBe(WatermarkLocation.BottomRight); + watermark.WidthPercent.ShouldBe(12); + watermark.Opacity.ShouldBe(55); + } + + [Test] + public async Task Initialize_Should_Be_Idempotent() + { + await using TvContext context = _db.CreateContext(); + + await DbInitializer.Initialize(context, CancellationToken.None); + await DbInitializer.Initialize(context, CancellationToken.None); + + context.ChannelWatermarks.Count(w => w.Name == "Channel Bug").ShouldBe(1); + } +} +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +```bash +dotnet test ErsatzTV.Tests/ErsatzTV.Tests.csproj --filter FullyQualifiedName~DbInitializerChannelBugWatermarkTests +``` +Expected: FAIL — `Sequence contains no elements` from `.Single(...)` (no seeded row yet). + +- [ ] **Step 3: Implement the seed** + +In `ErsatzTV.Infrastructure/Data/DbInitializer.cs`, add the call immediately after the existing `SeedChannelTemplates` call at `:134`: + +```csharp + await SeedChannelTemplates(context, cancellationToken); + await SeedChannelBugWatermark(context, cancellationToken); +``` + +Then add this private method directly after `SeedChannelTemplates` (which ends at `:186`): + +```csharp + // A single shared preset is all that's needed: ImageSource.ChannelLogo resolves each channel's + // own logo artwork at render time (WatermarkSelector), so one row makes every channel use its + // own logo as its on-screen bug. Name-guarded so an existing hand-made row keeps its tuned + // geometry — adopt, never overwrite. + private static async Task SeedChannelBugWatermark(TvContext context, CancellationToken cancellationToken) + { + if (await context.ChannelWatermarks.AnyAsync(w => w.Name == "Channel Bug", cancellationToken)) + { + return; + } + + await context.ChannelWatermarks.AddAsync( + new ChannelWatermark + { + Name = "Channel Bug", + Mode = ChannelWatermarkMode.Permanent, + ImageSource = ChannelWatermarkImageSource.ChannelLogo, + Image = null, + Location = WatermarkLocation.TopLeft, + Size = WatermarkSize.Scaled, + WidthPercent = 5.0, + HorizontalMarginPercent = 1.0, + VerticalMarginPercent = 1.0, + FrequencyMinutes = 0, + DurationSeconds = 0, + Opacity = 80, + PlaceWithinSourceContent = false, + ZIndex = 0 + }, + cancellationToken); + + await context.SaveChangesAsync(cancellationToken); + } +``` + +Add `using ErsatzTV.FFmpeg.State;` to the file's usings if `WatermarkLocation`/`WatermarkSize` do not already resolve. + +- [ ] **Step 4: Run the tests to verify they pass** + +```bash +dotnet test ErsatzTV.Tests/ErsatzTV.Tests.csproj --filter FullyQualifiedName~DbInitializerChannelBugWatermarkTests +``` +Expected: PASS, 3 tests. Confirm the build output shows `Build succeeded` / `0 Error(s)` — a failed build with `--no-build` elsewhere can silently run a stale dll. + +- [ ] **Step 5: Strip the BOM and commit** + +```bash +python3 - <<'EOF' +p = 'ErsatzTV.Infrastructure/Data/DbInitializer.cs' +b = open(p, 'rb').read() +if b.startswith(b'\xef\xbb\xbf'): + open(p, 'wb').write(b[3:]) + print('stripped', p) +EOF +head -c3 ErsatzTV.Infrastructure/Data/DbInitializer.cs | xxd -p # must NOT be efbbbf +git add ErsatzTV.Infrastructure/Data/DbInitializer.cs ErsatzTV.Tests/Infrastructure/DbInitializerChannelBugWatermarkTests.cs +git commit -m "feat(67): seed shared Channel Bug watermark preset (adopts an existing row untouched)" +``` + +--- + +### Task 2: Expose `imageSource` on the watermark picker DTO + +**Files:** +- Modify: `ErsatzTV.Core/Api/Watermarks/WatermarkResponseModel.cs` (whole file, 4 lines) +- Modify: `ErsatzTV.Application/Watermarks/Mapper.cs:9-10` (`ProjectToResponseModel`) +- Modify (generated, do not hand-edit): `ErsatzTV/wwwroot/openapi/v1.json`, `docs/endpoint-index.md`, `web/src/api/generated/v1.d.ts` +- Test: `ErsatzTV.Tests/Application/Watermarks/WatermarkMapperTests.cs` (create) + +**Interfaces:** +- Consumes: `ChannelWatermark.ImageSource`. +- Produces: `WatermarkResponseModel(int Id, string Name, ChannelWatermarkImageSource ImageSource)`, surfacing as `Watermark.imageSource` (`"Custom" | "ChannelLogo" | "Resource"`) in `web/src/api/pickers.ts`. Tasks 5 and 6 consume this. + +- [ ] **Step 1: Write the failing test** + +Create `ErsatzTV.Tests/Application/Watermarks/WatermarkMapperTests.cs`: + +```csharp +using ErsatzTV.Core.Api.Watermarks; +using ErsatzTV.Core.Domain; +using NUnit.Framework; +using Shouldly; +using static ErsatzTV.Application.Watermarks.Mapper; + +namespace ErsatzTV.Tests.Application.Watermarks; + +[TestFixture] +public class WatermarkMapperTests +{ + [Test] + public void ProjectToResponseModel_Should_Carry_ImageSource() + { + var watermark = new ChannelWatermark + { + Id = 7, + Name = "Channel Bug", + ImageSource = ChannelWatermarkImageSource.ChannelLogo + }; + + WatermarkResponseModel result = ProjectToResponseModel(watermark); + + result.Id.ShouldBe(7); + result.Name.ShouldBe("Channel Bug"); + result.ImageSource.ShouldBe(ChannelWatermarkImageSource.ChannelLogo); + } +} +``` + +`Mapper` is `internal static`, so `ErsatzTV.Application` must expose internals to the test project. Check first: + +```bash +grep -rn "InternalsVisibleTo" ErsatzTV.Application/ +``` +If `ErsatzTV.Tests` is not already listed, do NOT add an `InternalsVisibleTo` — instead assert the same behavior through the public query handler by writing the test against `GetAllWatermarksForApiHandler` with an `InMemoryTvContext` (same harness as Task 1), seeding one `ChannelWatermark` with `ImageSource = ChannelLogo` and asserting the returned `List` single item carries it. + +- [ ] **Step 2: Run the test to verify it fails** + +```bash +dotnet test ErsatzTV.Tests/ErsatzTV.Tests.csproj --filter FullyQualifiedName~WatermarkMapperTests +``` +Expected: FAIL to compile — `WatermarkResponseModel` does not contain a definition for `ImageSource`. + +- [ ] **Step 3: Add the field** + +`ErsatzTV.Core/Api/Watermarks/WatermarkResponseModel.cs` becomes: + +```csharp +#nullable enable +using ErsatzTV.Core.Domain; + +namespace ErsatzTV.Core.Api.Watermarks; + +// ImageSource lets a client identify logo-driven presets (the seeded "Channel Bug") without +// matching a user-editable name. Additive under the frozen /api/v1 contract (#286). +public record WatermarkResponseModel(int Id, string Name, ChannelWatermarkImageSource ImageSource); +``` + +`ErsatzTV.Application/Watermarks/Mapper.cs:9-10` becomes: + +```csharp + internal static WatermarkResponseModel ProjectToResponseModel(ChannelWatermark watermark) => + new(watermark.Id, watermark.Name, watermark.ImageSource); +``` + +- [ ] **Step 4: Run the test to verify it passes** + +```bash +dotnet test ErsatzTV.Tests/ErsatzTV.Tests.csproj --filter FullyQualifiedName~WatermarkMapperTests +``` +Expected: PASS. + +- [ ] **Step 5: Regenerate the OpenAPI artifacts — build FIRST** + +Order matters: `update-openapi.sh` runs `dotnet-getdocument` against the already-built `ErsatzTV.dll`, so a stale assembly silently serializes the old schema (`docs/api-conventions.md:308-312`). + +```bash +dotnet build ErsatzTV.sln +./scripts/update-openapi.sh +cd web && npm run generate:api && cd .. +git diff --stat -- ErsatzTV/wwwroot/openapi/v1.json docs/endpoint-index.md web/src/api/generated/v1.d.ts +``` +Expected: `v1.d.ts` now shows `"WatermarkResponseModel": { "id": number; "name": string; "imageSource": components["schemas"]["ChannelWatermarkImageSource"]; }`. + +- [ ] **Step 6: Verify the generated artifacts are in sync** + +```bash +cd web && npm run check:api && cd .. +``` +Expected: exit 0, no diff. + +- [ ] **Step 7: Strip the BOM and commit** + +```bash +python3 - <<'EOF' +p = 'ErsatzTV.Application/Watermarks/Mapper.cs' +b = open(p, 'rb').read() +if b.startswith(b'\xef\xbb\xbf'): + open(p, 'wb').write(b[3:]) + print('stripped', p) +EOF +head -c3 ErsatzTV.Application/Watermarks/Mapper.cs | xxd -p # must NOT be efbbbf +git add -A +git commit -m "feat(67): add imageSource to the watermark picker DTO (additive)" +``` + +--- + +### Task 3: The shared `` component + +**Files:** +- Create: `web/src/components/bugPreview.tsx` +- Create: `web/src/components/bugPreview.test.tsx` +- Modify: `web/src/components/index.ts` (add the re-export — check the file's existing export style first and match it) + +**Interfaces:** +- Produces, consumed by Tasks 4 and 5: + ```ts + export interface BugPreviewGeometry { + location: components['schemas']['WatermarkLocation']; + size: components['schemas']['WatermarkSize']; + width: number; // percent of frame width, used when size === 'Scaled' + horizontalMargin: number; // percent + verticalMargin: number; // percent + opacity: number; // 0-100 + } + export function bugPreviewStyle(geometry: BugPreviewGeometry): CSSProperties; + export function BugPreview(props: BugPreviewGeometry & { src: string | null; alt?: string }): JSX.Element; + ``` + The geometry math lives in the pure `bugPreviewStyle` so it is unit-testable without the DOM. + +- [ ] **Step 1: Write the failing test** + +Create `web/src/components/bugPreview.test.tsx`: + +```tsx +import { cleanup, render, screen } from '@testing-library/react'; +import { afterEach, describe, expect, it } from 'vitest'; +import { BugPreview, bugPreviewStyle, type BugPreviewGeometry } from './bugPreview'; + +const base: BugPreviewGeometry = { + horizontalMargin: 1, + location: 'TopLeft', + opacity: 80, + size: 'Scaled', + verticalMargin: 1, + width: 5 +}; + +describe('bugPreviewStyle', () => { + it('anchors TopLeft using the margins', () => { + const style = bugPreviewStyle(base); + expect(style.top).toBe('1%'); + expect(style.left).toBe('1%'); + expect(style.bottom).toBeUndefined(); + expect(style.right).toBeUndefined(); + }); + + it('anchors BottomRight using the margins', () => { + const style = bugPreviewStyle({ ...base, location: 'BottomRight' }); + expect(style.bottom).toBe('1%'); + expect(style.right).toBe('1%'); + expect(style.top).toBeUndefined(); + expect(style.left).toBeUndefined(); + }); + + it('centers horizontally for TopMiddle', () => { + const style = bugPreviewStyle({ ...base, location: 'TopMiddle' }); + expect(style.top).toBe('1%'); + expect(style.left).toBe('50%'); + expect(style.transform).toBe('translateX(-50%)'); + }); + + it('centers vertically for LeftMiddle', () => { + const style = bugPreviewStyle({ ...base, location: 'LeftMiddle' }); + expect(style.left).toBe('1%'); + expect(style.top).toBe('50%'); + expect(style.transform).toBe('translateY(-50%)'); + }); + + it('centers both axes for MiddleCenter', () => { + const style = bugPreviewStyle({ ...base, location: 'MiddleCenter' }); + expect(style.transform).toBe('translate(-50%, -50%)'); + }); + + it('scales width by percent when size is Scaled', () => { + expect(bugPreviewStyle({ ...base, width: 12 }).width).toBe('12%'); + }); + + it('leaves width unset at ActualSize', () => { + expect(bugPreviewStyle({ ...base, size: 'ActualSize' }).width).toBeUndefined(); + }); + + it('maps opacity percent to a 0-1 fraction', () => { + expect(bugPreviewStyle({ ...base, opacity: 80 }).opacity).toBeCloseTo(0.8); + }); +}); + +describe('BugPreview', () => { + afterEach(cleanup); + + it('renders the image when a src is supplied', () => { + render(); + expect(screen.getByAltText('Bug preview')).toHaveAttribute('src', '/iptv/logos/abc'); + }); + + it('renders an empty frame without a src', () => { + render(); + expect(screen.queryByRole('img')).toBeNull(); + }); +}); +``` + +- [ ] **Step 2: Run the test to verify it fails** + +```bash +cd web && npx vitest run src/components/bugPreview.test.tsx +``` +Expected: FAIL — cannot resolve `./bugPreview`. + +- [ ] **Step 3: Implement the component** + +Create `web/src/components/bugPreview.tsx`: + +```tsx +import type { CSSProperties } from 'react'; +import type { components } from '../api/generated/v1'; + +// Renders where the on-screen bug lands, using the same percent-of-frame geometry the FFmpeg +// watermark filters use (WatermarkScaleFilter scales to widthPercent of output width; margins are +// percentages). This is an approximation for editing feedback, not a frame-accurate render. +export interface BugPreviewGeometry { + horizontalMargin: number; + location: components['schemas']['WatermarkLocation']; + opacity: number; + size: components['schemas']['WatermarkSize']; + verticalMargin: number; + width: number; +} + +export function bugPreviewStyle(geometry: BugPreviewGeometry): CSSProperties { + const horizontal = `${geometry.horizontalMargin}%`; + const vertical = `${geometry.verticalMargin}%`; + + const placement: CSSProperties = (() => { + switch (geometry.location) { + case 'BottomLeft': + return { bottom: vertical, left: horizontal }; + case 'BottomMiddle': + return { bottom: vertical, left: '50%', transform: 'translateX(-50%)' }; + case 'BottomRight': + return { bottom: vertical, right: horizontal }; + case 'LeftMiddle': + return { left: horizontal, top: '50%', transform: 'translateY(-50%)' }; + case 'MiddleCenter': + return { left: '50%', top: '50%', transform: 'translate(-50%, -50%)' }; + case 'RightMiddle': + return { right: horizontal, top: '50%', transform: 'translateY(-50%)' }; + case 'TopLeft': + return { left: horizontal, top: vertical }; + case 'TopMiddle': + return { left: '50%', top: vertical, transform: 'translateX(-50%)' }; + case 'TopRight': + return { right: horizontal, top: vertical }; + default: + return { left: horizontal, top: vertical }; + } + })(); + + return { + ...placement, + objectFit: 'contain', + opacity: geometry.opacity / 100, + position: 'absolute', + width: geometry.size === 'Scaled' ? `${geometry.width}%` : undefined + }; +} + +export function BugPreview({ + alt = 'On-screen bug preview', + src, + ...geometry +}: BugPreviewGeometry & { alt?: string; src: null | string }) { + return ( +
+ {src && {alt}} +
+ ); +} +``` + +Then re-export it from `web/src/components/index.ts`, matching that file's existing export style (inspect it first — do not guess between `export *` and named re-exports). + +- [ ] **Step 4: Run the test to verify it passes** + +```bash +cd web && npx vitest run src/components/bugPreview.test.tsx +``` +Expected: PASS, 10 tests. + +- [ ] **Step 5: Commit** + +```bash +git add web/src/components/bugPreview.tsx web/src/components/bugPreview.test.tsx web/src/components/index.ts +git commit -m "feat(67): add shared BugPreview component with pure geometry helper" +``` + +--- + +### Task 4: Use `` on the Watermarks screen + +**Files:** +- Modify: `web/src/screens/WatermarksScreen.tsx:511-548` (the "Image" Row's raw `` block) +- Test: `web/src/screens/WatermarksScreen.test.tsx` (extend) + +**Interfaces:** +- Consumes: `BugPreview` from Task 3; the existing `Draft` (= `CreateWatermarkRequest`) fields `image`, `imageSource`, `location`, `size`, `width`, `horizontalMargin`, `verticalMargin`, `opacity`; `watermarkImageUrl` (already imported). + +- [ ] **Step 1: Write the failing test** + +Append to the `describe('WatermarksScreen', ...)` block in `web/src/screens/WatermarksScreen.test.tsx`: + +```tsx + it('renders a geometry preview of the watermark image in the editor', async () => { + window.history.pushState({}, '', '/app/watermarks/3'); + mockApi(); + render(); + + const preview = await screen.findByAltText('On-screen bug preview'); + expect(preview).toHaveAttribute('src', '/artwork/watermarks/logo.png'); + // BottomRight at 5% margins, 15% width, 100% opacity (see watermarkDetail fixture) + expect(preview).toHaveStyle({ bottom: '5%', right: '5%', width: '15%' }); + }); +``` + +If `/app/watermarks/3` is not the edit route, read the screen's routing block and use the real edit path — do not invent one. + +- [ ] **Step 2: Run the test to verify it fails** + +```bash +cd web && npx vitest run src/screens/WatermarksScreen.test.tsx +``` +Expected: FAIL — `Unable to find an element with the alt text: On-screen bug preview`. + +- [ ] **Step 3: Replace the raw ``** + +Add `BugPreview` to the existing import from `'../components'` (line 5). Then replace exactly this block (`web/src/screens/WatermarksScreen.tsx`, inside the "Image" Row): + +```tsx + {custom && draft.image && ( + Watermark preview + )} +``` + +with: + +```tsx + {custom && draft.image && ( + + )} +``` + +- [ ] **Step 4: Run the test to verify it passes** + +```bash +cd web && npx vitest run src/screens/WatermarksScreen.test.tsx +``` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add web/src/screens/WatermarksScreen.tsx web/src/screens/WatermarksScreen.test.tsx +git commit -m "feat(67): show resolved bug geometry on the watermarks screen" +``` + +--- + +### Task 5: The "Use logo as on-screen bug" toggle in the channel editor + +**Files:** +- Modify: `web/src/screens/ChannelEditScreen.tsx` — `BrandingPane` (`:646-760`) +- Test: `web/src/screens/ChannelEditScreen.test.tsx` (extend) + +**Interfaces:** +- Consumes: `BugPreview` (Task 3); `Watermark.imageSource` (Task 2); existing `data.watermarks`, `draft.watermarkId`, `previewSrc`, `set()`. +- Behavior contract: the toggle is a **pure reflection of stored state** — ticked iff `draft.watermarkId` points at a watermark whose `imageSource === 'ChannelLogo'`. Loading and re-saving a channel without touching the toggle must leave `watermarkId` byte-identical. The existing watermark `` stays — the toggle is a shortcut, not a replacement. +- Consumes: `BugPreview` (Task 3); `Watermark.imageSource` (Task 2); `getWatermark(id)` from `web/src/api/watermarks.ts`; existing `data.watermarks`, `draft.watermarkId`, `previewSrc`, `set()`. +- Produces, also used by Task 6 — add to `web/src/api/watermarks.ts` and export from the `'../api'` barrel: + ```ts + export function findLogoBugWatermark( + watermarks: T[] + ): T | null; + ``` + +**Behavior contract (read carefully — the first draft of this plan got this wrong):** +- **Ticked** iff the watermark that `draft.watermarkId` *points at* has `imageSource === 'ChannelLogo'`. Resolve it by **id lookup**, never by searching the list for the first `ChannelLogo` entry: `getWatermarks()` sorts by name (`web/src/api/pickers.ts:14`), so a search silently binds to the alphabetically-first logo-driven preset. Since the design explicitly invites users to create a second preset for per-channel geometry, that search would (a) show the toggle OFF for a channel that *is* logo-driven and (b) **repoint the channel** on tick — a data-changing action from a control documented as a pure reflection. +- **Tick-on target** is `findLogoBugWatermark(...)`: prefer the preset named exactly `Channel Bug`, else the first `ChannelLogo` preset, else `null`. With several logo-driven presets `imageSource` alone cannot identify *the* default, so the name is a deterministic tiebreak — not the identification mechanism. +- Loading and re-saving a channel without touching the toggle must leave `watermarkId` **byte-identical**, including when it points at a non-`ChannelLogo` watermark or at a `ChannelLogo` preset that is not the seeded one. +- The existing watermark ` Date: Mon, 20 Jul 2026 21:02:16 +0200 Subject: [PATCH 11/13] fix(67): cover the bug-preview geometry path, preserve a custom logo preset across toggling - Add ChannelEditScreen tests exercising the geometry-fetch/preview path (previously untested because the fixture's blank logo path kept BugPreview from ever rendering): asserts the fetched watermark geometry lands on the preview image with asymmetric values, and that an external logo URL suppresses the preview. - Fix a toggle-off/toggle-on data loss: re-enabling "use logo as on-screen bug" used to always rebind to the default 'Channel Bug' preset, discarding a channel's own per-geometry ChannelLogo preset. Remember the last referenced logo-driven watermark id in a ref and prefer it, falling back to the shared default. - Strip the geometry cache's `id` key before it reaches BugPreview's props (was leaking via a spread). - Drop the "(see #502)" issue reference from the visible help text; keep it in the code comment. - Constrain findLogoBugWatermark's imageSource param to the generated ChannelWatermarkImageSource union instead of `string`, and add unit tests for its preference/fallback/none-found behavior. Co-Authored-By: Claude Opus 4.8 (1M context) --- web/src/api/watermarks.test.ts | 31 ++++++ web/src/api/watermarks.ts | 6 +- web/src/screens/ChannelEditScreen.test.tsx | 104 +++++++++++++++++++++ web/src/screens/ChannelEditScreen.tsx | 31 +++++- 4 files changed, 166 insertions(+), 6 deletions(-) diff --git a/web/src/api/watermarks.test.ts b/web/src/api/watermarks.test.ts index b5ee2d4b5..61095d561 100644 --- a/web/src/api/watermarks.test.ts +++ b/web/src/api/watermarks.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { createWatermark, deleteWatermark, + findLogoBugWatermark, getWatermark, updateWatermark, watermarkImageUrl, @@ -84,3 +85,33 @@ describe('watermarks api client', () => { expect(watermarkImageUrl('abc.png')).toBe('/artwork/watermarks/abc.png'); }); }); + +describe('findLogoBugWatermark', () => { + it('prefers the row named "Channel Bug" over an earlier ChannelLogo row', () => { + const watermarks = [ + { id: 4, imageSource: 'ChannelLogo' as const, name: 'Alpha Bug' }, + { id: 9, imageSource: 'ChannelLogo' as const, name: 'Channel Bug' } + ]; + + expect(findLogoBugWatermark(watermarks)).toMatchObject({ id: 9 }); + }); + + it('falls back to the first ChannelLogo row when no "Channel Bug" exists', () => { + const watermarks = [ + { id: 2, imageSource: 'Custom' as const, name: 'Corner bug' }, + { id: 4, imageSource: 'ChannelLogo' as const, name: 'Alpha Bug' }, + { id: 6, imageSource: 'ChannelLogo' as const, name: 'Beta Bug' } + ]; + + expect(findLogoBugWatermark(watermarks)).toMatchObject({ id: 4 }); + }); + + it('returns null when none are ChannelLogo', () => { + const watermarks = [ + { id: 2, imageSource: 'Custom' as const, name: 'Corner bug' }, + { id: 3, imageSource: 'Resource' as const, name: 'Station ident' } + ]; + + expect(findLogoBugWatermark(watermarks)).toBeNull(); + }); +}); diff --git a/web/src/api/watermarks.ts b/web/src/api/watermarks.ts index b347c5e41..4d3c54c5b 100644 --- a/web/src/api/watermarks.ts +++ b/web/src/api/watermarks.ts @@ -16,9 +16,9 @@ export function getWatermark(id: number): Promise { // Identifies THE default logo-driven preset. imageSource alone is not enough once a second // ChannelLogo preset exists (the design invites creating one for per-channel geometry), and // getWatermarks() sorts by name — so prefer the seeded name, then fall back deterministically. -export function findLogoBugWatermark( - watermarks: T[] -): T | null { +export function findLogoBugWatermark< + T extends { id: number; imageSource: components['schemas']['ChannelWatermarkImageSource']; name: null | string } +>(watermarks: T[]): T | null { const logoDriven = watermarks.filter((watermark) => watermark.imageSource === 'ChannelLogo'); return logoDriven.find((watermark) => watermark.name === 'Channel Bug') ?? logoDriven[0] ?? null; } diff --git a/web/src/screens/ChannelEditScreen.test.tsx b/web/src/screens/ChannelEditScreen.test.tsx index 98947ed47..74a6e3b75 100644 --- a/web/src/screens/ChannelEditScreen.test.tsx +++ b/web/src/screens/ChannelEditScreen.test.tsx @@ -471,5 +471,109 @@ describe('ChannelEditScreen', () => { await waitFor(() => expect(puts).toHaveLength(1)); expect((puts[0] as { watermarkId: null | number }).watermarkId).toBe(2); }); + + // Regression: toggling off (watermarkId -> null) then back on used to always rebind to + // logoBugTarget (the 'Channel Bug' default), silently losing a channel's own per-geometry + // ChannelLogo preset. + it('restores the channel’s own ChannelLogo preset — not the default — when re-toggled on', async () => { + const puts: unknown[] = []; + mockApi({ + channelOverrides: { watermarkId: 4 }, + onPut: (body) => puts.push(body), + watermarks: [ + { id: 4, imageSource: 'ChannelLogo', name: 'Alpha Bug' }, + { id: 9, imageSource: 'ChannelLogo', name: 'Channel Bug' } + ] + }); + render(); + + await screen.findByDisplayValue('Cartoons'); + fireEvent.click(screen.getByRole('button', { name: /^Branding/ })); + + const toggle = await screen.findByRole('switch', { name: 'Use logo as on-screen bug' }); + expect(toggle).toHaveAttribute('aria-checked', 'true'); + + fireEvent.click(toggle); + expect(toggle).toHaveAttribute('aria-checked', 'false'); + + fireEvent.click(toggle); + expect(toggle).toHaveAttribute('aria-checked', 'true'); + + // watermarkId round-trips back to its original value across the off/on toggle, so the form + // isn't dirty from that alone — touch something unrelated to enable Save. + fireEvent.click(screen.getByRole('button', { name: /^General/ })); + fireEvent.change(screen.getByDisplayValue('Cartoons'), { target: { value: 'Renamed channel' } }); + + const saveButton = await screen.findByRole('button', { name: 'Save changes' }); + fireEvent.click(saveButton); + + await waitFor(() => expect(puts).toHaveLength(1)); + expect((puts[0] as { watermarkId: null | number }).watermarkId).toBe(4); + }); + }); + + describe('on-screen bug preview', () => { + it('renders the preview image using the fetched watermark geometry', async () => { + mockApi({ + channelOverrides: { + logo: { path: 'iptv/logos/cartoons.png', contentType: 'image/png' }, + watermarkId: 9 + }, + watermarks: [ + { id: 2, imageSource: 'Custom', name: 'Corner bug' }, + { + id: 9, + imageSource: 'ChannelLogo', + name: 'Channel Bug', + horizontalMargin: 3, + location: 'BottomLeft', + opacity: 60, + size: 'Scaled', + verticalMargin: 7, + width: 11 + } + ] + }); + render(); + + await screen.findByDisplayValue('Cartoons'); + fireEvent.click(screen.getByRole('button', { name: /^Branding/ })); + + const preview = await screen.findByAltText('On-screen bug preview'); + // Asymmetric values so a horizontal/vertical (or margin/width) prop swap fails this test. + expect(preview).toHaveAttribute('src', 'iptv/logos/cartoons.png'); + expect(preview).toHaveStyle({ + bottom: '7%', + left: '3%', + opacity: '0.6', + width: '11%' + }); + expect(preview.style.right).toBe(''); + expect(preview.style.top).toBe(''); + }); + + it('does not render the preview when an external logo URL is set', async () => { + mockApi({ + channelOverrides: { + logo: { path: 'iptv/logos/cartoons.png', contentType: 'image/png' }, + watermarkId: 9 + }, + watermarks: [{ id: 9, imageSource: 'ChannelLogo', name: 'Channel Bug' }] + }); + render(); + + await screen.findByDisplayValue('Cartoons'); + fireEvent.click(screen.getByRole('button', { name: /^Branding/ })); + + // Preview renders from the uploaded logo first. + await screen.findByAltText('On-screen bug preview'); + + const urlInput = screen.getByPlaceholderText('https://example.com/logo.png'); + fireEvent.change(urlInput, { target: { value: 'https://example.com/new-logo.png' } }); + + // An external URL can never resolve to a real bug (see the comment on the WatermarkSelector + // File.Exists behavior), so the preview must disappear once one is set. + await waitFor(() => expect(screen.queryByAltText('On-screen bug preview')).toBeNull()); + }); }); }); diff --git a/web/src/screens/ChannelEditScreen.tsx b/web/src/screens/ChannelEditScreen.tsx index 8bcde0d83..f4e9831c9 100644 --- a/web/src/screens/ChannelEditScreen.tsx +++ b/web/src/screens/ChannelEditScreen.tsx @@ -729,6 +729,18 @@ function BrandingPane({ const bugGeometry = fetchedGeometry && fetchedGeometry.id === draft.watermarkId ? fetchedGeometry : null; + // Remembers the last logo-driven watermark this channel actually referenced — which may be a + // per-channel-geometry ChannelLogo preset other than logoBugTarget (the shared default). Without + // this, toggling off (watermarkId -> null) then back on would always rebind to logoBugTarget, + // silently losing a custom preset's geometry. Only updated while the toggle reads as ON, so a + // toggle-off never clobbers the memory of what to restore. + const lastLogoWatermarkIdRef = useRef(null); + useEffect(() => { + if (logoBugEnabled && draft.watermarkId != null) { + lastLogoWatermarkIdRef.current = draft.watermarkId; + } + }, [logoBugEnabled, draft.watermarkId]); + return ( @@ -786,7 +798,7 @@ function BrandingPane({ logoBugTarget == null ? 'No logo-driven watermark preset exists yet.' : externalUrlLogo - ? 'An external logo URL cannot be used as the on-screen bug — upload an image instead (see #502).' + ? 'An external logo URL cannot be used as the on-screen bug — upload an image instead.' : 'Overlays this channel’s own logo on the stream, using the shared preset’s position and size.' } label="Use logo as on-screen bug" @@ -797,11 +809,24 @@ function BrandingPane({ disabled={hlsDirect || logoBugTarget == null} hideLabel label="Use logo as on-screen bug" - onChange={(next) => set({ watermarkId: next ? (logoBugTarget?.id ?? null) : null })} + onChange={(next) => + set({ watermarkId: next ? (lastLogoWatermarkIdRef.current ?? logoBugTarget?.id ?? null) : null }) + } size="sm" /> {logoBugEnabled && bugGeometry && previewSrc && !externalUrlLogo && ( - + // bugGeometry is cached keyed by `id` (see fetchedGeometry above); pass the + // geometry fields explicitly rather than spreading so the cache key never leaks + // into BugPreview's props. + )} -- 2.47.3 From c1bd8c067a4bc5cd21d3bae8aeacb5f0285662dc Mon Sep 17 00:00:00 2001 From: Timothy Date: Mon, 20 Jul 2026 21:05:38 +0200 Subject: [PATCH 12/13] feat(67): default new channels to the logo-bug watermark preset --- web/src/screens/ChannelsScreen.test.tsx | 53 +++++++++++++++++++++++++ web/src/screens/ChannelsScreen.tsx | 13 +++++- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/web/src/screens/ChannelsScreen.test.tsx b/web/src/screens/ChannelsScreen.test.tsx index 86716f5f9..3fa9a32ce 100644 --- a/web/src/screens/ChannelsScreen.test.tsx +++ b/web/src/screens/ChannelsScreen.test.tsx @@ -52,6 +52,8 @@ interface MockOptions { ffmpegSettings?: Record; // path -> ProblemDetails body returned with a 422 for a bulk mutation. mutationFailures?: Record; + watermarks?: unknown[] | null; + watermarksFail?: boolean; } interface MockHandle { @@ -65,6 +67,7 @@ function mockApi(options: MockOptions = {}): MockHandle { const channelStates = options.channelStates ?? []; const ffmpegSettings = options.ffmpegSettings ?? ffmpegSettingsFixture(); const mutationFailures = options.mutationFailures ?? {}; + const watermarks = options.watermarks ?? []; const fetchSpy = vi.spyOn(window, 'fetch').mockImplementation((input: RequestInfo | URL, init?: RequestInit) => { const url = input.toString(); @@ -81,6 +84,12 @@ function mockApi(options: MockOptions = {}): MockHandle { if (url === '/api/v1/channels' && method === 'GET') return Promise.resolve(jsonResponse(channels)); if (url === '/api/v1/channels/state' && method === 'GET') return Promise.resolve(jsonResponse(channelStates)); if (url === '/api/v1/settings/ffmpeg' && method === 'GET') return Promise.resolve(jsonResponse(ffmpegSettings)); + if (url === '/api/v1/watermarks' && method === 'GET') { + if (options.watermarksFail) { + return Promise.resolve(new Response(null, { status: 500 })); + } + return Promise.resolve(jsonResponse(watermarks)); + } if ( url === '/api/v1/channels/bulk/renumber' || @@ -564,4 +573,48 @@ describe('ChannelsScreen — bare create', () => { expect(window.location.pathname).toBe('/app/channels'); expect(screen.getByRole('button', { name: 'New blank channel' })).toBeInTheDocument(); }); + + // #67: one uploaded channel logo drives both the guide listing and the on-screen bug. A newly + // created channel should default to that shared preset rather than starting with no watermark. + it('defaults a newly created channel to the logo-bug watermark preset', async () => { + const handle = mockApi({ + channels: [channelRow()], + createChannelResponse: { id: 42 }, + watermarks: [{ id: 9, imageSource: 'ChannelLogo', name: 'Channel Bug' }] + }); + + render(); + expect(await screen.findByText('Retro Cartoons')).toBeInTheDocument(); + + fireEvent.click(screen.getByRole('button', { name: 'New blank channel' })); + + await waitFor(() => + expect(handle.fetchSpy).toHaveBeenCalledWith('/api/v1/channels', expect.objectContaining({ method: 'POST' })) + ); + + const body = requestBodyFor(handle, '/api/v1/channels'); + expect(body).toMatchObject({ watermarkId: 9 }); + }); + + // A watermark-list failure must degrade to no default, never block channel creation. + it('still creates the channel when the watermark lookup fails', async () => { + const handle = mockApi({ + channels: [channelRow()], + createChannelResponse: { id: 42 }, + watermarksFail: true + }); + + render(); + expect(await screen.findByText('Retro Cartoons')).toBeInTheDocument(); + + fireEvent.click(screen.getByRole('button', { name: 'New blank channel' })); + + await waitFor(() => + expect(handle.fetchSpy).toHaveBeenCalledWith('/api/v1/channels', expect.objectContaining({ method: 'POST' })) + ); + + const body = requestBodyFor(handle, '/api/v1/channels'); + expect(body).toMatchObject({ watermarkId: null }); + await waitFor(() => expect(window.location.pathname).toBe('/app/edit-channel/42')); + }); }); diff --git a/web/src/screens/ChannelsScreen.tsx b/web/src/screens/ChannelsScreen.tsx index d13bc84dd..618138708 100644 --- a/web/src/screens/ChannelsScreen.tsx +++ b/web/src/screens/ChannelsScreen.tsx @@ -17,7 +17,9 @@ import { bulkRenumberChannels, createChannel, deleteChannel, + findLogoBugWatermark, getFfmpegSettings, + getWatermarks, messageFromError, useChannelsScreenQuery, type ChannelState, @@ -347,6 +349,15 @@ export function ChannelsScreen() { return Number.isFinite(parsed) ? Math.max(max, parsed) : max; }, 0); + // New channels default to the seeded logo-driven preset so one uploaded logo drives both the + // guide listing and the on-screen bug (#67). A lookup failure must not block channel creation. + let watermarkId: null | number = null; + try { + watermarkId = findLogoBugWatermark(await getWatermarks())?.id ?? null; + } catch { + watermarkId = null; + } + const body: CreateChannelRequest = { categories: null, fallbackFillerId: null, @@ -374,7 +385,7 @@ export function ChannelsScreen() { streamSelectorMode: 'Default', subtitleMode: 'None', transcodeMode: 'OnDemand', - watermarkId: null + watermarkId }; const created = await createChannel(body); -- 2.47.3 From 893d4a63989b2a0cc2d09d67bf11d3c07a88bf76 Mon Sep 17 00:00:00 2001 From: Timothy Date: Mon, 20 Jul 2026 21:13:47 +0200 Subject: [PATCH 13/13] test(67): de-vacuify the template-stamping assertion; cover adopt-then-delete Whole-branch review found the stamping test was structurally vacuous: a bare foreach over ChannelTemplates.Where(IsSystem) passes with zero iterations, so the test would have stayed green if template seeding silently bailed out. Assert the collection is non-empty first. Proven non-vacuous by a negative control (forcing SeedChannelTemplates to bail makes exactly this test fail). Also cover the ACTUAL production sequence -- adopt an existing hand-made row, then delete it -- which the previous no-resurrect test did not exercise (it covered seed-then-delete). The marker is written on the adopt path too, so the deleted row must stay deleted. docs: note that a deleted preset degrades to no default rather than failing, and that the default applies to newly created channels, not retroactively. Refs #67 --- .../DbInitializerChannelBugWatermarkTests.cs | 38 ++++++++++++++++++- docs/channels.md | 7 +++- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/ErsatzTV.Tests/Infrastructure/DbInitializerChannelBugWatermarkTests.cs b/ErsatzTV.Tests/Infrastructure/DbInitializerChannelBugWatermarkTests.cs index 25ac12446..e89e0e88d 100644 --- a/ErsatzTV.Tests/Infrastructure/DbInitializerChannelBugWatermarkTests.cs +++ b/ErsatzTV.Tests/Infrastructure/DbInitializerChannelBugWatermarkTests.cs @@ -99,6 +99,37 @@ public class DbInitializerChannelBugWatermarkTests context.ChannelWatermarks.Any(w => w.Name == "Channel Bug").ShouldBeFalse(); } + // The production sequence is ADOPT (an existing hand-made row) and then, possibly, delete — + // not seed-then-delete. The marker is written on the adopt path too, so this must not resurrect. + [Test] + public async Task Initialize_Should_Not_Resurrect_An_Adopted_Preset_After_Deletion() + { + await using TvContext context = _db.CreateContext(); + await context.ChannelWatermarks.AddAsync( + new ChannelWatermark + { + Name = "Channel Bug", + Mode = ChannelWatermarkMode.Permanent, + ImageSource = ChannelWatermarkImageSource.ChannelLogo, + Location = WatermarkLocation.BottomRight, + Size = WatermarkSize.Scaled, + WidthPercent = 12, + Opacity = 55 + }); + await context.SaveChangesAsync(); + + // First run adopts the existing row and records the marker. + await DbInitializer.Initialize(context, CancellationToken.None); + + ChannelWatermark adopted = context.ChannelWatermarks.Single(w => w.Name == "Channel Bug"); + context.ChannelWatermarks.Remove(adopted); + await context.SaveChangesAsync(); + + await DbInitializer.Initialize(context, CancellationToken.None); + + context.ChannelWatermarks.Any(w => w.Name == "Channel Bug").ShouldBeFalse(); + } + [Test] public async Task Initialize_Should_Stamp_The_Preset_On_Freshly_Seeded_Templates() { @@ -107,7 +138,12 @@ public class DbInitializerChannelBugWatermarkTests await DbInitializer.Initialize(context, CancellationToken.None); ChannelWatermark watermark = context.ChannelWatermarks.Single(w => w.Name == "Channel Bug"); - foreach (ChannelTemplate template in context.ChannelTemplates.Where(t => t.IsSystem).ToList()) + List templates = context.ChannelTemplates.Where(t => t.IsSystem).ToList(); + + // Assert the collection is non-empty FIRST: a bare foreach over zero rows passes vacuously, + // so this test would stay green if template seeding silently bailed out. + templates.Count.ShouldBe(2); + foreach (ChannelTemplate template in templates) { template.WatermarkId.ShouldBe(watermark.Id); } diff --git a/docs/channels.md b/docs/channels.md index a8b9610bf..de4ab94d0 100644 --- a/docs/channels.md +++ b/docs/channels.md @@ -154,8 +154,11 @@ listing and the bug (#67). An existing `Channel Bug` row is adopted untouched, n a `ConfigElement` marker (`watermark.channel_bug_seeded`) makes the seed run once per database, so a deliberately deleted preset is not resurrected on the next restart. -Quick-add channel creation defaults to the preset, and the channel editor's Branding tab exposes it -as a "Use logo as on-screen bug" toggle. On a **fresh** install the seed also stamps the preset onto +Quick-add channel creation defaults to the preset (if the preset has been deleted, creation falls +back to no watermark rather than failing), and the channel editor's Branding tab exposes it as a +"Use logo as on-screen bug" toggle. The default applies to **newly created** channels — an existing +channel with no watermark does not gain one retroactively when a logo is uploaded; flip the toggle. +On a **fresh** install the seed also stamps the preset onto the system channel templates it creates, so the library-to-lineup builder (which inherits `WatermarkId` from the selected template) gets the default too. On an existing install the templates are left alone, so builder-created and auto-tuned channels there inherit whatever the template -- 2.47.3