Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Allow after_commit :do_something, :on => [:update, :create] #988

Closed
lighthouse-import opened this Issue · 15 comments

9 participants

@lighthouse-import

Imported from Lighthouse. Original ticket at: http://rails.lighthouseapp.com/projects/8994/tickets/6660
Created by Ary Borenszweig - 2011-03-31 17:48:22 UTC

It's very common to want to do the same thing on an update/create callback after commit (example: create/update a related file, but if the file was there then it is just overwritten).

I tried:

after_commit :do_something, :on => [:update, :create]

but I get:

/Users/asterite/.rvm/gems/ruby-1.9.2-p136@verbo/gems/activesupport-3.0.3/lib/active_support/callbacks.rb:400:in `class_eval': /Users/asterite/.rvm/gems/ruby-1.9.2-p136@verbo/gems/activesupport-3.0.3/lib/active_support/callbacks.rb:414: syntax error, unexpected '[', expecting tSTRING_CONTENT or tSTRING_DBEG or tSTRING_DVAR or tSTRING_END (SyntaxError)
if (transaction_include_action?(:[:create, :update]))
                                  ^

I tried doing this:

after_commit :do_something, :on => :create
after_commit :do_something, :on => :update

but the second callback overrides the first one. So for now I'm using two different methods, or just using an alias_method, but I think the :on => [...] is more DRY, useful and it also similar to the controllers :before_filter, :only => [...]

@lighthouse-import

Imported from Lighthouse.
Comment by bgentry - 2011-04-01 19:16:51 UTC

I just encountered this same bug today and was fortunate to find this bug report from a day ago. I have the same situation and I'd like to run the same actions but only on create/update, not destroy.

I definitely wouldn't expect that using after_commit twice would override the first action.

@lighthouse-import

Imported from Lighthouse.
Comment by bgentry - 2011-04-01 19:25:32 UTC

Also, I believe you can work around this exact issue by defining a single callback and only running it if the object is persisted:

after_commit :do_something, :if => :persisted?
@lighthouse-import

Imported from Lighthouse.
Comment by Ary Borenszweig - 2011-04-01 19:29:00 UTC

Yes, thanks! That worked. So what do we do about this? Close as invalid? Still propose to do it? (I will try to make a patch for this)

@lighthouse-import

Imported from Lighthouse.
Comment by bgentry - 2011-04-01 20:04:44 UTC

I still thing it's a valid bug, the behavior is certainly not as I'd expect. Callbacks are never supposed to override each other like this. At a glance it didn't look like the patch would be very simple, but go ahead and take a crack at it. Hopefully we can get some assistance from the core team.

@lighthouse-import

Imported from Lighthouse.
Comment by Michael Andrews - 2011-04-14 01:27:53 UTC

I also recently encountered this problem. Here is a unit test demonstrating this behavior:

#269

@lighthouse-import

Imported from Lighthouse.
Comment by Michael Andrews - 2011-04-14 02:11:52 UTC

Updated the pull request with a fix to prevent the callbacks in separate contexts from overriding one another.

@naoisegolden

Was this ever fixed? after_commit still doesn't accept an array (using Rails 3.2.11):

after_commit :do_something, :on => [:create, :update] # calls neither but doesn't raise error

Stacking them also doesn't work, as mentioned before:

after_commit :do_something, :on => :create
after_commit :do_something, :on => :update # only calls on :update

In the official documentation they suggest after_commit :do_something, :if => :persisted? for :update and :create but I still feel it makes sense that it accepted an array, as in other filters.

Re-open as a bug?

@rafaelfranca rafaelfranca reopened this
@nanego

I was trying to send a notification after both creation and deletion when I saw this issue.

That would be great if this peace of code could work :
after_commit :send_notification, :on => [:create, :destroy]

The :if => :persisted? trick doesn't work in this case.

@senny senny was assigned
@senny senny closed this in d98763a
@Frexuz

This is only for Rails 4, right? Any Rails 3.2.x work around?

@seako

@Frexuz the workaround i've used is to do the following

after_commit :do_something_on_create, on: :create
after_commit :do_something_on_destory, on: :destroy

alias :do_something_on_create :do_something
alias :do_something_on_destroy :do_something

or alternatively

after_commit ->(obj) { obj.do_something }, on: :create
after_commit ->(obj) { obj.do_something }, on: :update
@iamjoshua iamjoshua referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@naveda89

:+1: to @seako workaround!!

@kimiyash kimiyash referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@kimiyash kimiyash referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@deraru deraru referenced this issue in GetStream/stream-rails
Merged

Add active record 3 support #7

@mchaisse

Hi!

I just came through this bug again in the 4-1-stable rails branch.

My code looks like exactly the same as mentioned before:

  after_commit :my_method, on: :create # not fired
  after_commit :my_method, on: :update # fired

So here, my_method is called only on an update, but not on a create.

Does somebody else have the same bug with this branch?

Thanks :)

@rafaelfranca

It is not a bug. You are defining the same callback just with different arguments, so only the last one will be fired.

@mchaisse

Hi @rafaelfranca !
Thank you for your very quick answer. I thought that was possible on multiple lines like I did and saw on the above comment. My bad.
Thanks again :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.