added :callbacks => false functionality for create, save, update_attribute(s), and destroy #4885

Closed
wants to merge 1 commit into from

7 participants

@patrick99e99

Added the option of :callbacks => false to create, create!, save, save!, update_attribute, update_attributes, and destroy.

This will allow anyone to easily disable the before/after create/save/update/destroy callbacks.

@josevalim
Ruby on Rails member
@rafaelfranca
Ruby on Rails member

When you need to skip callbacks it seems like a abuse of them. I wrote the same code one year ago, but I agree that my case was a abuse of callbacks.

@patrick99e99

Well for me, often times when I am testing a model, I want to isolate some particular behavior, and if there are callbacks along the way of what I am testing then it quickly can become a bit of a pain. I will then have to have my test stub out the methods that are getting triggered via callbacks, and that makes my tests less clear, and more brittle.

Being able to turn off callbacks is a nice convenience, and doesn't have to indicate "abuse"...

@carlosantoniodasilva
Ruby on Rails member

@patrick99e99 your pull request does not apply cleanly anymore, perhaps you could bring it up-to-date with master and we can start the discussion again.

I just want to add that, if your use case is only for tests, I'd advice you to use observers instead of callbacks. You can disable them as you wish, at any time, without any change on how callbacks work. Thanks!

@josevalim
Ruby on Rails member

Closing this. If you want to turn off callbacks, you are poorly using them.

@josevalim josevalim closed this May 3, 2012
@patrick99e99

Closing this. If you want to turn off callbacks, you are poorly using them.

I just don't understand why it's ok for ActiveRecord to give options for skipping validation, and skipping protection for attr_accessible, but it's not ok to give you an option to skip callbacks. It seems inconsistent and inflexible. To say "you're poorly using them" doesn't really seem fair. There are a lot of instances I can think of where one might want to be able to choose to not go through the entire callback cycle-- I see can see this happening a lot via experimentation in the console.

@indrekj

See http://jamesgolick.com/2010/3/14/crazy-heretical-and-awesome-the-way-i-write-rails-apps.html

I suggest creating a separate class to handle this kind of logic. Then there aren't any callbacks and it is much easier to test. Also you can test that separate class in isolation without even loading rails.

@globetro

Luckily the rest of the Rails community doesn't agree with your blanket statement that if you need to skip any callbacks then you're using it wrong:

http://apidock.com/rails/ActiveRecord/Persistence/update_column

@hgtesta

I often need to skip callbacks for performance reasons when manipulating data in maintenance routines. An option to disable callbacks would help me a lot and the code would be much cleaner.

@carlosantoniodasilva
Ruby on Rails member

In general, the solution for such scenarios is usually not to use callbacks. If callbacks are made conditional from an outsider's perspective, then there might be no point in them being callbacks tied to the lifecycle of the object in question, and something might smell bad with the application architecture, that could probably be improved. Extracting objects out of AR models is usually a good improvement/solution for that.

@hgtesta

@carlosantoniodasilva Are you saying that I must give up my callbacks that are used all around the application (called from dozen of places) just because one background maintenance routine where I need more performance?

@carlosantoniodasilva
Ruby on Rails member

Not really, what I'm trying to say is that if there's only one use case where callbacks should not be triggered (from an outsider perspective), they are probably not supposed to be callbacks. It's like allowing a user to tell the controller whether the before filters should run :).

Anyway, each use case is different, but the idea of skipping callbacks has already been rejected due to reasons described above.

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