Not sure if this is a bug - but...

I have 3 roles:

anonymous
authenticated
staff

Some of the users are members of the 'Staff' role, but also get the authenticated role as standard.

I've then assigned weights to the roles:

anonymous 0
authenticated -1
staff -5

So based on the usual way weights work - when I call $role = module_invoke('role', 'weights_get_highest', $user->roles); I should get back array(2) { ["rid"]=> int(3) ["name"]=> string(5) "staff" }

However I'm getting back array(2) { ["rid"]=> int(2) ["name"]=> string(18) "authenticated user" } - as if the weights are being treated the opposite way around to the rest of the Drupal system (lower weights float to the top of the list) ?

The problem (if it is a problem) is probably down to the way the highest role is found, "// run through $roles, returning FIRST role matched" - maybe the array needs to be ordered correctly so that negative weights are first?

I wish to use the module with path_access but this is causing a bit of a problem - unless I've mixed something up!

CommentFileSizeAuthor
#11 role_weights_reverse3.patch3.39 KBpfaocle
#5 role_weights_reverse2.patch1.65 KBpfaocle
#4 role_weights_reverse1.patch1.67 KBpfaocle

Comments

budda’s picture

budda
Primary language English
Location Cheshire
commented
Status: Active » Needs review

A call to array_reverse() might be the cure:

function role_weights_get_highest($roles) {
  $role_weights = _role_weights_get_weights();
  arsort($role_weights);
  $role_weights = array_reverse($role_weights, TRUE);

  // run through $roles, returning FIRST role matched
  foreach ($role_weights as $rid => $weight) {
    if (array_key_exists($rid, $roles)) {
      return array('rid' => $rid, 'name' => $roles[$rid]);
    }
  }
}
pfaocle’s picture

pfaocle
He/Him
Primary language English
commented
Category: bug » feature
Status: Needs review » Active

Hmm - I think the behaviour is correct based on my original use for the module - the higher the weight, the more 'important' the role is. In your example, I would have set the weights something like:

anonymous 0
authenticated 1
staff 5

However, its a good point about weights elsewhere in Drupal: we should be consistent. Please feel free to comment further.

I've just got back from a week away so should get around to looking at the module's issue queue soon.

budda’s picture

budda
Primary language English
Location Cheshire
commented

Yeah I can see where you were originally coming from with the weight setting, but keeping the site admin interface consistent with the user interface is important.

pfaocle’s picture

pfaocle
He/Him
Primary language English
commented
Version: master » 5.x-1.x-dev
Assigned: Unassigned » pfaocle
Category: feature » task
Status: Active » Needs review
StatusFileSize
new role_weights_reverse1.patch1.67 KB

OK, I think we should reverse them to be more in line with other Drupal weights. I'll roll a couple of patches for both the 4.7 and 5.x branches, then release a 1.1 version of each with this in.

Note that as well as a quick array_reverse() call, the patch now includes a different initial weight for the authenticated role and an update to essentially reverse any existing role weights stored, so the user doesn't have to re-shuffle them all around.

Test0r, plox.

pfaocle’s picture

pfaocle
He/Him
Primary language English
commented
StatusFileSize
new role_weights_reverse2.patch1.65 KB

Silly me. arsort() -> asort() and lose the array_reverse call.

budda’s picture

budda
Primary language English
Location Cheshire
commented
Priority: Normal » Critical

Nearly a year on from the original issue :)

Just patched role_weights.module but am still having the same issue.

Here's some test code:

global $user;
print "user has:";var_dump($user->roles);
  $role = module_exists('role_weights') ? module_invoke('role_weights', 'get_highest', $user->roles) : array('name' => $user->roles[1]);
print "\nhighest:";var_dump($role);

And the output is:

user has:array(2) { [2]=> string(18) "authenticated user" [5]=> string(6) "member" }
highest:int(2)

Role id 5 "member" has a weight of -5, whilse role id 2 "authenticated user" has a weight of -1.

This confusion with the weights issue renders the module pretty useless at present, and breaks Path_Access from working fully too. Hopefully we can get to the bottom of this soon. Cheers.

budda’s picture

budda
Primary language English
Location Cheshire
commented
Status: Needs review » Reviewed & tested by the community

Update: it appears the patch words- however the database update messed up my existing role weights.
It changed my "member" role from weight -5 to +5, yet the "authenticated user" role stayed at -1

Either way - the patch works and is good to go IMO.

pfaocle’s picture

pfaocle
He/Him
Primary language English
commented
Status: Closed (fixed) » Reviewed & tested by the community

Thanks for testing. I tried to re-create your update problem and couldn't, I'm afraid: the simple update seemed to work fine. I'll be adding in a note to the project page/README.txt anyway regarding changing the 'highest' weight, so hopefully this'll reduce any confusion.

Before I commit this, i think the word 'highest' in the function name is the only ambiguous thing left - we're actually (in numerical terms) now returning the lowest weight, but what we're saying we're returning is the 'best', or 'most overriding' weight for a user's set of roles. Any suggestions for changing the function name? I really think 'highest' doesn't really make sense any more.

pfaocle’s picture

pfaocle
He/Him
Primary language English
commented

Oops, messed up that comment, Should've read:

"Before I commit this, i think the word 'highest' in the function name is the only ambiguous thing left - we're actually (in numerical terms) now returning the lowest weight, but what we're saying we're returning is the 'best', or 'most overriding' weight for a user's set of roles. Any suggestions for changing the function name? I really think 'highest' doesn't really make sense any more."

pfaocle’s picture

pfaocle
He/Him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Finally. Please update to 5.x-1.1!

pfaocle’s picture

pfaocle
He/Him
Primary language English
commented
StatusFileSize
new role_weights_reverse3.patch3.39 KB

And the patch I committed.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.

Status: Reviewed & tested by the community » Closed (fixed)

Correcting issue status, see #2125263: Issue status not taken from last comment during D7 migration.