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?

Answer 1

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 %>

Answer 2

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."