From d67c0de5966d92c6706879f2f3bee266e9db4e3b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 15 May 2026 13:21:09 -0400 Subject: [PATCH] =?UTF-8?q?refactor(playlists):=20#411=20R2=20=E2=80=94=20?= =?UTF-8?q?generic=20registry-driven=20system=20endpoints?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the per-kind refresh/shuffle handlers with one generic pair driven off the kind registry, in lockstep across both clients. Server: - systemPlaylistKind gains Singleton; RefreshableSystemKind(key) exported. for_you/discover singleton; songs_like_artist not. - New generic POST /api/playlists/system/{kind}/refresh and GET /api/playlists/system/{kind}/shuffle ({kind} = raw system_variant). Non-singleton/unknown kind → 404. Deleted playlists_{foryou,discover}_refresh.go and the per-kind shuffle wrappers; serveSystemPlaylistShuffle core kept. - playlistRowView.refreshable: server-derived flag so clients show the refresh affordance generically without hardcoding kinds. Web (not drift-cached → uses the server flag): - refreshSystem(variant) replaces refreshForYou/refreshDiscover; systemShuffle drops the for_you→for-you mapping (raw variant). - PlaylistCard + detail page gate the kebab/Refresh button on playlist.refreshable; label is "Refresh {name}". Tests reworked; obsolete refresh-foryou/discover api tests deleted. Flutter (list tiles are drift-cache-sourced → derive, no migration): - Playlist.refreshable getter = isSystem && variant != songs_like_artist (holds for all current + planned kinds). - refreshSystem/systemShuffle use the raw variant; PlaylistCard + detail screen gate kebab/Regenerate/source-tagging on refreshable so songs_like_artist plays via get() (no by-kind endpoint). Pure-plumbing refactor; CI verifies parity. Next (R3): the five discovery mixes — each a candidate query + one registry entry. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../lib/api/endpoints/playlists.dart | 25 ++--- flutter_client/lib/models/playlist.dart | 10 ++ .../lib/playlists/playlist_detail_screen.dart | 6 +- .../lib/playlists/widgets/playlist_card.dart | 28 ++--- internal/api/api.go | 6 +- internal/api/playlists.go | 8 +- internal/api/playlists_discover_refresh.go | 69 ------------ internal/api/playlists_foryou_refresh.go | 98 ---------------- internal/api/playlists_system_shuffle.go | 106 +++++++++++++++--- internal/playlists/system.go | 31 ++++- .../api/playlists.refresh-discover.test.ts | 28 ----- .../lib/api/playlists.refresh-foryou.test.ts | 28 ----- web/src/lib/api/playlists.ts | 54 +++------ web/src/lib/api/types.ts | 6 +- .../lib/components/AddToPlaylistMenu.test.ts | 3 + web/src/lib/components/PlaylistCard.svelte | 27 ++--- web/src/lib/components/PlaylistCard.test.ts | 100 +++++++---------- web/src/routes/page.test.ts | 1 + web/src/routes/playlists/[id]/+page.svelte | 26 ++--- .../routes/playlists/[id]/playlist.test.ts | 43 ++++--- web/src/routes/playlists/playlists.test.ts | 2 +- 21 files changed, 277 insertions(+), 428 deletions(-) delete mode 100644 internal/api/playlists_discover_refresh.go delete mode 100644 internal/api/playlists_foryou_refresh.go delete mode 100644 web/src/lib/api/playlists.refresh-discover.test.ts delete mode 100644 web/src/lib/api/playlists.refresh-foryou.test.ts diff --git a/flutter_client/lib/api/endpoints/playlists.dart b/flutter_client/lib/api/endpoints/playlists.dart index 92f6d49b..3455aaed 100644 --- a/flutter_client/lib/api/endpoints/playlists.dart +++ b/flutter_client/lib/api/endpoints/playlists.dart @@ -45,15 +45,13 @@ class PlaylistsApi { return PlaylistDetail.fromJson(r.data ?? const {}); } - /// GET /api/playlists/system/{variant}/shuffle (#415 stage 2/3). + /// GET /api/playlists/system/{kind}/shuffle (#415 / #411 R2). /// Same shape as get() but tracks are server-ordered rotation-aware - /// (unplayed-this-rotation first; resets when exhausted). Model - /// variant uses underscores; the route segment is hyphenated. - /// Intentionally uncached — varies per play. + /// (unplayed-this-rotation first; resets when exhausted). {kind} is + /// the raw system_variant. Intentionally uncached — varies per play. Future systemShuffle(String variant) async { - final seg = variant == 'for_you' ? 'for-you' : variant; final r = await _dio.get>( - '/api/playlists/system/$seg/shuffle', + '/api/playlists/system/$variant/shuffle', ); return PlaylistDetail.fromJson(r.data ?? const {}); } @@ -67,17 +65,14 @@ class PlaylistsApi { ); } - /// POST /api/playlists/system/{variant}/refresh. Synchronously - /// rebuilds the caller's system playlist for the given variant - /// ("for_you" | "discover") and returns the new playlist id, or - /// null when the library is empty so there's nothing to build. - /// - /// The variant uses underscores in the model (system_variant) but - /// the route segment is hyphenated ("for-you"), so map here. + /// POST /api/playlists/system/{kind}/refresh (#411 R2). Rebuilds + /// the caller's system playlists and returns the named kind's new + /// playlist id, or null when the library is empty. {kind} is the + /// raw system_variant — the server routes generically off the + /// kind registry, no hyphen mapping. Future refreshSystem(String variant) async { - final segment = variant == 'for_you' ? 'for-you' : variant; final r = await _dio.post>( - '/api/playlists/system/$segment/refresh', + '/api/playlists/system/$variant/refresh', data: const {}, ); return (r.data ?? const {})['playlist_id'] as String?; diff --git a/flutter_client/lib/models/playlist.dart b/flutter_client/lib/models/playlist.dart index 7916fde7..1ae01860 100644 --- a/flutter_client/lib/models/playlist.dart +++ b/flutter_client/lib/models/playlist.dart @@ -38,6 +38,16 @@ class Playlist { bool get isSystem => systemVariant != null; + /// Whether this playlist supports the generic by-kind refresh/ + /// shuffle endpoints (#411 R2) — i.e. a singleton system kind. + /// The server exposes a `refreshable` flag for JSON-sourced + /// playlists, but the list tiles are drift-cache-sourced (no + /// migration just for this), so derive it: every system kind is a + /// singleton except songs_like_artist (multi-per-user). This rule + /// holds for For-You/Discover and all planned discovery mixes; if + /// a future non-singleton kind appears, extend the exclusion. + bool get refreshable => isSystem && systemVariant != 'songs_like_artist'; + factory Playlist.fromJson(Map j) => Playlist( id: j['id'] as String, userId: j['user_id'] as String? ?? '', diff --git a/flutter_client/lib/playlists/playlist_detail_screen.dart b/flutter_client/lib/playlists/playlist_detail_screen.dart index 7f0f59d8..b53cc7d2 100644 --- a/flutter_client/lib/playlists/playlist_detail_screen.dart +++ b/flutter_client/lib/playlists/playlist_detail_screen.dart @@ -212,7 +212,7 @@ class _Body extends ConsumerWidget { ref.read(playerActionsProvider).playTracks( playableRefs, initialIndex: startIdx >= 0 ? startIdx : 0, - source: detail.playlist.isSystem + source: detail.playlist.refreshable ? detail.playlist.systemVariant : null, ); @@ -266,7 +266,7 @@ class _Header extends ConsumerWidget { ), const Spacer(), if (playable.isNotEmpty) ...[ - if (p.isSystem) + if (p.refreshable) OutlinedButton.icon( key: const Key('regenerate_playlist_button'), onPressed: () => _regenerate(context, ref), @@ -298,7 +298,7 @@ class _Header extends ConsumerWidget { final refs = playable.map(_toTrackRef).toList(growable: false); ref.read(playerActionsProvider).playTracks( refs, - source: p.isSystem ? p.systemVariant : null, + source: p.refreshable ? p.systemVariant : null, ); }, icon: const Icon(Icons.play_arrow), diff --git a/flutter_client/lib/playlists/widgets/playlist_card.dart b/flutter_client/lib/playlists/widgets/playlist_card.dart index 0026834b..16cfaef0 100644 --- a/flutter_client/lib/playlists/widgets/playlist_card.dart +++ b/flutter_client/lib/playlists/widgets/playlist_card.dart @@ -69,7 +69,7 @@ class PlaylistCard extends ConsumerWidget { // System playlists get a refresh affordance so the // user can force a fresh mix instead of waiting for // the daily 03:00 rebuild. Mirrors the web kebab. - if (playlist.isSystem) + if (playlist.refreshable) Positioned( top: 2, right: 2, @@ -123,11 +123,7 @@ class PlaylistCard extends ConsumerWidget { ); } - String get _refreshLabel => switch (playlist.systemVariant) { - 'for_you' => 'Refresh For You', - 'discover' => 'Refresh Discover', - _ => 'Refresh', - }; + String get _refreshLabel => 'Refresh ${playlist.name}'; /// Forces a server-side rebuild of this system playlist, then /// invalidates the aggregate list so the rotated UUID + new tracks @@ -155,11 +151,12 @@ class PlaylistCard extends ConsumerWidget { /// index 0. Mirrors the web PlaylistCard's onPlayClick. Future _playPlaylist(WidgetRef ref) async { final api = await ref.read(playlistsApiProvider.future); - // System playlists: fetch the server's rotation-aware order - // (#415) and play it as-is, tagged with the source so the - // play-events reporter advances that playlist's rotation. User - // playlists keep stored order, untagged. - final detail = playlist.isSystem + // Refreshable (singleton) system playlists: fetch the server's + // rotation-aware order (#415) and play as-is, tagged with the + // source so the reporter advances rotation. User playlists AND + // non-singleton system kinds (songs_like_artist — no by-kind + // endpoint) play the stored order via get(), untagged. + final detail = playlist.refreshable ? await api.systemShuffle(playlist.systemVariant!) : await api.get(playlist.id); final refs = []; @@ -177,14 +174,13 @@ class PlaylistCard extends ConsumerWidget { )); } if (refs.isEmpty) return; - // System playlists already arrive in rotation-aware order from - // the server (#415) — play as-is, tagged with the variant so the - // reporter advances rotation. No client shuffle. User playlists - // play in stored order, untagged. + // Rotation-aware kinds arrive pre-ordered from the server — play + // as-is, tagged so the reporter advances rotation. No client + // shuffle. Everything else plays stored order, untagged. await ref.read(playerActionsProvider).playTracks( refs, initialIndex: 0, - source: playlist.isSystem ? playlist.systemVariant : null, + source: playlist.refreshable ? playlist.systemVariant : null, ); } } diff --git a/internal/api/api.go b/internal/api/api.go index 3cbf2dbf..52878972 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -172,10 +172,8 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev authed.Delete("/playlists/{id}/tracks/{position}", h.handleRemovePlaylistTrack) authed.Put("/playlists/{id}/tracks", h.handleReorderPlaylist) authed.Get("/playlists/{id}/cover", h.handleGetPlaylistCover) - authed.Post("/playlists/system/discover/refresh", h.handleDiscoverRefresh) - authed.Post("/playlists/system/for-you/refresh", h.handleForYouRefresh) - authed.Get("/playlists/system/discover/shuffle", h.handleDiscoverShuffle) - authed.Get("/playlists/system/for-you/shuffle", h.handleForYouShuffle) + authed.Post("/playlists/system/{kind}/refresh", h.handleSystemPlaylistRefresh) + authed.Get("/playlists/system/{kind}/shuffle", h.handleSystemPlaylistShuffle) }) }) } diff --git a/internal/api/playlists.go b/internal/api/playlists.go index 1a94f5a9..bd2779a0 100644 --- a/internal/api/playlists.go +++ b/internal/api/playlists.go @@ -32,7 +32,12 @@ type playlistRowView struct { IsPublic bool `json:"is_public"` Kind string `json:"kind"` SystemVariant *string `json:"system_variant,omitempty"` - SeedArtistID *string `json:"seed_artist_id,omitempty"` + // Refreshable: a singleton system kind addressable by the generic + // /system/{kind}/{refresh,shuffle} endpoints. Lets clients show + // the per-tile refresh affordance generically without hardcoding + // which kinds support it (false for user + songs_like_artist). + Refreshable bool `json:"refreshable"` + SeedArtistID *string `json:"seed_artist_id,omitempty"` CoverURL string `json:"cover_url,omitempty"` TrackCount int32 `json:"track_count"` DurationSec int32 `json:"duration_sec"` @@ -435,6 +440,7 @@ func playlistRowToView(r *playlists.PlaylistRow) playlistRowView { IsPublic: r.IsPublic, Kind: r.Kind, SystemVariant: r.SystemVariant, + Refreshable: r.SystemVariant != nil && playlists.RefreshableSystemKind(*r.SystemVariant), TrackCount: r.TrackCount, DurationSec: r.DurationSec, CreatedAt: formatTimestamp(r.CreatedAt), diff --git a/internal/api/playlists_discover_refresh.go b/internal/api/playlists_discover_refresh.go deleted file mode 100644 index 7ec2cf9f..00000000 --- a/internal/api/playlists_discover_refresh.go +++ /dev/null @@ -1,69 +0,0 @@ -package api - -import ( - "errors" - "net/http" - "time" - - "github.com/jackc/pgx/v5" - - "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" - "git.fabledsword.com/bvandeusen/minstrel/internal/playlists" -) - -// discoverRefreshResp is the wire shape for POST -// /api/playlists/system/discover/refresh. playlist_id is null when -// the build succeeded but the user's library yielded no eligible -// tracks (degenerate empty-library); track_count is 0 in that case. -type discoverRefreshResp struct { - PlaylistID *string `json:"playlist_id"` - TrackCount int32 `json:"track_count"` -} - -// handleDiscoverRefresh re-runs BuildSystemPlaylists synchronously -// for the calling user, then returns the resulting Discover row's -// id + track_count. Used by the frontend's refresh affordances on -// the detail page header and the home tile kebab. -// -// Authenticated user only (gated by the authed sub-router's -// RequireUser middleware). Each user only ever refreshes their own -// Discover — no admin route, no cross-user impersonation. -func (h *handlers) handleDiscoverRefresh(w http.ResponseWriter, r *http.Request) { - user, ok := requireUser(w, r) - if !ok { - return - } - if err := playlists.BuildSystemPlaylists(r.Context(), h.pool, h.logger, user.ID, time.Now(), h.dataDir); err != nil { - h.logger.Error("discover refresh: build failed", - "user_id", uuidToString(user.ID), "err", err) - writeErr(w, apierror.InternalMsg("build failed", err)) - return - } - - sysVariant := "discover" - pl, err := dbq.New(h.pool).GetSystemPlaylistByVariantForUser(r.Context(), - dbq.GetSystemPlaylistByVariantForUserParams{ - UserID: user.ID, - SystemVariant: &sysVariant, - }) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - // Build succeeded but no Discover row produced — empty library. - writeJSON(w, http.StatusOK, discoverRefreshResp{ - PlaylistID: nil, TrackCount: 0, - }) - return - } - h.logger.Error("discover refresh: lookup failed", - "user_id", uuidToString(user.ID), "err", err) - writeErr(w, apierror.InternalMsg("lookup failed", err)) - return - } - - idStr := uuidToString(pl.ID) - writeJSON(w, http.StatusOK, discoverRefreshResp{ - PlaylistID: &idStr, - TrackCount: pl.TrackCount, - }) -} diff --git a/internal/api/playlists_foryou_refresh.go b/internal/api/playlists_foryou_refresh.go deleted file mode 100644 index eee83f8f..00000000 --- a/internal/api/playlists_foryou_refresh.go +++ /dev/null @@ -1,98 +0,0 @@ -package api - -import ( - "errors" - "net/http" - "time" - - "github.com/jackc/pgx/v5" - - "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" - "git.fabledsword.com/bvandeusen/minstrel/internal/playlists" -) - -// foryouRefreshResp is the wire shape for POST -// /api/playlists/system/for-you/refresh. track_ids is the playlist's -// tracks in playlist position order — the frontend enqueues this -// list directly to start playback without a second roundtrip. -// -// playlist_id is null + track_ids empty when the build succeeded but -// the user's library yielded no eligible candidates (degenerate -// empty-library case). -type foryouRefreshResp struct { - PlaylistID *string `json:"playlist_id"` - TrackCount int32 `json:"track_count"` - TrackIDs []string `json:"track_ids"` -} - -// handleForYouRefresh re-runs BuildSystemPlaylists synchronously for -// the calling user and returns their freshly-built For-You playlist. -// Used by the home Playlists row's For-You tile play button: click -// triggers refresh, the response carries the track IDs, the -// frontend enqueues + auto-plays. -// -// Authenticated user only — each user refreshes only their own -// For-You. -func (h *handlers) handleForYouRefresh(w http.ResponseWriter, r *http.Request) { - user, ok := requireUser(w, r) - if !ok { - return - } - if err := playlists.BuildSystemPlaylists(r.Context(), h.pool, h.logger, user.ID, time.Now(), h.dataDir); err != nil { - h.logger.Error("for-you refresh: build failed", - "user_id", uuidToString(user.ID), "err", err) - writeErr(w, apierror.InternalMsg("build failed", err)) - return - } - - q := dbq.New(h.pool) - sysVariant := "for_you" - pl, err := q.GetSystemPlaylistByVariantForUser(r.Context(), - dbq.GetSystemPlaylistByVariantForUserParams{ - UserID: user.ID, - SystemVariant: &sysVariant, - }) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - // Build succeeded but no For-You row — empty library. - writeJSON(w, http.StatusOK, foryouRefreshResp{ - PlaylistID: nil, - TrackCount: 0, - TrackIDs: []string{}, - }) - return - } - h.logger.Error("for-you refresh: lookup failed", - "user_id", uuidToString(user.ID), "err", err) - writeErr(w, apierror.InternalMsg("lookup failed", err)) - return - } - - // Fetch tracks in playlist position order so the frontend can - // enqueue them directly. - rows, err := q.ListPlaylistTracks(r.Context(), pl.ID) - if err != nil { - h.logger.Error("for-you refresh: list tracks failed", - "playlist_id", uuidToString(pl.ID), "err", err) - writeErr(w, apierror.InternalMsg("list tracks failed", err)) - return - } - trackIDs := make([]string, 0, len(rows)) - for _, row := range rows { - // Skip rows where the track was removed from the library - // (LiveTrackID won't be Valid). The snapshot row still - // exists in playlist_tracks, but there's nothing to enqueue. - if !row.LiveTrackID.Valid { - continue - } - trackIDs = append(trackIDs, uuidToString(row.LiveTrackID)) - } - - idStr := uuidToString(pl.ID) - writeJSON(w, http.StatusOK, foryouRefreshResp{ - PlaylistID: &idStr, - TrackCount: pl.TrackCount, - TrackIDs: trackIDs, - }) -} diff --git a/internal/api/playlists_system_shuffle.go b/internal/api/playlists_system_shuffle.go index e531f3f0..bca19717 100644 --- a/internal/api/playlists_system_shuffle.go +++ b/internal/api/playlists_system_shuffle.go @@ -4,7 +4,9 @@ import ( "errors" "math/rand" "net/http" + "time" + "github.com/go-chi/chi/v5" "github.com/jackc/pgx/v5" "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" @@ -12,27 +14,99 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/playlists" ) -// Stage 2 of #415. GET /api/playlists/system/{variant}/shuffle returns -// the caller's system playlist with tracks in rotation-aware play -// order: tracks not yet heard this rotation first (shuffled), then -// already-heard tracks (shuffled). When the whole snapshot has been -// heard the rotation resets and the full list reshuffles. +// Generic registry-driven system-playlist endpoints (#411 R2), +// replacing the former per-kind handlers. {kind} is the raw +// system_variant ('for_you' | 'discover' | future singleton kinds); +// only singleton kinds (playlists.RefreshableSystemKind) are +// addressable here — songs_like_artist is multi-per-user and played +// by playlist id, so it 404s. // -// Same JSON shape as GET /api/playlists/{id} so clients reuse their -// existing track parsing. Intentionally a separate, uncached endpoint -// (Option A): the cached playlist-detail GET stays pure for the -// "open the playlist to look at it" path; this one varies per play. +// GET /api/playlists/system/{kind}/shuffle — the caller's playlist +// for that kind with tracks in rotation-aware order (#415): unheard +// this rotation first (shuffled), then heard; resets when exhausted. +// Same JSON shape as GET /api/playlists/{id} so clients reuse track +// parsing. Intentionally uncached — it varies per play, unlike the +// cached detail GET. // -// Two explicit static routes (one per variant) mirror the refresh -// handlers and sidestep chi static-vs-param ambiguity under -// /playlists/system/. Future kinds add their own thin route. +// POST /api/playlists/system/{kind}/refresh — rebuilds the caller's +// system playlists and returns that kind's fresh row. -func (h *handlers) handleForYouShuffle(w http.ResponseWriter, r *http.Request) { - h.serveSystemPlaylistShuffle(w, r, "for_you") +func systemKindFromURL(w http.ResponseWriter, r *http.Request) (string, bool) { + kind := chi.URLParam(r, "kind") + if !playlists.RefreshableSystemKind(kind) { + writeErr(w, &apierror.Error{Status: 404, Code: "not_found", Message: "unknown system playlist"}) + return "", false + } + return kind, true } -func (h *handlers) handleDiscoverShuffle(w http.ResponseWriter, r *http.Request) { - h.serveSystemPlaylistShuffle(w, r, "discover") +func (h *handlers) handleSystemPlaylistShuffle(w http.ResponseWriter, r *http.Request) { + kind, ok := systemKindFromURL(w, r) + if !ok { + return + } + h.serveSystemPlaylistShuffle(w, r, kind) +} + +// systemRefreshResp unifies the prior for-you/discover shapes +// (for-you carried track_ids, discover didn't); clients ignore +// extra fields. playlist_id null + empty when the build produced no +// row for that kind (empty library). +type systemRefreshResp struct { + PlaylistID *string `json:"playlist_id"` + TrackCount int32 `json:"track_count"` + TrackIDs []string `json:"track_ids"` +} + +func (h *handlers) handleSystemPlaylistRefresh(w http.ResponseWriter, r *http.Request) { + user, ok := requireUser(w, r) + if !ok { + return + } + kind, ok := systemKindFromURL(w, r) + if !ok { + return + } + if err := playlists.BuildSystemPlaylists(r.Context(), h.pool, h.logger, user.ID, time.Now(), h.dataDir); err != nil { + h.logger.Error("system refresh: build failed", + "kind", kind, "user_id", uuidToString(user.ID), "err", err) + writeErr(w, apierror.InternalMsg("build failed", err)) + return + } + q := dbq.New(h.pool) + v := kind + pl, err := q.GetSystemPlaylistByVariantForUser(r.Context(), + dbq.GetSystemPlaylistByVariantForUserParams{UserID: user.ID, SystemVariant: &v}) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + writeJSON(w, http.StatusOK, systemRefreshResp{TrackIDs: []string{}}) + return + } + h.logger.Error("system refresh: lookup failed", + "kind", kind, "user_id", uuidToString(user.ID), "err", err) + writeErr(w, apierror.InternalMsg("lookup failed", err)) + return + } + rows, err := q.ListPlaylistTracks(r.Context(), pl.ID) + if err != nil { + h.logger.Error("system refresh: list tracks failed", + "kind", kind, "playlist_id", uuidToString(pl.ID), "err", err) + writeErr(w, apierror.InternalMsg("list tracks failed", err)) + return + } + trackIDs := make([]string, 0, len(rows)) + for _, row := range rows { + if !row.LiveTrackID.Valid { + continue + } + trackIDs = append(trackIDs, uuidToString(row.LiveTrackID)) + } + idStr := uuidToString(pl.ID) + writeJSON(w, http.StatusOK, systemRefreshResp{ + PlaylistID: &idStr, + TrackCount: pl.TrackCount, + TrackIDs: trackIDs, + }) } func (h *handlers) serveSystemPlaylistShuffle(w http.ResponseWriter, r *http.Request, variant string) { diff --git a/internal/playlists/system.go b/internal/playlists/system.go index 6e7cadc2..97b97e8f 100644 --- a/internal/playlists/system.go +++ b/internal/playlists/system.go @@ -246,8 +246,29 @@ type systemPlaylistProducer func( ) ([]builtPlaylist, error) type systemPlaylistKind struct { - Key string - Produce systemPlaylistProducer + Key string + // Singleton kinds produce exactly one playlist per user (For-You, + // Discover, the discovery mixes) and so can be addressed by kind: + // the generic /system/{kind}/{refresh,shuffle} endpoints + the + // per-tile refresh affordance only apply to these. Non-singleton + // kinds (songs_like_artist → up to 3 per user) are played by + // playlist id and have no by-kind endpoint. + Singleton bool + Produce systemPlaylistProducer +} + +// RefreshableSystemKind reports whether `key` is a known singleton +// system-playlist kind — i.e. addressable by the generic by-kind +// refresh/shuffle endpoints and eligible for the per-tile refresh +// affordance. The API layer + clients use this so neither hardcodes +// the kind list. +func RefreshableSystemKind(key string) bool { + for _, k := range systemPlaylistRegistry { + if k.Key == key { + return k.Singleton + } + } + return false } // systemPlaylistRegistry is the single source of truth for which @@ -258,9 +279,9 @@ type systemPlaylistKind struct { // it has no functional effect (atomic replace + per-playlist // collage are order-independent). var systemPlaylistRegistry = []systemPlaylistKind{ - {Key: "for_you", Produce: produceForYou}, - {Key: "songs_like_artist", Produce: produceSeedMixes}, - {Key: "discover", Produce: produceDiscover}, + {Key: "for_you", Singleton: true, Produce: produceForYou}, + {Key: "songs_like_artist", Singleton: false, Produce: produceSeedMixes}, + {Key: "discover", Singleton: true, Produce: produceDiscover}, } // produceForYou: today's seed from the user's top-5 played tracks diff --git a/web/src/lib/api/playlists.refresh-discover.test.ts b/web/src/lib/api/playlists.refresh-discover.test.ts deleted file mode 100644 index 98b72e43..00000000 --- a/web/src/lib/api/playlists.refresh-discover.test.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { refreshDiscover } from './playlists'; - -vi.mock('./client', () => ({ - api: { post: vi.fn() } -})); - -import { api } from './client'; - -describe('refreshDiscover', () => { - beforeEach(() => vi.clearAllMocks()); - - it('POSTs the correct path and returns the response', async () => { - const sample = { playlist_id: 'abc-123', track_count: 100 }; - (api.post as unknown as ReturnType).mockResolvedValueOnce(sample); - const got = await refreshDiscover(); - expect(api.post).toHaveBeenCalledWith('/api/playlists/system/discover/refresh', {}); - expect(got).toEqual(sample); - }); - - it('handles empty-library response', async () => { - const sample = { playlist_id: null, track_count: 0 }; - (api.post as unknown as ReturnType).mockResolvedValueOnce(sample); - const got = await refreshDiscover(); - expect(got.playlist_id).toBe(null); - expect(got.track_count).toBe(0); - }); -}); diff --git a/web/src/lib/api/playlists.refresh-foryou.test.ts b/web/src/lib/api/playlists.refresh-foryou.test.ts deleted file mode 100644 index de715232..00000000 --- a/web/src/lib/api/playlists.refresh-foryou.test.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { refreshForYou } from './playlists'; - -vi.mock('./client', () => ({ - api: { post: vi.fn() } -})); - -import { api } from './client'; - -describe('refreshForYou', () => { - beforeEach(() => vi.clearAllMocks()); - - it('POSTs the correct path and returns the response', async () => { - const sample = { playlist_id: 'pl-abc', track_count: 25, track_ids: ['t1', 't2'] }; - (api.post as unknown as ReturnType).mockResolvedValueOnce(sample); - const got = await refreshForYou(); - expect(api.post).toHaveBeenCalledWith('/api/playlists/system/for-you/refresh', {}); - expect(got).toEqual(sample); - }); - - it('handles empty-library response', async () => { - const sample = { playlist_id: null, track_count: 0, track_ids: [] }; - (api.post as unknown as ReturnType).mockResolvedValueOnce(sample); - const got = await refreshForYou(); - expect(got.playlist_id).toBe(null); - expect(got.track_ids).toEqual([]); - }); -}); diff --git a/web/src/lib/api/playlists.ts b/web/src/lib/api/playlists.ts index e1dfa192..bd340d74 100644 --- a/web/src/lib/api/playlists.ts +++ b/web/src/lib/api/playlists.ts @@ -21,13 +21,27 @@ export async function getPlaylist(id: string): Promise { // #415 stage 2/3: rotation-aware play order for a system playlist. // Same shape as getPlaylist but tracks are server-ordered (unplayed -// this rotation first, then heard; resets when exhausted). The model -// variant uses underscores; the route segment is hyphenated. +// this rotation first, then heard; resets when exhausted). // Intentionally uncached — it varies per play, unlike getPlaylist. +// {variant} is the raw system_variant (#411 R2: generic by-kind). export async function systemShuffle(variant: string): Promise { - const seg = variant === 'for_you' ? 'for-you' : variant; return api.get( - `/api/playlists/system/${encodeURIComponent(seg)}/shuffle`, + `/api/playlists/system/${encodeURIComponent(variant)}/shuffle`, + ); +} + +// #411 R2: generic by-kind refresh. Rebuilds the caller's system +// playlists and returns the named kind's fresh row. Replaces the +// former per-kind refreshForYou/refreshDiscover. +export type RefreshSystemResponse = { + playlist_id: string | null; + track_count: number; + track_ids: string[]; +}; +export async function refreshSystem(variant: string): Promise { + return api.post( + `/api/playlists/system/${encodeURIComponent(variant)}/refresh`, + {}, ); } @@ -91,35 +105,3 @@ export function createPlaylistQuery(id: string) { }); } -// System playlist refresh ---------------------------------------------------- - -export type RefreshDiscoverResponse = { - playlist_id: string | null; - track_count: number; -}; - -// Re-runs the daily system playlist build for the calling user and -// returns the resulting Discover playlist's id + track count. -// Used by the detail-page Refresh button and the home tile kebab. -export async function refreshDiscover(): Promise { - return api.post('/api/playlists/system/discover/refresh', {}); -} - -// For-You refresh ------------------------------------------------------------ - -export type RefreshForYouResponse = { - playlist_id: string | null; - track_count: number; - track_ids: string[]; -}; - -// POST /api/playlists/system/for-you/refresh — synchronously -// rebuilds the calling user's For-You playlist and returns the new -// id, track count, and track id list. -// -// Used by the home Playlists row's For-You tile play button: click -// triggers refresh, the frontend then calls getPlaylist(new_id) to -// get full TrackRef snapshots and enqueues for playback. -export async function refreshForYou(): Promise { - return api.post('/api/playlists/system/for-you/refresh', {}); -} diff --git a/web/src/lib/api/types.ts b/web/src/lib/api/types.ts index c58ded2f..d4235b42 100644 --- a/web/src/lib/api/types.ts +++ b/web/src/lib/api/types.ts @@ -59,7 +59,11 @@ export type Playlist = { description: string; is_public: boolean; kind: 'user' | 'system'; - system_variant: 'for_you' | 'songs_like_artist' | 'discover' | null; + system_variant: string | null; + // Server-derived: a singleton system kind addressable by the + // generic /system/{kind}/{refresh,shuffle} endpoints. Drives the + // per-tile refresh affordance without the client hardcoding kinds. + refreshable: boolean; seed_artist_id: string | null; cover_url: string; // empty string when no cover; UI renders glyph track_count: number; diff --git a/web/src/lib/components/AddToPlaylistMenu.test.ts b/web/src/lib/components/AddToPlaylistMenu.test.ts index 89ae38e5..8f0a123b 100644 --- a/web/src/lib/components/AddToPlaylistMenu.test.ts +++ b/web/src/lib/components/AddToPlaylistMenu.test.ts @@ -14,6 +14,7 @@ const playlistsData = vi.hoisted(() => ({ is_public: false, kind: 'user', system_variant: null, + refreshable: false, seed_artist_id: null, cover_url: '', track_count: 3, @@ -30,6 +31,7 @@ const playlistsData = vi.hoisted(() => ({ is_public: false, kind: 'user', system_variant: null, + refreshable: false, seed_artist_id: null, cover_url: '', track_count: 5, @@ -58,6 +60,7 @@ vi.mock('$lib/api/playlists', () => ({ is_public: false, kind: 'user', system_variant: null, + refreshable: false, seed_artist_id: null, cover_url: '', track_count: 0, diff --git a/web/src/lib/components/PlaylistCard.svelte b/web/src/lib/components/PlaylistCard.svelte index 92f41875..1436afca 100644 --- a/web/src/lib/components/PlaylistCard.svelte +++ b/web/src/lib/components/PlaylistCard.svelte @@ -3,7 +3,7 @@ import { useQueryClient } from '@tanstack/svelte-query'; import type { Playlist, PlaylistTrack, TrackRef } from '$lib/api/types'; import { user } from '$lib/auth/store.svelte'; - import { getPlaylist, systemShuffle, refreshDiscover, refreshForYou } from '$lib/api/playlists'; + import { getPlaylist, systemShuffle, refreshSystem } from '$lib/api/playlists'; import { playlistTrackToRef } from '$lib/playlists/playlistTrackToRef'; import { errCode } from '$lib/api/errors'; import { qk } from '$lib/api/queries'; @@ -13,12 +13,9 @@ let { playlist }: { playlist: Playlist } = $props(); const isOwn = $derived(user.value?.id === playlist.user_id); - const isDiscover = $derived(playlist.system_variant === 'discover'); - const isForYou = $derived(playlist.system_variant === 'for_you'); - const isSystem = $derived(playlist.system_variant != null); - const refreshLabel = $derived( - isDiscover ? 'Refresh Discover' : isForYou ? 'Refresh For You' : 'Refresh', - ); + // Server tells us which system playlists support by-kind refresh + // (#411 R2) — no hardcoded kind list here. + const refreshable = $derived(playlist.refreshable === true); const queryClient = useQueryClient(); @@ -72,16 +69,10 @@ e.preventDefault(); e.stopPropagation(); menuOpen = false; + if (!refreshable || playlist.system_variant == null) return; try { - if (isDiscover) { - await refreshDiscover(); - pushToast('Discover refreshed.'); - } else if (isForYou) { - await refreshForYou(); - pushToast('For You refreshed.'); - } else { - return; - } + await refreshSystem(playlist.system_variant); + pushToast(`${playlist.name} refreshed.`); await queryClient.invalidateQueries({ queryKey: qk.playlist(playlist.id) }); await queryClient.invalidateQueries({ queryKey: qk.playlists() }); } catch (err: unknown) { @@ -93,7 +84,7 @@ { menuOpen = false; }} />
- {#if isSystem} + {#if refreshable}
{/if} diff --git a/web/src/lib/components/PlaylistCard.test.ts b/web/src/lib/components/PlaylistCard.test.ts index 5eac8386..c5d3bf3c 100644 --- a/web/src/lib/components/PlaylistCard.test.ts +++ b/web/src/lib/components/PlaylistCard.test.ts @@ -17,6 +17,7 @@ vi.mock('$lib/api/playlists', () => ({ is_public: false, kind: 'user', system_variant: null, + refreshable: false, seed_artist_id: null, cover_url: '', track_count: 1, @@ -47,6 +48,7 @@ vi.mock('$lib/api/playlists', () => ({ is_public: false, kind: 'system', system_variant: 'for_you', + refreshable: true, seed_artist_id: null, cover_url: '', track_count: 1, @@ -68,8 +70,7 @@ vi.mock('$lib/api/playlists', () => ({ } ] } as PlaylistDetail), - refreshDiscover: vi.fn().mockResolvedValue({ playlist_id: 'p-1', track_count: 5 }), - refreshForYou: vi.fn().mockResolvedValue({ playlist_id: 'p-new', track_count: 25, track_ids: ['t-1'] }) + refreshSystem: vi.fn().mockResolvedValue({ playlist_id: 'p-1', track_count: 5, track_ids: ['t-1'] }) })); vi.mock('$lib/player/store.svelte', () => ({ @@ -92,6 +93,7 @@ const base: Playlist = { is_public: false, kind: 'user', system_variant: null, + refreshable: false, seed_artist_id: null, cover_url: '', track_count: 12, @@ -173,98 +175,73 @@ describe('PlaylistCard', () => { expect(playQueue).toHaveBeenCalled(); }); - test('shows kebab button on Discover playlists', () => { + test('shows kebab on refreshable (singleton system) playlists', () => { const discoverPlaylist: Playlist = { ...base, kind: 'system', system_variant: 'discover', + refreshable: true, name: 'Discover' }; render(PlaylistCard, { props: { playlist: discoverPlaylist } }); expect(screen.getByLabelText(/playlist actions/i)).toBeInTheDocument(); }); - test('shows kebab button on For You playlists', () => { - const forYouPlaylist: Playlist = { - ...base, - kind: 'system', - system_variant: 'for_you', - name: 'For You' - }; - render(PlaylistCard, { props: { playlist: forYouPlaylist } }); - expect(screen.getByLabelText(/playlist actions/i)).toBeInTheDocument(); - }); - - test('does not show kebab button on user playlists', () => { + test('does not show kebab on user playlists', () => { render(PlaylistCard, { props: { playlist: base } }); expect(screen.queryByLabelText(/playlist actions/i)).not.toBeInTheDocument(); }); - test('Refresh Discover item appears in Discover kebab', async () => { + test('does not show kebab on a non-refreshable system playlist', () => { + // e.g. songs_like_artist — system but multi-per-user, server + // reports refreshable:false. + const songsLike: Playlist = { + ...base, + kind: 'system', + system_variant: 'songs_like_artist', + refreshable: false, + name: 'Songs like X' + }; + render(PlaylistCard, { props: { playlist: songsLike } }); + expect(screen.queryByLabelText(/playlist actions/i)).not.toBeInTheDocument(); + }); + + test('Refresh item appears in the kebab, labelled by name', async () => { const discoverPlaylist: Playlist = { ...base, kind: 'system', system_variant: 'discover', + refreshable: true, name: 'Discover' }; render(PlaylistCard, { props: { playlist: discoverPlaylist } }); - const kebabBtn = screen.getByLabelText(/playlist actions/i); - await fireEvent.click(kebabBtn); + await fireEvent.click(screen.getByLabelText(/playlist actions/i)); expect(screen.getByRole('menuitem', { name: /refresh discover/i })).toBeInTheDocument(); }); - test('Refresh For You item appears in For You kebab', async () => { - const forYouPlaylist: Playlist = { - ...base, - kind: 'system', - system_variant: 'for_you', - name: 'For You' - }; - render(PlaylistCard, { props: { playlist: forYouPlaylist } }); - const kebabBtn = screen.getByLabelText(/playlist actions/i); - await fireEvent.click(kebabBtn); - expect(screen.getByRole('menuitem', { name: /refresh for you/i })).toBeInTheDocument(); - }); - - test('clicking Refresh Discover calls refreshDiscover', async () => { - const { refreshDiscover } = await import('$lib/api/playlists'); + test('clicking Refresh calls refreshSystem with the raw variant', async () => { + const { refreshSystem } = await import('$lib/api/playlists'); const discoverPlaylist: Playlist = { ...base, kind: 'system', system_variant: 'discover', + refreshable: true, name: 'Discover' }; render(PlaylistCard, { props: { playlist: discoverPlaylist } }); - const kebabBtn = screen.getByLabelText(/playlist actions/i); - await fireEvent.click(kebabBtn); - const refreshItem = screen.getByRole('menuitem', { name: /refresh discover/i }); - await fireEvent.click(refreshItem); - await waitFor(() => expect(refreshDiscover).toHaveBeenCalled()); + await fireEvent.click(screen.getByLabelText(/playlist actions/i)); + await fireEvent.click(screen.getByRole('menuitem', { name: /refresh discover/i })); + await waitFor(() => expect(refreshSystem).toHaveBeenCalledWith('discover')); }); - test('clicking Refresh For You calls refreshForYou', async () => { - const { refreshForYou } = await import('$lib/api/playlists'); - const forYouPlaylist: Playlist = { - ...base, - kind: 'system', - system_variant: 'for_you', - name: 'For You' - }; - render(PlaylistCard, { props: { playlist: forYouPlaylist } }); - const kebabBtn = screen.getByLabelText(/playlist actions/i); - await fireEvent.click(kebabBtn); - const refreshItem = screen.getByRole('menuitem', { name: /refresh for you/i }); - await fireEvent.click(refreshItem); - await waitFor(() => expect(refreshForYou).toHaveBeenCalled()); - }); - - test('For-You play calls systemShuffle, not getPlaylist or refreshForYou', async () => { - const { refreshForYou, getPlaylist, systemShuffle } = await import('$lib/api/playlists'); + test('For-You play calls systemShuffle, not getPlaylist', async () => { + const { getPlaylist, systemShuffle } = await import('$lib/api/playlists'); const { playQueue } = await import('$lib/player/store.svelte'); const forYouPlaylist: Playlist = { ...base, kind: 'system', system_variant: 'for_you', + refreshable: true, name: 'For You', track_count: 25 }; @@ -274,11 +251,10 @@ describe('PlaylistCard', () => { await fireEvent.click(playButton); await new Promise(resolve => setTimeout(resolve, 10)); - // #415: system play hits the rotation-aware shuffle endpoint, - // not the cached detail GET, and never refreshes on press. + // #415/#411: system play hits the rotation-aware shuffle endpoint + // (raw variant), not the cached detail GET, and never refreshes. expect(systemShuffle).toHaveBeenCalledWith('for_you'); expect(getPlaylist).not.toHaveBeenCalled(); - expect(refreshForYou).not.toHaveBeenCalled(); expect(playQueue).toHaveBeenCalled(); }); @@ -315,8 +291,8 @@ describe('PlaylistCard', () => { expect(playQueue).toHaveBeenCalledWith(expect.any(Array), 0); }); - test('non-For-You play button does NOT call refreshForYou', async () => { - const { refreshForYou, getPlaylist } = await import('$lib/api/playlists'); + test('user-playlist play does not refresh anything', async () => { + const { refreshSystem, getPlaylist } = await import('$lib/api/playlists'); const { playQueue } = await import('$lib/player/store.svelte'); render(PlaylistCard, { props: { playlist: base } }); @@ -324,7 +300,7 @@ describe('PlaylistCard', () => { await fireEvent.click(playButton); await new Promise(resolve => setTimeout(resolve, 10)); - expect(refreshForYou).not.toHaveBeenCalled(); + expect(refreshSystem).not.toHaveBeenCalled(); expect(getPlaylist).toHaveBeenCalledWith('p-1'); expect(playQueue).toHaveBeenCalled(); }); diff --git a/web/src/routes/page.test.ts b/web/src/routes/page.test.ts index 884dd067..a4d6b76f 100644 --- a/web/src/routes/page.test.ts +++ b/web/src/routes/page.test.ts @@ -84,6 +84,7 @@ describe('home Playlists section', () => { is_public: false, kind: 'system', system_variant: 'for_you', + refreshable: false, seed_artist_id: null, cover_url: '', track_count: 25, diff --git a/web/src/routes/playlists/[id]/+page.svelte b/web/src/routes/playlists/[id]/+page.svelte index 8182ad39..0767e0f4 100644 --- a/web/src/routes/playlists/[id]/+page.svelte +++ b/web/src/routes/playlists/[id]/+page.svelte @@ -12,7 +12,7 @@ deletePlaylist, removePlaylistTrack, reorderPlaylist, - refreshDiscover + refreshSystem } from '$lib/api/playlists'; import { qk } from '$lib/api/queries'; import { user } from '$lib/auth/store.svelte'; @@ -130,20 +130,20 @@ } } - // --- Discover refresh --- - let refreshingDiscover = $state(false); + // --- System-playlist refresh (#411 R2: generic by-kind) --- + let refreshingSystem = $state(false); - async function onRefreshDiscover() { - refreshingDiscover = true; + async function onRefreshSystem(variant: string) { + refreshingSystem = true; try { - await refreshDiscover(); - pushToast('Discover refreshed.'); + await refreshSystem(variant); + pushToast('Refreshed.'); await queryClient.invalidateQueries({ queryKey: qk.playlist(id) }); await queryClient.invalidateQueries({ queryKey: qk.playlists() }); } catch (e: unknown) { pushToast(`Refresh failed: ${errCode(e)}`, 'error'); } finally { - refreshingDiscover = false; + refreshingSystem = false; } } @@ -177,15 +177,15 @@ {#if !isOwner}· by {pl.owner_username}{/if}

- {#if pl.system_variant === 'discover'} + {#if pl.refreshable && pl.system_variant} {/if} {#if isOwner} diff --git a/web/src/routes/playlists/[id]/playlist.test.ts b/web/src/routes/playlists/[id]/playlist.test.ts index 4eac7aef..031d170c 100644 --- a/web/src/routes/playlists/[id]/playlist.test.ts +++ b/web/src/routes/playlists/[id]/playlist.test.ts @@ -19,7 +19,7 @@ vi.mock('$lib/api/playlists', () => ({ deletePlaylist: vi.fn().mockResolvedValue(undefined), removePlaylistTrack: vi.fn().mockResolvedValue(undefined), reorderPlaylist: vi.fn().mockResolvedValue(undefined), - refreshDiscover: vi.fn().mockResolvedValue({ playlist_id: 'p1', track_count: 5 }) + refreshSystem: vi.fn().mockResolvedValue({ playlist_id: 'p1', track_count: 5, track_ids: [] }) })); vi.mock('$lib/auth/store.svelte', () => ({ @@ -44,13 +44,13 @@ vi.mock('$lib/api/quarantine', () => emptyQuarantineMock()); vi.mock('$lib/api/admin/tracks', () => ({ removeTrack: vi.fn() })); import PlaylistDetailPage from './+page.svelte'; -import { createPlaylistQuery, deletePlaylist, refreshDiscover } from '$lib/api/playlists'; +import { createPlaylistQuery, deletePlaylist, refreshSystem } from '$lib/api/playlists'; const mockedQuery = createPlaylistQuery as ReturnType; const ownDetail: PlaylistDetail = { id: 'p1', user_id: 'u-self', owner_username: 'me', name: 'Mine', - description: '', is_public: false, kind: 'user', system_variant: null, seed_artist_id: null, cover_url: '', track_count: 2, duration_sec: 274, + description: '', is_public: false, kind: 'user', system_variant: null, refreshable: false, seed_artist_id: null, cover_url: '', track_count: 2, duration_sec: 274, created_at: '', updated_at: '', tracks: [ { position: 0, track_id: 't1', album_id: 'a1', artist_id: 'ar1', title: 'A', artist_name: 'X', album_title: 'Y', duration_sec: 137, stream_url: '/api/tracks/t1/stream', added_at: '' }, @@ -87,43 +87,58 @@ describe('Playlist detail page', () => { confirmSpy.mockRestore(); }); - test('renders Refresh button only on Discover playlists', () => { + test('renders Refresh button on a refreshable system playlist', () => { const discoverDetail: PlaylistDetail = { ...ownDetail, kind: 'system', - system_variant: 'discover' + system_variant: 'discover', + refreshable: true }; mockedQuery.mockReturnValue(mockQuery({ data: discoverDetail })); render(PlaylistDetailPage); - expect(screen.getByLabelText(/refresh discover/i)).toBeInTheDocument(); + expect(screen.getByLabelText(/refresh playlist/i)).toBeInTheDocument(); }); - test('does not render Refresh button on for_you playlists', () => { + test('renders Refresh button on For You too (also refreshable now)', () => { const forYouDetail: PlaylistDetail = { ...ownDetail, kind: 'system', - system_variant: 'for_you' + system_variant: 'for_you', + refreshable: true }; mockedQuery.mockReturnValue(mockQuery({ data: forYouDetail })); render(PlaylistDetailPage); - expect(screen.queryByLabelText(/refresh discover/i)).not.toBeInTheDocument(); + expect(screen.getByLabelText(/refresh playlist/i)).toBeInTheDocument(); + }); + + test('does not render Refresh on a non-refreshable system playlist', () => { + const songsLike: PlaylistDetail = { + ...ownDetail, + kind: 'system', + system_variant: 'songs_like_artist', + refreshable: false + }; + mockedQuery.mockReturnValue(mockQuery({ data: songsLike })); + render(PlaylistDetailPage); + expect(screen.queryByLabelText(/refresh playlist/i)).not.toBeInTheDocument(); }); test('does not render Refresh button on user playlists', () => { mockedQuery.mockReturnValue(mockQuery({ data: ownDetail })); render(PlaylistDetailPage); - expect(screen.queryByLabelText(/refresh discover/i)).not.toBeInTheDocument(); + expect(screen.queryByLabelText(/refresh playlist/i)).not.toBeInTheDocument(); }); - test('Refresh button calls refreshDiscover when clicked', async () => { + test('Refresh button calls refreshSystem with the variant', async () => { const discoverDetail: PlaylistDetail = { ...ownDetail, kind: 'system', - system_variant: 'discover' + system_variant: 'discover', + refreshable: true }; mockedQuery.mockReturnValue(mockQuery({ data: discoverDetail })); render(PlaylistDetailPage); - await fireEvent.click(screen.getByLabelText(/refresh discover/i)); - await waitFor(() => expect(refreshDiscover).toHaveBeenCalled()); + await fireEvent.click(screen.getByLabelText(/refresh playlist/i)); + await waitFor(() => expect(refreshSystem).toHaveBeenCalledWith('discover')); }); }); diff --git a/web/src/routes/playlists/playlists.test.ts b/web/src/routes/playlists/playlists.test.ts index e54247ee..a9fe445e 100644 --- a/web/src/routes/playlists/playlists.test.ts +++ b/web/src/routes/playlists/playlists.test.ts @@ -19,7 +19,7 @@ const mockedCreatePlaylist = createPlaylist as ReturnType; function p(over: Partial): Playlist { return { id: 'p1', user_id: 'u-self', owner_username: 'me', name: 'A', - description: '', is_public: false, kind: 'user', system_variant: null, seed_artist_id: null, cover_url: '', track_count: 0, duration_sec: 0, + description: '', is_public: false, kind: 'user', system_variant: null, refreshable: false, seed_artist_id: null, cover_url: '', track_count: 0, duration_sec: 0, created_at: '', updated_at: '', ...over }; }