Rails Ruby on Rails → 2.x

Comments and changes to this ticket

• You flagged this item as spam.
  

  Zach Dennis May 21st, 2008 @ 04:41 PM

  +1. This patch is simple and the test is clean and easy to read. The proposed behavior also seems to be correct: model callbacks should fire before external Observer callbacks.

• You flagged this item as spam.
  

  Jason Stewart May 21st, 2008 @ 05:05 PM

  +1. Looks good to me.

• You flagged this item as spam.
  

  Jason Stewart May 21st, 2008 @ 05:16 PM

  I'd also like to add that the tests look good, and firing the callbacks on the model before the observers seems to be the right thing to do.

• You flagged this item as spam.
  

  Brandon Keepers May 23rd, 2008 @ 05:06 AM

  I can't think of any reason why it should be the way it is. Seems logical that the callbacks defined closest to the model should be called first.

  +1 - patch and tests look good.

• You flagged this item as spam.
  

  Mark Van Holstyn May 23rd, 2008 @ 08:00 AM

  +1

  I think it makes much more sense for model callbacks to fire first, then observers. Tests all look good and pass.

• 

  josh May 25th, 2008 @ 12:29 AM

  • Milestone set to “2.1.1”
  • State changed from “new” to “open”
  • Assigned user set to “josh”
• 

  Repository May 28th, 2008 @ 04:16 PM

  • State changed from “open” to “resolved”

  (from [08763e606282f14fe98f4cf87a9f93d6836373cc]) Callbacks fire before notifying observers [#230 state:resolved]

  Signed-off-by: Joshua Peek

  http://github.com/rails/rails/co...

• 

  josh May 28th, 2008 @ 04:26 PM

  • Milestone changed from “2.1.1” to “2.x”
  • State changed from “resolved” to “open”

  After 2.1

• 

  Repository June 3rd, 2008 @ 07:38 PM

  • State changed from “open” to “resolved”

  (from [aa1771668877f20ca044e8f45a9736fbb7c8402e]) Callbacks fire before notifying observers [#230 state:resolved]

  Signed-off-by: Joshua Peek

  http://github.com/rails/rails/co...

• You flagged this item as spam.
  

  Brendon Muir November 26th, 2008 @ 06:40 AM

  • Tag set to “activerecord, patch, verified”

  Actually, this just broke my application. Here's why:

  I have ComponentInstances, a tree based model that's the core of my CMS. Instances of ComponentInstances are polymorphic so you can add Pages, Links, Podcasts etc... to your site and ComponentInstance handles the ordering and tree structuring.

  To make this nice and automatic when you create a Page (for example) there is an observer on the Page model that creates and links the ComponentInstance. Actually there's only one observer that watches all my models that need associated ComponentInstances (but that's nothing surprising).

  The issue comes when I have after_create callbacks on my models that need to use the associated component_instance. For example, I have a callback on one of our models that creates some children instances below this model instance. To do this we need to go page.component_instance.children << blah. But here's the catch, because this callback in the model is called before the callback in the observer, the component_instance doesn't exist yet.

  Obviously this is a fairly complex case, but there's nothing wrong with it as far as I'm aware. Can we come to a solution that works for all?

  Cheers,

  Brendon

• You flagged this item as spam.
  

  Craig Demyanovich November 26th, 2008 @ 02:41 PM

  That a model's callbacks fire before any of its observers are notified makes the most sense to me, at least as a default. Indeed, the surprise that they didn't work that way is the reason that I wrote the patch. Assuming that there's no good way to change your implementation to match this behavior, you could explore ways to configure the callback and observer notification order, perhaps on a per-model basis.

• You flagged this item as spam.
  

  Brendon November 27th, 2008 @ 08:39 AM

  Thanks Craig, I managed to work around it with superclasses. When you define callbacks in superclasses (and do self.abstract_class = true on that superclass of course), these callbacks get called first. Handily, I already had the superclass in place so it was an easy change and now I don't need to maintain a list of observed classes in my observer.

  However, what upset me about this change was that you mention that documenting the modification would have been nice, but you never actually did it. I searched for half a day (literally) trying to find an explanation as to why the behaviour had suddenly changed and finally stumbled upon this ticket. Needless to say that if the execution order was documented I would have found the problem much sooner as I scoured the API for any hint of it :) Perhaps could you add a line to the docs to this effect now?

  Cheers,

  Brendon

