Thursday, October 4, 2018
I agree with Rob that it is in the realm of taste and preference. I wouldn't have something like this in my view:
users = User.where(active: true).join(:transactions).merge(Transaction.where('total > 1000'))
form.collection_select users, :id, :name
This would be putting model level stuff in my view as the guide Rob linked to recommended against. That first line should be wrapped up in a model scope or something.
But in the case where I simply want all scoped users I think putting `policy_scope(User)` in the view is ok. You could create a helper method maybe like:
def scoped_users
policy_scope User
policy_scope User
end
Then in the view pass `scoped_users` to `collection_select`. But is that really much different than `policy_scope(User)`. IMHO the view is less interacting with the model layer but declaratively specifying to the helper how it should interact with the model layer.
Also the fact that Pundit has explicitly created this helper method of `policy_scope` means it considers it ok to call from the view.
Again this is getting in the realm of tasted. If I started coding in an app and I saw that the policy_scope was called in the controller and the result was passed to the view I wouldn't think bad of it.
Eric
Hi John,firstly - I don't know pundit, so this is only general advice.secondly - we're definitely in the realm of taste and preference here rather than 'ok' and 'not ok'having said that, my two pence:the rails style guide says 'Never call the model layer directly from a view'I like to think of views as pieces that will do their job of displaying a thing with minimal worrying about the details of the thing. The view shouldn't be worrying about things like user policyalso I wouldn't want authorisation logic in the views just because it is easy to lose track of (and that stuff is important).my suggestion would bea) You say you're calling a lot of policy_scope(User)this sounds like something where a lot of your calls probably need to find a single scoped user at the start. If this is the case, use a before_actionbefore_action :get_policy_scoped_user, only: [:show, :update, :destroy]I'd be tempted to have that before_action assigning an @policy_user which I can then pass down to the view.this keeps the authorisation stuff in the controller, and the view can just display a user's stuff without worrying about what is allowedso - back to your select. I'd write the view so that it will work when passed any kind of user and doesn't know or care what kind of user it is dealing with.It just so happens that your controller only passes down a scoped user.
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-talk+unsubscribe@googlegroups.com.
To post to this group, send email to rubyonrails-talk@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/2f473c1e-706d-4998-bb63-8bff53fdd73a%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Subscribe to:
Post Comments (Atom)
No comments:
Post a Comment