fix(tracks): removing a track writes a track_remove audit row (#6294)
release / govulncheck (push) Successful in 17s
release / web (push) Successful in 1m21s
release / go (push) Successful in 1m39s
release / integration (push) Successful in 5m13s
release / android (push) Successful in 5m19s
release / Build signed APK (releases and dev) (push) Successful in 5m32s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 16s
release / Verify release artifacts (tag releases only) (push) Skipped
release / govulncheck (push) Successful in 17s
release / web (push) Successful in 1m21s
release / go (push) Successful in 1m39s
release / integration (push) Successful in 5m13s
release / android (push) Successful in 5m19s
release / Build signed APK (releases and dev) (push) Successful in 5m32s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 16s
release / Verify release artifacts (tag releases only) (push) Skipped
RemoveTrack took the admin's id and dropped it, so a merge left an audit trail and the more destructive removal did not. It now reads the track's title, artist, album and file path before the delete and writes them, with whether the album and artist went too, under the new track_remove action. The audit action groups lose their plan-era U1/U2/U3 headings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -13,11 +13,13 @@ import (
|
||||
"fmt"
|
||||
"log/slog"
|
||||
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
"github.com/jackc/pgx/v5/pgxpool"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/audit"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/library"
|
||||
syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync"
|
||||
)
|
||||
@@ -57,11 +59,32 @@ func NewService(pool *pgxpool.Pool, logger *slog.Logger, dataDir string) *Servic
|
||||
// file cannot be removed it is a *library.FileRemoveError and NOTHING was
|
||||
// deleted — see library.DeleteTrackFile for why that order is the contract.
|
||||
//
|
||||
// adminID is accepted for the audit log but not yet written to it.
|
||||
// A removal is written to the audit log with adminID as the actor. The
|
||||
// track's names are read before the delete, since the row, and perhaps its
|
||||
// album and artist, are gone after it.
|
||||
func (s *Service) RemoveTrack(
|
||||
ctx context.Context,
|
||||
trackID, adminID pgtype.UUID, //nolint:revive // adminID reserved for audit-log wiring in a follow-up
|
||||
ctx context.Context, trackID, adminID pgtype.UUID,
|
||||
) (deletedAlbumID *pgtype.UUID, deletedArtistID *pgtype.UUID, err error) {
|
||||
q := dbq.New(s.pool)
|
||||
track, err := q.GetTrackByID(ctx, trackID)
|
||||
if err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
return nil, nil, ErrNotFound
|
||||
}
|
||||
return nil, nil, fmt.Errorf("get track: %w", err)
|
||||
}
|
||||
meta := map[string]any{
|
||||
"track_id": syncpkg.FormatUUID(track.ID),
|
||||
"title": track.Title,
|
||||
"file_path": track.FilePath,
|
||||
}
|
||||
if artist, err := q.GetArtistByID(ctx, track.ArtistID); err == nil {
|
||||
meta["artist"] = artist.Name
|
||||
}
|
||||
if album, err := q.GetAlbumByID(ctx, track.AlbumID); err == nil {
|
||||
meta["album"] = album.Title
|
||||
}
|
||||
|
||||
deleted, err := library.DeleteTrackFile(ctx, s.pool, s.logger, s.dataDir, trackID)
|
||||
if err != nil {
|
||||
if errors.Is(err, library.ErrTrackNotFound) {
|
||||
@@ -69,6 +92,9 @@ func (s *Service) RemoveTrack(
|
||||
}
|
||||
return nil, nil, fmt.Errorf("delete track: %w", err)
|
||||
}
|
||||
meta["album_deleted"] = deleted.AlbumID != nil
|
||||
meta["artist_deleted"] = deleted.ArtistID != nil
|
||||
audit.WriteOrLog(ctx, s.pool, s.logger, adminID, pgtype.UUID{}, audit.ActionTrackRemove, meta)
|
||||
return deleted.AlbumID, deleted.ArtistID, nil
|
||||
}
|
||||
|
||||
|
||||
@@ -137,6 +137,47 @@ func TestRemoveTrack_LocalFile_HappyPath(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestRemoveTrack_WritesAuditRow(t *testing.T) {
|
||||
pool := newPool(t)
|
||||
admin := seedAdmin(t, pool, "alice")
|
||||
if _, err := pool.Exec(context.Background(), `DELETE FROM audit_log`); err != nil {
|
||||
t.Fatalf("reset audit_log: %v", err)
|
||||
}
|
||||
|
||||
dir := t.TempDir()
|
||||
path := filepath.Join(dir, "track.mp3")
|
||||
if err := os.WriteFile(path, []byte("x"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
_, _, track := seedArtistAlbumTrack(t, pool, "Audited", "", path)
|
||||
|
||||
svc := NewService(pool, nil, "")
|
||||
if _, _, err := svc.RemoveTrack(context.Background(), track.ID, admin.ID); err != nil {
|
||||
t.Fatalf("RemoveTrack: %v", err)
|
||||
}
|
||||
|
||||
var actor pgtype.UUID
|
||||
var title, artist, album, filePath string
|
||||
var albumDeleted bool
|
||||
err := pool.QueryRow(context.Background(), `
|
||||
SELECT actor_id, metadata->>'title', metadata->>'artist', metadata->>'album',
|
||||
metadata->>'file_path', (metadata->>'album_deleted')::boolean
|
||||
FROM audit_log WHERE action = 'track_remove'`).
|
||||
Scan(&actor, &title, &artist, &album, &filePath, &albumDeleted)
|
||||
if err != nil {
|
||||
t.Fatalf("read audit row: %v", err)
|
||||
}
|
||||
if actor != admin.ID {
|
||||
t.Errorf("actor = %v, want the admin %v", actor, admin.ID)
|
||||
}
|
||||
if title != "Audited Track" || artist != "Audited Artist" || album != "Audited Album" || filePath != path {
|
||||
t.Errorf("metadata = %q / %q / %q / %q", title, artist, album, filePath)
|
||||
}
|
||||
if !albumDeleted {
|
||||
t.Errorf("album_deleted = false, want true: the track was its album's only one")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRemoveTrack_FileAlreadyMissing(t *testing.T) {
|
||||
pool := newPool(t)
|
||||
admin := seedAdmin(t, pool, "alice")
|
||||
|
||||
Reference in New Issue
Block a user