diff --git a/docs/plans/2026-07-30-live-remix-filtering.md b/docs/plans/2026-07-30-live-remix-filtering.md new file mode 100644 index 0000000..98c24f3 --- /dev/null +++ b/docs/plans/2026-07-30-live-remix-filtering.md @@ -0,0 +1,80 @@ +# Implement Live/Remix Filtering + +## Overview +- Implement support for "Live" and "Remix" secondary type filtering for artist discographies. +- Problem it solves: Users currently cannot filter out Live or Remix albums/singles, which can clutter the dashboard. +- Key benefits: Improved user experience and cleaner discography views. + +## Context (from discovery) +- Files/components involved: + - `internal/database/database.go` (ArtistSettings struct) + - `internal/musicbrainz/filter.go` (FilterOptions, ApplyTypeToggles) + - `internal/web/handlers.go` (Artist detail view) + - `internal/web/templates/artist.html` +- Related patterns found: Follows the existing pattern for `ignore_singles` and `ignore_compilations`. +- Dependencies identified: `database` package, `musicbrainz` package, `web` package. + +## Development Approach +- **Testing approach**: TDD (tests first) +- Complete each task fully before moving to the next +- Make small, focused changes +- **CRITICAL: every task MUST include new/updated tests** for code changes in that task +- **CRITICAL: all tests must pass before starting next task** - no exceptions +- **CRITICAL: update this plan file when scope changes during implementation** +- Run tests after each change +- Maintain backward compatibility + +## Testing Strategy +- **Unit tests**: required for every task (see Development Approach above) +- **E2E tests**: None required for this scope. + +## Progress Tracking +- Mark completed items with `[x]` immediately when done +- Add newly discovered tasks with ➕ prefix +- Document issues/blockers with ⚠️ prefix +- Update plan if implementation deviates from original scope +- Keep plan in sync with actual work done + +## What Goes Where +- **Implementation Steps** (`[ ]` checkboxes): code changes, tests, documentation updates +- **Post-Completion** (no checkboxes): manual testing of the scanner and web UI + +## Implementation Steps + +### Task 1: Update Database Schema and Model +- [x] Update `ArtistSettings` struct in `internal/database/database.go` to include `IgnoreLive` and `IgnoreRemix` fields +- [x] Create a migration or manual SQL script to add `ignore_live` and `ignore_remix` columns to `artist_settings` table +- [x] Update `SaveArtistSettings` and `UpdateArtistSettings` to handle the new fields +- [x] write unit tests for `ArtistSettings` struct and database operations +- [x] run project tests - must pass before next task + +### Task 2: Update Filtering Core Logic +- [ ] Update `FilterOptions` struct in `internal/musicbrainz/filter.go` to include `IgnoreLive` and `IgnoreRemix` +- [ ] Update `ApplyTypeToggles` in `internal/musicbrainz/filter.go` to include logic for "Live" and "Remix" types +- [ ] write unit tests for `ApplyTypeToggles` covering all four toggle types (Single, Compilation, Live, Remix) +- [ ] run project tests - must pass before next task + +### Task 3: Update Web UI and Handlers +- [ ] Update `ArtistData` or similar view models to include the new filter booleans +- [ ] Update `internal/web/handlers.go` to handle the new toggle POST requests +- [ ] Update `internal/web/templates/artist.html` to show new toggles for Live and Remix +- [ ] write tests for new web handlers +- [ ] run project tests - must pass before next task + +### Task 4: Verify and Document +- [ ] Verify the scanner correctly suppresses "Live" and "Remix" types when toggles are enabled +- [ ] Verify the Web UI correctly updates the database on toggle change +- [ ] Update `CLAUDE.md` or other docs if new patterns were discovered +- [ ] run full test suite +- [ ] verify no breaking changes were introduced + +## Technical Details +- **Database**: `ignore_live` (boolean, default false), `ignore_remix` (boolean, default false) +- **Filtering**: "Live" and "Remix" will be checked in both primary `Type` and `SecondaryTypes` slices. +- **Web**: New endpoints will mirror existing `/ignore-singles` logic. + +## Post-Completion +**Manual verification**: +- Verify that toggling "Live" in the Web UI actually removes "Live" results from the scanner output. +- Verify that toggling "Remix" in the Web UI actually removes "Remix" results from the scanner output. +- Verify that these filters do not affect "Single" or "Compilation" filtering. diff --git a/internal/database/artist_settings_test.go b/internal/database/artist_settings_test.go index 649dbab..350d2c5 100644 --- a/internal/database/artist_settings_test.go +++ b/internal/database/artist_settings_test.go @@ -592,3 +592,196 @@ func TestSaveArtistSettings_LastSyncedUpdated(t *testing.T) { t.Errorf("expected last_synced to be updated to %v, got %v", newTime, got.LastSynced) } } + +// TestGetArtistSettings_FoundWithNewFields verifies retrieving an existing artist with new fields. +func TestGetArtistSettings_FoundWithNewFields(t *testing.T) { + db, err := New(":memory:") + if err != nil { + t.Fatalf("New() error: %v", err) + } + defer db.Close() + + // Insert a row with all fields including new ones + _, err = db.Conn().Exec( + "INSERT INTO artist_settings (id, name, mbid, ignore_singles, ignore_compilations, ignore_live, ignore_remix, monitored, last_synced) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)", + "artist-1", "Test Artist", "", true, false, true, false, true, time.Now(), + ) + if err != nil { + t.Fatalf("insert: %v", err) + } + + s, err := GetArtistSettings(db, "artist-1") + if err != nil { + t.Fatalf("GetArtistSettings() error: %v", err) + } + + if s.ID != "artist-1" { + t.Errorf("expected ID 'artist-1', got %q", s.ID) + } + if s.Name != "Test Artist" { + t.Errorf("expected Name 'Test Artist', got %q", s.Name) + } + if !s.IgnoreSingles { + t.Error("expected IgnoreSingles true") + } + if s.IgnoreCompilations { + t.Error("expected IgnoreCompilations false") + } + if !s.IgnoreLive { + t.Error("expected IgnoreLive true") + } + if s.IgnoreRemix { + t.Error("expected IgnoreRemix false") + } + if !s.Monitored { + t.Error("expected Monitored true") + } +} + +// TestSaveArtistSettings_UpdateNewFields verifies that SaveArtistSettings works with new fields. +func TestSaveArtistSettings_UpdateNewFields(t *testing.T) { + db, err := New(":memory:") + if err != nil { + t.Fatalf("New() error: %v", err) + } + defer db.Close() + + // Insert initial row + s1 := &ArtistSettings{ + ID: "artist-1", + Name: "Original Name", + IgnoreSingles: false, + IgnoreCompilations: false, + IgnoreLive: false, + IgnoreRemix: false, + Monitored: true, + } + if err := SaveArtistSettings(db, s1); err != nil { + t.Fatalf("first SaveArtistSettings() error: %v", err) + } + + // Update with new fields + s2 := &ArtistSettings{ + ID: "artist-1", + Name: "Updated Name", + IgnoreSingles: true, + IgnoreCompilations: true, + IgnoreLive: true, + IgnoreRemix: true, + Monitored: false, + } + if err := SaveArtistSettings(db, s2); err != nil { + t.Fatalf("second SaveArtistSettings() error: %v", err) + } + + got, err := GetArtistSettings(db, "artist-1") + if err != nil { + t.Fatalf("GetArtistSettings() error: %v", err) + } + if got.Name != "Updated Name" { + t.Errorf("expected Name 'Updated Name', got %q", got.Name) + } + if !got.IgnoreSingles { + t.Error("expected IgnoreSingles true") + } + if !got.IgnoreCompilations { + t.Error("expected IgnoreCompilations true") + } + if !got.IgnoreLive { + t.Error("expected IgnoreLive true") + } + if !got.IgnoreRemix { + t.Error("expected IgnoreRemix false") + } + if got.Monitored { + t.Error("expected Monitored false") + } +} + +// TestUpdateArtistSettings_NewFields verifies updating the new fields works. +func TestUpdateArtistSettings_NewFields(t *testing.T) { + db, err := New(":memory:") + if err != nil { + t.Fatalf("New() error: %v", err) + } + defer db.Close() + + // Insert initial row + s := &ArtistSettings{ + ID: "artist-1", + Name: "Original", + IgnoreSingles: false, + IgnoreCompilations: false, + IgnoreLive: false, + IgnoreRemix: false, + Monitored: true, + } + if err := SaveArtistSettings(db, s); err != nil { + t.Fatalf("SaveArtistSettings() error: %v", err) + } + + // Update only new fields + updates := map[string]interface{}{ + "ignore_live": true, + "ignore_remix": true, + } + if err := UpdateArtistSettings(db, "artist-1", updates); err != nil { + t.Fatalf("UpdateArtistSettings() error: %v", err) + } + + got, err := GetArtistSettings(db, "artist-1") + if err != nil { + t.Fatalf("GetArtistSettings() error: %v", err) + } + if !got.IgnoreLive { + t.Error("expected IgnoreLive true") + } + if !got.IgnoreRemix { + t.Error("expected IgnoreRemix true") + } + // Unchanged fields should remain + if got.IgnoreSingles != false { + t.Error("expected IgnoreSingles unchanged (false)") + } + if got.IgnoreCompilations != false { + t.Error("expected IgnoreCompilations unchanged (false)") + } + if !got.Monitored { + t.Error("expected Monitored unchanged (true)") + } +} + +// TestUpdateArtistSettings_NewFieldsNotFound verifies updating a nonexistent artist returns error. +func TestUpdateArtistSettings_NewFieldsNotFound(t *testing.T) { + db, err := New(":memory:") + if err != nil { + t.Fatalf("New() error: %v", err) + } + defer db.Close() + + updates := map[string]interface{}{"ignore_live": true} + err = UpdateArtistSettings(db, "nonexistent", updates) + if err == nil { + t.Error("expected error for nonexistent artist, got nil") + } +} + +// TestUpdateArtistSettings_InvalidColumnNewFields verifies unknown columns are rejected. +func TestUpdateArtistSettings_InvalidColumnNewFields(t *testing.T) { + db, err := New(":memory:") + if err != nil { + t.Fatalf("New() error: %v", err) + } + defer db.Close() + + s := &ArtistSettings{ID: "artist-1", Name: "Test"} + if err := SaveArtistSettings(db, s); err != nil { + t.Fatalf("SaveArtistSettings() error: %v", err) + } + + updates := map[string]interface{}{"invalid_col": "value"} + err = UpdateArtistSettings(db, "artist-1", updates) + if err == nil { + t.Error("expected error for invalid column, got nil") + } +} diff --git a/internal/database/database_test.go b/internal/database/database_test.go index 4a1bcc6..2f48d6b 100644 --- a/internal/database/database_test.go +++ b/internal/database/database_test.go @@ -31,7 +31,7 @@ func TestNew_InitializationAndSchema(t *testing.T) { } } -// TestNew_MigrationIdempency verifies that calling New() twice (via migrate) does not fail. +// TestNew_MigrationIdempotency verifies that calling New() twice (via migrate) does not fail. func TestNew_MigrationIdempotency(t *testing.T) { db, err := New(":memory:") if err != nil { @@ -89,26 +89,27 @@ func TestArtistSettingsSchema(t *testing.T) { // Insert a row to verify column names and types. _, err = db.Conn().Exec( - "INSERT INTO artist_settings (id, name, ignore_singles, ignore_compilations, monitored) VALUES (?, ?, ?, ?, ?)", - "artist-1", "Test Artist", true, false, true, + "INSERT INTO artist_settings (id, name, ignore_singles, ignore_compilations, ignore_live, ignore_remix, monitored) VALUES (?, ?, ?, ?, ?, ?, ?)", + "artist-1", "Test Artist", true, false, false, false, true, ) if err != nil { t.Fatalf("insert into artist_settings: %v", err) } var id, name string + var ignoreLive, ignoreRemix bool var ignoreSingles, ignoreCompilations, monitored bool err = db.Conn().QueryRow( - "SELECT id, name, ignore_singles, ignore_compilations, monitored FROM artist_settings WHERE id = ?", + "SELECT id, name, ignore_singles, ignore_compilations, ignore_live, ignore_remix, monitored FROM artist_settings WHERE id = ?", "artist-1", - ).Scan(&id, &name, &ignoreSingles, &ignoreCompilations, &monitored) + ).Scan(&id, &name, &ignoreSingles, &ignoreCompilations, &ignoreLive, &ignoreRemix, &monitored) if err != nil { t.Fatalf("select from artist_settings: %v", err) } - if id != "artist-1" || name != "Test Artist" || !ignoreSingles || ignoreCompilations || !monitored { - t.Errorf("unexpected row values: id=%q name=%q ignoreSingles=%v ignoreCompilations=%v monitored=%v", - id, name, ignoreSingles, ignoreCompilations, monitored) + if id != "artist-1" || name != "Test Artist" || !ignoreSingles || ignoreCompilations || ignoreLive || ignoreRemix || !monitored { + t.Errorf("unexpected row values: id=%q name=%q ignoreSingles=%v ignoreCompilations=%v ignoreLive=%v ignoreRemix=%v monitored=%v", + id, name, ignoreSingles, ignoreCompilations, ignoreLive, ignoreRemix, monitored) } } @@ -205,11 +206,8 @@ func TestMigrationTracking(t *testing.T) { t.Fatalf("query migrations count: %v", err) } - // We have 9 recorded migrations: artist_settings, external_releases, - // local_albums, notifications_sent, cached_at column, secondary_types - // column, the external_releases.artist_id index, the artist_settings mbid - // column, and the artist_settings last_synced column. - if count != 9 { - t.Errorf("expected 9 applied migrations, got %d", count) + // We now have 10 recorded migrations. + if count != 10 { + t.Errorf("expected 10 applied migrations, got %d", count) } }