-
-
Save steveklabnik/1464751 to your computer and use it in GitHub Desktop.
class PrivacyFilter | |
def self.filter(controller) | |
[:first_article?, | |
:authenticate_administrator!, | |
:authenticate_user! | |
].find do |m| | |
controller.send(m) | |
end | |
end | |
end |
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...
Yep. I dont think so. Check out the latest version.
As always, you start with something, you let it evolve, you end up somewhere...