fix(playlists): one rendering for a playlist total, honest below a minute (E9 R1)
Epic-end deep review, round 1. Six findings survived verification; five of them were one defect wearing different hats. formatRuntime floored 1-59s to "0m". Harmless while it only ever formatted a Plex movie's length, but 12D repurposed it for an aggregate, and a total is the only input that can land under a minute: a playlist of Shorts reported "0m" while the rail beside it read "0:45". Fixed in the function, not behind a `>= 60` guard at each of the seven call sites — the invariant belongs where it cannot be forgotten. Its contract comment claimed "Plex movie/episode lengths" and was two epics stale. The rail rendered the same playlist total as formatDuration (1:23:45) while the new header rendered formatRuntime (1h 23m) — two shapes of one number, side by side on one screen. The rail now uses formatRuntime too; formatTotalHours had already recorded that H:MM:SS is wrong for an aggregate. The rename input kept min-w-0 without flex-1 in the compacted row, so it was the only shrinkable child beside a shrink-0 action cluster. The Plex half of this same epic already had the fix; this restores the symmetry. plex.browse.items had no plural forms, so a one-item Plex playlist read "1 items". Pre-existing, but the new header is the call site where count=1 is ordinary. Both languages get both forms, no key drift. formatRuntime had no unit tests at all across seven call sites. Added a tier table plus a sweep asserting no non-zero second ever renders as "0m". Dropped after 3/3 adversarial refutations: the meta-line segment ordering (the filtered count is labelled, and count-then-runtime is the convention) and the null-duration lower bound (the server's own SUM has skipped NULLs since long before this branch; changing only the client would split them).
This commit is contained in:
@@ -630,7 +630,7 @@ export default function Playlists() {
|
||||
if (e.key === "Escape") setRenaming(false);
|
||||
}}
|
||||
aria-label={t("playlists.rename")}
|
||||
className="min-w-0 text-lg font-semibold bg-surface border border-border rounded-md px-2 py-0.5 focus:outline-hidden focus:ring-2 focus:ring-accent/40"
|
||||
className="min-w-0 flex-1 text-lg font-semibold bg-surface border border-border rounded-md px-2 py-0.5 focus:outline-hidden focus:ring-2 focus:ring-accent/40"
|
||||
/>
|
||||
) : (
|
||||
<>
|
||||
|
||||
@@ -4,7 +4,7 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
|
||||
import { qk } from "../lib/queryKeys";
|
||||
import { ListVideo, Pin, Plus, RefreshCw, Youtube } from "lucide-react";
|
||||
import { api, type Playlist } from "../lib/api";
|
||||
import { formatDuration } from "../lib/format";
|
||||
import { formatRuntime } from "../lib/format";
|
||||
import { notify } from "../lib/notifications";
|
||||
import { notifyYouTubeActionError } from "../lib/youtubeErrors";
|
||||
import { LS, readAccountMerged, writeAccount } from "../lib/storage";
|
||||
@@ -247,7 +247,7 @@ export default function PlaylistsRail({
|
||||
<div className="text-2xs text-muted">
|
||||
{t("playlists.itemCount", { count: pl.item_count })}
|
||||
{pl.total_duration_seconds > 0 &&
|
||||
` · ${formatDuration(pl.total_duration_seconds)}`}
|
||||
` · ${formatRuntime(pl.total_duration_seconds)}`}
|
||||
</div>
|
||||
</div>
|
||||
</button>
|
||||
|
||||
@@ -350,7 +350,8 @@
|
||||
"browse": "Browse"
|
||||
},
|
||||
"browse": {
|
||||
"items": "{{count}} items"
|
||||
"items_one": "{{count}} item",
|
||||
"items_other": "{{count}} items"
|
||||
},
|
||||
"discover": {
|
||||
"tab": "Discover",
|
||||
|
||||
@@ -350,7 +350,8 @@
|
||||
"browse": "Böngészés"
|
||||
},
|
||||
"browse": {
|
||||
"items": "{{count}} elem"
|
||||
"items_one": "{{count}} elem",
|
||||
"items_other": "{{count}} elem"
|
||||
},
|
||||
"discover": {
|
||||
"tab": "Felfedezés",
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { RELATIVE_UNITS, relativeTime, relativeFromMs } from "./format";
|
||||
import { RELATIVE_UNITS, formatRuntime, relativeTime, relativeFromMs } from "./format";
|
||||
import en from "../i18n/locales/en/time.json";
|
||||
import hu from "../i18n/locales/hu/time.json";
|
||||
|
||||
@@ -108,3 +108,35 @@ describe("relativeFromMs", () => {
|
||||
expect(hu).toHaveProperty("justNow");
|
||||
});
|
||||
});
|
||||
|
||||
// formatRuntime went from "one Plex movie's length" to "a playlist's total playtime" (E9 12D), and a
|
||||
// TOTAL is the only input that can legitimately be 1-59 seconds. That tier had no coverage and no
|
||||
// implementation: 45s floored to "0m", so a playlist of Shorts reported no runtime while the rail
|
||||
// beside it read "0:45". The table pins every tier boundary, so the sub-minute branch cannot be
|
||||
// dropped again as "dead code" — it is the one branch a whole-playlist figure reaches.
|
||||
describe("formatRuntime", () => {
|
||||
const CASES: [number | null | undefined, string][] = [
|
||||
[null, ""],
|
||||
[undefined, ""],
|
||||
[0, ""], // nothing to show — the call sites suppress their separator on the empty string
|
||||
[1, "1s"],
|
||||
[45, "45s"], // one Short: the regression case
|
||||
[59, "59s"], // last second before the minute tier
|
||||
[60, "1m"],
|
||||
[90, "1m"], // floors, never rounds up — a total must not overstate
|
||||
[3599, "59m"],
|
||||
[3600, "1h 0m"], // the zero minutes stay: "1h" alone reads as an approximation
|
||||
[5025, "1h 23m"],
|
||||
[360000, "100h 0m"], // a large aggregate keeps counting hours rather than rolling to days
|
||||
];
|
||||
|
||||
it.each(CASES)("formats %o as %o", (sec, expected) => {
|
||||
expect(formatRuntime(sec)).toBe(expected);
|
||||
});
|
||||
|
||||
it("never reports a non-zero duration as zero", () => {
|
||||
for (let sec = 1; sec < 3600; sec++) {
|
||||
expect(formatRuntime(sec), `${sec}s`).not.toBe("0m");
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -96,10 +96,16 @@ export function formatDuration(sec: number | null): string {
|
||||
return h > 0 ? `${h}:${pad(m)}:${pad(s)}` : `${m}:${pad(s)}`;
|
||||
}
|
||||
|
||||
/** Compact human runtime ("1h 30m", "45m") from seconds — used for Plex movie/episode lengths.
|
||||
* Empty string for a missing/zero duration (nothing to show). */
|
||||
/** Compact human runtime ("1h 30m", "45m", "45s") from seconds. Formats BOTH a single item's
|
||||
* length (Plex movies/episodes) and an aggregate total (playlist playtime, on the YouTube and the
|
||||
* Plex side alike) — it is the one renderer for a playlist total, so a total never appears in two
|
||||
* shapes on one screen. Empty string for a missing/zero duration (nothing to show).
|
||||
* The sub-minute tier is load-bearing, not decoration: an aggregate is the only input that can land
|
||||
* between 1 and 59 seconds (a playlist of Shorts), and flooring that to a flat "0m" made a
|
||||
* non-empty playlist report no runtime at all. Keep it, or that case silently regresses. */
|
||||
export function formatRuntime(sec?: number | null): string {
|
||||
if (!sec) return "";
|
||||
if (sec < 60) return `${Math.floor(sec)}s`;
|
||||
const h = Math.floor(sec / 3600);
|
||||
const m = Math.floor((sec % 3600) / 60);
|
||||
return h ? `${h}h ${m}m` : `${m}m`;
|
||||
|
||||
Reference in New Issue
Block a user