CI's "Formatting (changed .cs conform to .editorconfig)" job failed with
`error CHARSET: Fix file encoding` on all six pre-existing BOM'd files this PR
touches. The #311 gate is fix-as-you-touch: any .cs a PR touches must conform to
.editorconfig (charset=utf-8), and these carried BOMs inherited from upstream.
BOM removal only — six files, one line each, zero content change. The three
files already without a BOM (GetChannelByIdForApiHandler + both new/changed
Tests files) needed nothing.
Note for the next person: this cannot be validated locally on this Mac —
`dotnet format --include` silently no-ops here, so the gate is only observable
in CI. Check `head -c3 <file> | xxd -p` for `efbbbf` on every touched .cs before
pushing instead of trusting a local format run.
Refs #72
Re-review, Low: domain-model.md still documented a "Problems" filter — a line
added by 50005b12 earlier in this same PR, so the PR contradicted itself once
38483681 renamed the filter to "No playout".
Ironic in exactly the way the rename is about: the next person implementing the
deferred fault classes would read the doc, see "Problems" documented, and
restore the over-promising label that ChannelsScreen's own comment asks them
not to. Stale docs re-introduce the bug the code comment defends against.
decisions.md needed no change — it only ever named the badge, never the filter.
Refs #72
Adversarial review, Medium: the "Problems" filter over-promised. The badge is
honestly named ("No playout"), but the filter claimed a taxonomy that does not
exist — decisions.md enumerates three fault classes this deliberately does NOT
compute (empty schedule behind a playout, broken source, origin). A user whose
Classic schedule is empty would read "Problems 0" as "lineup healthy" and ship
a dead channel. A false all-clear is worse than no affordance.
Renamed the filter to "No playout" so badge and filter both name exactly the
one fault the API can prove, leaving "Problems" free for when the taxonomy
behind it actually exists. Rationale recorded at the predicate so the next
person doesn't "improve" the label back.
Also from the review:
- The zero-playout test's comment claimed include coverage it does not provide
(it passes with or without the include — 0 == 0). Re-stated as what it is: a
mapper boundary check. Its two siblings are the include coverage.
- UpdateChannelHandler:179 is a fourth call site the "shared by all three"
framing excluded. Harmless (the controller discards the view model and
re-projects through GetChannelByIdForApi), but a trap: its query lacks the
MirrorSourceChannel include, so swapping in the shared helper would report 0
for a working mirror. Documented that the fix there is a QUERY change.
- Mirrored the rename into the design-system prototype.
The rename collided the badge and filter labels, so the screen tests now scope
the badge assertion to the table (spa-conventions §6). This also fixed a real
weakness: the mirror test's unscoped queryByText would have matched the filter
button and asserted nothing.
Review note: the reviewer's strongest hypothesis — that PUT and GET could
disagree on a mirror channel's count — was investigated and does NOT hold.
Refs #72
Same-PR docs obligations for #72:
- decisions.md — why health is the raw `PlayoutCount` fact on the lean list DTO
rather than a derived status enum or a new endpoint, and, more importantly,
why empty-schedule / broken-source / auto-tuned-origin are deliberately NOT
computed. The empty-schedule one is the trap worth writing down: it only
understands Classic ProgramSchedule playouts, so a badge driven off it would
silently lie on the other four schedule kinds.
- domain-model.md — a glossary row for channel health, drawing the line against
/channels/state's OnAir (runtime liveness, not "would play if tuned").
- design-system — mirror the "No playout" badge + Problems filter into the
Channels prototype so design-system/ stays a faithful mirror of prod
(design-sync #388). Nature Docs carries playouts: 0 so the fault state is
visible in the canvas.
Refs #72
Surfaces the per-channel playout count the previous commit put on the API as
the fault state #72 calls "the important one": a channel with no playout can
never play, and until now the lineup gave no sign of it.
- "No playout" renders as a Badge with visible text rather than another
single-letter marker like D/H. D/H flag states the user chose deliberately;
this is a fault they need to notice and act on, and a tooltip-only glyph is
invisible on touch and easy to miss in a long lineup.
- A "Problems" segmented filter answers the issue's actual goal — seeing the
health of a LARGE lineup at a glance — by collapsing it to just the faults.
- `willNeverPlay` is one predicate shared by the badge, the count and the
filter, so the three cannot disagree.
Mirror channels relay their source's playouts and the server folds those into
playoutCount, so a working mirror is not false-flagged (covered by a test).
Deliberately not approximated here, per the design pass: an empty schedule
behind an existing playout (EmptyScheduleHealthCheck only understands Classic
ProgramSchedule playouts — a badge driven by it would silently lie on Block,
Sequential, Scripted and ExternalJson channels) and broken/missing sources
(FileNotFound/Unavailable are server-wide counts with no channel attribution).
Both stay open on #72 rather than shipping a signal that is wrong for four of
the five schedule kinds.
The test fixture now sends playoutCount, matching the API, so the fault state
is opt-in per test instead of depending on an absent field.
Refs #72
The channels API could not answer "will this channel play?", which #72 needs
to flag a broken channel in the lineup at a glance.
Two defects, one root cause each:
1. `ChannelRepository.GetChannel` never included `Playouts`. The read is
AsNoTracking with no lazy-loading proxies, so the navigation came back
empty and `GetChannelByIdForApiHandler`'s `channel.Playouts?.Count ?? 0`
could only ever evaluate to 0 — `GET /api/v1/channels/{id}` reported
`playoutCount: 0` for every channel on the system. That silently disabled
the channel editor's playout-source guard (ChannelEditScreen:820, gated on
`playoutCount > 0`), so the "Cannot be changed once a generated channel has
a playout" control was always live. The server still enforces the invariant
(UpdateChannelHandler coerces Mirror back to Generated), so nothing was
corrupted — but the user's change was silently discarded. That silent
coercion is filed separately as #401.
2. The detail path counted only the channel's own playouts, never the mirror
source's, so a working Mirror channel would read as "no playout" even once
the include landed.
Both call sites now share `Mapper.GetPlayoutsCount` (previously private to
GetAllChannelsHandler), which handles the Mirror case. `ChannelResponseModel`
gains `PlayoutCount` so the list — #72's actual surface — can render it; the
count is free there, since `GetAll` already includes `Playouts` and
`MirrorSourceChannel.Playouts` and simply discarded them.
Tests run the real repository against a real context on purpose: a handler
test with a substituted IChannelRepository populates `Playouts` itself, so it
passes whether or not the query includes them. Proven non-vacuous — removing
the include again turns the 2-playout and mirror cases red (0 CS errors, so
no stale-dll false pass).
Refs #72
timothy