From 90fb2ee3bc59b61ce5d800c8f569a336b6c47184 Mon Sep 17 00:00:00 2001 From: ChrisBeWithYou Date: Sun, 28 Jun 2026 07:10:15 -0500 Subject: [PATCH] feat(v3): content-dependent playlist covers + custom art (#626) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(v3): content-dependent playlist covers + custom art upload Playlist cards were a tiny 🎵 emoji on an empty square. Now the cover reflects the playlist's contents, and you can override it with a custom image. Cover (in priority order): - custom uploaded cover, else - empty playlist -> the icon - a few songs -> the first song's album art - 4+ songs -> a 2x2 album-art mosaic Backend (server.py): - MetadataDB.list_playlists() returns each playlist's first few still-present songs' art URLs (`art_urls`) for the content cover. - GET /api/playlists and GET /api/playlists/{id} add `cover_url` when a custom cover exists. - POST/GET/DELETE /api/playlists/{id}/cover — store a small PNG thumbnail under CONFIG_DIR/playlist_covers/ (PIL-converted, mirroring song-art upload); the cover is deleted with the playlist. Cover mutators added to _MUTATING_ROUTES. Frontend (static/v3/playlists.js): playlistCoverHtml(p) renders the rules above; the playlist detail view gets "Cover" (pick an image) + "Remove cover". Tests: tests/test_playlists_api.py (art_urls + cover roundtrip / reject-non-image / delete-removes-cover — 11 pass) and tests/js/v3_playlist_cover.test.js. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF * fix(playlists): 400 (not 500) on non-string cover image + bust same-second cover cache Two review follow-ups on the playlist-cover endpoints: - POST /cover did `if "," in b64` before any type check, so a non-string image (e.g. {"image": 123} / null) raised TypeError -> 500. Guard with isinstance (mirrors the avatar/song-art upload) for a clean 400. +regression test covering number/null/object/list. - The cover URL busted only on int(st_mtime) (1s granularity) and GET /cover sent no cache headers, so a same-second replace/remove/re-upload could serve a stale image. Use st_mtime_ns in the cache-bust token and add the shared no-cache header (_ART_CACHE_HEADERS), matching song art. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 Co-authored-by: byrongamatos --- CHANGELOG.md | 1 + server.py | 119 +++++++++++++++++++++++++++-- static/v3/playlists.js | 43 ++++++++++- tests/js/v3_playlist_cover.test.js | 28 +++++++ tests/test_playlists_api.py | 64 ++++++++++++++++ 5 files changed, 247 insertions(+), 8 deletions(-) create mode 100644 tests/js/v3_playlist_cover.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index bf9e025..3bd13fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- **Playlists get content-dependent covers + custom art.** Playlist cards were a tiny `🎵` emoji on an empty square. Now a playlist's cover reflects its contents: **empty → the icon**, **a few songs → the first song's album art**, **4+ songs → a 2×2 art mosaic**. You can also **upload a custom cover** (a "Cover" button in the playlist detail view → image picker; "Remove cover" reverts to the content view). `MetadataDB.list_playlists()` now returns each playlist's first few song `art_urls`; `GET /api/playlists` and `GET /api/playlists/{id}` add `cover_url` when a custom cover exists. New routes `POST` / `GET` / `DELETE /api/playlists/{id}/cover` store a small PNG thumbnail under `CONFIG_DIR/playlist_covers/` (PIL-converted, like song-art upload); the cover is removed with the playlist. Frontend: `playlistCoverHtml(p)` in `static/v3/playlists.js`. Tests: `tests/test_playlists_api.py` (art_urls + cover roundtrip / reject-non-image / delete-cleanup), `tests/js/v3_playlist_cover.test.js`. - **v3 Songs: "Add to playlist" is now on each song's ⋮ "More" menu.** Previously a song could only be added to a playlist through select-mode (the checkbox → batch bar). The per-card overflow menu now has an **Add to playlist** row that targets that one song, reusing the same picker (choose a listed number or type a new name to create it). The select-mode batch flow and the single-song menu now share one extracted `addFilenamesToPlaylist(filenames)` helper in `static/v3/songs.js` (both grid and tree rows, since they share `openCardMenu`). Tests: `tests/js/v3_add_to_playlist_menu.test.js`. - **Resume where you left off — leaving a song now snapshots your place so an exit is recoverable, not a restart-from-zero.** Exiting the player (`showScreen()` teardown, before audio unload) writes `{song, arrangement, position, speed}` to `localStorage` (`feedBack.resumeSession`), and a non-blocking **"Resume practice"** pill offers it back on the next non-player screen (and on the next app launch). Clicking Resume re-enters the song, restores the arrangement + playback speed, and seeks to the saved position via the existing `_audioSeek` funnel; `playSong()` gains a `{ resume: {position, speed} }` option that arms a `song:ready`-consumed restore instead of the normal autostart, so the two never fight over playback. The snapshot is deliberately conservative — ignored for a song you barely started (< 3s) or had basically finished (within 5s of the end), cleared on natural song-end and once consumed, and expired after 24h. The pill is self-contained (inline-styled, body-appended, works identically in the classic and v3 shells with no Tailwind rebuild), never blocks, and a dismiss forgets the current snapshot for the session. This pairs with the Escape focus fix: now that Escape reliably leaves regardless of focus, an *accidental* exit is one tap to undo. Public surface: `window.resumeLastSession()` / `window.feedBack.resumeLastSession`. (The broader nav-state work — returning to a song after wandering into Settings → Tone Builder — is a separate, larger track; this lands the player-session slice.) Tests: `tests/browser/resume-session.spec.ts` (snapshot guards, staleness, pill show/hide/dismiss, resume consumption). - **Optional "Ask before leaving a song" confirm (Gameplay tab, default OFF).** A new client-only toggle (`confirmExitSong` in `localStorage`, in the v3 Gameplay settings + the Gameplay "Reset" set) for players who want a guard against an accidental exit. **Off by default — Escape leaves instantly, zero change for everyone else.** When on, a *user-initiated* exit (the player-scope Escape shortcut, or the player's ✕) opens a small true-modal confirm instead of leaving; auto-exit on song-end and a results screen's own Close are unaffected (they call `closeCurrentSong()` directly, which stays the unguarded actual-exit). The confirm honors the team's refined asks: **opening it pauses the song** (so it isn't running or being scored behind the prompt) and **Stay resumes exactly what was paused**; **Escape = Stay** — the dialog's capture-phase handler *dismisses* it (now consistent with every other modal and the generic `_confirmDialog`'s Esc=cancel), so a second Escape returns you to the (resumed) song rather than leaving; **Space/Enter (or click) Leave** by natively activating the default-focused "Leave" button ("just get me out"). Pause/resume run through the canonical `togglePlay()` path (HTML5 + `_juceMode`), guarded so a count-in, an already-paused song, or a teardown/seek/end behind the modal can't mis-resume. It's a real modal (`role="dialog" aria-modal="true"` / `.feedBack-modal`) with **Tab trapped inside it** and a **backdrop click that also Stays**, so the Escape/Space focus carve-outs treat it as a trap and don't fire player-back / play-pause behind it. The player Escape shortcut and the v3 ✕ route through a shared `window.requestExitSong()` gate (the ✕ also becomes origin-aware, matching Escape). Tests: `tests/browser/exit-confirm.spec.ts` (default-off instant exit, confirm-on opens + stays, second-Escape stays, backdrop stays, Stay/Leave, Enter-leaves); the audio pause/resume is verified manually on web + desktop (the mock song has no backing track). diff --git a/server.py b/server.py index d2c2d2c..6625b1b 100644 --- a/server.py +++ b/server.py @@ -220,6 +220,8 @@ _DEMO_BLOCKED: list[tuple[str, re.Pattern]] = [ ("POST", re.compile(r"^/api/playlists/[^/]+/songs$")), ("DELETE", re.compile(r"^/api/playlists/[^/]+/songs/.+$")), ("POST", re.compile(r"^/api/playlists/[^/]+/reorder$")), + ("POST", re.compile(r"^/api/playlists/[^/]+/cover$")), + ("DELETE", re.compile(r"^/api/playlists/[^/]+/cover$")), ("POST", re.compile(r"^/api/saved/toggle$")), # Progression (spec 010) write endpoints — demo mode stays read-only. ("POST", re.compile(r"^/api/progression/paths$")), @@ -1323,14 +1325,30 @@ class MetadataDB: return None def list_playlists(self) -> list[dict]: + from urllib.parse import quote rows = self.conn.execute( "SELECT id, name, system_key, created_at, updated_at FROM playlists " "ORDER BY (system_key IS NULL), name COLLATE NOCASE" ).fetchall() - return [{ - "id": r[0], "name": r[1], "system_key": r[2], - "created_at": r[3], "updated_at": r[4], "count": self._playlist_count(r[0]), - } for r in rows] + out = [] + for r in rows: + pid = r[0] + # First few still-present songs (in order) → art URLs, for a + # content-dependent playlist cover (single art / 2x2 mosaic). The + # JOIN drops dead songs, matching get_playlist's visibility. + arts = self.conn.execute( + "SELECT ps.filename FROM playlist_songs ps " + "JOIN songs s ON s.filename = ps.filename " + "WHERE ps.playlist_id = ? ORDER BY ps.position LIMIT 4", + (pid,), + ).fetchall() + out.append({ + "id": pid, "name": r[1], "system_key": r[2], + "created_at": r[3], "updated_at": r[4], + "count": self._playlist_count(pid), + "art_urls": [f"/api/song/{quote(a[0])}/art" for a in arts], + }) + return out def create_playlist(self, name: str, system_key: str | None = None) -> dict: with self._lock: @@ -5149,9 +5167,35 @@ def api_song_stats(filename: str): # ── Playlists / Saved for Later / Continue-Playing (fee[dB]ack v0.3.0) ──────── +def _playlist_cover_path(pid) -> Path | None: + """Filesystem path of a playlist's optional custom cover image (PNG), + stored under CONFIG_DIR. Returns None for a non-integer id.""" + try: + pid = int(pid) + except (TypeError, ValueError): + return None + return CONFIG_DIR / "playlist_covers" / f"{pid}.png" + + +def _playlist_cover_url(pid) -> str | None: + cover = _playlist_cover_path(pid) + if not cover or not cover.exists(): + return None + try: + # Nanosecond mtime so a same-second replace/remove/re-upload still + # changes the cache-bust token (int seconds could collide → stale image). + mt = cover.stat().st_mtime_ns + except OSError: + mt = 0 + return f"/api/playlists/{pid}/cover?v={mt}" + + @app.get("/api/playlists") def api_list_playlists(): - return meta_db.list_playlists() + lists = meta_db.list_playlists() + for pl in lists: + pl["cover_url"] = _playlist_cover_url(pl["id"]) + return lists @app.post("/api/playlists") @@ -5167,6 +5211,7 @@ def api_get_playlist(pid: int): pl = meta_db.get_playlist(pid) if pl is None: return JSONResponse({"error": "not found"}, status_code=404) + pl["cover_url"] = _playlist_cover_url(pid) return pl @@ -5193,6 +5238,12 @@ def api_delete_playlist(pid: int): return JSONResponse({"error": "System playlists cannot be deleted."}, status_code=400) if not meta_db.delete_playlist(pid): # vanished under us (concurrent delete) return JSONResponse({"error": "not found"}, status_code=404) + cover = _playlist_cover_path(pid) # drop any custom cover with the playlist + if cover and cover.exists(): + try: + cover.unlink() + except OSError: + pass return {"ok": True} @@ -5239,6 +5290,64 @@ def api_reorder_playlist(pid: int, data: dict): return meta_db.get_playlist(pid) +@app.post("/api/playlists/{pid}/cover") +async def api_set_playlist_cover(pid: int, data: dict): + """Set a playlist's custom cover from a base64 / data-URL image (PNG/JPG). + Overrides the content-dependent (song-art) cover. Stored as a small PNG + thumbnail under CONFIG_DIR/playlist_covers/.""" + if meta_db.get_playlist(pid) is None: + return JSONResponse({"error": "not found"}, status_code=404) + import base64 + import io + b64 = data.get("image", "") + # Guard the type before the `","` membership test — a non-string image + # (e.g. {"image": 123} / null) would otherwise raise TypeError → 500. + # Mirrors the avatar/song-art upload guard. + if not isinstance(b64, str) or not b64: + return JSONResponse({"error": "No image data"}, status_code=400) + if "," in b64: + b64 = b64.split(",", 1)[1] + if not b64: + return JSONResponse({"error": "No image data"}, status_code=400) + try: + img_data = base64.b64decode(b64) + except Exception: + return JSONResponse({"error": "Invalid base64"}, status_code=400) + cover = _playlist_cover_path(pid) + cover.parent.mkdir(parents=True, exist_ok=True) + try: + from PIL import Image + img = Image.open(io.BytesIO(img_data)).convert("RGB") + img.thumbnail((640, 640)) # covers stay small + tmp = cover.with_suffix(".png.tmp") + img.save(str(tmp), "PNG") + tmp.replace(cover) + except Exception as e: + return JSONResponse({"error": f"Invalid image: {e}"}, status_code=400) + return {"ok": True, "cover_url": _playlist_cover_url(pid)} + + +@app.get("/api/playlists/{pid}/cover") +def api_get_playlist_cover(pid: int): + cover = _playlist_cover_path(pid) + if not cover or not cover.exists(): + return JSONResponse({"error": "not found"}, status_code=404) + # no-cache (revalidate) like song art, so a replaced cover is never served + # stale — pairs with the mtime-ns cache-bust token on the URL. + return FileResponse(str(cover), media_type="image/png", headers=_ART_CACHE_HEADERS) + + +@app.delete("/api/playlists/{pid}/cover") +def api_delete_playlist_cover(pid: int): + cover = _playlist_cover_path(pid) + if cover and cover.exists(): + try: + cover.unlink() + except OSError: + pass + return {"ok": True} + + @app.post("/api/saved/toggle") def api_toggle_saved(data: dict): """Add/remove a song on the reserved Saved-for-Later playlist.""" diff --git a/static/v3/playlists.js b/static/v3/playlists.js index c336d65..b429391 100644 --- a/static/v3/playlists.js +++ b/static/v3/playlists.js @@ -28,6 +28,22 @@ } catch (e) { return null; } } + // Content-dependent playlist cover: a custom uploaded cover wins; otherwise + // the playlist's own song art — the icon when empty, one cover for a few + // songs, a 2×2 mosaic at 4+. `art_urls` / `cover_url` come from /api/playlists. + function playlistCoverHtml(p) { + const box = 'w-full aspect-square rounded-lg overflow-hidden bg-fb-bg/50 mb-3'; + const img = (u, cls) => ''; + if (p.cover_url) return '
' + img(p.cover_url, 'w-full h-full object-cover') + '
'; + const arts = Array.isArray(p.art_urls) ? p.art_urls : []; + if (!arts.length) { + return '
' + (p.system_key ? '🔖' : '🎵') + '
'; + } + if (arts.length < 4) return '
' + img(arts[0], 'w-full h-full object-cover') + '
'; + return '
' + + arts.slice(0, 4).map((u) => img(u, 'w-full h-full object-cover')).join('') + '
'; + } + function songRow(s, opts) { opts = opts || {}; const handle = opts.draggable @@ -96,8 +112,7 @@ (lists.length ? '
' + lists.map((p) => '').join('') + '
' @@ -126,8 +141,11 @@ '

' + esc(pl.name) + '

' + (isSystem ? '' : '
' + + '' + + (pl.cover_url ? '' : '') + '' + - '
') + + '' + + '') + '' + (pl.songs.length ? '
    ' + pl.songs.map((s) => songRow(s, { draggable: !isSystem })).join('') + '
' @@ -147,6 +165,25 @@ await fetch('/api/playlists/' + pid, { method: 'DELETE' }); renderPlaylists(); }); + // Custom cover: pick an image → upload as a data URL → the playlist card + // shows it (overriding the song-art cover). Re-render the detail so the + // Remove-cover button appears; the grid picks up the new cover on return. + const coverFile = root.querySelector('#v3-pl-cover-file'); + root.querySelector('#v3-pl-cover')?.addEventListener('click', () => coverFile && coverFile.click()); + coverFile?.addEventListener('change', () => { + const f = coverFile.files && coverFile.files[0]; + if (!f) return; + const reader = new FileReader(); + reader.onload = async (e) => { + await jsend('POST', '/api/playlists/' + pid + '/cover', { image: e.target.result }); + renderPlaylistDetail(pid); + }; + reader.readAsDataURL(f); + }); + root.querySelector('#v3-pl-cover-rm')?.addEventListener('click', async () => { + await fetch('/api/playlists/' + pid + '/cover', { method: 'DELETE' }); + renderPlaylistDetail(pid); + }); } // ── #v3-saved ─────────────────────────────────────────────────────────-- diff --git a/tests/js/v3_playlist_cover.test.js b/tests/js/v3_playlist_cover.test.js new file mode 100644 index 0000000..5a52c7d --- /dev/null +++ b/tests/js/v3_playlist_cover.test.js @@ -0,0 +1,28 @@ +// Guard for the content-dependent playlist cover (playlists.js). A custom +// uploaded cover wins; otherwise the playlist's song art decides: icon when +// empty, a single cover for a few songs, a 2×2 mosaic at 4+. (Rendering is DOM +// glue, so this is a source-level guard on the decision branches.) + +'use strict'; +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const PL = fs.readFileSync( + path.join(__dirname, '..', '..', 'static', 'v3', 'playlists.js'), 'utf8'); + +test('custom cover_url takes priority', () => { + assert.match(PL, /function playlistCoverHtml\(p\)/); + assert.match(PL, /if \(p\.cover_url\) return/); +}); + +test('empty → icon, <4 → single art, 4+ → 2×2 mosaic', () => { + assert.match(PL, /if \(!arts\.length\)[\s\S]{0,160}(🔖|🎵)/); // empty → icon + assert.match(PL, /arts\.length < 4\) return[\s\S]{0,120}arts\[0\]/); // a few → single cover + assert.match(PL, /grid-cols-2 grid-rows-2[\s\S]{0,120}slice\(0, 4\)/); // 4+ → mosaic +}); + +test('the card uses playlistCoverHtml (not the old static emoji box)', () => { + assert.match(PL, /playlistCoverHtml\(p\)/); +}); diff --git a/tests/test_playlists_api.py b/tests/test_playlists_api.py index 6f953f9..ae1caf6 100644 --- a/tests/test_playlists_api.py +++ b/tests/test_playlists_api.py @@ -106,3 +106,67 @@ def test_playlist_hides_dead_songs_when_library_populated(client, server): names = [s["filename"] for s in pl["songs"]] assert "live.archive" in names and "ghost.archive" not in names assert [p for p in client.get("/api/playlists").json() if p["id"] == pid][0]["count"] == 1 + + +# ── Playlist covers (content-dependent art + custom upload) ────────────────── + +def _png_b64(): + """A tiny base64 PNG with the data-URL prefix, like the browser sends.""" + import base64 + import io + from PIL import Image + buf = io.BytesIO() + Image.new("RGB", (4, 4), (200, 30, 60)).save(buf, "PNG") + return "data:image/png;base64," + base64.b64encode(buf.getvalue()).decode() + + +def test_list_includes_song_art_urls_for_content_cover(client, server): + for fn in ("x.archive", "y.archive"): + server.meta_db.put(fn, 0, 0, {}) + pid = client.post("/api/playlists", json={"name": "Arts"}).json()["id"] + client.post(f"/api/playlists/{pid}/songs", json={"filename": "x.archive"}) + client.post(f"/api/playlists/{pid}/songs", json={"filename": "y.archive"}) + pl = [p for p in client.get("/api/playlists").json() if p["id"] == pid][0] + assert pl["art_urls"] == ["/api/song/x.archive/art", "/api/song/y.archive/art"] + assert pl["cover_url"] is None # no custom cover yet + + +def test_custom_cover_roundtrip(client): + pid = client.post("/api/playlists", json={"name": "Cover"}).json()["id"] + r = client.post(f"/api/playlists/{pid}/cover", json={"image": _png_b64()}) + assert r.status_code == 200 and r.json()["ok"] is True + assert r.json()["cover_url"].startswith(f"/api/playlists/{pid}/cover") + # list + detail both report it + assert [p for p in client.get("/api/playlists").json() if p["id"] == pid][0]["cover_url"] + assert client.get(f"/api/playlists/{pid}").json()["cover_url"] + # served as a real PNG + img = client.get(f"/api/playlists/{pid}/cover") + assert img.status_code == 200 and img.headers["content-type"] == "image/png" + assert img.content[:8] == b"\x89PNG\r\n\x1a\n" + # removed + assert client.delete(f"/api/playlists/{pid}/cover").json() == {"ok": True} + assert client.get(f"/api/playlists/{pid}/cover").status_code == 404 + assert client.get(f"/api/playlists/{pid}").json()["cover_url"] is None + + +def test_cover_rejects_non_image(client): + pid = client.post("/api/playlists", json={"name": "Bad"}).json()["id"] + assert client.post(f"/api/playlists/{pid}/cover", + json={"image": "data:text/plain;base64,bm90IGFuIGltYWdl"}).status_code == 400 + assert client.post(f"/api/playlists/{pid}/cover", json={"image": ""}).status_code == 400 + + +def test_cover_rejects_non_string_image_with_400_not_500(client): + # A non-string `image` (number / null / object) must be a clean 400, not a + # 500 from `"," in ` raising TypeError before the type check. + pid = client.post("/api/playlists", json={"name": "Typed"}).json()["id"] + for bad in (123, None, {"x": 1}, ["a"]): + assert client.post(f"/api/playlists/{pid}/cover", json={"image": bad}).status_code == 400 + + +def test_deleting_playlist_removes_custom_cover(client, server): + pid = client.post("/api/playlists", json={"name": "Doomed"}).json()["id"] + client.post(f"/api/playlists/{pid}/cover", json={"image": _png_b64()}) + assert server._playlist_cover_path(pid).exists() + client.delete(f"/api/playlists/{pid}") + assert not server._playlist_cover_path(pid).exists()