9 tasks covering: SQL cascade-cleanup queries, internal/tracks
service with Lidarr-aware RemoveTrack, DELETE /api/admin/tracks/{id}
handler, frontend admin API helper, audio-store playNext addition,
TrackMenuItem + TrackMenuDivider primitives, full TrackMenu rewrite
with all 9 entries (Add to playlist… reserved as a disabled slot for
#352), and TrackRow/PlayerBar test adjustments.
Plan reflects the no-in-task-tests rule — implementers write test
files but don't run them; CI handles verification. Migration 0014
that the spec marked conditional is dropped: all relevant track_id
FKs already cascade.
55 KiB
M7 — Track-level actions menu — Implementation Plan
For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (
- [ ]) syntax for tracking.
Goal: Replace the M5b-era single-entry <TrackMenu> (which only offers "Flag this track…") with a 9-entry kebab menu in 4 logical groups (queue / collection / navigation / lifecycle). Reserves an "Add to playlist…" slot for #352. Lands a new admin-only DELETE /api/admin/tracks/{id} endpoint with cascade album → artist tidy-up and Lidarr-aware deletion for managed tracks.
Architecture: Backend adds a single admin endpoint backed by a new internal/tracks service. Lidarr-managed tracks reuse the existing lidarrquarantine.DeleteViaLidarr primitive; non-Lidarr tracks use direct file removal. All track_id FKs already cascade. Frontend rewrites <TrackMenu> from one FlagPopover-only entry into nine entries via two new primitives (<TrackMenuItem>, <TrackMenuDivider>). The component's prop API (track, direction) stays compatible so existing callers (<TrackRow>, <PlayerBar>) pick up the new entries with no code change.
Tech Stack: Go 1.23 · pgx/v5 + sqlc · Postgres (no migration) · SvelteKit 2 / Svelte 5 (runes) · TanStack Query · Vitest · Lucide icons · FabledSword design tokens.
Spec: docs/superpowers/specs/2026-05-03-m7-track-actions-menu-design.md. Read it before starting — every decision is explained there.
Memory dependencies:
feedback_no_in_task_tests.md— HARD RULE. Do NOT runnpm test,npm run check,go test ./...,flutter test, etc. inside any task. Write the test file as code, write the production code, commit. CI runs the suite. The plan reflects this — there are noRun: npm teststeps.feedback_v1_is_full_product.md— this slice ships complete, not as scaffolding for a later iteration.project_design_system.md— FabledSword tokens (no hardcoded colors), Lucide icons, sentence case.project_no_github.md— Forgejo MCP for any PR/issue ops; neverghCLI.project_subsonic_legacy.md—/api/*is primary; this slice adds nothing under/rest/*.project_git_workflow.md— commit ondev; PR tomainis a separate step.
File map
Backend — create
internal/tracks/service.go—RemoveTrack(ctx, trackID, adminID)returning(deletedAlbumID, deletedArtistID, err). Typed errors:ErrNotFound,ErrLidarrUnreachable,ErrLidarrUnauthorized,ErrLidarrServerError.internal/tracks/service_test.go— six service-level cases.internal/api/admin_tracks.go—handleRemoveTrackHTTP handler.internal/api/admin_tracks_test.go— admin-auth cases + four error codes + happy path envelope.
Backend — modify
internal/db/queries/tracks.sql— appendDeleteTrack,CountTracksByAlbum(already exists; verify reuse).internal/db/queries/albums.sql— appendDeleteAlbumIfEmpty,CountAlbumsByArtist.internal/db/queries/artists.sql— appendDeleteArtistIfEmpty.internal/db/dbq/*— regenerated bysqlc generate.internal/api/api.go— registeradmin.Delete("/tracks/{id}", h.handleRemoveTrack).
Frontend — create
web/src/lib/components/TrackMenuItem.svelte— single menu entry (icon + label + onclick + aria-disabled).web/src/lib/components/TrackMenuItem.test.tsweb/src/lib/components/TrackMenuDivider.svelte— visual separator (<hr>styled to--fs-pewter).web/src/lib/api/admin/tracks.ts—removeTrack(id)+RemoveTrackResulttype.web/src/lib/api/admin/tracks.test.ts
Frontend — modify
web/src/lib/components/TrackMenu.svelte— full rewrite from FlagPopover-only to multi-entry menu. Preservestrack: TrackRefanddirection: 'up' | 'down'props.web/src/lib/components/TrackMenu.test.ts— full rewrite.web/src/lib/components/TrackRow.test.ts— adjust assertions for the new 9-entry menu (no production code change).web/src/lib/components/PlayerBar.test.ts— same.web/src/lib/player/store.svelte.ts— addplayNext(t: TrackRef)(splices at_index + 1).enqueueTrack(t)already exists for "Add to queue" semantics; reused.web/src/lib/player/store.test.ts— extend with playNext cases.
Task list
Task 1 — SQL queries for cascade cleanup
Files:
-
Modify:
internal/db/queries/tracks.sql(appendDeleteTrack) -
Modify:
internal/db/queries/albums.sql(appendDeleteAlbumIfEmpty,CountAlbumsByArtist) -
Modify:
internal/db/queries/artists.sql(appendDeleteArtistIfEmpty) -
Regenerate:
internal/db/dbq/* -
Step 1.1: Append
DeleteTracktointernal/db/queries/tracks.sql
-- name: DeleteTrack :one
-- M7 #372: hard delete with FK cascade. The CASCADE on track_id from
-- play_events / general_likes_tracks / lidarr_quarantine /
-- lidarr_quarantine_actions handles their cleanup. RETURNING gives us
-- album_id + artist_id for the album-empty / artist-empty cascade
-- checks the service does next.
DELETE FROM tracks WHERE id = $1
RETURNING id, album_id, artist_id, file_path, mbid;
- Step 1.2: Append
DeleteAlbumIfEmpty+CountAlbumsByArtisttointernal/db/queries/albums.sql
-- name: DeleteAlbumIfEmpty :one
-- M7 #372: deletes the album row only when it has no remaining tracks.
-- Used after a track delete to tidy up orphaned albums. RETURNING gives
-- us the artist_id so the service can chain into the artist-empty check.
-- Returns no rows when the album still has tracks; the service treats
-- pgx.ErrNoRows as "album not orphaned, skip cascade".
DELETE FROM albums
WHERE id = $1
AND NOT EXISTS (SELECT 1 FROM tracks WHERE album_id = $1)
RETURNING id, artist_id;
-- name: CountAlbumsByArtist :one
SELECT count(*) FROM albums WHERE artist_id = $1;
- Step 1.3: Append
DeleteArtistIfEmptytointernal/db/queries/artists.sql
-- name: DeleteArtistIfEmpty :one
-- M7 #372: deletes the artist row only when it has no remaining albums
-- AND no remaining tracks (defensive — tracks can in principle exist
-- without an album, though the scanner doesn't write that shape today).
DELETE FROM artists
WHERE id = $1
AND NOT EXISTS (SELECT 1 FROM albums WHERE artist_id = $1)
AND NOT EXISTS (SELECT 1 FROM tracks WHERE artist_id = $1)
RETURNING id;
- Step 1.4: Regenerate sqlc
cd /home/bvandeusen/Nextcloud/Projects/Minstrel/minstrel
sqlc generate
Expected: internal/db/dbq/tracks.sql.go, internal/db/dbq/albums.sql.go, internal/db/dbq/artists.sql.go updated with the new methods.
- Step 1.5: Commit
git add internal/db/queries/tracks.sql internal/db/queries/albums.sql internal/db/queries/artists.sql internal/db/dbq/
git commit -F - <<'EOF'
feat(db): cascade-cleanup queries for M7 #372 track removal
DeleteTrack returns album_id + artist_id so the calling service can
chain the album-empty / artist-empty cascade. DeleteAlbumIfEmpty and
DeleteArtistIfEmpty are no-ops when other rows still reference the
parent — the service treats pgx.ErrNoRows as "not orphaned, skip".
EOF
Task 2 — internal/tracks/service.go — RemoveTrack
Files:
- Create:
internal/tracks/service.go - Create:
internal/tracks/service_test.go
This task introduces a new service module. RemoveTrack drives the whole removal flow: fetch the track, delegate file removal (Lidarr-managed → Lidarr API; non-Lidarr → direct os.Remove), execute DB deletes in a single transaction, and return the IDs of any cascaded album/artist rows so the API caller can invalidate caches.
- Step 2.1: Write
internal/tracks/service.go
package tracks
import (
"context"
"errors"
"fmt"
"log/slog"
"os"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"
"github.com/jackc/pgx/v5/pgxpool"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrquarantine"
)
// Typed errors. Mapped to wire codes by the API layer.
var (
ErrNotFound = errors.New("track not found")
ErrLidarrUnreachable = errors.New("lidarr unreachable")
ErrLidarrUnauthorized = errors.New("lidarr unauthorized")
ErrLidarrServerError = errors.New("lidarr server error")
)
// LidarrDeleter is the subset of lidarrquarantine.Service the tracks
// service needs. Defined here so tests can stub it without spinning up
// a real Lidarr stub.
type LidarrDeleter interface {
DeleteViaLidarr(ctx context.Context, trackID, adminID pgtype.UUID) (dbq.LidarrQuarantineAction, int, error)
}
type Service struct {
pool *pgxpool.Pool
logger *slog.Logger
lidarrDeleter LidarrDeleter
}
func NewService(pool *pgxpool.Pool, logger *slog.Logger, lidarrDeleter LidarrDeleter) *Service {
if logger == nil {
logger = slog.Default()
}
return &Service{pool: pool, logger: logger, lidarrDeleter: lidarrDeleter}
}
// RemoveTrack deletes the track and (cascade-style) any album / artist
// rows that become orphaned. For Lidarr-managed tracks, Lidarr is told
// to delete first so it doesn't re-import on the next sync.
//
// Returns the deleted track id plus optional album_id and artist_id when
// those parent rows were tidied up too. The API layer surfaces these so
// the client can invalidate the matching cached query keys.
func (s *Service) RemoveTrack(ctx context.Context, trackID, adminID pgtype.UUID) (deletedAlbumID, deletedArtistID *pgtype.UUID, err error) {
q := dbq.New(s.pool)
track, err := q.GetTrackByID(ctx, trackID)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
return nil, nil, ErrNotFound
}
return nil, nil, fmt.Errorf("get track: %w", err)
}
// Lidarr-managed → delegate file removal to Lidarr (so Lidarr stops
// re-importing). Track is "Lidarr-managed" when its mbid is set —
// scanner only writes the mbid for Lidarr-imported files.
if track.Mbid.Valid && s.lidarrDeleter != nil {
_, _, lerr := s.lidarrDeleter.DeleteViaLidarr(ctx, trackID, adminID)
switch {
case lerr == nil:
// Lidarr deleted the file. Proceed to DB cleanup below.
case errors.Is(lerr, lidarrquarantine.ErrLidarrUnreachable):
return nil, nil, ErrLidarrUnreachable
case errors.Is(lerr, lidarrquarantine.ErrLidarrUnauthorized):
return nil, nil, ErrLidarrUnauthorized
case errors.Is(lerr, lidarrquarantine.ErrLidarrServerError):
return nil, nil, ErrLidarrServerError
default:
return nil, nil, fmt.Errorf("lidarr delete: %w", lerr)
}
} else if track.FilePath != "" {
// Non-Lidarr track → direct file removal. Already-missing is fine;
// the goal is DB consistency.
if rerr := os.Remove(track.FilePath); rerr != nil && !errors.Is(rerr, os.ErrNotExist) {
s.logger.Warn("track delete: file remove failed",
"path", track.FilePath, "track_id", trackID, "err", rerr)
// We still proceed — leaving an orphan file is recoverable;
// leaving an orphan DB row would cause the row to keep showing
// up in the UI with no playable file.
}
}
// DB cleanup in a single transaction so a failure midway leaves the
// schema consistent.
tx, err := s.pool.Begin(ctx)
if err != nil {
return nil, nil, fmt.Errorf("begin tx: %w", err)
}
defer func() { _ = tx.Rollback(ctx) }() // no-op once Commit succeeds
tq := dbq.New(tx)
deleted, err := tq.DeleteTrack(ctx, trackID)
if err != nil {
return nil, nil, fmt.Errorf("delete track: %w", err)
}
// Album-empty cascade.
albumRow, err := tq.DeleteAlbumIfEmpty(ctx, deleted.AlbumID)
switch {
case err == nil:
deletedAlbumID = &albumRow.ID
// Artist-empty cascade only relevant when the album was tidied up.
artistRow, aerr := tq.DeleteArtistIfEmpty(ctx, albumRow.ArtistID)
switch {
case aerr == nil:
deletedArtistID = &artistRow.ID
case errors.Is(aerr, pgx.ErrNoRows):
// Artist still has other albums or stray tracks; leave it.
default:
return nil, nil, fmt.Errorf("delete artist if empty: %w", aerr)
}
case errors.Is(err, pgx.ErrNoRows):
// Album still has other tracks; leave it.
default:
return nil, nil, fmt.Errorf("delete album if empty: %w", err)
}
if err := tx.Commit(ctx); err != nil {
return nil, nil, fmt.Errorf("commit: %w", err)
}
return deletedAlbumID, deletedArtistID, nil
}
- Step 2.2: Write
internal/tracks/service_test.go
package tracks_test
import (
"context"
"errors"
"net/http/httptest"
"os"
"path/filepath"
"testing"
"github.com/jackc/pgx/v5/pgtype"
"github.com/jackc/pgx/v5/pgxpool"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig"
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrquarantine"
"git.fabledsword.com/bvandeusen/minstrel/internal/tracks"
)
// fakeLidarrDeleter records calls and returns a configurable error.
type fakeLidarrDeleter struct {
calls []pgtype.UUID
err error
}
func (f *fakeLidarrDeleter) DeleteViaLidarr(ctx context.Context, trackID, adminID pgtype.UUID) (dbq.LidarrQuarantineAction, int, error) {
f.calls = append(f.calls, trackID)
return dbq.LidarrQuarantineAction{}, 0, f.err
}
// Helpers seedUser, seedTrack, newPool follow the same pattern as
// existing service tests (see internal/lidarrquarantine/service_test.go
// for reference). Reuse them via copy or the existing test fixtures.
func TestRemoveTrack_NotFound(t *testing.T) {
pool := newPool(t)
user := seedUser(t, pool, "admin")
svc := tracks.NewService(pool, nil, &fakeLidarrDeleter{})
_, _, err := svc.RemoveTrack(context.Background(), randomUUID(), user.ID)
if !errors.Is(err, tracks.ErrNotFound) {
t.Errorf("err = %v, want ErrNotFound", err)
}
}
func TestRemoveTrack_NonLidarr_HappyPath(t *testing.T) {
pool := newPool(t)
user := seedUser(t, pool, "admin")
dir := t.TempDir()
path := filepath.Join(dir, "song.mp3")
if err := os.WriteFile(path, []byte("audio"), 0o644); err != nil {
t.Fatalf("write fixture: %v", err)
}
track := seedTrackAt(t, pool, "Song", path, "" /* no mbid → non-Lidarr */)
svc := tracks.NewService(pool, nil, &fakeLidarrDeleter{})
_, _, err := svc.RemoveTrack(context.Background(), track.ID, user.ID)
if err != nil {
t.Fatalf("RemoveTrack: %v", err)
}
if _, statErr := os.Stat(path); !errors.Is(statErr, os.ErrNotExist) {
t.Errorf("file still present at %s", path)
}
q := dbq.New(pool)
if _, gerr := q.GetTrackByID(context.Background(), track.ID); gerr == nil {
t.Error("track row still in DB")
}
}
func TestRemoveTrack_NonLidarr_FileAlreadyMissing(t *testing.T) {
pool := newPool(t)
user := seedUser(t, pool, "admin")
track := seedTrackAt(t, pool, "Song", "/tmp/does-not-exist-xyz", "")
svc := tracks.NewService(pool, nil, &fakeLidarrDeleter{})
if _, _, err := svc.RemoveTrack(context.Background(), track.ID, user.ID); err != nil {
t.Fatalf("RemoveTrack: %v (file-already-missing should be tolerated)", err)
}
}
func TestRemoveTrack_Lidarr_HappyPath(t *testing.T) {
pool := newPool(t)
user := seedUser(t, pool, "admin")
track := seedLidarrTrack(t, pool, "Song", "track-mbid")
deleter := &fakeLidarrDeleter{}
svc := tracks.NewService(pool, nil, deleter)
if _, _, err := svc.RemoveTrack(context.Background(), track.ID, user.ID); err != nil {
t.Fatalf("RemoveTrack: %v", err)
}
if len(deleter.calls) != 1 {
t.Errorf("DeleteViaLidarr call count = %d, want 1", len(deleter.calls))
}
}
func TestRemoveTrack_Lidarr_ErrorPropagation(t *testing.T) {
cases := []struct {
name string
lidarr error
wantErr error
}{
{"unreachable", lidarrquarantine.ErrLidarrUnreachable, tracks.ErrLidarrUnreachable},
{"unauthorized", lidarrquarantine.ErrLidarrUnauthorized, tracks.ErrLidarrUnauthorized},
{"server_error", lidarrquarantine.ErrLidarrServerError, tracks.ErrLidarrServerError},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
pool := newPool(t)
user := seedUser(t, pool, "admin")
track := seedLidarrTrack(t, pool, "Song", "mbid")
deleter := &fakeLidarrDeleter{err: tc.lidarr}
svc := tracks.NewService(pool, nil, deleter)
_, _, err := svc.RemoveTrack(context.Background(), track.ID, user.ID)
if !errors.Is(err, tc.wantErr) {
t.Errorf("err = %v, want %v", err, tc.wantErr)
}
// DB row must still exist — Lidarr failure should not orphan rows.
q := dbq.New(pool)
if _, gerr := q.GetTrackByID(context.Background(), track.ID); gerr != nil {
t.Errorf("track row was deleted despite Lidarr failure: %v", gerr)
}
})
}
}
func TestRemoveTrack_Cascade_AlbumEmpty(t *testing.T) {
pool := newPool(t)
user := seedUser(t, pool, "admin")
// Single track in a single album, both belong to a single artist that
// has another album with another track. The track delete should also
// drop the album, but the artist persists.
artist := seedArtist(t, pool, "Solo")
albumA := seedAlbumForArtist(t, pool, "OnlyAlbum", artist.ID)
trackA := seedTrackInAlbum(t, pool, "OnlyTrack", albumA.ID, artist.ID)
albumB := seedAlbumForArtist(t, pool, "OtherAlbum", artist.ID)
_ = seedTrackInAlbum(t, pool, "OtherTrack", albumB.ID, artist.ID)
svc := tracks.NewService(pool, nil, &fakeLidarrDeleter{})
deletedAlbum, deletedArtist, err := svc.RemoveTrack(context.Background(), trackA.ID, user.ID)
if err != nil {
t.Fatalf("RemoveTrack: %v", err)
}
if deletedAlbum == nil || deletedAlbum.Bytes != albumA.ID.Bytes {
t.Errorf("deletedAlbumID = %v, want albumA.ID", deletedAlbum)
}
if deletedArtist != nil {
t.Errorf("deletedArtistID = %v, want nil (artist still has albumB)", deletedArtist)
}
}
func TestRemoveTrack_Cascade_ArtistEmpty(t *testing.T) {
pool := newPool(t)
user := seedUser(t, pool, "admin")
artist := seedArtist(t, pool, "Solo")
album := seedAlbumForArtist(t, pool, "OnlyAlbum", artist.ID)
track := seedTrackInAlbum(t, pool, "OnlyTrack", album.ID, artist.ID)
svc := tracks.NewService(pool, nil, &fakeLidarrDeleter{})
deletedAlbum, deletedArtist, err := svc.RemoveTrack(context.Background(), track.ID, user.ID)
if err != nil {
t.Fatalf("RemoveTrack: %v", err)
}
if deletedAlbum == nil {
t.Error("deletedAlbumID = nil, want album.ID")
}
if deletedArtist == nil {
t.Error("deletedArtistID = nil, want artist.ID")
}
}
The seed helpers (newPool, seedUser, seedArtist, seedAlbumForArtist, seedTrackInAlbum, seedTrackAt, seedLidarrTrack, randomUUID) follow the same pattern used in internal/lidarrquarantine/service_test.go. The implementer either copies them into a service_test_helpers.go in this package or extracts the existing ones into a shared internal/dbtest package — operator preference, but copying into the new package is the path of least surprise.
- Step 2.3: Commit
git add internal/tracks/service.go internal/tracks/service_test.go
git commit -F - <<'EOF'
feat(tracks): RemoveTrack service with Lidarr-aware delete + cascade
RemoveTrack handles both Lidarr-managed (delegates to existing
lidarrquarantine.DeleteViaLidarr) and non-Lidarr (direct os.Remove)
file deletion, then runs the cascade album-if-empty / artist-if-empty
DB cleanup in a single transaction. Lidarr errors propagate as typed
errors so the API layer can map them to the existing wire codes.
EOF
Task 3 — internal/api/admin_tracks.go — HTTP handler
Files:
-
Create:
internal/api/admin_tracks.go -
Create:
internal/api/admin_tracks_test.go -
Modify:
internal/api/api.go(register the route) -
Modify:
internal/api/api.goand the handler-construction site (wire the new tracks service) -
Step 3.1: Write
internal/api/admin_tracks.go
package api
import (
"encoding/json"
"errors"
"net/http"
"github.com/go-chi/chi/v5"
"github.com/jackc/pgx/v5/pgtype"
"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
"git.fabledsword.com/bvandeusen/minstrel/internal/tracks"
)
// removeTrackResponse mirrors the wire shape from the spec. The
// optional fields are omitted when their corresponding cascade did
// not fire (album/artist still has other rows).
type removeTrackResponse struct {
DeletedTrackID string `json:"deleted_track_id"`
DeletedAlbumID *string `json:"deleted_album_id,omitempty"`
DeletedArtistID *string `json:"deleted_artist_id,omitempty"`
}
// handleRemoveTrack implements DELETE /api/admin/tracks/{id}.
// Admin-only. Cascades album / artist if the track removal leaves
// either parent empty.
func (h *handlers) handleRemoveTrack(w http.ResponseWriter, r *http.Request) {
idStr := chi.URLParam(r, "id")
var trackID pgtype.UUID
if err := trackID.Scan(idStr); err != nil {
writeErr(w, http.StatusBadRequest, "bad_request", "invalid track id")
return
}
admin, ok := auth.UserFromContext(r.Context())
if !ok {
writeErr(w, http.StatusUnauthorized, "unauthenticated", "no session")
return
}
deletedAlbum, deletedArtist, err := h.tracks.RemoveTrack(r.Context(), trackID, admin.ID)
switch {
case err == nil:
// fall through
case errors.Is(err, tracks.ErrNotFound):
writeErr(w, http.StatusNotFound, "not_found", "track not found")
return
case errors.Is(err, tracks.ErrLidarrUnreachable):
writeErr(w, http.StatusServiceUnavailable, "lidarr_unreachable", "Lidarr is unreachable")
return
case errors.Is(err, tracks.ErrLidarrUnauthorized):
writeErr(w, http.StatusServiceUnavailable, "lidarr_unauthorized", "Lidarr rejected the API key")
return
case errors.Is(err, tracks.ErrLidarrServerError):
writeErr(w, http.StatusBadGateway, "lidarr_server_error", "Lidarr returned an error")
return
default:
h.logger.Error("api: remove track failed", "err", err, "track_id", trackID)
writeErr(w, http.StatusInternalServerError, "server_error", "remove failed")
return
}
resp := removeTrackResponse{DeletedTrackID: idStr}
if deletedAlbum != nil {
s := uuidToString(*deletedAlbum)
resp.DeletedAlbumID = &s
}
if deletedArtist != nil {
s := uuidToString(*deletedArtist)
resp.DeletedArtistID = &s
}
w.Header().Set("Content-Type", "application/json")
_ = json.NewEncoder(w).Encode(resp)
}
If uuidToString doesn't exist in the api package, add it next to convert.go's helpers — single line: func uuidToString(u pgtype.UUID) string { return uuid.UUID(u.Bytes).String() } (using github.com/google/uuid, which the api package already imports for similar conversions; check convert.go to confirm the existing helper name and reuse it instead of duplicating).
- Step 3.2: Wire the tracks service into
handlers
In internal/api/api.go, the Mount function takes a number of services. Read the existing signature, then add tracksSvc *tracks.Service (with import "git.fabledsword.com/bvandeusen/minstrel/internal/tracks"). Add a corresponding tracks field to the handlers struct. The construction at the call site in internal/server/server.go builds the new service via tracks.NewService(s.Pool, s.Logger, lidarrQuar) (where lidarrQuar is the existing *lidarrquarantine.Service).
The handler check h.tracks reaches the new service.
- Step 3.3: Register the route in
internal/api/api.go
Inside the r.Route("/api", ...) → authed.Group(...) → authed.Route("/admin", ...) block, add:
admin.Delete("/tracks/{id}", h.handleRemoveTrack)
Place it adjacent to the other admin routes (alongside admin.Post("/quarantine/{track_id}/delete-via-lidarr", ...) since the operations are conceptually neighboring).
- Step 3.4: Write
internal/api/admin_tracks_test.go
package api_test
import (
"net/http"
"net/http/httptest"
"testing"
"git.fabledsword.com/bvandeusen/minstrel/internal/tracks"
)
func TestRemoveTrack_NotAdmin_403(t *testing.T) {
srv, _ := newTestServer(t) // existing helper from api_test fixtures
user := seedNonAdminUser(t, srv.pool)
req := httptest.NewRequest(http.MethodDelete, "/api/admin/tracks/"+randomUUIDString(), nil)
req = withSession(req, user)
rr := httptest.NewRecorder()
srv.handler.ServeHTTP(rr, req)
if rr.Code != http.StatusForbidden {
t.Errorf("status = %d, want 403", rr.Code)
}
}
func TestRemoveTrack_NoSession_401(t *testing.T) {
srv, _ := newTestServer(t)
req := httptest.NewRequest(http.MethodDelete, "/api/admin/tracks/"+randomUUIDString(), nil)
rr := httptest.NewRecorder()
srv.handler.ServeHTTP(rr, req)
if rr.Code != http.StatusUnauthorized {
t.Errorf("status = %d, want 401", rr.Code)
}
}
func TestRemoveTrack_NotFound_404(t *testing.T) {
srv, _ := newTestServer(t)
admin := seedAdminUser(t, srv.pool)
req := httptest.NewRequest(http.MethodDelete, "/api/admin/tracks/"+randomUUIDString(), nil)
req = withSession(req, admin)
rr := httptest.NewRecorder()
srv.handler.ServeHTTP(rr, req)
if rr.Code != http.StatusNotFound {
t.Errorf("status = %d, want 404", rr.Code)
}
if !contains(rr.Body.String(), `"error":"not_found"`) {
t.Errorf("body = %s, want not_found code", rr.Body.String())
}
}
func TestRemoveTrack_LidarrError_503(t *testing.T) {
cases := []struct {
name string
lidarr error
code string
status int
}{
{"unreachable", tracks.ErrLidarrUnreachable, "lidarr_unreachable", http.StatusServiceUnavailable},
{"unauthorized", tracks.ErrLidarrUnauthorized, "lidarr_unauthorized", http.StatusServiceUnavailable},
{"server_error", tracks.ErrLidarrServerError, "lidarr_server_error", http.StatusBadGateway},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
srv, _ := newTestServer(t, withLidarrError(tc.lidarr)) // tunable test fixture
admin := seedAdminUser(t, srv.pool)
track := seedLidarrTrack(t, srv.pool, "Song", "mbid")
req := httptest.NewRequest(http.MethodDelete, "/api/admin/tracks/"+uuidString(track.ID), nil)
req = withSession(req, admin)
rr := httptest.NewRecorder()
srv.handler.ServeHTTP(rr, req)
if rr.Code != tc.status {
t.Errorf("status = %d, want %d", rr.Code, tc.status)
}
if !contains(rr.Body.String(), `"error":"`+tc.code+`"`) {
t.Errorf("body = %s, want code %s", rr.Body.String(), tc.code)
}
})
}
}
func TestRemoveTrack_Happy_200WithCascade(t *testing.T) {
srv, _ := newTestServer(t)
admin := seedAdminUser(t, srv.pool)
// Set up a single-track album owned by a single-album artist so we
// see the full cascade in the response.
artist := seedArtist(t, srv.pool, "Solo")
album := seedAlbumForArtist(t, srv.pool, "Album", artist.ID)
track := seedTrackInAlbum(t, srv.pool, "Song", album.ID, artist.ID)
req := httptest.NewRequest(http.MethodDelete, "/api/admin/tracks/"+uuidString(track.ID), nil)
req = withSession(req, admin)
rr := httptest.NewRecorder()
srv.handler.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Fatalf("status = %d, want 200", rr.Code)
}
body := rr.Body.String()
for _, want := range []string{`"deleted_track_id":`, `"deleted_album_id":`, `"deleted_artist_id":`} {
if !contains(body, want) {
t.Errorf("body missing %s: %s", want, body)
}
}
}
The helpers (newTestServer, seedAdminUser, seedNonAdminUser, seedArtist, etc.) follow the patterns in the existing api_test files. withLidarrError is a fixture option this slice introduces — the implementer adds it to newTestServer so any test can stub the Lidarr deleter to return a specific typed error.
- Step 3.5: Commit
git add internal/api/admin_tracks.go internal/api/admin_tracks_test.go internal/api/api.go internal/server/server.go
git commit -F - <<'EOF'
feat(api): DELETE /api/admin/tracks/{id} for M7 #372
Admin-only handler; maps tracks.RemoveTrack typed errors to the
project's existing wire codes (not_found, lidarr_unreachable,
lidarr_unauthorized, lidarr_server_error). Response envelope returns
deleted_track_id plus optional deleted_album_id / deleted_artist_id
when the cascade tidied up parent rows.
EOF
Task 4 — Frontend admin API helper
Files:
-
Create:
web/src/lib/api/admin/tracks.ts -
Create:
web/src/lib/api/admin/tracks.test.ts -
Step 4.1: Write
web/src/lib/api/admin/tracks.ts
import { apiFetch } from '$lib/api/client';
export type RemoveTrackResult = {
deleted_track_id: string;
deleted_album_id?: string;
deleted_artist_id?: string;
};
/**
* DELETE /api/admin/tracks/{id} — admin-only. Returns the deleted
* track id plus optional album/artist ids when their parent row was
* tidied up by the server-side cascade. The caller is responsible for
* invalidating any TanStack Query keys the deleted entities backed.
*/
export function removeTrack(id: string): Promise<RemoveTrackResult> {
return apiFetch<RemoveTrackResult>(`/api/admin/tracks/${encodeURIComponent(id)}`, {
method: 'DELETE',
});
}
- Step 4.2: Write
web/src/lib/api/admin/tracks.test.ts
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { removeTrack } from './tracks';
const originalFetch = globalThis.fetch;
beforeEach(() => {
globalThis.fetch = vi.fn();
});
afterEach(() => {
globalThis.fetch = originalFetch;
});
describe('removeTrack', () => {
it('DELETEs /api/admin/tracks/:id and parses the envelope', async () => {
(globalThis.fetch as ReturnType<typeof vi.fn>).mockResolvedValueOnce(
new Response(JSON.stringify({
deleted_track_id: 't-1',
deleted_album_id: 'al-1',
}), { status: 200, headers: { 'Content-Type': 'application/json' } })
);
const r = await removeTrack('t-1');
expect(r.deleted_track_id).toBe('t-1');
expect(r.deleted_album_id).toBe('al-1');
expect(r.deleted_artist_id).toBeUndefined();
const [url, init] = (globalThis.fetch as ReturnType<typeof vi.fn>).mock.calls[0];
expect(url).toBe('/api/admin/tracks/t-1');
expect(init.method).toBe('DELETE');
});
it('surfaces lidarr_unreachable as ApiError with that code', async () => {
(globalThis.fetch as ReturnType<typeof vi.fn>).mockResolvedValueOnce(
new Response(JSON.stringify({ error: 'lidarr_unreachable' }), {
status: 503,
headers: { 'Content-Type': 'application/json' },
})
);
await expect(removeTrack('t-1')).rejects.toMatchObject({
code: 'lidarr_unreachable',
});
});
});
- Step 4.3: Commit
git add web/src/lib/api/admin/tracks.ts web/src/lib/api/admin/tracks.test.ts
git commit -F - <<'EOF'
feat(web): admin removeTrack API helper for M7 #372
Single function that hits DELETE /api/admin/tracks/{id} and returns
the typed envelope. Reuses the existing apiFetch + ApiError surface
so error codes (lidarr_unreachable etc.) flow into copyForCode the
same way as every other admin operation.
EOF
Task 5 — Audio store: playNext()
Files:
- Modify:
web/src/lib/player/store.svelte.ts - Modify:
web/src/lib/player/store.test.ts
enqueueTrack(t) already exists for "Add to queue" semantics — reuse it as-is. This task adds playNext(t) for the "Play next" entry which splices into _queue[_index + 1].
- Step 5.1: Add
playNexttoweb/src/lib/player/store.svelte.ts
Append next to the existing enqueueTrack export (around line 208):
/**
* M7 #372: insert a track immediately after the currently-playing one.
* If the queue is empty, behaves like enqueueTrack and starts the queue
* with this track at index 0.
*/
export function playNext(t: TrackRef): void {
_radioSeedId = null; // M4c: any user-driven enqueue clears the radio refresh state
if (_queue.length === 0) {
_queue = [t];
_index = 0;
return;
}
const next = _index + 1;
_queue = [..._queue.slice(0, next), t, ..._queue.slice(next)];
}
- Step 5.2: Extend
web/src/lib/player/store.test.tswith playNext cases
Append (or wherever the file's existing enqueueTrack block lives, place these alongside):
import { describe, it, expect, beforeEach } from 'vitest';
import { playNext, enqueueTrack, getQueue, setQueue, getIndex, setIndex } from './store.svelte';
// (Adjust imports to match the file's actual export shape; the existing
// store.test.ts will tell you what's exposed for tests today.)
describe('playNext', () => {
beforeEach(() => {
setQueue([]);
setIndex(0);
});
it('splices the track immediately after the current index', () => {
const a = { id: 'a', title: 'A', albumId: 'x', artistId: 'y' } as const;
const b = { id: 'b', title: 'B', albumId: 'x', artistId: 'y' } as const;
const c = { id: 'c', title: 'C', albumId: 'x', artistId: 'y' } as const;
setQueue([a, b]);
setIndex(0);
playNext(c);
expect(getQueue().map((t) => t.id)).toEqual(['a', 'c', 'b']);
expect(getIndex()).toBe(0);
});
it('seeds an empty queue with the track at index 0', () => {
const t = { id: 't', title: 'T', albumId: 'x', artistId: 'y' } as const;
playNext(t);
expect(getQueue().map((x) => x.id)).toEqual(['t']);
expect(getIndex()).toBe(0);
});
});
If the existing store test file uses a different test harness (mounting a Svelte component to reach the runes-state), adapt to whatever pattern is already in place — don't fight the existing setup. The two assertions above are what matters: splice-at-index+1 and empty-queue seeding.
- Step 5.3: Commit
git add web/src/lib/player/store.svelte.ts web/src/lib/player/store.test.ts
git commit -F - <<'EOF'
feat(player): playNext for M7 #372 track-actions menu
Inserts at _queue[_index + 1] so the next-up slot is overwritten with
the chosen track. Empty-queue seeding mirrors enqueueTrack's behavior.
Clears _radioSeedId since user-driven enqueue invalidates the M4c
auto-refresh trigger.
EOF
Task 6 — <TrackMenuItem> primitive
Files:
- Create:
web/src/lib/components/TrackMenuItem.svelte - Create:
web/src/lib/components/TrackMenuItem.test.ts
A single row in the menu: icon + label + optional onclick, with aria-disabled support so disabled-but-visible entries (admin-only for non-admin, "Add to playlist…" pre-#352) keep menu height stable.
- Step 6.1: Write
web/src/lib/components/TrackMenuItem.svelte
<script lang="ts">
import type { Snippet } from 'svelte';
import type { Component } from 'svelte';
let {
icon,
label,
onclick,
disabled = false,
danger = false,
title,
}: {
/** Lucide icon component (or any Svelte component matching the Lucide signature). */
icon: Component<{ size?: number; strokeWidth?: number; class?: string }>;
label: string;
onclick?: () => void;
disabled?: boolean;
/** Renders in oxblood for destructive actions ("Remove from library"). */
danger?: boolean;
/** Tooltip text — used to explain disabled state ("Coming with playlists"). */
title?: string;
} = $props();
const Icon = $derived(icon);
function fire() {
if (disabled || !onclick) return;
onclick();
}
</script>
<button
type="button"
role="menuitem"
aria-disabled={disabled}
{title}
onclick={fire}
class="flex w-full items-center gap-2 rounded px-2 py-1.5 text-sm
{disabled ? 'cursor-not-allowed text-text-muted' : 'text-text-primary hover:bg-surface-hover'}
{danger && !disabled ? 'text-oxblood' : ''}"
>
<Icon size={14} strokeWidth={1} />
{label}
</button>
- Step 6.2: Write
web/src/lib/components/TrackMenuItem.test.ts
import { describe, it, expect, vi } from 'vitest';
import { render, fireEvent, screen } from '@testing-library/svelte';
import { Music2 } from 'lucide-svelte';
import TrackMenuItem from './TrackMenuItem.svelte';
describe('TrackMenuItem', () => {
it('renders icon, label, and fires onclick', async () => {
const onclick = vi.fn();
render(TrackMenuItem, { props: { icon: Music2, label: 'Play next', onclick } });
const btn = screen.getByRole('menuitem', { name: /play next/i });
await fireEvent.click(btn);
expect(onclick).toHaveBeenCalledOnce();
expect(btn.getAttribute('aria-disabled')).toBe('false');
});
it('aria-disabled blocks onclick and reflects in DOM', async () => {
const onclick = vi.fn();
render(TrackMenuItem, {
props: { icon: Music2, label: 'Add to playlist…', onclick, disabled: true, title: 'Coming with playlists' },
});
const btn = screen.getByRole('menuitem', { name: /add to playlist/i });
expect(btn.getAttribute('aria-disabled')).toBe('true');
expect(btn.getAttribute('title')).toBe('Coming with playlists');
await fireEvent.click(btn);
expect(onclick).not.toHaveBeenCalled();
});
});
- Step 6.3: Commit
git add web/src/lib/components/TrackMenuItem.svelte web/src/lib/components/TrackMenuItem.test.ts
git commit -F - <<'EOF'
feat(web): TrackMenuItem primitive for M7 #372
Single-entry button used by TrackMenu. Supports aria-disabled (kept
visible to preserve menu height when admin-only or pre-#352 entries
are non-functional) and a `danger` flag for the oxblood-tinted
"Remove from library" entry.
EOF
Task 7 — <TrackMenuDivider> primitive
Files:
- Create:
web/src/lib/components/TrackMenuDivider.svelte
Trivial visual separator. No test — it's a styled <hr>.
- Step 7.1: Write
web/src/lib/components/TrackMenuDivider.svelte
<hr class="my-1 border-border" role="separator" />
- Step 7.2: Commit
git add web/src/lib/components/TrackMenuDivider.svelte
git commit -F - <<'EOF'
feat(web): TrackMenuDivider primitive for M7 #372 menu groups
EOF
Task 8 — <TrackMenu> rewrite (multi-entry)
Files:
- Modify (rewrite):
web/src/lib/components/TrackMenu.svelte - Modify (rewrite):
web/src/lib/components/TrackMenu.test.ts
This is the centerpiece. Existing track and direction props stay. Internal state grows from the existing menuOpen / popoverOpen pair to also track confirmation flow for "Remove from library" (a non-modal confirm — the entry is two-click: first click flips it to "Click again to confirm", second click fires the API).
- Step 8.1: Rewrite
web/src/lib/components/TrackMenu.svelte
<script lang="ts">
import {
MoreVertical,
ListPlus,
Plus,
Heart,
HeartOff,
ListMusic,
Album,
Disc3,
Flag,
EyeOff,
Eye,
Trash2,
} from 'lucide-svelte';
import { goto } from '$app/navigation';
import { useQueryClient } from '@tanstack/svelte-query';
import FlagPopover from './FlagPopover.svelte';
import TrackMenuItem from './TrackMenuItem.svelte';
import TrackMenuDivider from './TrackMenuDivider.svelte';
import { likeTrack, unlikeTrack } from '$lib/api/likes';
import { quarantineTrack, unquarantineTrack } from '$lib/api/quarantine';
import { removeTrack } from '$lib/api/admin/tracks';
import { qk } from '$lib/api/queries';
import { meStore } from '$lib/stores/me';
import { likedIdsStore } from '$lib/stores/likedIds';
import { hiddenIdsStore } from '$lib/stores/hiddenIds';
import { playNext as playNextStore, enqueueTrack } from '$lib/player/store.svelte';
import { copyForCode } from '$lib/api/error-copy';
import type { TrackRef } from '$lib/api/types';
let {
track,
direction = 'down',
}: {
track: TrackRef;
direction?: 'up' | 'down';
} = $props();
let menuOpen = $state(false);
let popoverOpen = $state(false);
let removeArmed = $state(false); // two-click confirm
let toast = $state<string | null>(null);
const client = useQueryClient();
const me = $derived($meStore);
const liked = $derived($likedIdsStore.tracks.has(track.id));
const hidden = $derived($hiddenIdsStore.has(track.id));
function toggleMenu(e: MouseEvent) {
e.stopPropagation();
menuOpen = !menuOpen;
removeArmed = false;
}
function closeAll() {
menuOpen = false;
popoverOpen = false;
removeArmed = false;
}
function showToast(msg: string) {
toast = msg;
setTimeout(() => (toast = null), 4000);
}
// --- Entry handlers ---
function onPlayNext() {
playNextStore(track);
closeAll();
}
function onAddToQueue() {
enqueueTrack(track);
closeAll();
}
async function onToggleLike() {
closeAll();
try {
if (liked) {
await unlikeTrack(track.id);
} else {
await likeTrack(track.id);
}
await client.invalidateQueries({ queryKey: qk.likedTracks() });
} catch (e) {
showToast(copyForCode((e as { code?: string })?.code ?? 'unknown'));
}
}
async function onToggleHide() {
closeAll();
try {
if (hidden) {
await unquarantineTrack(track.id);
} else {
await quarantineTrack(track.id);
}
await client.invalidateQueries({ queryKey: qk.hiddenTracks() });
} catch (e) {
showToast(copyForCode((e as { code?: string })?.code ?? 'unknown'));
}
}
function onGoToAlbum() {
closeAll();
goto(`/albums/${track.albumId}`);
}
function onGoToArtist() {
closeAll();
goto(`/artists/${track.artistId}`);
}
function onFlag() {
menuOpen = false;
removeArmed = false;
popoverOpen = true;
}
async function onRemoveFromLibrary() {
if (!removeArmed) {
removeArmed = true;
return;
}
closeAll();
try {
const r = await removeTrack(track.id);
// Invalidate caches for any vanished entity.
await client.invalidateQueries({ queryKey: qk.albums(track.albumId) });
await client.invalidateQueries({ queryKey: qk.artists(track.artistId) });
await client.invalidateQueries({ queryKey: qk.home() });
if (r.deleted_album_id) {
await client.invalidateQueries({ queryKey: qk.albums(r.deleted_album_id) });
}
if (r.deleted_artist_id) {
await client.invalidateQueries({ queryKey: qk.artists(r.deleted_artist_id) });
}
} catch (e) {
showToast(copyForCode((e as { code?: string })?.code ?? 'unknown'));
}
}
function onKeydown(e: KeyboardEvent) {
if (!menuOpen) return;
if (e.key === 'Escape') {
e.preventDefault();
closeAll();
}
}
</script>
<svelte:window onclick={() => (menuOpen = false)} onkeydown={onKeydown} />
<div class="relative inline-block">
<button
type="button"
aria-label={`Track actions for ${track.title}`}
aria-haspopup="menu"
aria-expanded={menuOpen}
onclick={toggleMenu}
class="rounded p-1 text-text-muted hover:text-text-primary"
>
<MoreVertical size={16} strokeWidth={1} />
</button>
{#if menuOpen}
<div
role="menu"
class="absolute right-0 z-20 w-56 rounded-md border border-border bg-surface p-1 shadow-lg
{direction === 'up' ? 'bottom-full mb-1' : 'top-full mt-1'}"
onclick={(e) => e.stopPropagation()}
>
<!-- Group 1: queue -->
<TrackMenuItem icon={ListPlus} label="Play next" onclick={onPlayNext} />
<TrackMenuItem icon={Plus} label="Add to queue" onclick={onAddToQueue} />
<TrackMenuDivider />
<!-- Group 2: collection -->
<TrackMenuItem
icon={liked ? HeartOff : Heart}
label={liked ? 'Unlike' : 'Like'}
onclick={onToggleLike}
/>
<TrackMenuItem
icon={ListMusic}
label="Add to playlist…"
disabled
title="Coming with playlists"
/>
<TrackMenuDivider />
<!-- Group 3: navigation -->
<TrackMenuItem icon={Album} label="Go to album" onclick={onGoToAlbum} />
<TrackMenuItem icon={Disc3} label="Go to artist" onclick={onGoToArtist} />
<TrackMenuDivider />
<!-- Group 4: lifecycle -->
<TrackMenuItem icon={Flag} label="Flag this track…" onclick={onFlag} />
<TrackMenuItem
icon={hidden ? Eye : EyeOff}
label={hidden ? 'Unhide' : 'Hide'}
onclick={onToggleHide}
/>
{#if me?.is_admin}
<TrackMenuItem
icon={Trash2}
label={removeArmed ? 'Click again to confirm' : 'Remove from library'}
onclick={onRemoveFromLibrary}
danger
/>
{/if}
</div>
{/if}
{#if popoverOpen}
<FlagPopover {track} onClose={closeAll} />
{/if}
{#if toast}
<div
role="status"
class="absolute right-0 top-full mt-1 w-64 rounded-md border border-border bg-surface px-3 py-2 text-sm text-text-primary shadow-lg"
>
{toast}
</div>
{/if}
</div>
The imports at the top reference $lib/stores/me, $lib/stores/likedIds, $lib/stores/hiddenIds — verify those exist (they should, given the existing LikeButton + Hide-row pattern). If the names differ in the actual codebase, adjust to match. Same for $lib/api/quarantine (quarantineTrack / unquarantineTrack). Read the files before writing if unsure.
If any of those stores/helpers genuinely don't exist (e.g., hiddenIdsStore), they need to land first or this task gets stretched. Most likely path: the existing LikeButton component already imports a "set of liked ids" store; reuse that pattern. The Hide/quarantine equivalent has shipped in M5b — check internal/api/quarantine.go for the helper names and web/src/lib/api/ for the existing TS surface.
- Step 8.2: Rewrite
web/src/lib/components/TrackMenu.test.ts
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { render, fireEvent, screen, waitFor } from '@testing-library/svelte';
import { writable } from 'svelte/store';
import TrackMenu from './TrackMenu.svelte';
import type { TrackRef } from '$lib/api/types';
// ---- Mocks ----
vi.mock('$lib/stores/me', () => ({
meStore: writable<{ id: string; username: string; is_admin: boolean } | null>({
id: 'u-1', username: 'admin', is_admin: true,
}),
}));
vi.mock('$lib/stores/likedIds', () => ({
likedIdsStore: writable({ tracks: new Set<string>(), albums: new Set<string>(), artists: new Set<string>() }),
}));
vi.mock('$lib/stores/hiddenIds', () => ({
hiddenIdsStore: writable(new Set<string>()),
}));
vi.mock('$lib/player/store.svelte', () => ({
playNext: vi.fn(),
enqueueTrack: vi.fn(),
}));
vi.mock('$lib/api/likes', () => ({
likeTrack: vi.fn().mockResolvedValue(undefined),
unlikeTrack: vi.fn().mockResolvedValue(undefined),
}));
vi.mock('$lib/api/quarantine', () => ({
quarantineTrack: vi.fn().mockResolvedValue(undefined),
unquarantineTrack: vi.fn().mockResolvedValue(undefined),
}));
vi.mock('$lib/api/admin/tracks', () => ({
removeTrack: vi.fn().mockResolvedValue({ deleted_track_id: 't-1' }),
}));
vi.mock('@tanstack/svelte-query', async (orig) => {
const actual = (await orig()) as Record<string, unknown>;
return {
...actual,
useQueryClient: () => ({ invalidateQueries: vi.fn().mockResolvedValue(undefined) }),
};
});
const track: TrackRef = {
id: 't-1', title: 'Roygbiv',
albumId: 'al-1', albumTitle: 'Geogaddi',
artistId: 'art-1', artistName: 'Boards of Canada',
durationMs: 137000, trackNumber: 4,
} as unknown as TrackRef;
beforeEach(() => {
vi.clearAllMocks();
});
afterEach(() => {
vi.clearAllMocks();
});
describe('TrackMenu', () => {
it('kebab toggles aria-expanded; menu opens with all 9 entries for an admin', async () => {
render(TrackMenu, { props: { track } });
const kebab = screen.getByRole('button', { name: /track actions/i });
expect(kebab.getAttribute('aria-expanded')).toBe('false');
await fireEvent.click(kebab);
expect(kebab.getAttribute('aria-expanded')).toBe('true');
expect(screen.getByRole('menuitem', { name: /play next/i })).toBeInTheDocument();
expect(screen.getByRole('menuitem', { name: /add to queue/i })).toBeInTheDocument();
expect(screen.getByRole('menuitem', { name: /^like$/i })).toBeInTheDocument();
expect(screen.getByRole('menuitem', { name: /add to playlist/i })).toBeInTheDocument();
expect(screen.getByRole('menuitem', { name: /go to album/i })).toBeInTheDocument();
expect(screen.getByRole('menuitem', { name: /go to artist/i })).toBeInTheDocument();
expect(screen.getByRole('menuitem', { name: /flag this track/i })).toBeInTheDocument();
expect(screen.getByRole('menuitem', { name: /^hide$/i })).toBeInTheDocument();
expect(screen.getByRole('menuitem', { name: /remove from library/i })).toBeInTheDocument();
});
it('hides "Remove from library" for non-admin users', async () => {
const { meStore } = await import('$lib/stores/me');
(meStore as unknown as { set: (v: unknown) => void }).set({ id: 'u-2', username: 'alice', is_admin: false });
render(TrackMenu, { props: { track } });
await fireEvent.click(screen.getByRole('button', { name: /track actions/i }));
expect(screen.queryByRole('menuitem', { name: /remove from library/i })).not.toBeInTheDocument();
});
it('"Add to playlist…" is disabled with a tooltip until #352 ships', async () => {
render(TrackMenu, { props: { track } });
await fireEvent.click(screen.getByRole('button', { name: /track actions/i }));
const item = screen.getByRole('menuitem', { name: /add to playlist/i });
expect(item.getAttribute('aria-disabled')).toBe('true');
expect(item.getAttribute('title')).toBe('Coming with playlists');
});
it('Play next dispatches to the player store', async () => {
const { playNext } = await import('$lib/player/store.svelte');
render(TrackMenu, { props: { track } });
await fireEvent.click(screen.getByRole('button', { name: /track actions/i }));
await fireEvent.click(screen.getByRole('menuitem', { name: /play next/i }));
expect(playNext).toHaveBeenCalledWith(track);
});
it('Remove from library is two-click — first click arms, second confirms', async () => {
const { removeTrack } = await import('$lib/api/admin/tracks');
render(TrackMenu, { props: { track } });
await fireEvent.click(screen.getByRole('button', { name: /track actions/i }));
await fireEvent.click(screen.getByRole('menuitem', { name: /remove from library/i }));
expect(screen.getByRole('menuitem', { name: /click again to confirm/i })).toBeInTheDocument();
expect(removeTrack).not.toHaveBeenCalled();
await fireEvent.click(screen.getByRole('menuitem', { name: /click again to confirm/i }));
await waitFor(() => expect(removeTrack).toHaveBeenCalledWith(track.id));
});
it('Escape closes the menu and returns aria-expanded to false', async () => {
render(TrackMenu, { props: { track } });
const kebab = screen.getByRole('button', { name: /track actions/i });
await fireEvent.click(kebab);
expect(kebab.getAttribute('aria-expanded')).toBe('true');
await fireEvent.keyDown(window, { key: 'Escape' });
await waitFor(() => expect(kebab.getAttribute('aria-expanded')).toBe('false'));
});
});
If $lib/stores/me, $lib/stores/likedIds, or $lib/stores/hiddenIds don't exist by those exact paths, update both the production component (Step 8.1) and the mock here to match the actual paths. The functional shape stays the same.
- Step 8.3: Commit
git add web/src/lib/components/TrackMenu.svelte web/src/lib/components/TrackMenu.test.ts
git commit -F - <<'EOF'
feat(web): TrackMenu rewrite — 9 entries in 4 groups (M7 #372)
Replaces the M5b-era FlagPopover-only menu with the full track-actions
surface: queue (Play next, Add to queue), collection (Like/Unlike,
Add to playlist… reserved for #352), navigation (Go to album/artist),
lifecycle (Flag, Hide/Unhide, Remove from library — admin-only).
Remove from library uses a two-click confirm (no modal) so the user
sees the next click is destructive without a dialog interrupting the
flow. The component's prop API (track, direction) is unchanged so
TrackRow + PlayerBar pick up the new entries with no code change.
EOF
Task 9 — Adjust <TrackRow> and <PlayerBar> test expectations
Files:
- Modify:
web/src/lib/components/TrackRow.test.ts - Modify:
web/src/lib/components/PlayerBar.test.ts
The component code for TrackRow + PlayerBar doesn't change — both already use <TrackMenu> with the same prop API. But existing tests in those files assert "the menu opens to a single Flag entry" (or similar M5b-era expectations). After Task 8 those assertions are wrong.
- Step 9.1: Open
web/src/lib/components/TrackRow.test.tsand find any test that asserts on<TrackMenu>'s open-state contents (search for "Flag this track", "menuitem", "aria-haspopup"). Replace those assertions with one that confirms the kebab is present andaria-haspopup="menu"is set:
it('renders a track-actions kebab with aria-haspopup="menu"', () => {
render(TrackRow, { props: { track: fakeTrack } });
const kebab = screen.getByRole('button', { name: /track actions/i });
expect(kebab.getAttribute('aria-haspopup')).toBe('menu');
});
The detailed menu-content tests live in TrackMenu.test.ts (Task 8) — TrackRow's job is just to mount the menu, so its tests should not duplicate the entry-list assertions.
- Step 9.2: Same change in
web/src/lib/components/PlayerBar.test.ts. PlayerBar passesdirection="up"to TrackMenu (drop-up); add an assertion confirming that prop reaches the kebab if the existing test suite covers that surface:
it('mounts TrackMenu with drop-up direction', () => {
render(PlayerBar, { props: { /* ...existing setup... */ } });
const kebab = screen.getByRole('button', { name: /track actions/i });
expect(kebab).toBeInTheDocument();
// Direction="up" is an internal CSS class concern; not assertable
// through aria. The existing snapshot or DOM-class check (if any)
// will need a manual update after the menu rewrite.
});
- Step 9.3: Commit
git add web/src/lib/components/TrackRow.test.ts web/src/lib/components/PlayerBar.test.ts
git commit -F - <<'EOF'
test(web): adjust TrackRow + PlayerBar tests for new TrackMenu shape
The 9-entry menu rewrite (Task 8) breaks any pre-existing assertion
that the menu opens to a single Flag entry. Strip those — TrackMenu's
own test file covers the entry list end-to-end. TrackRow + PlayerBar
just need to confirm the kebab mounts with aria-haspopup="menu".
EOF
Self-review
-
Spec coverage:
- §2 Goals — all 9 entries land in Task 8; Like/Hide menu duplication shipped; "Add to playlist…" reserved as a disabled slot; "Go to artist" single-target; "Remove from library" admin-only with cascade. ✓
- §3 Architecture (backend) — Tasks 1, 2, 3 cover SQL queries, RemoveTrack service, HTTP handler. ✓
- §3 Architecture (frontend) — Task 4 (admin API helper), Task 5 (audio store), Tasks 6-8 (component primitives + TrackMenu rewrite). ✓
- §3 Accessibility — kebab
aria-haspopup/aria-expanded,role="menu",role="menuitem",aria-disabled, Escape-to-close all in Task 8. ✓ - §4 File map — every file in the plan's File map maps to a task. ✓
- §5 API contract — Task 3.1 implements the response envelope; Task 3.4 tests the four error codes. ✓
- §6 Error handling — Task 8 wires
copyForCode()for toast surface. ✓ - §7 Testing — every test file from §7 has a writing step. ✓
- §8 Distribution / migration — confirmed no migration needed (all
track_idFKs already CASCADE). The "conditional 0014_track_cascades.up.sql" from the spec is dropped here as redundant. ✓ (this is an intentional plan deviation from the spec's optional clause)
-
Placeholder scan: No "TBD"/"TODO"/"add validation" in any task. The phrase "if unsure" appears once in Task 8 ("verify those exist (they should)") which is direction to the implementer, not a placeholder.
-
Type consistency:
TrackRef,RemoveTrackResult,removeTrack(id),playNext(t),enqueueTrack(t),qk.albums(id),qk.artists(id),qk.home(),qk.likedTracks(),qk.hiddenTracks()all consistent across tasks. Backend types:tracks.Service,tracks.RemoveTrack,tracks.ErrNotFound,tracks.ErrLidarrUnreachableall consistent. -
One spec deviation noted explicitly: the spec says migration
0014_track_cascades.up.sqlis "conditional, only if any FK is RESTRICT". I checked — all fourtrack_idFKs (play_events,general_likes_tracks,lidarr_quarantine,lidarr_quarantine_actions) already useON DELETE CASCADE. No migration is needed; the plan does not include one.