Update tranasaction state when record gets commited #6226

Merged
merged 3 commits into from May 9, 2012

6 participants

@gnufied

The pull request ensures that, when record gets committed to DB, transaction state gets updated from latest values. Without the patch, calling save from after_commit :on => :create callback will trigger after_commit :on => :create again, which I think is wrong. The current behaviour is also not historically consistent with, https://github.com/freelancing-god/after_commit .

@jeremy
Ruby on Rails member

After the change, does calling save from after_commit trigger another after_commit :on => :update ?

@gnufied

Yup, it does. I can additional test to prove that.

@jeremy
Ruby on Rails member

❤️

@erichmenge

Closes #6207

@tenderlove tenderlove merged commit 9bf1a0d into rails:master May 9, 2012
@jjb

would be great if this were backported to 3.2-stable #8577 /cc @sidonath

This commit breaks the functionality of after_commit(on: :create) when a model instance is saved within the same transaction in which it was created (see #8937)

I think functionality only breaks when you have multiple after_create blocks registered. I can't imagine why anyone will call save! from following callbacks:

after_commit {} # without condition
after_commit(on: :update) {} 

Both will trigger infinite loop.

@gnufied it breaks when there is only one after_create block in which save is called. There's no need to call save in after_commit to trigger the problem.

@rafaelfranca rafaelfranca added a commit that referenced this pull request Apr 10, 2013
@rafaelfranca rafaelfranca Revert "Merge pull request #6226 from gnufied/master"
This reverts commit 9bf1a0d, reversing
changes made to fed9709.

Conflicts:
	activerecord/test/cases/transaction_callbacks_test.rb

Reason: This fix introduces another issue described at #8937, so we are
reverting it to restore the behavior of 3-2-stable.

We will fix both issues when we come out with a better solution
9d2146a
@n-studio n-studio commented on the diff Jan 10, 2016
activerecord/test/cases/transaction_callbacks_test.rb
+ after_commit :call_update, :on => :update
+ attr_accessor :cached, :record_updated
+
+ def call_update
+ self.record_updated = true
+ end
+
+ def cache_topic
+ unless cached
+ self.cached = true
+ self.save
+ else
+ self.cached = false
+ end
+ end
+ end
@n-studio
n-studio added a line comment Jan 10, 2016

Why does the callback still need a condition for not being called again?
Isn't the purpose of this PR to prevent the callback from being called again after a save?

def cache_topic
  self.cached = true
  self.save
end

Shouldn't this code work too?

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