#4087 ActiveRecord observers can't be used for "before" callbacks - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#4087 ✓committed

ActiveRecord observers can't be used for "before" callbacks

Reported by Mislav | March 2nd, 2010 @ 05:30 AM

ActiveRecord::Observer hooks into observed models by registering a _notify_observers_for_#{method} callback on them, which calls notify_observers(:#{method}). For a before_create, this would result in something like:

## in MyModel
def _notify_observers_for_before_create
  notify_observers(:before_create)
end
before_create :_notify_observers_for_before_create

The problem is, the underlying implementation uses Observable module from Ruby stdlib, which implements notify_observers in a way that it returns false on success. Needless to say, this halts the callback chain on "before" callbacks and the record is not saved.

Comments and changes to this ticket

You flagged this item as spam.
Mislav March 2nd, 2010 @ 06:11 AM
- Tag changed from “activerecord, callbacks, observers” to “activerecord, callbacks, observers, rails3”

Sven Fuchs March 2nd, 2010 @ 09:07 AM

Gosh, I ran into the same thing today.

As long as you don't care about the return value this hax seems to work for now

ActiveRecord::Observer.class_eval do
  def add_observer!(klass)
    super
    self.class.observed_methods.each do |method|
      callback = :"_notify_observers_for_#{method}"
      if (klass.instance_methods & [callback, callback.to_s]).empty?
        klass.class_eval "def #{callback}; notify_observers(:#{method}); true; end"
        klass.send(method, callback)
      end
    end
  end
end

You flagged this item as spam.
Mislav March 2nd, 2010 @ 03:09 PM
That's the exact monkeypatch I did in Radiant
Yehuda Katz (wycats) March 27th, 2010 @ 11:12 AM
- Assigned user set to “Yehuda Katz (wycats)”
You flagged this item as spam.
Mislav April 16th, 2010 @ 04:21 PM
- Tag changed from “activerecord, callbacks, observers, rails3” to “activerecord, callbacks, observers, rails3, regression”
Failing test
- observers-test.patch 4.9 KB
You flagged this item as spam.
Mislav April 16th, 2010 @ 06:55 PM
Taken a stab at fixing this. This patch includes changes from the above patch. Detailed explanation of the changes in commit messages, but here is the last (and most important):
improve how ActiveRecord::Observer defines callbacks on observed models

Instead of using a single notify_observers call for every callback type, each observer now registers a unique callback for itself. Example:

before_save :_notify_user_observer_for_before_save def _notify_user_observer_for_before_save observer.update(:before_save, self) end

Benefit: "before" callbacks halt when observer.update returns false. This way, ActiveRecord observers can prevent records from saving.
- observers-fix.patch 9.7 KB
Repository April 16th, 2010 @ 09:54 PM
- State changed from “new” to “committed”
(from [2161b8745a22379356b466a60b9aa763c0593f9b]) improve how ActiveRecord::Observer defines callbacks on observed models

Instead of using a single notify_observers call for every callback type,
each observer now registers a unique callback for itself. Example:

before_save :_notify_user_observer_for_before_save

def _notify_user_observer_for_before_save
```
observer.update(:before_save, self)
```
end

Benefit: "before" callbacks halt when observer.update returns false.
This way, ActiveRecord observers can prevent records from saving.

[#4087 state:committed]

Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/2161b8745a22379356b466a60b9aa7...