Multiple Roles per User

Finally! ... Another great patch.

It looks like you are doing what I would have hoped. A user's permissions are the union of all permissions granted to his roles. That is, if a user has editor and authenticated user roles, he will benefit from the permissions granted to editor and from the permissions granted to authenticated user.

About your open issues:
- i like switching to $user->roles so it is clear that users may have more than one role. I don't care much though.

The patch allows multiple roles per user, but did this mean that if I am administrator, and I give myself also the role of the 'authenticated user', I can switch between these roles? Or do I get both of all permissions?

BTW, just a little question: The default role names ('anonymous user' && 'authenticated user') are not translatable.. Could you also take care of that?

Looks good. Here are some suggestions:

1. I suggest we introduce $user->roles and drop both $user->rid and $user->role.
2. Some contributed modules will need to be updated. When this patch gets committed, you'll want to document the changes at Converting 4.4 modules to HEAD.
3. Can't you use a JOIN here?
   

    $result = db_query("SELECT r.rid, r.name FROM {role} r, {users_roles} ur WHERE ur.uid = '%d' AND r.rid = ur.rid", $user->uid);
   
    $result = db_query("SELECT DISTINCT p.perm FROM {role} r, {permission} p, {users_roles} ur WHERE ur.uid = '%d' AND r.rid = ur.rid AND p.rid = r.rid", $user->uid);
   

4. In SQL queries, you should not put single quotes around %d's. Here are two examples, but there are many more:

     $result = db_query("SELECT r.rid, r.name FROM {role} r, {users_roles} ur WHERE ur.uid = '%d' AND r.rid = ur.rid", $user->uid); 
      // this query is used at two locations
   
     $rolesresult = db_query("SELECT r.name FROM {users_roles} ur, {role} r WHERE ur.uid = '%d' AND r.rid = ur.rid", $account->uid); 
       // can't we use '$result' instead of '$rolesresult'?
    

5. Try using single quotes for constant strings. For example, use 'small' instead of "small".

Do you mean use an explicit INNER JOIN rather than that implied by the comma? I can do that. I just tried to be consistent. User.module uses both INNER JOIN and the comma syntax, so I tried to use whatever was already there when I modified a query.

Yes, please.

This is a habit. I wrap all value types in queries with single quotes as an added measure of protection (i.e. from mischievous user input). There is certainly nothing wrong with it syntactically, but it is not the "standard" for Drupal, so I will change them, I guess.

IIRC, it breaks PostgreSQL compatibility.

Again, I used whatever was already there. User.module is peppered with both single- and double-quoted string constants. Do you want me to fix them all?

I know there are a lot of double-quoted strings left, but we're trying to update them step-by-step. You don't have to update them all, but it would be nice if you could update some (eg. the ones in the code you modified).

Thanks Jim.

First, after applying this patch, update.php is broken. This is because the upgrade scripts attempts to load the current user so it can perform an access check. To work around this problem the user will have to create the users_roles table manually before running the upgrades. I suggest we add instructions to update.php's main page like we did for the session table in Drupal 4.3.0 and the system table changes in Drupal 4.4.0.

Secondly, does everyone understand what selecting multiple roles means and how it works? As we're dealing with permissions, there should be no confusion. I think we should communicate clearly how each role's permissions are 'merged' when multiple roles are selected. I'd add that explanation below the role selection menu.

Third, any idea how many additional SQL queries are introduced when, for example, viewing the main page?

Looking better, every time!

Committed to HEAD. Thanks Jim H. Riggs. :-)

Committed to HEAD. Thanks. Marking this fixed.