From 9da12262a6672b058861f7ea8edad328fd385ce4 Mon Sep 17 00:00:00 2001 From: zerovox <933064+zerovox@users.noreply.github.com> Date: Sun, 16 Aug 2026 21:00:18 -0600 Subject: [PATCH] fix: only move album annotations if moving a track would empty the old album --- core/maintenance.go | 17 ++++++++++++----- core/maintenance_test.go | 16 +++++++++++++++- scanner/phase_2_missing_tracks_test.go | 6 +++--- tests/mock_mediafile_repo.go | 19 ++++++++++++------- 4 files changed, 42 insertions(+), 16 deletions(-) diff --git a/core/maintenance.go b/core/maintenance.go index b31d6c0c2..5994fb83a 100644 --- a/core/maintenance.go +++ b/core/maintenance.go @@ -93,13 +93,20 @@ func (s *maintenanceService) RemapMissingFile(ctx context.Context, missingID, ta return fmt.Errorf("delete discarded track: %w", err) } - // Reassign album annotations (starred, rating) and preserve album created_at on album change if oldAlbumID != newAlbumID { - if err := tx.Album(ctx).ReassignAnnotation(oldAlbumID, newAlbumID); err != nil { - log.Warn(ctx, "Could not reassign album annotations", "from", oldAlbumID, "to", newAlbumID, err) + // Reassign album annotations (starred, rating) if the old album is now empty + oldAlbumTracks, err := tx.MediaFile(ctx).CountAll(model.QueryOptions{Filters: squirrel.Eq{"album_id": oldAlbumID}}) + if err != nil { + return fmt.Errorf("get old album tracks: %w", err) } - if err := tx.Album(ctx).CopyAttributes(oldAlbumID, newAlbumID, "created_at"); err != nil && !errors.Is(err, model.ErrNotFound) { - log.Warn(ctx, "Could not copy album created_at", "from", oldAlbumID, "to", newAlbumID, err) + if oldAlbumTracks == 0 { + if err := tx.Album(ctx).ReassignAnnotation(oldAlbumID, newAlbumID); err != nil { + return fmt.Errorf("reassign album annotations: %w", err) + } + // Copy across the create_at timestamp from the old album + if err := tx.Album(ctx).CopyAttributes(oldAlbumID, newAlbumID, "created_at"); err != nil && !errors.Is(err, model.ErrNotFound) { + return fmt.Errorf("copy album attributes: %w", err) + } } } return nil diff --git a/core/maintenance_test.go b/core/maintenance_test.go index dfb8bf8af..9515e1963 100644 --- a/core/maintenance_test.go +++ b/core/maintenance_test.go @@ -272,18 +272,32 @@ var _ = Describe("Maintenance", func() { Expect(ds.GCCalled).To(BeTrue()) }) - It("reassigns album annotations when the album changes", func() { + It("reassigns album annotations when the old album is emptied", func() { albumRepo := ds.MockedAlbum.(*extendedAlbumRepo) mfRepo.SetData(model.MediaFiles{ {ID: "m1", AlbumID: "album1", Missing: true}, {ID: "t1", AlbumID: "album2", Missing: false}, }) + mfRepo.SetCountAll(0) Expect(service.RemapMissingFile(ctx, "m1", "t1")).To(Succeed()) Expect(albumRepo.ReassignAnnotationCalls).To(HaveKeyWithValue("album1", "album2")) }) + It("does not reassign album annotations when the old album is not emptied", func() { + albumRepo := ds.MockedAlbum.(*extendedAlbumRepo) + mfRepo.SetData(model.MediaFiles{ + {ID: "m1", AlbumID: "album1", Missing: true}, + {ID: "m2", AlbumID: "album1", Missing: false}, + {ID: "t1", AlbumID: "album2", Missing: false}, + }) + + Expect(service.RemapMissingFile(ctx, "m1", "t1")).To(Succeed()) + + Expect(albumRepo.ReassignAnnotationCalls).To(BeEmpty()) + }) + It("does not reassign annotations when the album is unchanged", func() { albumRepo := ds.MockedAlbum.(*extendedAlbumRepo) mfRepo.SetData(model.MediaFiles{ diff --git a/scanner/phase_2_missing_tracks_test.go b/scanner/phase_2_missing_tracks_test.go index d54ceee40..f93b166c1 100644 --- a/scanner/phase_2_missing_tracks_test.go +++ b/scanner/phase_2_missing_tracks_test.go @@ -311,7 +311,7 @@ var _ = Describe("phaseMissingTracks", func() { When("PurgeMissing is 'always'", func() { BeforeEach(func() { conf.Server.Scanner.PurgeMissing = consts.PurgeMissingAlways - mr.CountAllValue = 3 + mr.SetCountAll(3) mr.DeleteAllMissingValue = 3 }) It("should purge missing files", func() { @@ -325,7 +325,7 @@ var _ = Describe("phaseMissingTracks", func() { When("PurgeMissing is 'full'", func() { BeforeEach(func() { conf.Server.Scanner.PurgeMissing = consts.PurgeMissingFull - mr.CountAllValue = 2 + mr.SetCountAll(2) mr.DeleteAllMissingValue = 2 }) It("should not purge missing files if not a full scan", func() { @@ -346,7 +346,7 @@ var _ = Describe("phaseMissingTracks", func() { When("PurgeMissing is 'never'", func() { BeforeEach(func() { conf.Server.Scanner.PurgeMissing = consts.PurgeMissingNever - mr.CountAllValue = 1 + mr.SetCountAll(1) mr.DeleteAllMissingValue = 1 }) It("should not purge missing files", func() { diff --git a/tests/mock_mediafile_repo.go b/tests/mock_mediafile_repo.go index 58e84785b..d4b444655 100644 --- a/tests/mock_mediafile_repo.go +++ b/tests/mock_mediafile_repo.go @@ -24,8 +24,9 @@ type MockMediaFileRepo struct { model.MediaFileRepository Data map[string]*model.MediaFile Err bool - // Add fields and methods for controlling CountAll and DeleteAllMissing in tests - CountAllValue int64 + // Add fields and methods for controlling CountAll and DeleteAllMissing in tests. + // A nil CountAllValue is unset, and CountAll falls back to counting rows in Data. + CountAllValue *int64 CountAllOptions model.QueryOptions DeleteAllMissingValue int64 Options model.QueryOptions @@ -40,6 +41,10 @@ func (m *MockMediaFileRepo) SetError(err bool) { m.Err = err } +func (m *MockMediaFileRepo) SetCountAll(count int64) { + m.CountAllValue = &count +} + func (m *MockMediaFileRepo) SetData(mfs model.MediaFiles) { m.Data = make(map[string]*model.MediaFile) for i, mf := range mfs { @@ -259,11 +264,11 @@ func (m *MockMediaFileRepo) CountAll(opts ...model.QueryOptions) (int64, error) if m.Err { return 0, errors.New("error") } - if m.CountAllValue != 0 { - if len(opts) > 0 { - m.CountAllOptions = opts[0] - } - return m.CountAllValue, nil + if len(opts) > 0 { + m.CountAllOptions = opts[0] + } + if m.CountAllValue != nil { + return *m.CountAllValue, nil } return int64(len(m.Data)), nil }