feat(67): one logo drives both the listing and the on-screen bug #504
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: timothy/ersatztv#504
Reference in New Issue
Block a user
Closes #67.
What this does
One uploaded channel logo now drives both the guide listing and the on-screen bug (watermark), by default, with a preview that shows where the bug will actually land.
The finding that shaped the design
Most of #67 already existed.
ChannelWatermarkImageSource.ChannelLogoresolves each channel's ownArtworkKind.Logoartwork at FFmpeg render time, at every watermark precedence level — so one shared preset serves every channel. Production has been running exactly this pattern by hand since before the issue was filed: 43 channels all pointing at one hand-madeChannel Bugrow.So this PR productizes a proven mechanism rather than inventing one. The real gaps were the default, the preview, and the wiring — not the capability. The issue body's premise ("Channel already has separate
LogoandWatermarkfields… mostly a create/edit-flow convenience") is wrong on the details and has been corrected in the issue.No database migration. No change to how watermarks render.
Changes
Backend
DbInitializer.SeedChannelBugWatermarkseeds one sharedChannel Bugpreset (ImageSource=ChannelLogo, Permanent, TopLeft, Scaled 5% width, 1%/1% margins, 80% opacity — the values production already proved).ConfigElementmarker (watermark.channel_bug_seeded) makes it seed once per database, not once per name-absence.ChannelWatermarkhas noIsSystemflag andInitializeruns at every startup, so a name-only guard would resurrect a deliberately deleted preset forever.WatermarkResponseModelgainsimageSource(additive under the frozen-additive/api/v1, #286) so a client identifies logo-driven presets generically instead of matching a user-editable name.SPA
<BugPreview>component — a 16:9 frame placing the image by location/size/margins/opacity, with the geometry math in a pure, unit-testedbugPreviewStyle.GET /api/v1/watermarks/{id}).<img>becomes<BugPreview>, so the screen where geometry is edited finally shows it.nullrather than blocking creation.Production safety
The toggle is a pure reflection of stored state: ticked iff the watermark
watermarkIdpoints at hasimageSource === 'ChannelLogo', resolved by id lookup.An earlier draft of the plan derived it by searching the list for the first
ChannelLogoentry. BecausegetWatermarks()sorts by name, that would have shown the toggle OFF for a logo-driven channel bound to a non-first preset — and silently repointed it on tick. Caught in independent review before any code was written; pinned now by a two-preset regression test plus a "re-save untouched leaveswatermarkIdbyte-identical" test.Scope notes
WatermarkSelectorresolves it to the URL thenFile.Exists-checks it, which is never true. Pre-existing defect, filed as #502; this PR only stops the preview from promising it.MiddleCenterrenders bottom-right in FFmpeg (missing switch arm) — filed as #503. The preview keeps the semantically-correct centered rendering rather than encoding the defect.ChannelBuilderandPOST /api/v1/channels/auto-tuneare deliberately untouched on existing installs.Verification
check:apidotnet format --verify-no-changesThree vacuous-test blind spots were found and closed during the run, the last by the whole-branch review: the template-stamping assertion was a
foreachover a possibly-empty collection. It is now proven non-vacuous by a negative control — forcing template seeding to bail makes exactly that test fail.Reviews
Design and plan reviewed cold before implementation (found the repointing blocker, the seed-resurrection gap, and the hardcoded-preview-geometry problem — all fixed pre-implementation). Every task independently reviewed; whole-branch review returned MERGEABLE.
🤖 Generated with Claude Code
Fable review (cold, review-only) found a blocker in the plan's own code: Task 5/6 selected the logo-bug preset by searching for the first imageSource==='ChannelLogo' entry, but getWatermarks() sorts by name (pickers.ts:14), so with a second logo-driven preset -- which this design explicitly invites users to create -- the toggle would read OFF for a logo-driven channel and REPOINT it on tick, from a control documented as a pure reflection of stored state. Now resolved by id lookup, with the tick-on target chosen by a shared findLogoBugWatermark() helper and pinned by a two-preset regression test. Also adopted: - ConfigElement seed marker (watermark.channel_bug_seeded): ChannelWatermark has no IsSystem flag and Initialize runs every startup, so a name-only guard resurrected a deliberately deleted preset forever. - Channel-editor preview now fetches the referenced preset's REAL geometry via the existing GET /api/v1/watermarks/{id} instead of hardcoding the seeded defaults -- which would have been wrong for exactly the users who tuned theirs. - The 're-save untouched leaves watermarkId unchanged' test the spec promised and the plan had omitted, plus a create-path degrade test. - Reversed the ChannelBuilder exclusion (operator decision): fresh installs stamp the preset onto the templates the seed creates; existing installs are untouched. - External-URL logos never render a bug (File.Exists against a URL, WatermarkSelector.cs:269-286) -- verified, filed as #502, preview no longer promises it. - Dropped Task 2's InternalsVisibleTo branch: already present (ErsatzTV.Application.csproj:30-32). Refs #67 #502Live-E2E: PASSED (real running instance, fresh database)
Ran per
docs/e2e-local.mdagainst a fresh config dir on an isolated port — not prod, not a shared instance.1. Seed produces the preset, with the exact specified geometry
This also confirms the additive
imageSourcefield is live on the picker DTO, and that the by-id call the editor's preview depends on returns the real geometry.2. Fresh-install template stamping works
3. Write path round-trips through a subsequent read (the #229 lazy-materialisation net)
4. The marker genuinely prevents resurrection — the behaviour the unit test asserts, now proven against a real app: deleted the seeded preset, restarted the process against the same database, and it stayed deleted (
COUNT(*) = 0, marker still present). A name-only guard would have recreated it here.Full verification
check:apidotnet format --verify-no-changesReviews
ChannelLogopreset would have silently repointed channels bound to a non-first one, becausegetWatermarks()sorts by name. Fixed before any code was written; now pinned by a two-preset regression test.foreachover a possibly-empty collection). Fixed in893d4a63and proven non-vacuous by a negative control — forcing template seeding to bail makes exactly that test fail.Review-verdict: MERGEABLE @
893d4a6398