fix(60): re-review fixups for channel preview
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 10s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 13s
PR Gates / Docs update reminder (pull_request) Successful in 12s
PR Gates / decisions lifecycle (pull_request) Successful in 18s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 14m11s
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Successful in 17m26s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 21m44s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 22m57s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 10s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 13s
PR Gates / Docs update reminder (pull_request) Successful in 12s
PR Gates / decisions lifecycle (pull_request) Successful in 18s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 14m11s
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Successful in 17m26s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 21m44s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 22m57s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
- ChannelPreviewPanel: a manual play-button click on a video already
showing a fatal error was clearing the error, silently hiding the
fault the panel exists to reveal. onPlaying now ignores the event
while a fatal error is showing (tracked via a ref, reset in an
effect keyed on channel.id); Retry remains the only way to clear it.
- shell.css: .ctv-preview-facts spacing was dead — equal-specificity
.ctv-detail-infogrid{margin:0} later in the file won. Raised
specificity with a compound selector instead of touching
.ctv-detail-infogrid, which MediaDetailScreen also relies on.
- ChannelPreviewTests: added two cases exercising two simultaneously-
true Unavailable causes, so the documented guard precedence in
Mapper.GetPreview is actually pinned by a test.
- design doc: fixed a garbled sentence describing which DTO gained
the Preview field.
This commit is contained in:
@@ -77,6 +77,40 @@ public class ChannelPreviewTests
|
||||
result.UnavailableReason.ShouldBe("Channel has no playout");
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Jwt_Enabled_Takes_Precedence_Over_Disabled_Channel()
|
||||
{
|
||||
// Both causes are true at once: JWT enabled (checked first) must win over the channel being
|
||||
// disabled (checked second), per the precedence order documented on GetPreview.
|
||||
ChannelPreviewResponseModel result = Mapper.GetPreview(
|
||||
StreamingMode.HttpLiveStreamingSegmenter,
|
||||
"12.1",
|
||||
iptvJwtEnabled: true,
|
||||
isEnabled: false,
|
||||
playoutCount: 0);
|
||||
|
||||
result.Availability.ShouldBe("Unavailable");
|
||||
result.ManifestUrl.ShouldBeNull();
|
||||
result.UnavailableReason.ShouldBe("IPTV JWT authentication is enabled");
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Disabled_Channel_Takes_Precedence_Over_Zero_Playout_Count()
|
||||
{
|
||||
// Both causes are true at once: channel disabled (checked second) must win over zero
|
||||
// playouts (checked third), per the precedence order documented on GetPreview.
|
||||
ChannelPreviewResponseModel result = Mapper.GetPreview(
|
||||
StreamingMode.HttpLiveStreamingSegmenter,
|
||||
"12.1",
|
||||
iptvJwtEnabled: false,
|
||||
isEnabled: false,
|
||||
playoutCount: 0);
|
||||
|
||||
result.Availability.ShouldBe("Unavailable");
|
||||
result.ManifestUrl.ShouldBeNull();
|
||||
result.UnavailableReason.ShouldBe("Channel is disabled");
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Channel_Number_Is_Used_Verbatim_In_The_Manifest_Url()
|
||||
{
|
||||
|
||||
@@ -75,9 +75,9 @@ Following `api.healthcheck-remediation-dto` (server-declared metadata on an addi
|
||||
the SPA renders and acts on it, it does not derive labels itself) and `api.artwork-rooted-urls`
|
||||
(rooted, directly-usable URLs):
|
||||
|
||||
`ChannelResponseModel` (the list DTO the SPA previews from) gains: `ChannelDetailResponseModel`
|
||||
does not — nothing consumes it there, so it stays out deliberately rather than being added
|
||||
speculatively.
|
||||
`ChannelResponseModel` (the list DTO the SPA previews from) gains a `Preview` field.
|
||||
`ChannelDetailResponseModel` deliberately does not — nothing consumes it there, so it stays out
|
||||
rather than being added speculatively.
|
||||
|
||||
```
|
||||
Preview: {
|
||||
|
||||
@@ -229,6 +229,30 @@ describe('ChannelPreviewPanel', () => {
|
||||
expect(screen.getByText('playing')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('does not let a subsequent playing event (e.g. a manual play-button click) erase a fatal error', () => {
|
||||
// A fatal error must be reported and stay reported: it must never be silently erased by a later
|
||||
// `playing` event, such as the one fired when the operator manually clicks the visible play
|
||||
// button on the <video controls> element after the failure banner is already showing.
|
||||
render(
|
||||
<ChannelPreviewPanel channel={channel(available)} nowPlaying={null} onClose={vi.fn()} open />
|
||||
);
|
||||
|
||||
const errorHandler = getErrorHandler();
|
||||
act(() => {
|
||||
errorHandler({}, { details: 'manifestLoadError', fatal: true, type: 'networkError' });
|
||||
});
|
||||
expect(screen.getByText(/failed — networkError: manifestloaderror/i)).toBeInTheDocument();
|
||||
|
||||
const video = document.body.querySelector('video');
|
||||
expect(video).not.toBeNull();
|
||||
act(() => {
|
||||
video?.dispatchEvent(new Event('playing'));
|
||||
});
|
||||
|
||||
expect(screen.getByText(/failed — networkError: manifestloaderror/i)).toBeInTheDocument();
|
||||
expect(screen.queryByText('playing')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
describe('switching to a different channel while mounted (render-phase reset)', () => {
|
||||
it('does not auto-start a forced-hls-only channel switched in from an available one', () => {
|
||||
const { rerender } = render(
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useCallback, useState } from 'react';
|
||||
import { useCallback, useEffect, useRef, useState } from 'react';
|
||||
import type { ChannelPreviewAvailability } from '../../api/channels';
|
||||
import { Button } from '../../components/forms';
|
||||
import { SlideOver } from '../../components/overlay';
|
||||
@@ -49,6 +49,14 @@ export function ChannelPreviewPanel({ channel, nowPlaying, onClose, open }: Chan
|
||||
// no-op; HlsPlayer tears down and re-attaches whenever this changes, even with the same src.
|
||||
const [playToken, setPlayToken] = useState(0);
|
||||
|
||||
// Tracks whether the panel is currently showing a fatal error, mirrored alongside `state` so
|
||||
// onPlaying (a stable-identity callback, see below) can check it without needing `state` in its
|
||||
// dependency array. A fatal error must never be erased by a later `playing` event — including the
|
||||
// native <video controls> `playing` event fired when the operator manually clicks play on a
|
||||
// buffered/partial stream after the failure banner is already showing (that would silently hide
|
||||
// the fault the panel exists to reveal). Only user-initiated Retry may clear it.
|
||||
const failedRef = useRef(false);
|
||||
|
||||
// Reset when the panel is reopened for a different channel. Adjusted synchronously during
|
||||
// render (React's documented "adjusting state when a prop changes" pattern) rather than in a
|
||||
// useEffect body, which would trip the react-hooks "no set-state-in-effect" rule (spa-conventions
|
||||
@@ -62,15 +70,29 @@ export function ChannelPreviewPanel({ channel, nowPlaying, onClose, open }: Chan
|
||||
setPlayToken(0);
|
||||
}
|
||||
|
||||
// failedRef mutations are confined to event handlers and this effect — never render-phase — per
|
||||
// the react-hooks/refs rule (mutating a ref during render is disallowed even for the documented
|
||||
// "adjust state during render" pattern above, which is fine only for React state setters). This
|
||||
// runs after the render-phase reset above has committed, before any user interaction or media
|
||||
// event can reach onPlaying, so there is no window where a stale `true` could suppress it.
|
||||
useEffect(() => {
|
||||
failedRef.current = false;
|
||||
}, [channel.id]);
|
||||
|
||||
// Stable identity: HlsPlayer keeps onError in its attach effect's dependency array, so an inline
|
||||
// arrow would tear down and restart the stream on every render.
|
||||
const onError = useCallback((message: string) => {
|
||||
failedRef.current = true;
|
||||
setError(message);
|
||||
setState('failed');
|
||||
}, []);
|
||||
|
||||
// Stable identity for the same reason as onError above.
|
||||
// Stable identity for the same reason as onError above. Ignores the event entirely while a fatal
|
||||
// error is showing (see failedRef above) so it cannot erase the failure.
|
||||
const onPlaying = useCallback(() => {
|
||||
if (failedRef.current) {
|
||||
return;
|
||||
}
|
||||
setError(null);
|
||||
setState('playing');
|
||||
}, []);
|
||||
@@ -84,6 +106,7 @@ export function ChannelPreviewPanel({ channel, nowPlaying, onClose, open }: Chan
|
||||
// retries hides the fault it exists to reveal). Retry re-issues the manifest request by bumping
|
||||
// playToken and clears the previous error so a stale failure doesn't linger over a fresh attempt.
|
||||
const onRetry = useCallback(() => {
|
||||
failedRef.current = false;
|
||||
setError(null);
|
||||
setState('starting');
|
||||
setPlayToken((current) => current + 1);
|
||||
|
||||
+4
-1
@@ -1130,7 +1130,10 @@ body {
|
||||
margin: 0 0 var(--space-6, 12px);
|
||||
}
|
||||
|
||||
.ctv-preview-facts {
|
||||
/* Two classes (higher specificity than .ctv-detail-infogrid alone) so this margin actually wins
|
||||
over that shared rule's `margin: 0`, without weakening .ctv-detail-infogrid for its other
|
||||
consumer (MediaDetailScreen), which relies on margin:0 there. */
|
||||
.ctv-preview-facts.ctv-detail-infogrid {
|
||||
margin-top: var(--space-6, 12px);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user