search

pat / after_commit

A Ruby on Rails plugin to add an after_commit callback. This can be used to trigger things only after the entire transaction is complete.
MIT License
62 stars 32 forks source link

after_commit_on_update with multiple updates to the same object per transaction #8

Closed ctaintor closed 11 years ago

ctaintor commented 12 years ago

This issue is really biting us in the butt. I'm happy to commit a change to fix this, but I wanted to get some perspective before I do.

Here's the issue we're having:

we have a after_commit_on_update callback for a model. I've changed the callback to just print when it is called. Inside our code, we do something similar to this (of course these updates aren't right next to each other).

User.transaction do
  user.update_attribute(:first_name, "asdf")
  user.update_attribute(:last_name, "fdsa")
end

Will result in

CALLING COMMIT UPDATE
CALLING COMMIT UPDATE

Now, I admit that we probably shouldn't be doing these things, but we are, and it's hard to go through a relatively large codebase to fix these double-updated.

I think it makes sense to make the :committed_records_on_update collection only have one instance of an object in the collection, no matter how many times the object was updated inside the transaction. Thoughts?

pat commented 12 years ago

Hi Case

To be honest, I don't really use this project at all beyond Thinking Sphinx, and even then, most of my focus is on Rails 3 and newer. So, you're in a better position to make the call about whether this patch should happen. If you want to submit a pull request, please do - if you can add tests for it, even better.

xaviershay commented 12 years ago

I totally just lost the better part of my day to this :(

Will investigate a fix, but I think I'll probably use my time to refactor away our usage of this instead.

ctaintor commented 12 years ago

xaviershay - I ended up just editing my version of after_commit, which was probably not the best thing to do (since it gave you pain), but I knew it worked for our use case and didn't want to worry about someone else's use case. Basically, in after_commit.rb, change add_to_collection to be this:

def self.add_to_collection(collection, connection, record)
  current_collection = Thread.current[collection][connection.unique_transaction_key]
  #Only add to the collection if this specific object isn't already into the collection
  current_collection << record unless current_collection.any?{|existing_record| existing_record.object_id == record.object_id}
end