Problem with skipping callbacks after upgrade to Rails #931

Closed
pawandubey opened this Issue Jul 17, 2016 · 5 comments

Comments

Projects
None yet
6 participants
@pawandubey

We have a model Company with an after_create callback.

class Company < ApplicationRecord
  after_create :do_something

  # other methods
  def do_something
     # work
  end
end

In our factories, we are skipping callbacks like so:

after(:build) do |company|
      company.class.skip_callback(:create, :after, :set_default_company_preference)
end

This worked fine and all our tests pass on Rails 4.2, but after upgrading to Rails 5, we are getting

ArgumentError:
       After create callback :do_work has not been defined

This is happening with all our models in which we have callbacks defined. Did Rails/Factory Girl change something in an unexpected way?

@mgrunberg

This comment has been minimized.

Show comment
Hide comment
@mgrunberg

mgrunberg Aug 3, 2016

Looking at the changelog of Rails 5:

ActiveSupport::Callbacks#skip_callback now raises an ArgumentError if an unrecognized callback is removed.

Here is the pull request with the message you are seeing.

So I think that the callback :do_work not exists and in previous versions, the skip_callback call was failing silently

Looking at the changelog of Rails 5:

ActiveSupport::Callbacks#skip_callback now raises an ArgumentError if an unrecognized callback is removed.

Here is the pull request with the message you are seeing.

So I think that the callback :do_work not exists and in previous versions, the skip_callback call was failing silently

@baramik

This comment has been minimized.

Show comment
Hide comment
@baramik

baramik Aug 13, 2016

Have some promlems here also. Callback is defined in model. But tests being failed for some reason

baramik commented Aug 13, 2016

Have some promlems here also. Callback is defined in model. But tests being failed for some reason

@joshuaclayton

This comment has been minimized.

Show comment
Hide comment
@joshuaclayton

joshuaclayton Aug 16, 2016

Member

@pawandubey FactoryGirl's callbacks are entirely decoupled from ActiveRecords', apart from the fact that FG calls save!, which will trigger callbacks. Apart from that, FG doesn't interfere/interact whatsoever. This is very likely an issue with the upgrade to Rails 5 (you'd probably be able to confirm by calling Company.skip_callback by hand in the console, without using FG).

Member

joshuaclayton commented Aug 16, 2016

@pawandubey FactoryGirl's callbacks are entirely decoupled from ActiveRecords', apart from the fact that FG calls save!, which will trigger callbacks. Apart from that, FG doesn't interfere/interact whatsoever. This is very likely an issue with the upgrade to Rails 5 (you'd probably be able to confirm by calling Company.skip_callback by hand in the console, without using FG).

@savmac

This comment has been minimized.

Show comment
Hide comment
@savmac

savmac Jan 11, 2017

I came across this issue as well after upgrading to Rails 5. What I was able to figure out is that after creating one instance of a factory, the callback method is no longer defined on the model for any subsequent instances that are created. Rails 4 ignored this but Rails 5 raises the argument error. You can confirm in the console by calling Foo.skip_callback(:create, :after, :bar) and it will work the first time but then fail if you do it again.

I was able to get test passing again by changing the factory callback to:

after(:build) do |foo|
  class << foo
    def bar; true; end
  end
end

Which I found here

Not sure if this is the best approach if anyone has any suggestions. I suppose you could also call set_callback in an after(:create) factory callback.

savmac commented Jan 11, 2017

I came across this issue as well after upgrading to Rails 5. What I was able to figure out is that after creating one instance of a factory, the callback method is no longer defined on the model for any subsequent instances that are created. Rails 4 ignored this but Rails 5 raises the argument error. You can confirm in the console by calling Foo.skip_callback(:create, :after, :bar) and it will work the first time but then fail if you do it again.

I was able to get test passing again by changing the factory callback to:

after(:build) do |foo|
  class << foo
    def bar; true; end
  end
end

Which I found here

Not sure if this is the best approach if anyone has any suggestions. I suppose you could also call set_callback in an after(:create) factory callback.

@juliends juliends referenced this issue in OFriedel/aulo Mar 14, 2017

Merged

Install sidekiq #73

@s2t2

This comment has been minimized.

Show comment
Hide comment
@s2t2

s2t2 Jun 10, 2017

Try passing , raise: false to make the error message go away while keeping the callback setting/skipping functionality:

after(:build) do |company|
  company.class.skip_callback(:create, :after, :set_default_company_preference, raise: false)
end

s2t2 commented Jun 10, 2017

Try passing , raise: false to make the error message go away while keeping the callback setting/skipping functionality:

after(:build) do |company|
  company.class.skip_callback(:create, :after, :set_default_company_preference, raise: false)
end

@guillaumebihet guillaumebihet referenced this issue in sudo-chess/chess-app Nov 30, 2017

Merged

Added valid_move? for bishop #53

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