docs(m7): spec for #372 track-level actions menu
Replaces the M5b-era single-entry <TrackMenu> with a 9-entry kebab menu in 4 groups (queue / collection / navigation / lifecycle). Reserves the "Add to playlist..." slot for #352. Lands a new admin-only DELETE /api/admin/tracks/{id} for "Remove from library" with cascade album -> artist tidy-up and Lidarr-aware deletion for managed tracks. Track metadata editing split out as #373 — different feature concern (crosses into Lidarr's territory + scanner reconciliation).
This commit is contained in:
@@ -0,0 +1,240 @@
|
||||
# M7 — Track-level actions menu
|
||||
|
||||
> **Status:** Draft for review · 2026-05-03
|
||||
>
|
||||
> **Phase:** M7 task #372. Web-side polish slice. Replaces the single-entry `<TrackMenu>` (built as scaffolding for M5b's quarantine flag) with a real per-track action set. Lands before M7 #352 (Playlists CRUD) so the "Add to playlist…" entry has a menu to slot into. Mirrored on Flutter in a later slice once the web surface is settled.
|
||||
|
||||
## 1. Goal
|
||||
|
||||
Per-track kebab menu wherever `<TrackRow>` or the `<PlayerBar>` track display is used, exposing the full set of actions a user can take on a track right now: queue manipulation, library state changes (Like, Hide), navigation (Go to album/artist), Lidarr-aware track removal, and a placeholder slot for "Add to playlist…" that #352 fills.
|
||||
|
||||
## 2. Goals and non-goals
|
||||
|
||||
### Goals
|
||||
|
||||
- `<TrackMenu>` exposes nine entries, grouped four ways:
|
||||
1. **Queue:** Play next · Add to queue
|
||||
2. **Collection:** Like / Unlike · Add to playlist… (slot reserved; wired by #352)
|
||||
3. **Navigation:** Go to album · Go to artist
|
||||
4. **Lifecycle:** Flag this track… · Hide / Unhide · Remove from library *(admin-only)*
|
||||
- "Like / Unlike" and "Hide / Unhide" entries duplicate functionality already on the row (heart and eye icon buttons). Reason: keyboard discoverability — once the kebab is focused, arrow-keys reach every action; mouse users still get one-tap heart/eye on the row.
|
||||
- "Add to playlist…" reserves the menu slot but the submenu of playlists is wired in #352, not here. The slot renders as disabled with a tooltip "Coming with playlists" until #352 ships.
|
||||
- "Go to artist" is single-target (`track.artistId`); compilation/feature artists aren't modeled in the schema today and that's a schema problem to solve before this entry needs a submenu.
|
||||
- "Remove from library" is admin-only — gated client-side by `currentUser.is_admin`, server-side by the existing `auth.RequireAdmin()` middleware on the new `/api/admin/tracks/{id}` route. New backend endpoint deletes the track row, removes the file from disk (or via Lidarr for Lidarr-managed tracks), and cascades album → artist tidy-up if either becomes empty.
|
||||
- Keyboard accessibility: kebab is `<button aria-haspopup="menu" aria-expanded>`; opened menu has `role="menu"` with each entry `role="menuitem"`. Up/Down arrows cycle, Enter activates, Escape closes and returns focus to the kebab.
|
||||
- Error envelope: existing `copyForCode()` helper resolves all surface errors (`lidarr_unreachable`, `lidarr_unauthorized`, `lidarr_server_error`, `not_found`, `unauthorized`, `connection_refused`).
|
||||
|
||||
### Non-goals (this slice)
|
||||
|
||||
- Per-track metadata editing (title / artist / track_number / genre / year / etc.) — filed as M7 #373 ("Track metadata editing"). Crosses into Lidarr's territory and the scanner's reconciliation logic; deserves its own spec.
|
||||
- "Add to playlist…" submenu wiring — lands in #352 (Playlists CRUD slice 1).
|
||||
- "Remove from library" exposed to non-admin users — admin-only matches the existing model (file management is an operator concern).
|
||||
- Multi-artist disambiguation on "Go to artist" — schema doesn't model collaborator artists today; expanding the menu to a submenu earns its keep only after the schema does.
|
||||
- Submenu UI infrastructure beyond the playlists hook — no other entries need submenus in this slice.
|
||||
- Flutter mirror — separate task once web settles (#356 follow-up slice).
|
||||
|
||||
## 3. Architecture
|
||||
|
||||
### Backend
|
||||
|
||||
**One new admin endpoint:** `DELETE /api/admin/tracks/{id}`. Handler in `internal/api/admin_tracks.go` (new file). Calls a new `internal/tracks` package that owns the removal logic.
|
||||
|
||||
`internal/tracks/service.go` exposes `RemoveTrack(ctx, trackID, adminID)` returning `(deletedAlbumID *string, deletedArtistID *string, err)`. Steps:
|
||||
|
||||
1. Look up the track. Need: `file_path`, `lidarr_managed`, `album_id`, `artist_id`. Returns `ErrNotFound` if missing.
|
||||
2. **For Lidarr-managed tracks:** call the existing Lidarr client to delete the track via Lidarr first. The current `lidarrquarantine.DeleteViaLidarr` handles this for quarantined tracks; the same primitive applies. If Lidarr is unreachable / unauthorized / errors, return the same typed errors (`ErrLidarrUnreachable`, `ErrLidarrUnauthorized`, `ErrLidarrServerError`) that the quarantine flow already maps. The DB row is NOT deleted on Lidarr failure — Lidarr would re-import on next sync and the row would come back inconsistent.
|
||||
3. **For non-Lidarr tracks:** `os.Remove(file_path)`. If the file is already missing, log `slog.Warn("track delete: file already missing", "path", file_path, "track_id", id)` and proceed — the goal is DB consistency.
|
||||
4. Begin a single DB transaction:
|
||||
- `DELETE FROM tracks WHERE id=$1`. Existing FK `ON DELETE CASCADE` handles `play_events`, `general_likes_tracks`, `lidarr_quarantine`. (Future `playlist_tracks` will also need `ON DELETE CASCADE` — locked when #352's migration lands.)
|
||||
- Verify all relevant FKs are CASCADE: read `internal/db/migrations/*.up.sql`. If any are RESTRICT or have no FK rule, fix as part of this slice via migration `0014_track_cascades.up.sql` — only if needed.
|
||||
- Count remaining tracks for the album. If 0, `DELETE FROM albums WHERE id=$2`. Set `deletedAlbumID`.
|
||||
- If album was deleted: count remaining albums for the artist. If 0, `DELETE FROM artists WHERE id=$3`. Set `deletedArtistID`.
|
||||
- Commit.
|
||||
5. Return `(deletedAlbumID, deletedArtistID, nil)`.
|
||||
|
||||
**API handler** maps the typed errors:
|
||||
- `ErrNotFound` → 404 `{"error": "not_found"}`
|
||||
- `ErrLidarrUnreachable` → 503 `{"error": "lidarr_unreachable"}`
|
||||
- `ErrLidarrUnauthorized` → 503 `{"error": "lidarr_unauthorized"}`
|
||||
- `ErrLidarrServerError` → 502 `{"error": "lidarr_server_error"}`
|
||||
- success → 200 `{"deleted_track_id": "...", "deleted_album_id": "...?", "deleted_artist_id": "...?"}`
|
||||
|
||||
### Frontend
|
||||
|
||||
`<TrackMenu>` is rewritten from "kebab → FlagPopover only" to "kebab → menu with grouped entries". The menu API stays compatible (same `track: TrackRef` and `direction: 'up'|'down'` props), so existing callers (`<TrackRow>`, `<PlayerBar>`) pick up the new entries with no code change.
|
||||
|
||||
Two new component primitives:
|
||||
|
||||
- `<TrackMenuItem>` — single row of the menu (`<button role="menuitem">` + Lucide icon + label + optional shortcut indicator).
|
||||
- `<TrackMenuDivider>` — `<hr>` styled to FabledSword's pewter/slate.
|
||||
|
||||
`<TrackMenu>`'s state machine:
|
||||
|
||||
- `menuOpen: boolean` — set true on kebab click; toggles `aria-expanded`.
|
||||
- `popoverOpen: boolean` — set true when "Flag this track…" fires; menuOpen flips to false.
|
||||
- Outside-click handler closes both.
|
||||
|
||||
Audio queue mutations are pure client-state. The existing audio store gets two methods:
|
||||
|
||||
```ts
|
||||
function playNext(track: TrackRef) {
|
||||
// Splice immediately after the current index.
|
||||
_queue.splice(_index + 1, 0, track);
|
||||
}
|
||||
|
||||
function addToQueue(track: TrackRef) {
|
||||
_queue.push(track);
|
||||
}
|
||||
```
|
||||
|
||||
Like / Unlike + Hide / Unhide reuse the existing `likeTrack` / `unlikeTrack` and `quarantineTrack` / `unquarantineTrack` API helpers + their TanStack Query invalidations.
|
||||
|
||||
"Remove from library" calls a new `web/src/lib/api/admin/tracks.ts::removeTrack(id)`. On success, invalidate `qk.albums(track.albumId)`, `qk.artists(track.artistId)`, `qk.home()`, plus any list-page query keys currently rendered. The response payload's optional `deleted_album_id` / `deleted_artist_id` lets the client also evict those entity caches.
|
||||
|
||||
Navigation entries call `goto('/albums/' + track.albumId)` and `goto('/artists/' + track.artistId)` directly — no async, no error path.
|
||||
|
||||
### Accessibility
|
||||
|
||||
- Kebab `<button>` carries `aria-label="Track actions"`, `aria-haspopup="menu"`, `aria-expanded={menuOpen}`.
|
||||
- Open menu has `role="menu"`. Each entry: `<button role="menuitem">`. Disabled entries (admin-only for non-admin users; "Add to playlist…" pre-#352) have `aria-disabled="true"` rather than being absent — keeps the menu height stable so muscle memory works for both user types.
|
||||
- Up/Down arrow keys cycle focus through visible enabled items. Home/End jump to first/last. Escape closes and returns focus to the kebab. Enter activates the focused item.
|
||||
- The submenu plumbing #352 lands will use arrow-right to enter, arrow-left to exit, with focus following.
|
||||
|
||||
## 4. File map
|
||||
|
||||
### Backend — create
|
||||
|
||||
- `internal/tracks/service.go` — `RemoveTrack(ctx, id, adminID)` + typed errors.
|
||||
- `internal/tracks/service_test.go` — unit tests covering the six service paths (Lidarr happy, non-Lidarr happy, file-already-gone, Lidarr error, album-empty cascade, artist-empty cascade).
|
||||
- `internal/api/admin_tracks.go` — `handleRemoveTrack(w, r)`.
|
||||
- `internal/api/admin_tracks_test.go` — HTTP tests (admin-only auth, 4 error codes, response envelope).
|
||||
|
||||
### Backend — modify
|
||||
|
||||
- `internal/api/api.go` — register `admin.Delete("/tracks/{id}", h.handleRemoveTrack)`.
|
||||
- `internal/db/queries/tracks.sql` — append `DeleteTrack`, `DeleteAlbumIfEmpty`, `DeleteArtistIfEmpty`, `CountTracksByAlbum`, `CountAlbumsByArtist`.
|
||||
- `internal/db/dbq/*` — regenerated by `sqlc generate`.
|
||||
- `internal/db/migrations/0014_track_cascades.up.sql` — **conditional**, only if any of `play_events`, `general_likes_tracks`, `lidarr_quarantine` lacks `ON DELETE CASCADE` on its `track_id` FK. Read existing migrations first; skip if all cascades are already correct.
|
||||
|
||||
### Frontend — create
|
||||
|
||||
- `web/src/lib/components/TrackMenuItem.svelte` — single-entry primitive.
|
||||
- `web/src/lib/components/TrackMenuItem.test.ts`
|
||||
- `web/src/lib/components/TrackMenuDivider.svelte` — visual separator.
|
||||
- `web/src/lib/api/admin/tracks.ts` — `removeTrack(id)`.
|
||||
- `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.
|
||||
- `web/src/lib/components/TrackMenu.test.ts` — full rewrite.
|
||||
- `web/src/lib/components/TrackRow.svelte` — no code change required (uses `<TrackMenu>`'s same prop API).
|
||||
- `web/src/lib/components/PlayerBar.svelte` — no code change required.
|
||||
- `web/src/lib/components/TrackRow.test.ts` — adjust expectations (menu now has 9 entries instead of 1).
|
||||
- `web/src/lib/components/PlayerBar.test.ts` — same.
|
||||
- `web/src/lib/api/queries.ts` — no new query keys. The Remove flow invalidates existing `qk.albums(id)` / `qk.artists(id)` / `qk.home()` plus any list-page keys currently rendered. There is no admin-tracks list view in this slice.
|
||||
- `web/src/lib/stores/audio.ts` — add `playNext(track)` and `addToQueue(track)` methods.
|
||||
- `web/src/lib/stores/audio.test.ts` — extend to cover the new methods.
|
||||
|
||||
## 5. API contract
|
||||
|
||||
### `DELETE /api/admin/tracks/{id}`
|
||||
|
||||
**Auth:** admin session.
|
||||
|
||||
**Request:** no body.
|
||||
|
||||
**Response (200 OK):**
|
||||
|
||||
```json
|
||||
{
|
||||
"deleted_track_id": "uuid",
|
||||
"deleted_album_id": "uuid",
|
||||
"deleted_artist_id": "uuid"
|
||||
}
|
||||
```
|
||||
|
||||
`deleted_album_id` and `deleted_artist_id` are optional. Present only when removing the track left the parent empty and the row was deleted too.
|
||||
|
||||
**Errors:**
|
||||
|
||||
| Status | Code | Trigger |
|
||||
|---|---|---|
|
||||
| 401 | `unauthenticated` | no session |
|
||||
| 403 | `unauthorized` | non-admin session |
|
||||
| 404 | `not_found` | track id doesn't exist |
|
||||
| 502 | `lidarr_server_error` | Lidarr returned 5xx |
|
||||
| 503 | `lidarr_unreachable` | network error reaching Lidarr |
|
||||
| 503 | `lidarr_unauthorized` | Lidarr rejected the API key |
|
||||
| 500 | `server_error` | unexpected error (DB, filesystem) |
|
||||
|
||||
## 6. Error handling (frontend)
|
||||
|
||||
`removeTrack(id)` reuses `apiFetch` with the existing dual-envelope handling. Errors surface as a toast via `copyForCode(code)`:
|
||||
|
||||
- `lidarr_unreachable` → "Lidarr is unreachable right now. Try again, or check Admin → Integrations." (existing copy)
|
||||
- `lidarr_unauthorized` → existing copy
|
||||
- `lidarr_server_error` → existing copy
|
||||
- `not_found` → "Track no longer exists." Trigger refetch of parent list query.
|
||||
- `unauthorized` → "Admins only." (Defensive — UI should hide the entry for non-admins.)
|
||||
- `connection_refused` → existing copy.
|
||||
|
||||
Audio queue mutations have no error path — they are local state writes. Defensive guard: if `_queue` is `null` (audio store uninitialized), the action is a no-op.
|
||||
|
||||
## 7. Testing
|
||||
|
||||
### Backend
|
||||
|
||||
- `internal/tracks/service_test.go` — unit tests, table-driven where it helps:
|
||||
- Non-Lidarr track happy path. File present, deletes cleanly. DB row gone. Album / artist persist.
|
||||
- Lidarr-managed track happy path. Lidarr returns 200; file removed by Lidarr; DB row gone.
|
||||
- File-already-missing path. `os.Remove` returns `ErrNotExist`. Service logs warning, proceeds. DB row gone.
|
||||
- Lidarr error propagation. `lidarr_unreachable` / `lidarr_unauthorized` / `lidarr_server_error` each surface as the matching `Err…`. DB row stays.
|
||||
- Album-empty cascade. Track was the album's only track. Album row deleted, artist persists if has other albums. `deletedAlbumID` returned.
|
||||
- Artist-empty cascade. Track was the artist's only track. Album AND artist deleted. Both IDs returned.
|
||||
- `internal/api/admin_tracks_test.go` — HTTP tests:
|
||||
- Non-admin → 403 `unauthorized`.
|
||||
- No session → 401 `unauthenticated`.
|
||||
- Unknown id → 404 `not_found`.
|
||||
- Each typed Lidarr error → matching response code + envelope.
|
||||
- Happy path → 200 with `deleted_track_id` (and optionally `deleted_album_id` / `deleted_artist_id`).
|
||||
|
||||
### Frontend
|
||||
|
||||
- `TrackMenu.test.ts` (rewrite) covers:
|
||||
- Each of the 9 entries renders for an admin user.
|
||||
- "Remove from library" is hidden for non-admin (or renders disabled with `aria-disabled="true"`).
|
||||
- "Add to playlist…" renders disabled with the "Coming with playlists" tooltip.
|
||||
- Kebab `aria-haspopup="menu"`, `aria-expanded` cycles on open/close.
|
||||
- Up/Down arrow keys cycle focus; Escape closes; Enter activates.
|
||||
- Outside click closes.
|
||||
- Submenu plumbing (`role="menu"` on the playlist hook) renders even though disabled.
|
||||
- `TrackMenuItem.test.ts` — primitive renders icon, label, click handler fires, `aria-disabled` honored.
|
||||
- `tracks.test.ts` (new admin API helper) — `removeTrack` posts the correct method+URL, parses success envelope, surfaces errors.
|
||||
- `audio.test.ts` (extend) — `playNext(track)` splices at `_index+1`; `addToQueue(track)` appends; both no-op on empty queue.
|
||||
- `TrackRow.test.ts` and `PlayerBar.test.ts` (adjust) — existing assertions about the menu's sole "Flag" entry need updating to match the new 9-entry structure. The compatible prop API means no other test changes needed.
|
||||
|
||||
## 8. Distribution / migration notes
|
||||
|
||||
- **Migration `0014`:** conditional. Read the existing FKs from migrations 0002 (tracks), 0005 (play_events), 0006 (general_likes), 0011 (lidarr_quarantine). If any FK on `track_id` is `RESTRICT` or `NO ACTION`, write a migration to `ALTER TABLE … DROP CONSTRAINT … ADD CONSTRAINT … ON DELETE CASCADE`. If all four are already CASCADE, skip the migration entirely.
|
||||
- No client breaking changes. The `<TrackMenu>` prop API is unchanged.
|
||||
- `min_client_version` bump in `internal/server/version.go`: not required — the new endpoint only matters for admins, and the menu degrades gracefully on old clients (they don't show entries they don't know about).
|
||||
|
||||
## 9. Origin
|
||||
|
||||
M7 #372. Brainstorm 2026-05-03. Decisions locked:
|
||||
|
||||
1. Per-track metadata editing is out of scope — filed as M7 #373.
|
||||
2. Menu structure: 9 entries in 4 groups. Like/Hide duplicated in menu for keyboard discoverability.
|
||||
3. `Go to artist` single-target — multi-artist data model isn't there yet.
|
||||
4. `Remove from library` admin-only, with cascade album → artist tidy-up.
|
||||
5. Submenu plumbing only matters for #352's `Add to playlist…` and is reserved as a slot here.
|
||||
|
||||
## 10. Related
|
||||
|
||||
- M7 #352 — Playlists CRUD. Wires `Add to playlist…` submenu into the menu slot reserved here. Brainstorm parked at task-log id 44.
|
||||
- M7 #373 — Track metadata editing. Split out from #372 in this brainstorm.
|
||||
- M5b quarantine — `<FlagPopover>` and the `quarantineTrack` / `unquarantineTrack` helpers reused by the Hide/Unhide entries.
|
||||
- M2 likes — `likeTrack` / `unlikeTrack` and the existing heart icon, now also surfaced as a menu entry.
|
||||
- M5a Lidarr — Lidarr-aware delete reuses the same primitive `lidarrquarantine.DeleteViaLidarr` uses today.
|
||||
- M7 #356 — Flutter mirror in a later slice once the web menu settles.
|
||||
Reference in New Issue
Block a user