• You flagged this item as spam.
  

  Craig Demyanovich February 4th, 2009 @ 06:56 PM

  I was searching the book, The Rails Way, for an answer to another question when I stumbled upon the "Timing" section on p. 284. It states that, "Observers are notified before the in-object callbacks are triggered. Otherwise, it wouldn't be possible to act on the whole object in something like a before_destroy observer without having the object's own callbacks executed first."

  Does this new information make anyone else want to remove my patch?

• You flagged this item as spam.
  

  Brendon February 4th, 2009 @ 11:38 PM

  Yes please. It makes sense to reverse the change (and document it in an obvious place) especially since a prominent book explicitly talks of it and states a very good reason to have it the original order.

• You flagged this item as spam.
  

  Zach Dennis February 5th, 2009 @ 12:31 AM

  I don't think the order should be reverted. Trying to appeal to the book as the authority is not a solid argument, and unless the book expands in more detail on why it is such a good thing, I don't see the sentence that Craig found that compelling.

  In what concrete example would you expect or need an external callback to be called first?

  After a model object is destroyed it still retains all of its properties in memory, so any external before_destroy callback would still be able to operate just fine. The only time it wouldn't would be if it needed to operate on the object itself (by updating and saving the object), but that seems pointless since it's in the process of being deleted.

  An object's internal callbacks should take precedence over external callbacks.

• You flagged this item as spam.
  

  Brendon February 5th, 2009 @ 03:06 AM

  I gave a perfectly fine example above where I wanted to create dependent objects after the creation of the observed object, and then work on these as a group via internal callbacks specific to certain objects.

• You flagged this item as spam.
  

  Paul April 21st, 2009 @ 10:32 AM

  i am in trouble now with post notifying observers ...

  Assume you want to prevent object destruction based on observing before_destroy.

  There are two problems now:

  1) the return value of notified callback is thrown away. This seems to be ok for me, as i think an observer should not have an easy to use way of changing model behaviour.

  but now

  2) i do not have a chance to define something like model.please_do_not_destoy_as_external_conditions_are_not_met = true and react on it within the model before_destroy callback itself ...

  What could be a solution to this problem now?

• You flagged this item as spam.
  

  tom_302 July 22nd, 2009 @ 07:42 PM

  This change broke my app, too. (I use before_save() to set a belongs_to association to a new record, but now that record doesn't get saved.)

  Perhaps the best of both worlds is to call notify() and run_callback() in different order based on whether the prefix is 'before' or 'after'. That way, observers have access to the latest data (after some event) and the ability to modify data (before some event).

• You flagged this item as spam.
  

  legolin August 31st, 2009 @ 02:57 PM

  +1 @tom_302

  I completely agree that before and after callbacks should be handled differently.
  

  In the case of before callbacks, observers can (1) process external information that the model shouldn't have to worry about, (2) change the state of the model appropriately, so (3) the model can make an informed final decision on whether to follow through with the action.
  

  In the case of "after" observers like after_create, the model can do it's own post-processing first, providing a complete model for the observer to use when, say, sending an email.

• You flagged this item as spam.
  

  Obie January 29th, 2010 @ 02:19 AM

  +1 @legolin Well-reasoned option, should be considered.

• You flagged this item as spam.
  

  csnk May 18th, 2011 @ 08:24 AM

  • Importance changed from “” to “”

  We are the professional uniforms manufacturer, uniforms supplier, uniforms factory, custom uniforms.

• You flagged this item as spam.
  

  csnk May 18th, 2011 @ 08:24 AM

  We are the professional uniforms manufacturer, uniforms supplier, uniforms factory, custom uniforms.