Update: new controller methods inspired by Nicolás Sanguinetti
All to often Controllers verify if a certain user can say “create or edit a resource”, which at first seems to be a natural place for this kind of logic, but alas it is not!
When building your views you need access to the same knowledge, if the user can edit this movie, show the edit link.
Is it mine?
At first i solved this with a simple is_mine?(obj) on the user, and let the user decide if he can edit a record.
def is_mine?(obj) case obj when Movie then obj.id == id when Participation then obj.movie.id == id ..... end
Can i read/write it ?
But this still left too much logic for my helpers and controllers, so i added another bit: read/write, where a new User symbolizes no user, so user.can_write?(obj) returns false if it is a new_record.
def can_read?(obj) raise "new objects cannot be read" if obj.new_record? #new_records can only be created, never viewed... case obj when Address,Order,OrderItem #not readable by others/public is_mine?(obj) #i can read an order/address if it is mine else not new_record?#only a logged in user can read something... end end def can_write?(obj) ... end
Controller-logic be gone!
Now in my controller i use can_read for show, can_write for edit/update/create/new/destroy, which i inherit on all controllers.
before_filter :can_read, :only => [:show] before_filter :can_write, :only => [:destroy,:update,:edit,:new,:create] def can_write access_denied unless can_write?(requested_object) end def can_write?(object) user = current_user || User.new user.can_write?(object) end helper_method :can_write?
The magic starts in requested_object: Create a new object of the current controller class, or find it by id. This can be a bit tricky with nested resources, but can be as simple as (see below) with make_resourceful.
def requested_object case params[:action] when 'new','create' then build_object else current_object end end
Clean Tests
Testing, once cumbersome “get :new, response.should”-madness now looks like this, which is orders of magnitude faster, easier to write and more fun to read.
it "reads/writes movies" do #changeable by owner can_read_and_write(@owner,@movie) can_read_not_write(@other,@movie) can_write(@owner,Movie.new) cannot_read_and_write(@no_user,@movie) cannot_write(@no_user,Movie.new) end ... def can_read(user,item) user.can_read?(item).should be_true end ...
You got the point static rules are ridiculous 🙂
Nice. The only thing I don’t like is how the business logic gets lost inside the model’s can_read? and can_write? methods (my User classes tend to be among the largest in my apps, since users are usually tied to so many things).
Probably some class method (ala “has_many”) would clean that up.
class User lambda {|o| is_mine?(o) }
can_read_on :everything, :unless => lambda {|o| o.new_record? }
end
What do you think? Those can generate dinamycally the can_read/write methods to match the provided rules, and will be much more declarative than burying the logic in the middle of the class.
argh, your blog ate my content 😦
http://pastie.org/234460
(I’m unsure if instance_eval’ing those guards makes sense or it’s better to pass the user and the object to them, but even so, you get the idea :))
sounds good, this syntax seems like a good step forward
-business logic gets lost inside the [user]
i only keep the simple logic inside the user.rb and move everything more complex into the corresponding model (order.is_locked?)
updated the controller helpers thanks to nicolas 😉