From a4c426f640b566c06f03f5ca133353c8c1c1aaa2 Mon Sep 17 00:00:00 2001 From: Vladimir Zagainov Date: Sun, 19 Jul 2026 18:59:38 +0300 Subject: [PATCH] fix: address code review findings --- internal/musicbrainz/sync.go | 27 +++++++++++++++++++++++---- internal/normalize/normalize.go | 9 +++++++-- internal/normalize/normalize_test.go | 3 +++ 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/internal/musicbrainz/sync.go b/internal/musicbrainz/sync.go index 60270c1..b25b969 100644 --- a/internal/musicbrainz/sync.go +++ b/internal/musicbrainz/sync.go @@ -44,12 +44,30 @@ func SyncArtistDiscography( return nil, fmt.Errorf("sync artist discography: cache check failed: %w", err) } - // Step 2: If we have cached data, return it. + // Step 2: If we have cached data, return it. Re-apply the per-artist type + // toggles even on a cache hit so user changes to ignore_singles / + // ignore_compilations take effect without waiting for cache expiry. + // (Status/type inclusion was already applied when the rows were first + // synced and stored, so only the toggles can change.) if len(cachedReleases) > 0 { if err := ctx.Err(); err != nil { return nil, fmt.Errorf("sync artist discography: %w", err) } - return cachedReleases, nil + opts, err := getArtistFilterOptions(db, artistID) + if err != nil { + return nil, fmt.Errorf("sync artist discography: read artist filter options: %w", err) + } + filtered := cachedReleases[:0] + for _, r := range cachedReleases { + if opts.IgnoreSingles && r.Type == "Single" { + continue + } + if opts.IgnoreCompilations && r.Type == "Compilation" { + continue + } + filtered = append(filtered, r) + } + return filtered, nil } // Step 3: Cache miss — fetch from MusicBrainz API. @@ -137,11 +155,12 @@ func SyncArtistDiscography( // getArtistFilterOptions reads per-artist type filtering preferences. // Defaults to no filtering if artist_settings row doesn't exist. -func getArtistFilterOptions(db *database.DB, artistMBID string) (FilterOptions, error) { +// artistID is the Navidrome artist ID (artist_settings.id), not the MusicBrainz ID. +func getArtistFilterOptions(db *database.DB, artistID string) (FilterOptions, error) { var opts FilterOptions err := db.Conn().QueryRow( "SELECT COALESCE(ignore_singles, 0), COALESCE(ignore_compilations, 0) FROM artist_settings WHERE id = ?", - artistMBID, + artistID, ).Scan(&opts.IgnoreSingles, &opts.IgnoreCompilations) if err == sql.ErrNoRows { return opts, nil diff --git a/internal/normalize/normalize.go b/internal/normalize/normalize.go index 0b9b1cc..18fdb6f 100644 --- a/internal/normalize/normalize.go +++ b/internal/normalize/normalize.go @@ -37,8 +37,13 @@ func NormalizeString(s string) string { // Remove parenthesized content (e.g., (Deluxe), (Remastered)) s = parenRe.ReplaceAllString(s, "") - // Remove years (4-digit numbers between 1000-2999) - s = yearRe.ReplaceAllString(s, "") + // Remove years (4-digit numbers between 1000-2999). If stripping the + // year would empty the entire string (e.g. an album literally titled + // "1989" or "2112"), keep the original form so the title can still match. + stripped := yearRe.ReplaceAllString(s, "") + if strings.TrimSpace(stripped) != "" { + s = stripped + } // Replace common separators with spaces before stripping other special chars s = strings.ReplaceAll(s, "-", " ") diff --git a/internal/normalize/normalize_test.go b/internal/normalize/normalize_test.go index 3cf7089..02992a8 100644 --- a/internal/normalize/normalize_test.go +++ b/internal/normalize/normalize_test.go @@ -36,6 +36,9 @@ func TestNormalizeString_Basic(t *testing.T) { // Digits that are not years should stay {"30 Seconds to Mars", "30 seconds to mars"}, {"1941 - The Greatest Hits", "the greatest hits"}, + // Year-only title is preserved (not collapsed to empty) so it can still match + {"1989", "1989"}, + {"2112", "2112"}, } for _, tt := range tests {