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).
This commit is contained in:
@@ -205,10 +205,13 @@ func (s *Service) Resolve(ctx context.Context, trackID, adminID pgtype.UUID) (db
|
|||||||
if err := q.DeleteQuarantineForTrack(ctx, trackID); err != nil {
|
if err := q.DeleteQuarantineForTrack(ctx, trackID); err != nil {
|
||||||
return dbq.LidarrQuarantineAction{}, fmt.Errorf("delete rows: %w", err)
|
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{
|
return q.WriteQuarantineAction(ctx, dbq.WriteQuarantineActionParams{
|
||||||
TrackID: trackID, TrackTitle: snap.TrackTitle, ArtistName: snap.ArtistName,
|
TrackID: trackID, TrackTitle: snap.TrackTitle, ArtistName: snap.ArtistName,
|
||||||
AlbumTitle: snap.AlbumTitle, Action: dbq.LidarrQuarantineActionKindResolved,
|
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
|
// tracks row is gone; ON DELETE CASCADE on lidarr_quarantine.track_id
|
||||||
// already cleared the per-user rows. We don't call
|
// 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{
|
return q.WriteQuarantineAction(ctx, dbq.WriteQuarantineActionParams{
|
||||||
TrackID: trackID, TrackTitle: snap.TrackTitle, ArtistName: snap.ArtistName,
|
TrackID: trackID, TrackTitle: snap.TrackTitle, ArtistName: snap.ArtistName,
|
||||||
AlbumTitle: snap.AlbumTitle, Action: dbq.LidarrQuarantineActionKindDeletedFile,
|
AlbumTitle: snap.AlbumTitle, Action: dbq.LidarrQuarantineActionKindDeletedFile,
|
||||||
AdminID: adminID, LidarrAlbumMbid: snap.LidarrAlbumMBID, AffectedUsers: affected,
|
AdminID: adminID, LidarrAlbumMbid: nil, AffectedUsers: affected,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -253,12 +253,37 @@ func TestResolve_ClearsRowsAndWritesAudit(t *testing.T) {
|
|||||||
if audit.Action != dbq.LidarrQuarantineActionKindResolved {
|
if audit.Action != dbq.LidarrQuarantineActionKindResolved {
|
||||||
t.Errorf("action = %v, want resolved", audit.Action)
|
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)
|
n, _ := dbq.New(pool).CountQuarantineForTrack(context.Background(), track.ID)
|
||||||
if n != 0 {
|
if n != 0 {
|
||||||
t.Errorf("rows after resolve = %d, want 0", n)
|
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) {
|
func TestResolve_TrackNotFound(t *testing.T) {
|
||||||
pool := newPool(t)
|
pool := newPool(t)
|
||||||
user := seedUser(t, pool, "alice")
|
user := seedUser(t, pool, "alice")
|
||||||
@@ -305,6 +330,9 @@ func TestDeleteFile_RemovesFileAndAuditsAffected(t *testing.T) {
|
|||||||
if audit.Action != dbq.LidarrQuarantineActionKindDeletedFile {
|
if audit.Action != dbq.LidarrQuarantineActionKindDeletedFile {
|
||||||
t.Errorf("action = %v, want deleted_file", audit.Action)
|
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) {
|
if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) {
|
||||||
t.Errorf("file still exists: %v", err)
|
t.Errorf("file still exists: %v", err)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user