active_record_upsert icon indicating copy to clipboard operation
active_record_upsert copied to clipboard

Callbacks are fired even though upsert was skipped

Open hammady opened this issue 3 years ago • 2 comments

If the upsert operation is protected by an arel_condition which results in no records upserted, the model callbacks still fire producing errors. If no records are upserted, no callbacks should be fired. Take the below as an example:

class Subscription < ApplicationRecord
  attr_accessible :last_event_id
  belongs_to :user
  after_save {
    user.touch
  }
  upsert_keys [:store_id]
end

subscription1 = Subscription.create({store_id: 10, user_id: 5, last_event_id: 12}) # succeeds, after_save fires correctly
subscription2 = Subscription.upsert!({store_id: 10, user_id: 5, last_event_id: 11}, arel_condition: self.arel_table[:last_event_id].lt(11)) # fails, after_save should not be called in this case

In the above example, the first subscription is created and the after_save is called. However, in the second subscription, the upsert results in no records touched because the new event id is less than the older event it. While the upsert SQL command succeeds, the returned subscription has all nil fields and the callback fails.

hammady avatar Feb 17 '22 17:02 hammady

Hi, I was looking into this and couldn't find any good seam where we could insert a check. We can introspect the result of the SQL query, so in theory it's possible to know when no record was upserted. But I can't find a nice way of conditionally skipping callbacks based upon this info. Help appreciated!

jesjos avatar Nov 19 '22 15:11 jesjos

@jesjos In my application code, I am doing this check:

subscription2 = Subscription.upsert!...
unless subscription2.id.nil?
  # do manual callbacks
end

Or:

after_save {
  user.touch if user
}

hammady avatar Feb 03 '23 15:02 hammady