bvandeusen/minstrel
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 38 Wiki Activity

refactor(playlists): #411 R2 — generic registry-driven system endpoints

Browse Source 
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) <noreply@anthropic.com>
This commit is contained in:
Bryan Van Deusen
2026-05-15 13:21:09 -04:00
parent e3957b8eed
commit d67c0de596
21 changed files with 277 additions and 428 deletions
Download Patch File Download Diff File Expand all files Collapse all files
flutter_client/lib/api/endpoints/playlists.dart
+10 -15
View File
@@ -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<PlaylistDetail> systemShuffle(String variant) async {
final seg = variant == 'for_you' ? 'for-you' : variant;
final r = await _dio.get<Map<String, dynamic>>(
'/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<String?> refreshSystem(String variant) async {
final segment = variant == 'for_you' ? 'for-you' : variant;
final r = await _dio.post<Map<String, dynamic>>(
'/api/playlists/system/$segment/refresh',
'/api/playlists/system/$variant/refresh',
data: const <String, dynamic>{},
);
return (r.data ?? const {})['playlist_id'] as String?;
flutter_client/lib/models/playlist.dart
+10
View File
@@ -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<String, dynamic> j) => Playlist(
id: j['id'] as String,
userId: j['user_id'] as String? ?? '',
flutter_client/lib/playlists/playlist_detail_screen.dart
+3 -3
View File
@@ -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),
flutter_client/lib/playlists/widgets/playlist_card.dart
+12 -16
View File
@@ -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<void> _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 = <TrackRef>[];
@@ -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,
);
}
}
internal/api/api.go
+2 -4
View File
@@ -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)
})
})
}
internal/api/playlists.go
+7 -1
View File
@@ -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),
internal/api/playlists_discover_refresh.go
-69
View File
@@ -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,
})
}
internal/api/playlists_foryou_refresh.go
-98
View File
@@ -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,
})
}
internal/api/playlists_system_shuffle.go
+90 -16
View File
@@ -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) {
internal/playlists/system.go
+26 -5
View File
@@ -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
web/src/lib/api/playlists.refresh-discover.test.ts
-28
View File
@@ -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<typeof vi.fn>).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<typeof vi.fn>).mockResolvedValueOnce(sample);
const got = await refreshDiscover();
expect(got.playlist_id).toBe(null);
expect(got.track_count).toBe(0);
});
});
web/src/lib/api/playlists.refresh-foryou.test.ts
-28
View File
@@ -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<typeof vi.fn>).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<typeof vi.fn>).mockResolvedValueOnce(sample);
const got = await refreshForYou();
expect(got.playlist_id).toBe(null);
expect(got.track_ids).toEqual([]);
});
});
web/src/lib/api/playlists.ts
+18 -36
View File
@@ -21,13 +21,27 @@ export async function getPlaylist(id: string): Promise<PlaylistDetail> {
// #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<PlaylistDetail> {
const seg = variant === 'for_you' ? 'for-you' : variant;
return api.get<PlaylistDetail>(
`/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<RefreshSystemResponse> {
return api.post<RefreshSystemResponse>(
`/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<RefreshDiscoverResponse> {
return api.post<RefreshDiscoverResponse>('/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<RefreshForYouResponse> {
return api.post<RefreshForYouResponse>('/api/playlists/system/for-you/refresh', {});
}
web/src/lib/api/types.ts
+5 -1
View File
@@ -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;
web/src/lib/components/AddToPlaylistMenu.test.ts
+3
View File
@@ -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,
web/src/lib/components/PlaylistCard.svelte
+9 -18
View File
@@ -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 @@
<svelte:window onclick={() => { menuOpen = false; }} />
<div class="card relative">
{#if isSystem}
{#if refreshable}
<div class="kebab-wrap absolute right-1 top-1 z-10">
<button
type="button"
@@ -122,7 +113,7 @@
class="flex w-full items-center gap-2 rounded px-2 py-1.5 text-sm text-text-primary hover:bg-surface-hover"
>
<RefreshCcw size={14} strokeWidth={1} />
{refreshLabel}
Refresh {playlist.name}
</button>
</div>
{/if}
web/src/lib/components/PlaylistCard.test.ts
+38 -62
View File
@@ -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();
});
web/src/routes/page.test.ts
+1
View File
@@ -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,
web/src/routes/playlists/[id]/+page.svelte
+13 -13
View File
@@ -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;
}
}
</script>
@@ -177,15 +177,15 @@
{#if !isOwner}· by {pl.owner_username}{/if}
</p>
</div>
{#if pl.system_variant === 'discover'}
{#if pl.refreshable && pl.system_variant}
<button
type="button"
disabled={refreshingDiscover}
onclick={onRefreshDiscover}
disabled={refreshingSystem}
onclick={() => onRefreshSystem(pl.system_variant as string)}
class="rounded-md border border-border px-3 py-1.5 text-sm text-text-muted hover:text-text-primary"
aria-label="Refresh Discover"
aria-label="Refresh playlist"
>
{refreshingDiscover ? 'Refreshing…' : 'Refresh'}
{refreshingSystem ? 'Refreshing…' : 'Refresh'}
</button>
{/if}
{#if isOwner}
web/src/routes/playlists/[id]/playlist.test.ts
+29 -14
View File
@@ -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<typeof vi.fn>;
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'));
});
});
web/src/routes/playlists/playlists.test.ts
+1 -1
View File
@@ -19,7 +19,7 @@ const mockedCreatePlaylist = createPlaylist as ReturnType<typeof vi.fn>;
function p(over: Partial<Playlist>): 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
};
}