1
\$\begingroup\$

I am building an app that currently allows the users to have 3 different roles:

  1. Consumer
  2. Merchant
  3. Admin

I have three tables:

  1. user (Name, email, password etc with a status=0/1)
  2. user_role (The list of roles available)
  3. user_user_role (The table joining users to user roles in a many to many relationship. Also a status column indicating if the role is disabled for the user.)

But only two models:

  1. User_model
  2. User_role_model

In my User_model:

/**
 * @param $user_data
 * @param array $roles
 * @return bool|object
 */
function insert( $user_data, $roles = [] ){
    // Hash password
    if ( isset( $user_data['password'] ) ) {
        $user_data['password'] = password_hash($user_data['password'], PASSWORD_DEFAULT);
    }
    if ( $user_id = $this->db->insert( $this->table_name, $user_data ) ) {

        // Add user roles
        if (count($roles)) {
            foreach ($roles as $role_id) {
                $this->add_role($user_id, $role_id);
            }
        }

        return $user_id;

    } else {
        return FALSE;
    }
}

/**
 * Add role to user
 * @param $user_id
 * @param $role_id
 */
function add_role( $user_id, $role_id ){
    $this->db->insert(self::User_user_role_table_name, [
        'user_id' => $user_id,
        'user_role_id' => $role_id,
        'status' => self::Status_active,
        'created_on' => NULL,
        'modified_on' => NULL
    ]);
}

My questions:

  1. Is my database schema ideal? (Each role has access to a specific area with each area's controller extended from a core controller. i.e. Dashboard extends Admin_controller - which extends from a global controller. I don't foresee a need to have complex permissions in a typical RBAC.)
  2. Should I have 1 model instead of 2?
  3. Should I have the roles parameter in my insert function?
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

I think the schema is fine with one possible exception. I don't see any value in the status column in the user_user_role table. The same effect is achieved by removing the row connecting the user and a given role. If the connections isn't there then that role is 'disabled' for that user.

Unless you intend to add functionality that adds, removes or otherwise manages roles then one model is all you need.

Roles as a param to insert makes sense but perhaps a default value (beyond an empty array) should be provided. Otherwise, you could end up with users that don't have permission to do anything. Or, keep the definition $roles = [] and then if(empty($roles)), throw an exception.

I think you have a problem in the line

if ( $user_id = $this->db->insert( $this->table_name, $user_data ) ) {

$this->db->insert() returns either true or false. To get the id for the newly inserted records you will need to use $this->db->insert_id(); perhaps like this.

if ($this->db->insert( $this->table_name, $user_data ) ) {
    $user_id = $this->db->insert_id();
    ...

Also, in the interest of elegant code instead of

return $user_id;
} else {
    return FALSE;
}

use

} //end of if ($this->db->insert(...

return isset($user_id) ? $user_id : FALSE;
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.