Boss Nass
Boss Nass

Reputation: 3522

is there a cleaner way of achieving this?

im attempting to only display certain records from a table based on different information passed through, and if none of the requirements are met it redirects to home. the code is all functioning, just wanted to see how other would have tackled this

if current_user.admin?
  @schedules = Schedule.all
elsif current_user.team_id?
  @schedules = Schedule.find_all_by_team_id(current_user[:team_id])
else
  redirect_to root_path, :status => 301, :alert => "Please contact your club administrator to be assigned to a team."
  return
end

Upvotes: 2

Views: 68

Answers (2)

carpamon
carpamon

Reputation: 6623

You should always move complex logic away from your controllers.

class Schedule
  def self.for(user)
    case user.role #you should define the role method in User
      when User::ADMIN
        scoped
      when User::TEAM
        where(team_id: user[:team_id])
    end
  end
end

in your controller:

@schedules = Schedule.for(current_user)

redirect_to root_path, :status => 301, :alert => "Please contact your club administrator to be assigned to a team." unless @schedules

Upvotes: 6

MurifoX
MurifoX

Reputation: 15089

An approach to your problem.

@schedules = Schedule.all if current_user.admin?
@schedules = Schedule.find_all_by_team_id(current_user[:team_id]) if current_user.team_id?
if @schedules.nil?
  redirect_to root_path, :status => 301, :alert => "Please contact your club administrator 
    to be assigned to a team."
else
  #Your normal redirect
end

Upvotes: 0

Related Questions