A spec for a filter

privacy_filter.rb

class PrivacyFilter
  def self.filter(controller)
    [:first_article?,
     :authenticate_administrator!,
     :authenticate_user!
    ].find do |m|
      controller.send(m)
    end
  end
end

privacy_filter_spec.rb

require "app/models/privacy_filter"

describe PrivacyFilter do

  let(:controller) do
    double(:first_article? => false,
           :authenticate_administrator! => false,
           :authenticate_user! => false)
  end

  it "allows access to the root article" do
    controller.should_receive(:first_article?).and_return(true)
    PrivacyFilter.filter(controller).should be_true
  end

  it "allows access for administrators" do
    controller.should_receive(:authenticate_administrator!).and_return(true)
    PrivacyFilter.filter(controller).should be_true
  end

  it "allows access to users" do
    controller.should_receive(:authenticate_user!).and_return(true)
    PrivacyFilter.filter(controller).should be_true
  end

  it "denies all others" do
    PrivacyFilter.filter(controller).should be_false
  end
end

Why not instantiate it?

PrivacyFilter.new(controller).filter

and set the controller instance var in intitialize?

It always seems strange to me to set state in a singleton object, unless I meant for that state to be used later on.. "stashing" the object to save a few keystrokes seems like it makes it a bit less clear what is happening. What about something like this:

class PrivacyFilter
  class << self 
    def filter(controller)
      new(controller).allow?
    end
  end

  def initialize(controller)
    @controller = controller
  end

  def allow?
    first_article? or administrator? or user?
  end

private

  def first_article?
    @controller.params[:id] == 1  # or Hash#fetch if params[:id] is always expected
  end

  def administrator?
    @controller.authenticate_administrator!
  end

  def user?
    @controller.authenticate_user!
  end

end

@svenfuchs Rails :/

before_filter PrivacyFilter

Hmm, now that I see this here it might be overkill ;) All well, just another pov to consider.

... what dkubb said :)

@dkubb I wouldn't even make it a singleton, if the Rails API wasn't that self.filter is how this stuff gets called. I ended up moving the first_article? method into the controller, so that I just straight-up delegate, which seems to be the right way to do it.

do the !bang methods raise errors?

also, is it completely unacceptable to use a one liner? there's not a lot of inherent complexity to what this filter does right now, so is there a great need to add cognitive complexity to the code?

@dennyabraham I don't think so. They're just standard Devise stuff. I think the bang is because they can redirect.

I'm torn between what's more readable, a one liner, or this.

Now that I'm thinking about it, that might mean that this won't even work, as a user will hit the admin filter, which will redirect to admin login...

I should really be using CanCan for this, I guess.

Late to the party, but I really want this to be AccessPolicy.has_access?(user, article). (Raptor will let you write exactly this and specify it directly in a route... some day. garybernhardt/raptor@3e1b48e ;)

Yeah, that seems like the right API. Rails makes it awkward.

For the app, I just ended up using cancan, which is the Right Way anywaay...