feat: complete Task 1 - update database schema and model for live/remix filtering

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.

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")
+	}
+}

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)
 	}
 }