I can’t quite speak to whether there’s specifically a bug, but here’s some thoughts after looking at your repo, and playing with it in console.
-
delete_all goes straight to SQL, without hitting callbacks. This might be why there’s no errors squawking from the following notes.
-
request.items = [...] appears to work by replacing the collection in the DB. i.e., the moment you call r.items = [...], it executes the sql to replace the collection (delete and insert based on r.id).
- If you were to follow up your test with one more move:
r.save! after calling #new you’d find that this is unable to be inserted, because AR is treating it as a new record, as opposed to an existing record.
Finally, I get to the main observation: It looks like the collection setter (request.items = [...]) attempts to replace the collection immediately in the DB if the record was already persisted. It succeeds at deleting the existing records based on id, but it never follows up with inserting the records. This seems to be related to calling .new and passing in an existing id, rather than querying for the existing record. The AR instance sees this as a new record, but it in fact conflicts with an existing record so it cannot be saved. Thus, it cannot get an ID in order to populate the associations.
The more usual pattern would be to use find_or_initialize_by, letting AR know that you are pulling up an already-persisted record if such an ID already exists.
Here’s the bit of Rails code in question: rails/collection_association.rb at 291a3d2ef29a3842d1156ada7526f4ee60dd2b59 · rails/rails · GitHub
You can see that there’s a chunk that checks the ar attribute: #new_record? which is true from a #new, but wouldn’t be from a #find, etc.
Here’s some things to try in a console – specifically watch the DB logs for each variation:
r = Request.new(request_type: :wfh, status: :draft)
# => In memory record, not persisted
r.items = [RequestItem.new(start_at: Time.now, end_at: Time.now + 36000)]
# => Still in memory, nothing is persisted. Now, table this observation until the last example.
# Let's actually get a persisted record using the same example code:
r = Request.create(request_type: :wfh, status: :draft)
# => `r` is a new AR object, and it's persisted in DB due to #create
r.items = [RequestItem.new(start_at: Time.now, end_at: Time.now + 36000)]
# => Instantly triggers a db insert to add the collection, even without calling `r.save`.
# Just to check how this looks, let's replace that again before we lose this object:
#
r.items = [RequestItem.new(start_at: Time.now, end_at: Time.now + 36000)]
# => delete -> insert
#
# Notice that `=` triggered a full replacement: delete and insert.
# Now let's reconsistute this record:
#
# First, let's watch the case where AR knows a record was pulled from the DB:
# You can use `#find` or any other method that tells AR to query the db
r = Request.find(r.id)
# => loads from db. r is a new AR object, and it came from persistence. `new_record?` is false.
r.items = [RequestItem.new(start_at: Time.now, end_at: Time.now + 36000)]
# => delete -> insert
#
# Still works as expected.
# Finally, the reported case as above:
r = Request.new(id: r.id)
# No db query, only in memory.
r.items = []
# => Attempts to replace immediately, but curiously, triggers a delete, and nothing else.
#
# This is likely due to the fact that it doesn't have the persisted db PK from r,
# since r itself cound't be saved, so it can't persist the associated records yet either.
# I think using `delete_all` rather than `destroy` is helping "cover up"
# this issue, but isn't necessary the issue itself.
r.save!
# => RecordNotUnique rollback
In conclusion, I’m not sure that I agree it’s a bug with delete_all, but instead the unexpected usage of instantiating a record with an id which can be a bit dangerous since that id may already exist in the DB. That said, I think it’s unexpected behavior that the delete statement triggers and doesn’t get rolled back when in fact the record couldn’t be saved; however, since it’s a #new_record?, it is expected that the collection wouldn’t be written until the #new_record? is persisted, since it needs the id.