Multi-User Roles in Laravel

Question

I am very new to Laravel. I am building a website where multiple users can have one or more roles. I have a users table and a user_roles table. Each user_roles column has a user_id and an enum field where they can be a "referee", "coach", or "player". Whenever I want to check if the User is a certain member of a role I call a referee, coach, or player function in my model from the controller. Is my structure and code efficient, or is there any way to clean it up?

User.php

public function roles()
{
    return $this->hasMany('App\Role');
}

public function getRoles()
{
    $roles_collection = $this->roles;
    $roles = [];
    foreach ($roles_collection as $role) {
        $roles[] = $role['role'];
    }
    return $roles;
}

public function referee()
{
    if (in_array('referee', $this->getRoles())) {
        return true;
    } else {
        return false;
    }
}

public function player()
{
    if (in_array('player', $this->getRoles())) {
        return true;
    } else {
        return false;
    }
}

public function coach()
{
    if (in_array('coach', $this->getRoles())) {
        return true;
    } else {
        return false;
    }
}

Example of checking the User role, called from the controller:

if (Auth::user()->referee()) {
       echo 'referee';
}

Answer 1

I do not have much experience with Laravel, so I will comment on your code as it is presented, but keep in mind that I may miss something which is already handled by the framework.

The first thing which strikes my mind is the getRoles() method. To me it seems you are doing the work of the native function array_column(). This means the method can be reduced to:

public function getRoles() {

    return array_column($this->roles, 'role');

}

I even think you could extract only the role names into a property. I am not familiar with how Laravel deals with databases, but I am sure you can extract only the role names associated with the user directly from the database. If you then extract the roles during instantiation of the class, you can remove the getRoles() method altogether.

---

I would also remove the methods player, coach and referee and replace them with a generic method. This is to make it easier in the future if you add/remove new roles. Consider the following method.

public function hasRole($role) {

    return in_array($roles, $this->getRoles());

}

You would then check if a user by simply writing the role name.

if(Auth::user()->hasRole('referee')) {
    // Do awesome stuff here
}

This would reduce the number of methods inside your class and make it so you wouldn't have to edit your exiting code just to add a new role. This follows the open-closed principle which states that a class should be open for extension, but closed for modification.

Now you can also pass the role name as variable (if you ever need to) without the fear of an undefined method error as the method would simply return false.

Happy coding!

Answer 2

You shouldn't have individual functions for each role, and a user probably shouldn't be assigned more than one role. Especially seeing as you can't be all three

---

public function getRoles()
{
    $roles_collection = $this->roles;
    $roles = [];
    foreach ($roles_collection as $role) {
        $roles[] = $role['role'];
    }
      return $roles;
}

In another function you use a $this-> style item as a parameter, so I would suggest removing $roles_collection and staying consistent.

---

For referee(), player() & coach(), you can use boolean expressions as valid variables:

public function referee()
{
    if (in_array('referee', $this->getRoles())) {
        return true;
    } else {
        return false;
    }
}

into:

public function referee()
{
    return (in_array('referee', $this->getRoles()));
}

repeated for all of the functions.