views:

30

answers:

2

I have multiple user types in a system that shows each user different views and templates of the stored information, often based on whether they are logged in and what current_user.user_type is. As a result, I have lots of this:

#controller
@project = Project.find(params[:id])
if current_user.user_type == "Company"
  redirect_to :controller => "companies", :action => "home"
elsif current_user.user_type == "Contractor"
  @contractor = Contractor.find(current_user.user_type_id)
  redirect_to :controller => "contractors", :action => "home"
elsif current_user.user_type == "Customer"
  redirect_to :controller => "companies", :action => "list"
end

This is my first Rails project and I am pretty sure this is poor design. What are simple clean ways of doing this in a better way?

A: 

When a user hit login in your application, you must have some code to redirect them somewhere.
This is where you want to put your code. and you probably want to use:

redirect_to companies_path
Nicolas Viennot
+1  A: 

If you're having a lot of code like this, is a code smell that your controller is really serving multiple purposes. Assuming your controller is something like InfoController, which is a REST view for some Info model, ask yourself:

  • What is the central piece of your actions, the data or the user who access it?

  • Are you taking decisions based on who requests the actions? (Like saving, deleting, etc)

  • Can these decisions be taken in the Info model, rather than in the controller?

To me, seems like you should create different controllers for each model, and make just one redirect per action. In your views, you can use things like polymorphic_paths to link up to your controllers.

If you decide not to do this, I'd just put that code in a case statement instead of an if.

Chubas
Chubas, it seems to me that I should just do the case statement. I have places where, for example, someone signs in as a user but they are redirected to a different controller's home page depending on their user_type. If this is simply a redirect issue (and not a data decision one) then a case statement should be sufficient. Does this seem reasonable?
sscirrus