Factoring the Ruby Way

Question

I have Rails site where I've rolled my own authentication with a User class, but to address the need for different roles I have separate Customer, CustomerServiceRep, and Repairman classes. The User table has a user_type column that indicates what type of user it is.

My question is how to take advantage of Ruby to most efficiently code different views that correspond to the user. For instance, after logging in, I will direct each user to a different place based on their user_type.

I could do it this way in my SessionsController create action, but I'm pretty sure it's the "wrong" way:

if user.user_type == 'customer'         
  redirect_back_or root_path
elsif user.user_type == 'repairman'
  redirect_to repairman_panel_path, :flash => { :success => "Welcome Back Mr. #{user.repairman.last_name}" }
elsif user.user_type == 'customer_service_rep'
  redirect_to customer_service_panel_path, :flash => { :success => "Welcome Back #{user.customer_service_rep.first_name}" }
end

Is my intuition right that there's a much better way to use symbols or some such thing to avoid listing out if-else structures every time I want to funnel every user through the same system then back out to their appropriate areas?

Answer 1

Although it is not recommended to mix models with your URL structure, unless you have vanity URLs that you plan to keep around, then you could store this info in the model classes. Let's take a single class as an example - Repairman.

class Repairman
  # this allows you to use route helpers inside the Model
  include Rails.application.routes.url_helpers

  def greeting
    "Welcome Back Mr. #{last_name}"
  end

  def home_path
    repairman_panel_path
  end
end

Define a greeting and a home_path method in each class. Now when redirecting a user you can rely on these two methods without worrying about what type of user it is.

Inside the SessionController#create, you could now do,

class SessionController
  def create
    ..
    redirect_to user.home_path, flash: { success: user.greeting }
  end
end

Answer 2

If this switch is used in multiple places in your controller, you could consider using a mixin for your controller. Just define a new module in a different file and mix it in to your controller. You could have a method named redirect_for_user.

module ControllerMixin
  def redirect_params_for_user user
    case user.user_type
    when 'customer'
      root_path
    when 'repairman'
      repairman_panel_path, :flash => { :success => "Welcome Back Mr. #{user.repairman.last_name}" }
    when 'customer_service_rep'
      customer_service_panel_path, :flash => { :success => "Welcome Back #{user.customer_service_rep.first_name}" }
    end
  end
end

Then in your controller you just include it and use it:

class YourController < ApplicationController
  include ControllerMixin

  def your_action
    ... #logic here
    redirect_to redirect_params_for_user(user)
  end
end

Keeps you from repeating yourself and separates it nicely for testing.

Answer 3

The most obvious refactoring of this is to switch it up and use a case statement:

case (user.user_type)
when 'customer'         
  redirect_back_or root_path
when 'repairman'
  redirect_to repairman_panel_path, :flash => { :success => "Welcome Back Mr. #{user.repairman.last_name}" }
when 'customer_service_rep'
  redirect_to customer_service_panel_path, :flash => { :success => "Welcome Back #{user.customer_service_rep.first_name}" }
end

That's useful for when you're comparing the same thing against many things. It also has the advantage of never modifying that attribute as can be the case if you mistakenly use = instead of ==. Using symbols wouldn't necessarily be much of a gain here if the user_type is stored as a string anyway.

Generally having this sort of branching logic is a sign you're not doing things properly from an Object Oriented perspective, but in the MVC world you sometimes have to compromise. It would be more ideal if you had a method on user you could call and dispatch on rather than hard-wire the logic based on user_type. For instance, split this up:

case (user.home_action)
when :repairman_panel
  redirect_to repairman_panel_path, :flash => { :success => "Welcome Back Mr. #{user.repairman.last_name}" }
when :customer_service_panel
  redirect_to customer_service_panel_path, :flash => { :success => "Welcome Back #{user.customer_service_rep.first_name}" }
else
  redirect_back_or root_path
end

You can then define a home_action method that figures out what "kind" of user you're dealing with:

def home_action
  case (self.user_type)
  when 'repairman'
    :repairman_panel
  when 'customer_service_rep'
    :customer_service_panel
  else
    :root
  end
end

Splitting up the logic this way makes it easier to test since you can verify the correct home_action behavior independently of the actual redirect.