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
    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` 
<https://github.com/varvet/pundit/blob/master/lib/pundit.rb#L154> 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

On Thursday, October 4, 2018 at 4:19:47 AM UTC-4, Rob Jonson wrote:
>
> 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'
> https://github.com/rubocop-hq/rails-style-guide#no-direct-model-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 policy
>
> also 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 be
>
> a) 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_action
>
> before_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 allowed
>
> so - 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+unsubscr...@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.

Reply via email to