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

ActionController filter initialization #4219

Closed
pmeinhardt opened this issue Dec 28, 2011 · 5 comments
Closed

ActionController filter initialization #4219

pmeinhardt opened this issue Dec 28, 2011 · 5 comments

Comments

@pmeinhardt
Copy link

Hi, I encountered this strange behaviour, when trying to set up a controller *_filter from an incuded module:

The module code looks similar to this (shortened it for readability)

module Tracking

  module ControllerAdditions
    def track(selectors = [])
      selectors.each do |sel|
        before_filter sel do |con|
          puts "#{sel}d #{con.class} #{con}"
        end
        before_filter sel, nil # rails bug? dummy
      end
    end
  end

  def self.included(base)
    base.extend ControllerAdditions
  end

end

It is then included in a controller as follows:

class SomeController < ApplicationController
  include Tracking

  track [:foo]

  def foo
    redirect_to "http://example.org"
  end
end

Problem is, that

  • if I use after_filter instead of before_filter and leave out the dummy filter, the controller will throw a "multiple redirects" error
  • if I use before_filter without the dummy, nothing gets logged at all

I use Rails 3.1.3.

Thanks for your help.

Regards,
Paul

@arunagw
Copy link
Member

arunagw commented Feb 14, 2012

Hey @pmeinhardt have you got any solution for this?? Or this problem still exists for you?

@pmeinhardt
Copy link
Author

Hey, I haven't had the time to dig into it since then. However, if you have any clue on how to solve this, I'd appreciate any hints.

It's strange I have to call the before_filter method twice.

Another solution that I considered in the meantime was using cuts to hook into arbitrary methods outside the regular Rails initialize, create, update and destroy hooks. There's a performance penalty to it (method execution is about ~13 times slower than with explicit "tracking" calls), but it allows me to prevent scattering the tracking methods all over the place.

This (instrumenting methods) should be a common problem in web applications/analytics so if anyone can point me to sth. better, I'd be more than grateful.

@carlosantoniodasilva
Copy link
Member

Hey @pmeinhardt, I think I may not be understanding quite right what you're trying to achieve here.

By giving a symbol to before_filter, you're telling this filter to run a method, not to run on a specific action. By giving a block, you're asking to run that block by passing the controller instance to it.

So, if I got your idea right, it'd look something like this:

module Tracking
  module ControllerAdditions
    def track(*actions)
      before_filter only: actions do |con|
        puts "Tracking [#{con.action_name}]: #{con.class} #{con}"
      end
    end
  end

  def self.included(base)
    base.extend ControllerAdditions
  end
end

# controller
class PostsController < ApplicationController
  include Tracking
  track :index

  def index
    @posts = Post.all
  end
end

# logger output
# Tracking [index]: PostsController #<PostsController:0x007fa064161ac8>

This was tested with a newly generated app and Rails 3.2.3. If that's not the path you're trying to go here, please try to explain a little bit better so we can verify it that's a real issue. Thanks.

ps: about your question related to instrumentation, the best thing for it nowadays I believe would be AS::Notifications.

@pmeinhardt
Copy link
Author

Argh. Yes @carlosantoniodasilva, you're absolutely right. I completely used the before_filter method in a wrong way here. What I was really trying to achieve was "before :action do ...". It seems totally obvious now, thanks to you. When I threw this together, I should've given it another look. Thanks :)

Thanks also for the hint about AS::Notifications, I'll dig right into it.

Cheers,
Paul

@carlosantoniodasilva
Copy link
Member

Ok, great, thanks :). AS::Notifications it totally worth a try. Cheers.

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

3 participants