Making controller methods available to view, but slightly differently?

Question

In my header, I have a link to a Materials page that I don't want just anyone to access, so I need a condition set in my view and a before_filter in my MaterialsController.

I wrote a successful helper method for the view, but in the spirit of DRY, I wanted to write that method just once in the ApplicationController and make it available to the view using helper_method:

ApplicationController:

helper_method :user_is_admin_or_teacher_or_student_with_a_class

def user_is_admin_or_teacher_or_student_with_a_class
  if user_signed_in? and ( current_user.admin || current_user.type == "Teacher" || ( (current_user.type == "Student") and current_user.groups.any? ) )
  else redirect_to root_path, alert: "You are not authorised to view the Materials page."
  end
end

This worked perfectly in my MaterialsController:

before_action :user_is_admin_or_teacher_or_student_with_a_class, only: [:index, :show]

It has the desired effect.

Moving onto the helper side of things, I placed this in my view (_header.html.erb):

<% if user_is_admin_or_teacher_or_student_with_a_class %>
  <li><%= link_to "Materials", materials_path %></li>
<% end %>

But when trying to load my homepage in the browser, I get the 'this page has a redirect loop' browser error. I assume this is to do with the redirect_to root_path command in the controller method.

My crude solution was to remove the helper_method declaration from ApplicationController and write an almost identical method in ApplicationHelper:

def user_is_admin_or_teacher_or_student_with_a_class?
    user_signed_in? and ( current_user.admin || current_user.type == "Teacher" || ( (current_user.type == "Student") and current_user.groups.any? ) )
end

Of this works, but it's not DRY. How can I DRY this up and write the method just once and use it in Controllers and in Views?

You could put the common code in your model and then invoke in your `before_action` and your view. — steve klein, May 05 '15 at 00:51

Krzysztof Rygielski · Accepted Answer · 2015-05-06T19:51:48.227

I'd split the logic. You could put this method in your model (I assume it's the user):

class User < ActiveRecord::Base
  # ...

  def can_view_materials?
    # note no need for parentheses here if '&&' is used instead of 'and' operator
    admin || type == "Teacher" || type == "Student" && groups.any?
  end

  # ...
end

then in MaterialsController:

before_action :require_authorization_to_view_materials, only: [:index, :show]

def require_authorization_to_view_materials
  unless user_signed_in? && current_user.can_view_materials?
    redirect_to root_path, alert: "You are not authorised to view the Materials page."
  end
end

Finally, in your view:

<% if user_signed_in? && current_user.can_view_materials? %>
  <li><%= link_to "Materials", materials_path %></li>
<% end %>

It's just polished version of your approach. Could be achieved in few other, maybe better ways, introducing additional authorization logic, user roles, etc. But it all depends how complex your solution is going to be and if you'll really need it.

Note no helper methods made from controller methods ;)

If you REALLY want to make a controller/view common method to check user permissions you could do this in ApplicationController:

helper_method :user_can_view_materials?
def user_can_view_materials?
  user_signed_in? && current_user.can_view_materials?
end

and in MaterialsController:

def require_authorization_to_view_materials
  redirect_to root_path, alert: "You are not authorised to view the Materials page." unless user_can_view_materials?
end

and in view:

<% if user_can_view_materials? %>
  <li><%= link_to "Materials", materials_path %></li>
<% end %>

Very nice. One question - why would you put code for the method in `ApplicationController` instead of `MaterialsController`? — steve klein, May 05 '15 at 12:07
You mean `require_authorization_to_view_materials` method? You can put it in `MaterialsController` of course. The `user_can_view_materials?` should be in `ApplicationController` however, as it's used in the `_header` partial, which I assume is rendered always, with navigation for entire page etc. (I hope it makes sense ;) — Krzysztof Rygielski, May 06 '15 at 08:47
Thanks. I used the first technique (model method and controller method, plus invocation in the controller and view). There IS a small mistake in your MaterialsController method - it should be user_signed_in? && (not user_signed_in? || ). This is only the second time I've used a model method - I didn't know you could just write 'admin' and 'type' etc without doing self.admin, self.type or something? Is this shorthand? Could you link to a relevant piece of documentation for me? — Yorkshireman, May 06 '15 at 16:43
Of course you're right. It should be `&&` (`unless` confuses me sometimes...). I've updated the example. As for attributes, they can be called like any other method to return their value. Best and most concise answer to question when to use `self` or not you can find here: http://stackoverflow.com/a/4701389/2361698 — Krzysztof Rygielski, May 06 '15 at 19:56

daslicious · Answer 2 · 2015-05-05T02:12:55.177

0

You're root_path points to the same controller you're redirecting from. Change the redirection path or your root path

# routes.rb
Rails.application.routes.draw do
  root change_whatevers_here
end

or

redirect_to change_whatevers_here, alert: "You are not authorised to view the Materials page."

edited May 05 '15 at 02:12

answered May 05 '15 at 02:05

daslicious

1,535
10
18

Making controller methods available to view, but slightly differently?

2 Answers2