Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

It's impossible to add a custom callback to AR class #4200

Closed
romanbsd opened this issue Dec 27, 2011 · 4 comments
Closed

It's impossible to add a custom callback to AR class #4200

romanbsd opened this issue Dec 27, 2011 · 4 comments

Comments

@romanbsd
Copy link
Contributor

Consider the following:

class Post
  define_model_callbacks :approve, :only => :after

    def approve!
      run_callbacks(:approve) do
        update_attribute(:approved, true)
      end
    end
end

class PostObserver
  def after_approve(post)
    # do something
  end
end

This won't work as expected, because ActiveRecord::Observer defines the notify methods using the following code:

      def define_callbacks(klass)
        observer = self
        observer_name = observer.class.name.underscore.gsub('/', '__')

        ActiveRecord::Callbacks::CALLBACKS.each do |callback|
          next unless respond_to?(callback)
          callback_meth = :"_notify_#{observer_name}_for_#{callback}"
          unless klass.respond_to?(callback_meth)
            klass.send(:define_method, callback_meth) do |&block|
              observer.update(callback, self, &block)
            end
            klass.send(callback, callback_meth)
          end
        end
      end

Therefore only ActiveRecord::Callbacks::CALLBACKS are going to be used.

@lacco
Copy link

lacco commented Jan 27, 2012

It's a pity that this doesn't work out of the box. Workaround for the moment:

class Post
  define_model_callbacks :approve, :only => :after

  def approve!
    run_callbacks(:approve) do
      update_attribute(:approved, true)
      notify_observers :after_approve
    end
  end
end

@lacco
Copy link

lacco commented Jan 27, 2012

I moved the notifying call into an own method, so that you just have to use define_model_callbacks_for_observersinstead of ``define_model_callbacks` (you might want to put this into a module):

  def define_model_callbacks_for_observers(*args)
    types = Array.wrap(args.extract_options![:only] || [:before, :around, :after])

    callbacks = define_model_callbacks(*args)

    callbacks.each do |callback|
      types.each do |filter|
        set_callback(callback, filter) do
          notify_observers :"#{filter}_#{callback}"
          true
        end
      end
    end
  end

@ahawkins
Copy link

@romanbsd It is possible. You have to use notify_observers. Here is the snippet I use in my code:

def notify_of_finished
   changed ; Todo.notify_observers :finished, self ; true
end

This works perfectly. I think this the correct approach. I don't like extending the Rails callbacks using the proposed approach.

@lacco interested in submitting a Pull Request?

@carlosantoniodasilva
Copy link
Member

Hey, I'm closing this as stale, if anyone wants to try out a pull request, please go ahead. You can gather more feedback first if you want in the Rails Core mailing list. Thanks.

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

No branches or pull requests

4 participants