docs(spec): add M5b quarantine workflow design
Per-user track-level soft-hide with reason taxonomy, aggregated admin queue at /admin/quarantine with Resolve / Delete file / Delete via Lidarr actions, audit log for admin actions, soft-hide honored by /api/* read endpoints (Subsonic /rest/* unchanged per legacy rule). Lidarr client extends with LookupAlbumByMBID + DeleteAlbum (deleteFiles + addImportListExclusion).
This commit is contained in:
@@ -0,0 +1,389 @@
|
||||
# M5b — Quarantine workflow + admin resolution UI
|
||||
|
||||
> **Status:** Draft for review · 2026-04-30
|
||||
>
|
||||
> **Sub-plan of:** M5 (Lidarr integration + quarantine workflow). Decomposition recap from the M5a spec:
|
||||
>
|
||||
> - **M5a** — Lidarr connection + search/add + admin shell. Shipped on `dev`.
|
||||
> - **M5b (this spec)** — Quarantine workflow (per-user soft-hide of tracks, admin resolution UI).
|
||||
> - **M5c** — Radio "suggested additions" (out-of-library MBIDs surfaced in `/api/radio` responses; SPA add affordance).
|
||||
>
|
||||
> Each ships as its own PR with its own brainstorm/spec/plan cycle.
|
||||
|
||||
## 1. Goal
|
||||
|
||||
Authenticated users can flag any local track as broken with a reason and optional notes. A flagged track disappears from that user's library, search, browse, and `/api/radio` responses — but appears on a dedicated `/library/hidden` page where they can review and un-hide. Admins see an aggregated queue at `/admin/quarantine` (one row per track with reason distribution + per-user details + inline playback) and resolve each row by clearing the reports, deleting the local file, or telling Lidarr to remove the parent album with import-list exclusion.
|
||||
|
||||
The dominant user mental model is **data quality**: "this track is a bad rip / wrong file / wrong tags / duplicate." Personal preference framing is out of scope.
|
||||
|
||||
## 2. Goals and non-goals
|
||||
|
||||
### Goals
|
||||
|
||||
- Authenticated user can flag any track with reason ∈ {`bad_rip`, `wrong_file`, `wrong_tags`, `duplicate`, `other`} plus optional notes.
|
||||
- Trigger affordance is a `<TrackMenu>` overflow (kebab) on every track row and on the now-playing player bar — sibling to `<LikeButton>` but one-click-removed to prevent miss-clicks.
|
||||
- Quarantined tracks are hidden from that user's `/api/albums/:id`, `/api/artists/:id`, library track lists, search, `/api/radio`, and contextual radio. They remain visible only on `/library/hidden`.
|
||||
- `/library/hidden` page lists the user's own quarantines with un-hide affordance.
|
||||
- Admin queue at `/admin/quarantine` aggregates by track, shows reason distribution + count, expandable per-user reports, inline playback, and the three resolution actions.
|
||||
- Admin resolution actions: **Resolve** (clear the row), **Delete file** (rm file from disk + tracks row + clear), **Delete via Lidarr** (call Lidarr `DELETE /api/v1/album/{id}?deleteFiles=true&addImportListExclusion=true`, cascade Minstrel rows, clear).
|
||||
- Admin actions write to a `lidarr_quarantine_actions` audit log with snapshot fields so the log stays readable after the underlying tracks/albums are deleted.
|
||||
- Subsonic API (`/rest/*`) stays unfiltered — quarantine is `/api/*`-only.
|
||||
|
||||
### Non-goals (this slice)
|
||||
|
||||
- Per-album / per-artist quarantine. Tracks only.
|
||||
- Quarantine on shared playlists, queues, or Subsonic clients.
|
||||
- Auto-resolve rules ("if 3 users flag, auto-quarantine globally"). Admin always decides.
|
||||
- Notifying users when their report is acted on (toast / email). → polish slot.
|
||||
- Bulk admin operations (multi-select + apply). → revisit if queue grows beyond what one-by-one handles.
|
||||
|
||||
## 3. Architecture
|
||||
|
||||
### New Go packages
|
||||
|
||||
- **`internal/lidarrquarantine/`** — `Service` owning the quarantine lifecycle:
|
||||
- `Flag(ctx, userID, trackID, reason, notes)` — upsert a `lidarr_quarantine` row.
|
||||
- `Unflag(ctx, userID, trackID)` — remove the caller's row.
|
||||
- `ListMine(ctx, userID)` — caller's own quarantines (drives `/library/hidden`).
|
||||
- `ListAdminQueue(ctx)` — aggregated by track. Returns `[]AdminQueueRow{TrackID, TrackTitle, ArtistName, AlbumTitle, AlbumID, LidarrAlbumMBID, ReportCount, ReasonCounts, LatestAt, Reports []UserReport}`.
|
||||
- `Resolve(ctx, trackID, adminID)` — delete all per-user rows for the track + write audit row.
|
||||
- `DeleteFile(ctx, trackID, adminID)` — call `library.DeleteTrackFile` then Resolve.
|
||||
- `DeleteViaLidarr(ctx, trackID, adminID)` — look up the parent album in Lidarr by MBID, call `Client.DeleteAlbum(id, deleteFiles=true, addImportListExclusion=true)`, remove Minstrel rows for all tracks of that album, clear all related quarantine rows, write audit row.
|
||||
|
||||
### Lidarr client additions (`internal/lidarr`)
|
||||
|
||||
- `LookupArtistByMBID(ctx, mbid) (LidarrArtist, error)` — `GET /api/v1/artist?mbId=…`.
|
||||
- `LookupAlbumByMBID(ctx, mbid) (LidarrAlbum, error)` — `GET /api/v1/album?foreignAlbumId=…`.
|
||||
- `DeleteAlbum(ctx, lidarrAlbumID, deleteFiles bool, addImportListExclusion bool) error` — `DELETE /api/v1/album/{id}?deleteFiles=...&addImportListExclusion=...`. M5b admin path always passes both `true`.
|
||||
|
||||
Returns the same typed errors the existing client uses: `ErrUnreachable`, `ErrAuthFailed`, `ErrLookupFailed`, plus `ErrNotFound` for the lookup methods when no row matches.
|
||||
|
||||
### Library deletion (`internal/library`)
|
||||
|
||||
- New `DeleteTrackFile(ctx, trackID) error` — removes the file from disk and the row from `tracks`. Album/artist rows stay (other tracks may reference them). The reconciler's MBID lookup will simply find no match next pass; M5a's reconcile is unaffected.
|
||||
|
||||
### Soft-hide enforcement
|
||||
|
||||
Every endpoint that returns track lists in user-context joins against `lidarr_quarantine`:
|
||||
|
||||
```sql
|
||||
WHERE NOT EXISTS (
|
||||
SELECT 1 FROM lidarr_quarantine q
|
||||
WHERE q.user_id = $userID AND q.track_id = tracks.id
|
||||
)
|
||||
```
|
||||
|
||||
Affected queries (modified, not new):
|
||||
- `GetAlbumDetail` (track list filter)
|
||||
- `GetArtistDetail` / library artist views (transitively, via track filter)
|
||||
- `Search` (track facet)
|
||||
- Library track-list endpoints
|
||||
- Radio generator + contextual radio endpoints + similarity-driven autoplay
|
||||
|
||||
`/rest/*` Subsonic queries are NOT modified — Subsonic clients see the unfiltered library.
|
||||
|
||||
### Wiring
|
||||
|
||||
- `cmd/minstrel/main.go` constructs `lidarrquarantine.Service` and injects into `internal/api/handlers`. No new background worker.
|
||||
- New handler files: `internal/api/quarantine.go` (user-facing), `internal/api/admin_quarantine.go` (admin endpoints).
|
||||
- Reuses M5a's `RequireAdmin` middleware — `/api/admin/quarantine/*` mounts on the existing admin route group.
|
||||
|
||||
### SPA additions
|
||||
|
||||
- `<TrackMenu>` Svelte component — kebab icon button + dropdown menu. Renders a single "Flag this track…" item for M5b; designed for future actions.
|
||||
- `<FlagPopover>` Svelte component — opens from `<TrackMenu>` with the reason `<select>` + notes textarea + Cancel/Flag buttons. Pre-fills if the user has an existing flag (re-flag is upsert).
|
||||
- `/library/hidden` page — caller's quarantines with un-hide affordance.
|
||||
- `/admin/quarantine` page — aggregated admin queue with the three resolution actions, inline playback, expandable per-user reports.
|
||||
- `Shell.svelte` — add `Hidden` to the main nav (visible to all auth'd users).
|
||||
- `AdminSidebar.svelte` — promote `Quarantine` from placeholder to real link.
|
||||
- `<TrackRow>` and `<PlayerBar>` — mount `<TrackMenu>` next to `<LikeButton>`.
|
||||
|
||||
### Data flow — happy paths
|
||||
|
||||
**User flag → soft-hide:**
|
||||
1. User clicks the `<TrackMenu>` kebab on a track row → "Flag this track…" → popover opens.
|
||||
2. User picks reason, optionally types notes, clicks Flag → SPA `POST /api/quarantine {track_id, reason, notes?}`.
|
||||
3. Server upserts `lidarr_quarantine` row, returns 201.
|
||||
4. SPA optimistically removes the row from the visible list and toasts "Hidden — review on the Hidden tab."
|
||||
5. Subsequent reads from this user join the quarantine table and silently exclude the track.
|
||||
|
||||
**Admin Resolve:**
|
||||
1. Admin opens `/admin/quarantine`, sees aggregated row, clicks Resolve.
|
||||
2. SPA `POST /api/admin/quarantine/:track_id/resolve`.
|
||||
3. Service deletes all per-user rows for the track in a single statement, writes audit row, returns 200 with `{action_id, affected_users}`.
|
||||
4. SPA removes the row from the queue and toasts the count cleared.
|
||||
|
||||
**Admin Delete file:**
|
||||
1. Admin clicks Delete file → modal-confirm.
|
||||
2. SPA `POST /api/admin/quarantine/:track_id/delete-file`.
|
||||
3. Service calls `library.DeleteTrackFile` (rm + DELETE FROM tracks), then deletes per-user rows, writes audit row.
|
||||
4. SPA removes the row.
|
||||
|
||||
**Admin Delete via Lidarr:**
|
||||
1. Admin clicks Delete via Lidarr → typed-confirm modal ("DELETE").
|
||||
2. SPA `POST /api/admin/quarantine/:track_id/delete-via-lidarr`.
|
||||
3. Service:
|
||||
- Looks up the track's parent album in the local DB (gets `lidarr_album_mbid`).
|
||||
- 404 with `album_mbid_missing` if the track has no MBID.
|
||||
- Calls `Client.LookupAlbumByMBID` to translate MBID → Lidarr-internal album ID.
|
||||
- Calls `Client.DeleteAlbum(id, true, true)`. **No partial state**: if Lidarr fails, returns the typed error; nothing in Minstrel changes; admin retries.
|
||||
- On Lidarr success, deletes Minstrel rows for all tracks of the parent album, clears related quarantine rows, writes audit row with `affected_users` count + `lidarr_album_mbid`.
|
||||
4. SPA removes the row, toasts "Removed — Lidarr will not redownload."
|
||||
|
||||
## 4. Schema — migration `0011_lidarr_quarantine`
|
||||
|
||||
### `lidarr_quarantine` (per-user complaints)
|
||||
|
||||
```sql
|
||||
CREATE TYPE lidarr_quarantine_reason AS ENUM (
|
||||
'bad_rip', 'wrong_file', 'wrong_tags', 'duplicate', 'other'
|
||||
);
|
||||
|
||||
CREATE TABLE lidarr_quarantine (
|
||||
user_id uuid NOT NULL REFERENCES users(id) ON DELETE CASCADE,
|
||||
track_id uuid NOT NULL REFERENCES tracks(id) ON DELETE CASCADE,
|
||||
reason lidarr_quarantine_reason NOT NULL,
|
||||
notes text,
|
||||
created_at timestamptz NOT NULL DEFAULT now(),
|
||||
PRIMARY KEY (user_id, track_id)
|
||||
);
|
||||
|
||||
CREATE INDEX lidarr_quarantine_track_idx ON lidarr_quarantine (track_id);
|
||||
CREATE INDEX lidarr_quarantine_user_idx ON lidarr_quarantine (user_id, created_at DESC);
|
||||
```
|
||||
|
||||
### `lidarr_quarantine_actions` (admin audit log)
|
||||
|
||||
```sql
|
||||
CREATE TYPE lidarr_quarantine_action AS ENUM (
|
||||
'resolved', 'deleted_file', 'deleted_via_lidarr'
|
||||
);
|
||||
|
||||
CREATE TABLE lidarr_quarantine_actions (
|
||||
id uuid PRIMARY KEY DEFAULT gen_random_uuid(),
|
||||
track_id uuid NOT NULL, -- not FK: track may be gone
|
||||
track_title text NOT NULL, -- snapshot
|
||||
artist_name text NOT NULL, -- snapshot
|
||||
album_title text, -- snapshot
|
||||
action lidarr_quarantine_action NOT NULL,
|
||||
admin_id uuid REFERENCES users(id) ON DELETE SET NULL,
|
||||
lidarr_album_mbid text, -- non-null on deleted_via_lidarr
|
||||
affected_users int NOT NULL,
|
||||
created_at timestamptz NOT NULL DEFAULT now()
|
||||
);
|
||||
|
||||
CREATE INDEX lidarr_quarantine_actions_track_idx ON lidarr_quarantine_actions (track_id);
|
||||
CREATE INDEX lidarr_quarantine_actions_created_idx ON lidarr_quarantine_actions (created_at DESC);
|
||||
```
|
||||
|
||||
**Shape notes:**
|
||||
- `(user_id, track_id)` PK mirrors the `general_likes` pattern. Re-flagging upserts (replaces reason/notes).
|
||||
- `lidarr_quarantine_actions.track_id` deliberately is NOT a foreign key — `deleted_via_lidarr` removes the track row. Snapshot text columns keep the audit log readable post-delete.
|
||||
- `affected_users` lets the admin see "delete-via-Lidarr at 2026-05-02 affected 4 users" after the fact.
|
||||
- `lidarr_album_mbid` is non-null only on `deleted_via_lidarr`; gives recovery context if the operator later wants to re-add the album in Lidarr.
|
||||
|
||||
### Down migration
|
||||
|
||||
Drops in reverse order: indexes → tables → enum types.
|
||||
|
||||
## 5. API surface
|
||||
|
||||
All endpoints under `/api/*`, JSON, `{error: {code, message}}` envelope on errors. `/api/admin/*` passes through `RequireAdmin` (M5a).
|
||||
|
||||
### User-facing (any authenticated user)
|
||||
|
||||
| Method | Path | Behavior |
|
||||
|---|---|---|
|
||||
| `POST` | `/api/quarantine` | Body: `{track_id, reason, notes?}`. Upserts a `lidarr_quarantine` row for the caller. Returns `201 {track_id, reason, notes, created_at}`. 400 `bad_reason` if reason is not in the enum. 404 `track_not_found` if track_id doesn't exist. |
|
||||
| `DELETE` | `/api/quarantine/:track_id` | Removes the caller's row. 204 on success. 404 `quarantine_not_found` if no row exists for this user+track. |
|
||||
| `GET` | `/api/quarantine/mine` | Returns the caller's quarantined tracks with full track detail (joined). Used by `/library/hidden`. Ordered by `created_at DESC`. |
|
||||
|
||||
### Admin-only
|
||||
|
||||
| Method | Path | Behavior |
|
||||
|---|---|---|
|
||||
| `GET` | `/api/admin/quarantine` | Aggregated queue, one row per track. Each row: `{track_id, track_title, artist_name, album_title, album_id, lidarr_album_mbid?, report_count, reason_counts: {bad_rip:N,...}, latest_at, reports: [{user_id, username, reason, notes, created_at}]}`. Ordered by `latest_at DESC`. |
|
||||
| `POST` | `/api/admin/quarantine/:track_id/resolve` | Clears all `lidarr_quarantine` rows for the track. Writes audit row. Returns `200 {action_id, affected_users}`. |
|
||||
| `POST` | `/api/admin/quarantine/:track_id/delete-file` | Deletes file from disk + `tracks` row, then clears all quarantine rows, writes audit row. Returns `200 {action_id, affected_users}`. 404 `track_not_found` if already gone. 500 `file_delete_failed` on OS error. |
|
||||
| `POST` | `/api/admin/quarantine/:track_id/delete-via-lidarr` | Calls Lidarr DELETE on the parent album with `deleteFiles=true&addImportListExclusion=true`, then removes Minstrel rows for all tracks of that album, clears related quarantine rows, writes audit row. Returns `200 {action_id, affected_users, deleted_track_count}`. 503 `lidarr_disabled`/`lidarr_unreachable`/`lidarr_auth_failed`. 404 `album_mbid_missing` if track has no resolvable album MBID. 502 `lidarr_album_lookup_failed` if Lidarr returns no album for the MBID. |
|
||||
| `GET` | `/api/admin/quarantine/actions?limit=` | Recent admin actions log for audit/recovery. Default `limit=50`, max 200. Ordered by `created_at DESC`. |
|
||||
|
||||
### Modified read endpoints (soft-hide enforcement)
|
||||
|
||||
Existing endpoints get a quarantine join when called with an authenticated user context:
|
||||
- `GET /api/albums/:id`
|
||||
- `GET /api/artists/:id`
|
||||
- `GET /api/library` and friends
|
||||
- `GET /api/search` (track facet)
|
||||
- `GET /api/radio` and contextual radio endpoints
|
||||
|
||||
The filter is the `WHERE NOT EXISTS` clause described in §3. `/rest/*` (Subsonic) stays unchanged.
|
||||
|
||||
### Error codes added
|
||||
|
||||
`bad_reason`, `quarantine_not_found`, `track_not_found`, `album_mbid_missing`, `lidarr_album_lookup_failed`, `file_delete_failed`. Reuses M5a's: `lidarr_disabled`, `lidarr_unreachable`, `lidarr_auth_failed`, `not_authorized`.
|
||||
|
||||
## 6. UI surfaces
|
||||
|
||||
All against the FabledSword design tokens established in M5a. Voice rule: sentence case, "understated mythic" register on errors and empty states.
|
||||
|
||||
### `<TrackMenu>` — overflow menu component
|
||||
|
||||
Replaces the originally-considered standalone Flag button. Mounted in:
|
||||
- `TrackRow.svelte` — sibling to `<LikeButton>` in the row's right-cluster.
|
||||
- `PlayerBar.svelte` — next to the like button in the player's right cluster.
|
||||
|
||||
Layout: Lucide `MoreHorizontal` (or `MoreVertical` — finalize during implementation) at 16px / 1px stroke, `text-text-muted` default, `text-text-primary` on hover. Click toggles a small dropdown anchored to the button. Click-outside and Escape both close.
|
||||
|
||||
For M5b the menu has a single item: **Flag this track…** (Lucide `Flag` icon + sentence-case label). The component takes a `track: TrackRef` prop and is structured for future items (Add to queue, Add to playlist, View album, etc.).
|
||||
|
||||
### `<FlagPopover>` — flagging UI
|
||||
|
||||
Opens from `<TrackMenu>`. NOT a full modal — a small popover anchored near the trigger so the track context stays visible.
|
||||
|
||||
Contents:
|
||||
- Header: "Flag this track as broken" (Inter 13/500).
|
||||
- Reason `<select>`: "Bad rip", "Wrong file", "Wrong tags", "Duplicate", "Other".
|
||||
- Notes `<textarea>` (optional, placeholder "What's wrong with it? (optional)"). 200-char soft limit.
|
||||
- Actions: Cancel (Pewter ghost) · Flag (Bronze + Lucide `Flag` icon).
|
||||
|
||||
If the user already has a quarantine on this track, the popover pre-fills the saved reason/notes and the action button reads "Update flag." Re-submitting upserts.
|
||||
|
||||
On submit: optimistic hide of the track row + toast "Hidden — review on the Hidden tab." Failure rolls back the optimistic hide and shows an error toast.
|
||||
|
||||
### `/library/hidden` (user-facing)
|
||||
|
||||
New nav item under the existing Library section in `Shell.svelte`. Position: between Liked and Search (defer to plan time if a different position reads better).
|
||||
|
||||
Page shows the user's quarantined tracks ordered by `created_at DESC`. Row anatomy mirrors `/requests` for visual consistency:
|
||||
- 56px album art square (Slate fallback).
|
||||
- Pills: kind ("Track") + reason (`bad_rip` etc., accent-tint).
|
||||
- Title (Parchment) + meta line "by Artist · Album · flagged 2d ago".
|
||||
- Notes (Vellum, italic) — only when present.
|
||||
- Action: Un-hide (Pewter ghost + Lucide `RotateCcw` icon). One click — no confirmation. Optimistic remove.
|
||||
|
||||
Empty state: "Nothing hidden yet." (voice rule)
|
||||
|
||||
### `/admin/quarantine` (admin)
|
||||
|
||||
`AdminSidebar.svelte` — promote Quarantine from `placeholder: true` to a real link.
|
||||
|
||||
Page header: H2 "Quarantine" + count pill in `accent-tint` when `report_count > 0`.
|
||||
|
||||
Aggregated row (one per track):
|
||||
- 56px album art (left).
|
||||
- Title (Parchment) · meta: "artist · album · 3 reports — latest 4h ago".
|
||||
- **Reason distribution row**: pills with `<count>× <reason>` (`2× bad_rip`, `1× wrong_tags`), accent-tint background.
|
||||
- **Reports details** (collapsed by default): expand caret reveals per-user list with `username · reason · notes · 2h ago`.
|
||||
- **Inline play button** (Lucide `Play`, accent-colored — qualifies as a brand moment per the design system Hybrid rule): plays via the existing `enqueueTrack` action so admin can verify the issue.
|
||||
- Action cluster (right-aligned):
|
||||
- Resolve — Pewter ghost + Lucide `RotateCcw` icon.
|
||||
- Delete file — Bronze + Lucide `Trash2` icon. Modal-confirm.
|
||||
- Delete via Lidarr — Oxblood + Lucide `Trash2` + `Cloud` icon. Typed-confirm modal ("DELETE", trimmed equality, matching the M5a Disconnect pattern).
|
||||
|
||||
Modal-confirm copy for Delete file: "Remove `<track title>` from disk and clear `<N>` reports? Lidarr may auto-redownload depending on its monitor settings." Cancel (Pewter ghost) · Delete (Bronze).
|
||||
|
||||
Typed-confirm copy for Delete via Lidarr: "This will tell Lidarr to remove `<album title>` (artist `<artist name>`) and add it to the import-list exclusion. Affects all `<N>` tracks on the album. Type `DELETE` to confirm." Cancel (Pewter ghost) · Delete (Oxblood, disabled until trimmed input equals "DELETE").
|
||||
|
||||
When Delete via Lidarr fails, the modal stays open with an inline error: "Couldn't reach Lidarr — try again, or check Settings → Integrations." Same understated-mythic register as the M5a toast.
|
||||
|
||||
When `lidarr_album_mbid` is missing on a row, the Delete-via-Lidarr button renders dimmed with title-attribute tooltip "Local-only track — no Lidarr album to remove." Defers operator confusion ("why doesn't this button work?") at minimal UX cost.
|
||||
|
||||
Empty state: "Nothing to triage right now." (voice rule)
|
||||
|
||||
## 7. Error handling
|
||||
|
||||
### Lidarr unreachable / auth-failed during admin actions
|
||||
|
||||
- `Client.LookupAlbumByMBID` and `Client.DeleteAlbum` return the same typed errors as M5a: `ErrUnreachable`, `ErrAuthFailed`, `ErrLookupFailed`, plus `ErrNotFound` for lookups.
|
||||
- Admin handler maps to the same HTTP codes M5a established (503 + JSON envelope).
|
||||
- Failure leaves Minstrel state untouched. The quarantine rows stay; admin retries. **No partial-state writes** — Minstrel deletion fires only after Lidarr DELETE confirms.
|
||||
|
||||
### Track has no `lidarr_album_mbid`
|
||||
|
||||
- Some legacy/imported tracks may have empty MBIDs. The Delete-via-Lidarr action 404s with `album_mbid_missing`. UI dims the button on those rows (see §6).
|
||||
|
||||
### File-deletion errors
|
||||
|
||||
- `library.DeleteTrackFile` failure (file already gone, permission error) returns the OS error wrapped. Admin handler maps to 500 with `file_delete_failed`. The quarantine row stays so admin can retry. **No partial state.**
|
||||
|
||||
### User-facing flag flow
|
||||
|
||||
- `POST /api/quarantine` is idempotent on (user_id, track_id) — re-flagging upserts. Errors: 400 `bad_reason`, 404 `track_not_found`. Optimistic SPA hide rolls back on failure with an error toast.
|
||||
|
||||
### Soft-hide query joins
|
||||
|
||||
- The `WHERE NOT EXISTS` filter is gated on user context. Subsonic clients (legacy API) skip the join entirely. If the join fails (DB error), the entire request fails — quarantine is an integrity boundary, not "best effort."
|
||||
|
||||
## 8. Testing
|
||||
|
||||
### Unit tests (no DB)
|
||||
|
||||
- `internal/lidarr/` — additions: table-driven tests for `LookupArtistByMBID`, `LookupAlbumByMBID`, `DeleteAlbum`. Covers happy + auth-fail + 5xx + 404 + bad-JSON for each, against canned fixtures in `internal/lidarr/testdata/`.
|
||||
|
||||
### Integration tests (gated on `MINSTREL_TEST_DATABASE_URL`)
|
||||
|
||||
- `internal/lidarrquarantine.Service`:
|
||||
- Flag (insert + upsert behaviors).
|
||||
- Unflag (cleanup).
|
||||
- ListMine.
|
||||
- ListAdminQueue — verify aggregation: 3 users × 1 track = 1 admin row with `report_count=3`, correct reason distribution.
|
||||
- Resolve — writes audit row, clears all per-user rows.
|
||||
- DeleteFile — track row gone, quarantine cleared, audit written.
|
||||
- DeleteViaLidarr — with stub Lidarr server: lookup returns album, DELETE called with both flags `true`, Minstrel rows for all album tracks removed, audit row written, `affected_users` count correct. Failure stub: nothing changes locally.
|
||||
|
||||
- Soft-hide enforcement — seed 2 users + 1 quarantined track on user A. Verify:
|
||||
- User A's `GET /api/albums/:id` excludes the track.
|
||||
- User B's includes it.
|
||||
- Admin's `/api/admin/quarantine` shows it aggregated.
|
||||
- Same shape for search and radio endpoints.
|
||||
|
||||
### HTTP tests (handler level)
|
||||
|
||||
- `internal/api/quarantine.go` — POST (with valid + invalid reasons), DELETE happy + not-found, GET mine.
|
||||
- `internal/api/admin_quarantine.go` — GET aggregated queue shape, POST resolve / delete-file / delete-via-lidarr (with stub Lidarr), non-admin 403 across the board, action-log GET.
|
||||
|
||||
### Frontend tests (vitest)
|
||||
|
||||
- `<TrackMenu>` — opens on click, closes on outside-click + Escape, single "Flag this track…" item present, takes track prop.
|
||||
- `<FlagPopover>` — submits with reason, optional notes; Cancel does not submit; pre-fills when re-flagging; "Update flag" button label when re-flagging.
|
||||
- `/library/hidden` page — renders user's quarantines, un-hide removes row.
|
||||
- `/admin/quarantine` page — aggregated rows render with reason distribution, expandable per-user list, Play button calls the player, Resolve/Delete-file/Delete-via-Lidarr each fire the correct API and remove the row on success. Modal-confirm gates Delete file. Typed-confirm gates Delete via Lidarr. Error states surface inline. Dimmed Delete-via-Lidarr button when MBID missing.
|
||||
|
||||
### Coverage targets
|
||||
|
||||
- `internal/lidarr/` (now extended) ≥ 80%.
|
||||
- `internal/lidarrquarantine/` ≥ 80%.
|
||||
- `internal/api/quarantine.go` + `internal/api/admin_quarantine.go` — handler coverage measured combined ≥ 70%.
|
||||
|
||||
## 9. Decisions ledger
|
||||
|
||||
| # | Decision | Rationale |
|
||||
|---|---|---|
|
||||
| 1 | Per-user soft-hide; admin sees aggregate | Fits the data-quality framing — user files complaint, admin treats as bug queue, but each user's hide stays personal until admin acts |
|
||||
| 2 | Hide everywhere except `/library/hidden` | Strongest hide; matches "I never want to see this until admin fixes it"; `/library/hidden` keeps un-hide reachable |
|
||||
| 3 | Subsonic API stays unfiltered | Per `project_subsonic_legacy` memory; new behavior is `/api/*`-only |
|
||||
| 4 | Track-level only (no album/artist quarantine) | Matches M5a §10 framing; album-level flag is rare enough to not need a dedicated affordance |
|
||||
| 5 | Admin actions: Resolve / Delete file / Delete via Lidarr | Three buckets covering false alarm, bad file, bad release. The §10 carve-out specifically named "delete via Lidarr" |
|
||||
| 6 | Always `deleteFiles=true + addImportListExclusion=true` on Lidarr DELETE | Otherwise "Delete via Lidarr" doesn't actually prevent the same broken release from re-downloading |
|
||||
| 7 | Audit log for admin actions, not for per-user flags | Per-user rows are conceptually transient (flag → unflag or resolve); admin destructive actions need recovery context |
|
||||
| 8 | Trigger via `<TrackMenu>` overflow (kebab), not standalone Flag button | Prevents miss-clicks against `<LikeButton>`; gives a home for future track actions |
|
||||
| 9 | Aggregated admin queue, not per-user feed | Admin acts on tracks, not on users; one row per track keeps the queue actionable |
|
||||
| 10 | Reason fixed enum + optional notes | Aggregation gets actionable buckets; notes give the long tail an escape hatch |
|
||||
| 11 | Inline Play button in admin queue is brand-moment accent | Per the design-system Hybrid rule — Minstrel-feature interaction (audition the issue) gets the accent |
|
||||
| 12 | Flag widget is a popover, not a full modal | Keeps track context visible while flagging; smaller commitment than the M5a Add modal |
|
||||
|
||||
## 10. Out of scope (this slice)
|
||||
|
||||
- Album / artist quarantine.
|
||||
- Bulk admin operations.
|
||||
- Auto-resolve thresholds.
|
||||
- User notifications when reports are acted on.
|
||||
- Subsonic API quarantine honoring.
|
||||
- "Delete via Lidarr" with finer-grained options (deleteFiles=false, exclusion=false). Always full-cascade in v1.
|
||||
|
||||
## 11. Open questions
|
||||
|
||||
- **Default position of `/library/hidden` in nav:** between Liked and Search? Or under a future Library submenu? — defer to plan time.
|
||||
- **Track without `lidarr_album_mbid`:** UI dims the button with tooltip (current proposal) vs. hides it entirely — confirm during plan time. Leaning toward dimmed-with-tooltip so the operator understands why the action isn't available.
|
||||
- **Reason "duplicate" follow-up:** the admin might want to know which other track is the duplicate. Out of scope here; potential M5b polish task or M5c overlap with similarity. — defer.
|
||||
Reference in New Issue
Block a user