Legacy photos/songs rows are orphaned on record delete — add dependent: :destroy + FK constraints

[holden]

Problem

Record has_many :photos (app/models/record.rb:80) and has_many :legacy_songs (:81) have no dependent: option, and neither photos.record_id nor songs.record_id has a FK constraint to records (confirmed — add_foreign_key in db/schema.rb has no entry for either).

So destroying a record silently strands its legacy photos and songs rows. No error, no cascade, no cleanup.

These are the legacy Refile-era models kept for backwards-compatible URLs (/files/photos/:id, /files/songs/:id). They degrade gracefully when orphaned — Photo#active_storage_attachment returns nil via safe navigation, and FilesController returns head :not_found — so this is silent junk accumulation rather than a crash.

Measurements

Audited against the dev DB (20,388 records).

There is no existing mess

Check	Count
Orphaned photos rows (dangling record_id)	0
Orphaned songs rows	0
photos / songs with NULL record_id	0
Orphaned active_storage_attachments → missing Record	0
Dangling records.{price,artist,label,genre,record_format,discogs_release}_id	0 (FK-enforced)

No cleanup migration or backfill rake task is needed. The database is clean, and the FK constraints will apply with zero data repair.

But the forward exposure is large

Exposure	Count	% of records
Records with ≥1 legacy photos row	10,962	54%
Records with ≥1 legacy songs row	4,710	23%
Avg photos rows per affected record	3.0	—
Max photos rows on one record	25	—

Roughly every second delete would strand ~3 legacy rows. This is a forward-looking bug, not a backlog.

Why this is urgent now

Record deletion is currently hard to reach (see #384 — the public route doesn't exist and the only working button is buried in a collapsed admin panel), so the bug is latent. #384 makes deletion a first-class, easily-reachable action.

Shipping #384 before this fix would manufacture the backlog we don't currently have. This issue blocks #384.

Fix

has_many :photos, dependent: :destroy
has_many :legacy_songs, class_name: "Song", foreign_key: "record_id", dependent: :destroy

Plus a migration adding the FK constraints so the database enforces it independently of the model layer.

Tasks

• Add dependent: :destroy to has_many :photos and has_many :legacy_songs on Record
• Migration adding FK constraints on photos.record_id and songs.record_id
• Re-run the audit script against production and confirm 0 orphans there too
• Test: destroying a record cascades to its legacy Photo / Song rows
• Test: destroying a record does not delete its Price (the price guide is shared reference data — 252,228 of 264,914 prices have no record attached at all)

Done when: a Record.destroy in the console leaves zero dangling rows and the price guide entry intact.

Risk: low. If the FK migration is uncomfortable on a 32K-row table, ship dependent: :destroy alone and defer the constraint.

Audit script

Save and run with bin/rails runner orphan_audit.rb:

def one(sql) = ActiveRecord::Base.connection.select_value(sql)

puts "=== ORPHANED Active Storage attachments (point at a missing Record) ==="
puts "images: #{one("SELECT COUNT(*) FROM active_storage_attachments a LEFT JOIN records r ON r.id = a.record_id WHERE a.record_type = 'Record' AND a.name = 'images' AND r.id IS NULL")}"
puts "songs:  #{one("SELECT COUNT(*) FROM active_storage_attachments a LEFT JOIN records r ON r.id = a.record_id WHERE a.record_type = 'Record' AND a.name = 'songs' AND r.id IS NULL")}"

puts "\n=== EXPOSURE: how many deletes would create orphans? ==="
puts "records w/ >=1 legacy photo row: #{one("SELECT COUNT(DISTINCT record_id) FROM photos WHERE record_id IS NOT NULL")}"
puts "records w/ >=1 legacy song row:  #{one("SELECT COUNT(DISTINCT record_id) FROM songs WHERE record_id IS NOT NULL")}"
puts "avg photos per such record:      #{one("SELECT ROUND(AVG(c),1) FROM (SELECT COUNT(*) c FROM photos WHERE record_id IS NOT NULL GROUP BY record_id) t")}"
puts "max photos on one record:        #{one("SELECT MAX(c) FROM (SELECT COUNT(*) c FROM photos WHERE record_id IS NOT NULL GROUP BY record_id) t")}"

puts "\n=== dangling record FKs (FK-protected, expect 0) ==="
{ "price" => "prices", "artist" => "artists", "label" => "labels", "genre" => "genres",
  "record_format" => "record_formats", "discogs_release" => "discogs_releases" }.each do |col, tbl|
  puts "records.#{col}_id -> missing #{tbl}: #{one("SELECT COUNT(*) FROM records r LEFT JOIN #{tbl} x ON x.id = r.#{col}_id WHERE r.#{col}_id IS NOT NULL AND x.id IS NULL")}"
end

puts "\n=== catalog rows with no record attached (informational) ==="
puts "prices with no record:           #{one("SELECT COUNT(*) FROM prices p LEFT JOIN records r ON r.price_id = p.id WHERE r.id IS NULL")}  (of #{one("SELECT COUNT(*) FROM prices")} total)"
puts "discogs_releases with no record: #{one("SELECT COUNT(*) FROM discogs_releases d LEFT JOIN records r ON r.discogs_release_id = d.id WHERE r.id IS NULL")}  (of #{one("SELECT COUNT(*) FROM discogs_releases")} total)"

Related

• Record deletion: unreachable from the UI, mis-describes its own blast radius, and needs media-aware confirmation #384 Record deletion — blocked by this issue
• Remove Refile dependencies and update file serving to Active Storage #173 Remove Refile dependencies (origin of the legacy Photo/Song models)