mirror of
https://github.com/myronblair/kino-app
synced 2026-07-30 14:32:40 -05:00
Fix 8 code-review findings across transcode queue, requests, cleanup, music
- Episode transcode (single + bulk) now accepts storage_type=scan, matching stream_episode and the movie-transcode endpoint — Library-Scan-imported TV episodes can finally be transcoded, not just streamed. - retry_job now forwards content_type, fixing retry for failed/cancelled episode jobs (was silently treating them as movies and failing again). - cancel_job now updates the correct collection (movies vs episodes) based on content_type instead of always writing to db.movies. - approve_request atomically claims the request (pending -> approving) before contacting Radarr/Sonarr, so a double-click or two concurrent admins can't both add the same content; releases the claim back to pending on any failure so a fixable error can be retried. - Music enrichment's update_many now re-checks album_art_url is still empty, so it no longer clobbers art an admin manually set via PATCH. - Library Cleanup's dangling-refs scan now also checks episode_progress for rows referencing a deleted episode (previously only watchlist/ progress against movie_id were checked). - Settings now surfaces a failed bulk-TMDB-enrich run as an error instead of a false "Enrichment done" success summary. - Music player's isPlaying now syncs from the audio element's native play/pause events instead of only being set inside togglePlay/track- change, so it can't drift from actual playback state (buffering stalls, OS media-key pauses, etc).
This commit is contained in:
@@ -102,6 +102,12 @@ async def scan(db, videos_dir: Path, hls_dir: Path) -> Dict[str, List[Dict]]:
|
|||||||
if r.get("movie_id") and r["movie_id"] not in movie_ids:
|
if r.get("movie_id") and r["movie_id"] not in movie_ids:
|
||||||
findings["dangling_refs"].append({"id": r["id"] if "id" in r else f"{r['profile_id']}:{r['movie_id']}", "content_type": coll_name, "label": r["movie_id"], "detail": f"references deleted movie {r['movie_id']}", "profile_id": r.get("profile_id"), "movie_id": r.get("movie_id")})
|
findings["dangling_refs"].append({"id": r["id"] if "id" in r else f"{r['profile_id']}:{r['movie_id']}", "content_type": coll_name, "label": r["movie_id"], "detail": f"references deleted movie {r['movie_id']}", "profile_id": r.get("profile_id"), "movie_id": r.get("movie_id")})
|
||||||
|
|
||||||
|
# --- Dangling episode_progress rows (reference a deleted episode) ---
|
||||||
|
ep_rows = await db.episode_progress.find({}, {"_id": 0}).to_list(20000)
|
||||||
|
for r in ep_rows:
|
||||||
|
if r.get("episode_id") and r["episode_id"] not in episode_ids:
|
||||||
|
findings["dangling_refs"].append({"id": f"{r['profile_id']}:{r['episode_id']}", "content_type": "episode_progress", "label": r["episode_id"], "detail": f"references deleted episode {r['episode_id']}", "profile_id": r.get("profile_id"), "episode_id": r.get("episode_id")})
|
||||||
|
|
||||||
return findings
|
return findings
|
||||||
|
|
||||||
|
|
||||||
@@ -155,6 +161,9 @@ async def fix_one(db, videos_dir: Path, hls_dir: Path, category: str, item: Dict
|
|||||||
await db.subtitles.delete_one({"id": item_id})
|
await db.subtitles.delete_one({"id": item_id})
|
||||||
|
|
||||||
elif category == "dangling_refs":
|
elif category == "dangling_refs":
|
||||||
|
if content_type == "episode_progress":
|
||||||
|
await db.episode_progress.delete_one({"profile_id": item.get("profile_id"), "episode_id": item.get("episode_id")})
|
||||||
|
else:
|
||||||
coll = {"watchlist": db.watchlist, "progress": db.progress}[content_type]
|
coll = {"watchlist": db.watchlist, "progress": db.progress}[content_type]
|
||||||
await coll.delete_one({"profile_id": item.get("profile_id"), "movie_id": item.get("movie_id")})
|
await coll.delete_one({"profile_id": item.get("profile_id"), "movie_id": item.get("movie_id")})
|
||||||
|
|
||||||
|
|||||||
+24
-9
@@ -692,8 +692,9 @@ async def cancel_job(job_id: str, user: dict = Depends(require_admin)):
|
|||||||
raise HTTPException(status_code=400, detail="Cannot cancel a running job")
|
raise HTTPException(status_code=400, detail="Cannot cancel a running job")
|
||||||
await db.transcode_queue.update_one({"id": job_id}, {"$set": {"status": "cancelled", "finished_at": datetime.now(timezone.utc).isoformat()}})
|
await db.transcode_queue.update_one({"id": job_id}, {"$set": {"status": "cancelled", "finished_at": datetime.now(timezone.utc).isoformat()}})
|
||||||
if job["status"] == "pending":
|
if job["status"] == "pending":
|
||||||
# Reset movie hls_status AND clear stale hls_path so the row no longer shows as transcoded
|
# Reset hls_status AND clear stale hls_path so the row no longer shows as transcoded
|
||||||
await db.movies.update_one({"id": job["movie_id"]}, {"$set": {"hls_status": None, "hls_path": None}})
|
coll = db.movies if job.get("content_type", "movie") == "movie" else db.episodes
|
||||||
|
await coll.update_one({"id": job["movie_id"]}, {"$set": {"hls_status": None, "hls_path": None}})
|
||||||
return {"ok": True}
|
return {"ok": True}
|
||||||
|
|
||||||
|
|
||||||
@@ -703,7 +704,7 @@ async def retry_job(job_id: str, user: dict = Depends(require_admin)):
|
|||||||
if not job: raise HTTPException(status_code=404, detail="Job not found")
|
if not job: raise HTTPException(status_code=404, detail="Job not found")
|
||||||
if job["status"] not in ("failed", "cancelled"):
|
if job["status"] not in ("failed", "cancelled"):
|
||||||
raise HTTPException(status_code=400, detail="Only failed/cancelled jobs can be retried")
|
raise HTTPException(status_code=400, detail="Only failed/cancelled jobs can be retried")
|
||||||
new_job = await _enqueue_transcode(job["movie_id"], job["quality"], triggered_by=job.get("triggered_by", "manual"))
|
new_job = await _enqueue_transcode(job["movie_id"], job["quality"], triggered_by=job.get("triggered_by", "manual"), content_type=job.get("content_type", "movie"))
|
||||||
return {"ok": True, "job_id": new_job["id"]}
|
return {"ok": True, "job_id": new_job["id"]}
|
||||||
|
|
||||||
|
|
||||||
@@ -935,12 +936,21 @@ async def approve_request(request_id: str, payload: RequestApprove, user: dict =
|
|||||||
"""Looks up the request's title in Radarr/Sonarr, adds the best match with an immediate
|
"""Looks up the request's title in Radarr/Sonarr, adds the best match with an immediate
|
||||||
search enabled, and marks the request approved. Content actually lands via the normal
|
search enabled, and marks the request approved. Content actually lands via the normal
|
||||||
Radarr/Sonarr download pipeline — this only kicks that off."""
|
Radarr/Sonarr download pipeline — this only kicks that off."""
|
||||||
req = await db.requests.find_one({"id": request_id}, {"_id": 0})
|
|
||||||
if not req: raise HTTPException(status_code=404, detail="Request not found")
|
|
||||||
if payload.content_type not in ("movie", "tv"):
|
if payload.content_type not in ("movie", "tv"):
|
||||||
raise HTTPException(status_code=400, detail="content_type must be movie or tv")
|
raise HTTPException(status_code=400, detail="content_type must be movie or tv")
|
||||||
|
# Atomically claim the request so two concurrent approve calls (double-click, two admins)
|
||||||
|
# can't both pass a check-then-act race and each add the content to Radarr/Sonarr.
|
||||||
|
req = await db.requests.find_one_and_update(
|
||||||
|
{"id": request_id, "status": "pending"}, {"$set": {"status": "approving"}},
|
||||||
|
projection={"_id": 0}, return_document=False,
|
||||||
|
)
|
||||||
|
if not req:
|
||||||
|
existing = await db.requests.find_one({"id": request_id}, {"_id": 0, "status": 1})
|
||||||
|
if not existing: raise HTTPException(status_code=404, detail="Request not found")
|
||||||
|
raise HTTPException(status_code=409, detail=f"Request is already {existing['status']}")
|
||||||
s = await get_settings()
|
s = await get_settings()
|
||||||
|
|
||||||
|
try:
|
||||||
if payload.content_type == "movie":
|
if payload.content_type == "movie":
|
||||||
if not s.get("radarr_url") or not s.get("radarr_api_key"):
|
if not s.get("radarr_url") or not s.get("radarr_api_key"):
|
||||||
raise HTTPException(status_code=400, detail="Radarr not configured")
|
raise HTTPException(status_code=400, detail="Radarr not configured")
|
||||||
@@ -973,6 +983,11 @@ async def approve_request(request_id: str, payload: RequestApprove, user: dict =
|
|||||||
raise HTTPException(status_code=502, detail=f"Sonarr add error: {e}")
|
raise HTTPException(status_code=502, detail=f"Sonarr add error: {e}")
|
||||||
updates = {"status": "approved", "content_type": "tv", "sonarr_id": added["sonarr_id"],
|
updates = {"status": "approved", "content_type": "tv", "sonarr_id": added["sonarr_id"],
|
||||||
"matched_title": added.get("title"), "matched_year": added.get("year")}
|
"matched_title": added.get("title"), "matched_year": added.get("year")}
|
||||||
|
except HTTPException:
|
||||||
|
# Release the claim so a fixable failure (e.g. Radarr temporarily down) can be retried,
|
||||||
|
# instead of leaving the request stuck in "approving" forever.
|
||||||
|
await db.requests.update_one({"id": request_id}, {"$set": {"status": "pending"}})
|
||||||
|
raise
|
||||||
|
|
||||||
res = await db.requests.find_one_and_update(
|
res = await db.requests.find_one_and_update(
|
||||||
{"id": request_id}, {"$set": updates}, projection={"_id": 0}, return_document=True,
|
{"id": request_id}, {"$set": updates}, projection={"_id": 0}, return_document=True,
|
||||||
@@ -1212,7 +1227,7 @@ async def bulk_transcode_eps(payload: dict, user: dict = Depends(require_admin))
|
|||||||
for eid in ids:
|
for eid in ids:
|
||||||
ep = await db.episodes.find_one({"id": eid}, {"_id": 0})
|
ep = await db.episodes.find_one({"id": eid}, {"_id": 0})
|
||||||
if not ep or ep.get("hls_status") in ("running", "pending"): continue
|
if not ep or ep.get("hls_status") in ("running", "pending"): continue
|
||||||
if ep.get("storage_type") not in ("local", "sonarr") or not ep.get("storage_path"): continue
|
if ep.get("storage_type") not in ("local", "sonarr", "scan") or not ep.get("storage_path"): continue
|
||||||
await _enqueue_transcode(eid, quality, triggered_by="manual", content_type="episode")
|
await _enqueue_transcode(eid, quality, triggered_by="manual", content_type="episode")
|
||||||
queued += 1
|
queued += 1
|
||||||
return {"ok": True, "queued": queued}
|
return {"ok": True, "queued": queued}
|
||||||
@@ -1244,8 +1259,8 @@ async def transcode_episode(episode_id: str, payload: Optional[dict] = None, use
|
|||||||
if quality not in ("quick", "abr"): raise HTTPException(status_code=400, detail="Invalid quality")
|
if quality not in ("quick", "abr"): raise HTTPException(status_code=400, detail="Invalid quality")
|
||||||
ep = await db.episodes.find_one({"id": episode_id}, {"_id": 0})
|
ep = await db.episodes.find_one({"id": episode_id}, {"_id": 0})
|
||||||
if not ep: raise HTTPException(status_code=404, detail="Episode not found")
|
if not ep: raise HTTPException(status_code=404, detail="Episode not found")
|
||||||
if ep.get("storage_type") not in ("local", "sonarr") or not ep.get("storage_path"):
|
if ep.get("storage_type") not in ("local", "sonarr", "scan") or not ep.get("storage_path"):
|
||||||
raise HTTPException(status_code=400, detail="Only local/sonarr episodes can be transcoded")
|
raise HTTPException(status_code=400, detail="Only local/sonarr/scanned episodes can be transcoded")
|
||||||
if ep.get("hls_status") in ("running", "pending"):
|
if ep.get("hls_status") in ("running", "pending"):
|
||||||
raise HTTPException(status_code=409, detail="Already transcoding or queued")
|
raise HTTPException(status_code=409, detail="Already transcoding or queued")
|
||||||
job = await _enqueue_transcode(episode_id, quality, triggered_by="manual", content_type="episode")
|
job = await _enqueue_transcode(episode_id, quality, triggered_by="manual", content_type="episode")
|
||||||
@@ -1591,7 +1606,7 @@ async def _run_music_enrich():
|
|||||||
art_url = await asyncio.to_thread(musicbrainz_client.cover_art_url, release["mbid"])
|
art_url = await asyncio.to_thread(musicbrainz_client.cover_art_url, release["mbid"])
|
||||||
if art_url:
|
if art_url:
|
||||||
res = await db.tracks.update_many(
|
res = await db.tracks.update_many(
|
||||||
{"artist": artist, "album": album},
|
{"artist": artist, "album": album, "album_art_url": {"$in": [None, ""]}},
|
||||||
{"$set": {"album_art_url": art_url}},
|
{"$set": {"album_art_url": art_url}},
|
||||||
)
|
)
|
||||||
_music_enrich_status["updated"] += res.modified_count
|
_music_enrich_status["updated"] += res.modified_count
|
||||||
|
|||||||
@@ -41,29 +41,36 @@ export const MusicPlayerProvider = ({ children }) => {
|
|||||||
const audio = audioRef.current;
|
const audio = audioRef.current;
|
||||||
if (!current) { audio.pause(); return; }
|
if (!current) { audio.pause(); return; }
|
||||||
audio.src = getTrackStreamUrl(current);
|
audio.src = getTrackStreamUrl(current);
|
||||||
audio.play().then(() => setIsPlaying(true)).catch(() => setIsPlaying(false));
|
audio.play().catch(() => {}); // isPlaying reflects reality via the native play/pause listeners below
|
||||||
}, [current]);
|
}, [current]);
|
||||||
|
|
||||||
|
// isPlaying always mirrors the audio element's own state, so it stays correct through anything
|
||||||
|
// that changes playback outside togglePlay — a stall, an OS media-key pause, autoplay being blocked.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
const audio = audioRef.current;
|
const audio = audioRef.current;
|
||||||
const onTime = () => setProgress({ position: audio.currentTime, duration: audio.duration || 0 });
|
const onTime = () => setProgress({ position: audio.currentTime, duration: audio.duration || 0 });
|
||||||
const onEnded = () => {
|
const onEnded = () => {
|
||||||
if (index >= 0 && index < queue.length - 1) setIndex(index + 1);
|
if (index >= 0 && index < queue.length - 1) setIndex(index + 1);
|
||||||
else setIsPlaying(false);
|
|
||||||
};
|
};
|
||||||
|
const onPlay = () => setIsPlaying(true);
|
||||||
|
const onPause = () => setIsPlaying(false);
|
||||||
audio.addEventListener("timeupdate", onTime);
|
audio.addEventListener("timeupdate", onTime);
|
||||||
audio.addEventListener("ended", onEnded);
|
audio.addEventListener("ended", onEnded);
|
||||||
|
audio.addEventListener("play", onPlay);
|
||||||
|
audio.addEventListener("pause", onPause);
|
||||||
return () => {
|
return () => {
|
||||||
audio.removeEventListener("timeupdate", onTime);
|
audio.removeEventListener("timeupdate", onTime);
|
||||||
audio.removeEventListener("ended", onEnded);
|
audio.removeEventListener("ended", onEnded);
|
||||||
|
audio.removeEventListener("play", onPlay);
|
||||||
|
audio.removeEventListener("pause", onPause);
|
||||||
};
|
};
|
||||||
}, [index, queue]);
|
}, [index, queue]);
|
||||||
|
|
||||||
const togglePlay = useCallback(() => {
|
const togglePlay = useCallback(() => {
|
||||||
const audio = audioRef.current;
|
const audio = audioRef.current;
|
||||||
if (!current) return;
|
if (!current) return;
|
||||||
if (audio.paused) audio.play().then(() => setIsPlaying(true));
|
if (audio.paused) audio.play().catch(() => {});
|
||||||
else { audio.pause(); setIsPlaying(false); }
|
else audio.pause();
|
||||||
}, [current]);
|
}, [current]);
|
||||||
|
|
||||||
const next = useCallback(() => {
|
const next = useCallback(() => {
|
||||||
|
|||||||
@@ -140,8 +140,12 @@ export default function Settings() {
|
|||||||
setEnrich(data);
|
setEnrich(data);
|
||||||
if (!data.running) {
|
if (!data.running) {
|
||||||
clearInterval(t);
|
clearInterval(t);
|
||||||
|
if (data.error) {
|
||||||
|
toast.error(`Enrichment failed: ${data.error}`);
|
||||||
|
} else {
|
||||||
toast.success(`Enrichment done — ${data.enriched} enriched, ${data.skipped} already complete, ${data.failed} not found`);
|
toast.success(`Enrichment done — ${data.enriched} enriched, ${data.skipped} already complete, ${data.failed} not found`);
|
||||||
}
|
}
|
||||||
|
}
|
||||||
}, 2000);
|
}, 2000);
|
||||||
return () => clearInterval(t);
|
return () => clearInterval(t);
|
||||||
}, [enrich?.running]);
|
}, [enrich?.running]);
|
||||||
@@ -210,7 +214,12 @@ export default function Settings() {
|
|||||||
<button type="button" onClick={enrichAll} disabled={enrich?.running} className="text-xs uppercase tracking-[0.2em] border border-[#222] hover:border-[#D9381E] hover:text-[#D9381E] text-[#8A8A8A] px-4 py-2 transition-colors disabled:opacity-50" data-testid="enrich-all-button">
|
<button type="button" onClick={enrichAll} disabled={enrich?.running} className="text-xs uppercase tracking-[0.2em] border border-[#222] hover:border-[#D9381E] hover:text-[#D9381E] text-[#8A8A8A] px-4 py-2 transition-colors disabled:opacity-50" data-testid="enrich-all-button">
|
||||||
{enrich?.running ? `Enriching… ${enrich.done}/${enrich.total}` : "Bulk enrich all movies →"}
|
{enrich?.running ? `Enriching… ${enrich.done}/${enrich.total}` : "Bulk enrich all movies →"}
|
||||||
</button>
|
</button>
|
||||||
{enrich && !enrich.running && enrich.total > 0 && (
|
{enrich && !enrich.running && enrich.error && (
|
||||||
|
<span className="text-xs text-[#fca5a5]" data-testid="enrich-all-error">
|
||||||
|
Last run failed: {enrich.error}
|
||||||
|
</span>
|
||||||
|
)}
|
||||||
|
{enrich && !enrich.running && !enrich.error && enrich.total > 0 && (
|
||||||
<span className="text-xs text-[#8A8A8A]" data-testid="enrich-all-summary">
|
<span className="text-xs text-[#8A8A8A]" data-testid="enrich-all-summary">
|
||||||
Last run: {enrich.enriched} enriched · {enrich.skipped} already complete · {enrich.failed} not found on TMDB
|
Last run: {enrich.enriched} enriched · {enrich.skipped} already complete · {enrich.failed} not found on TMDB
|
||||||
</span>
|
</span>
|
||||||
|
|||||||
Reference in New Issue
Block a user