Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Callbacks are fired even though upsert was skipped #125

Open
hammady opened this issue Feb 17, 2022 · 2 comments
Open

Callbacks are fired even though upsert was skipped #125

hammady opened this issue Feb 17, 2022 · 2 comments

Comments

@hammady
Copy link

hammady commented Feb 17, 2022

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.

@jesjos
Copy link
Owner

jesjos commented Nov 19, 2022

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!

@hammady
Copy link
Author

hammady commented Feb 3, 2023

@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
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants