From ef3fffb3ea7bbf9e71f47ef587353fe5d1f5633f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 30 Apr 2026 19:42:19 -0400 Subject: [PATCH] fix(lidarrquarantine): keep LidarrAlbumMbid nil on Resolve/DeleteFile audit rows + cover idempotent Resolve The audit log treats lidarr_album_mbid as a 'this row triggered Lidarr' marker. Setting it on Resolve/DeleteFile (where Lidarr is never contacted) muddied that semantic. Reverting to plan: only DeleteViaLidarr writes the mbid. Adds an idempotency test for Resolve over a track with zero rows (audit row written, affected_users=0, mbid nil). --- internal/lidarrquarantine/service.go | 10 +++++--- internal/lidarrquarantine/service_test.go | 28 +++++++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/internal/lidarrquarantine/service.go b/internal/lidarrquarantine/service.go index 571f6b62..3a402342 100644 --- a/internal/lidarrquarantine/service.go +++ b/internal/lidarrquarantine/service.go @@ -205,10 +205,13 @@ func (s *Service) Resolve(ctx context.Context, trackID, adminID pgtype.UUID) (db if err := q.DeleteQuarantineForTrack(ctx, trackID); err != nil { return dbq.LidarrQuarantineAction{}, fmt.Errorf("delete rows: %w", err) } + // LidarrAlbumMbid stays nil here: Lidarr was not contacted. The audit + // log uses the field as a "this row triggered Lidarr" marker, so only + // DeleteViaLidarr sets it. return q.WriteQuarantineAction(ctx, dbq.WriteQuarantineActionParams{ TrackID: trackID, TrackTitle: snap.TrackTitle, ArtistName: snap.ArtistName, AlbumTitle: snap.AlbumTitle, Action: dbq.LidarrQuarantineActionKindResolved, - AdminID: adminID, LidarrAlbumMbid: snap.LidarrAlbumMBID, AffectedUsers: affected, + AdminID: adminID, LidarrAlbumMbid: nil, AffectedUsers: affected, }) } @@ -269,11 +272,12 @@ func (s *Service) DeleteFile(ctx context.Context, trackID, adminID pgtype.UUID) } // tracks row is gone; ON DELETE CASCADE on lidarr_quarantine.track_id // already cleared the per-user rows. We don't call - // DeleteQuarantineForTrack separately. + // DeleteQuarantineForTrack separately. LidarrAlbumMbid stays nil — + // Lidarr was not contacted. return q.WriteQuarantineAction(ctx, dbq.WriteQuarantineActionParams{ TrackID: trackID, TrackTitle: snap.TrackTitle, ArtistName: snap.ArtistName, AlbumTitle: snap.AlbumTitle, Action: dbq.LidarrQuarantineActionKindDeletedFile, - AdminID: adminID, LidarrAlbumMbid: snap.LidarrAlbumMBID, AffectedUsers: affected, + AdminID: adminID, LidarrAlbumMbid: nil, AffectedUsers: affected, }) } diff --git a/internal/lidarrquarantine/service_test.go b/internal/lidarrquarantine/service_test.go index 903aac86..a0708765 100644 --- a/internal/lidarrquarantine/service_test.go +++ b/internal/lidarrquarantine/service_test.go @@ -253,12 +253,37 @@ func TestResolve_ClearsRowsAndWritesAudit(t *testing.T) { if audit.Action != dbq.LidarrQuarantineActionKindResolved { t.Errorf("action = %v, want resolved", audit.Action) } + if audit.LidarrAlbumMbid != nil { + t.Errorf("lidarr_album_mbid = %v, want nil for resolve", audit.LidarrAlbumMbid) + } n, _ := dbq.New(pool).CountQuarantineForTrack(context.Background(), track.ID) if n != 0 { t.Errorf("rows after resolve = %d, want 0", n) } } +func TestResolve_NoExistingRowsStillWritesAudit(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + track, _, _ := seedTrack(t, pool, "T", "x") + + svc := NewService(pool, lidarrconfig.New(pool), nil) + // No flags applied — resolve a track with zero quarantine rows. + audit, err := svc.Resolve(context.Background(), track.ID, user.ID) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if audit.AffectedUsers != 0 { + t.Errorf("affected_users = %d, want 0", audit.AffectedUsers) + } + if audit.Action != dbq.LidarrQuarantineActionKindResolved { + t.Errorf("action = %v, want resolved", audit.Action) + } + if audit.LidarrAlbumMbid != nil { + t.Errorf("lidarr_album_mbid = %v, want nil for resolve", audit.LidarrAlbumMbid) + } +} + func TestResolve_TrackNotFound(t *testing.T) { pool := newPool(t) user := seedUser(t, pool, "alice") @@ -305,6 +330,9 @@ func TestDeleteFile_RemovesFileAndAuditsAffected(t *testing.T) { if audit.Action != dbq.LidarrQuarantineActionKindDeletedFile { t.Errorf("action = %v, want deleted_file", audit.Action) } + if audit.LidarrAlbumMbid != nil { + t.Errorf("lidarr_album_mbid = %v, want nil for delete_file", audit.LidarrAlbumMbid) + } if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) { t.Errorf("file still exists: %v", err) }