Display highest attained role?

Hi Khalid,

Thanks for cleaning this up. I like how you are handling the uid & points internally. Unfortunately there's some errors :(

Drupal is saying this:

* warning: Invalid argument supplied for foreach() in /Library/WebServer/Documents/drupal/sites/all/modules/userpoints_contrib/userpoints_role/userpoints_role.module on line 191.
* warning: array_pop() [function.array-pop]: The argument should be an array in /Library/WebServer/Documents/drupal/sites/all/modules/userpoints_contrib/userpoints_role/userpoints_role.module on line 202.

I believe is has to do with the fact that asort may not work on multidimensional associative arrays?

  // Get a list of roles, and sort them
  $role_points = asort(_userpoints_load_role_points());

I too thought the $role_points array should be sorted, however my research shows ma arrays need to be sorted using usort(). Perhaps we need to go that way?

Anyway, I had to massage your code as follows to get a good output -- however, now it's returning the first role above the user's highest role.

function userpoints_role_highest_role($uid = 0) {
  global $user;

  // If no user specified, then it is for the current user
  if (!$uid) {
    $uid = $user->uid;
  }

  // Get the points the user has
  $current_points = userpoints_get_current_points($uid);

  // Get a list of roles
  $role_points = _userpoints_load_role_points();

  foreach($role_points as $role) {
		
    if ($current_points > $role['points']) {
      continue;
    }
    else {
      return $role['name'];
    }
  }

  // The user has more points than the highest role, so return that
  $max = array_pop($role_points);
  return $max['name'];
}

I believe it's skipping (continuing) for each roll the user is in, and then returning the first role the user isn't in. This is why I put all the roles the user is in into an array and used the max() return the one with the higest points value. I thought that may not be the simplest way to do it, but I couldn't figure out a better way.

Hi Khalid,

Sorry. No joy. Still returning the wrong role.

Here's a version does work. It's a combination of your enhancements and my original code:

/**
 * Public function to return the name of the highest * role a user has reached.
 *
 * @param $userpoints
 *  current points for user
 *
 * @return 
 *  name of their role
 *
 */
function userpoints_role_highest_role($uid = 0) {
  global $user;

  // If no user specified, then it is for the current user
  if (!$uid) {
    $uid = $user->uid;
  }

  // Get the points the user has
  $current_points = userpoints_get_current_points($uid);

  // Get a list of roles
  $role_points = _userpoints_load_role_points();
  $roles_within = array();
  foreach($role_points as $role) {
    //put all the roles user is in into an array.
    if ($current_points > $role['points']){
      $roles_within[ $role['points'] ] = $role['name'];
    }
  }
  // return the maximum role name 
  // max() returns the value of the largest key
  return max($roles_within);
}

Hi Khalid,

"Thanks for your continued perseverance."

No problem. I enjoy doing this!

OK...Let's see if I can answer this...

"Two things concern me:

1. The _userpoints_load_role_points() function does not guarantee that the returned array is sorted by points. So, what happens if they are unsorted? We get back the wrong role, and it will be very hard to debug such a case."

  // Get a list of roles
  $role_points = _userpoints_load_role_points();

I don't believe the array $role_points needs to be sorted. The array can be totally out of order and the loop will still iterate over the entire array, putting all elements into $roles_within that fit our condition. In our case the condition is if the userpoints of the current user is higher than the value of userpoints threshold for this role.

  foreach($role_points as $role) {
    //put all the roles user is in into an array.
    if ($current_points > $role['points']){
      $roles_within[ $role['points'] ] = $role['name'];
    }
  }

So now we have all the roles the user is in put into $roles_within as an associative array with points as the key and name as the value. Now if we run the max() function on $roles_within

  return max($roles_within);

it will return the value (name) of the element with the largest key (point) value. Again, I believe the key thing here (no pun intended) is that the $roles_within array doesn't need to be sorted...max() will find the largest value for us.

"2. Is there a possibility of edge cases where you reach the return at the end with nothing in the $roles_within array, and hence the max() returns nothing? What can we do about that?"

I believe this function will always return a value as long as the user has a userpoints value that fits within one of our userpoints role thresholds. How do we ensure this? For starters, I believe it would be up to the administrator to be sure they have the roles points set up properly. Beyond that...we must ensure the user's userpoints never goes below the lowest userpoints role. This can be achieved by the following:

1. make the lowest role userpoint value == -1 and use the do not allow negative userpoints contrib module.
2. make the lowest role userpoint value equal to a very low negative number? eg -1000000

What do you think?

"For #2, I don't like magic values. They are trouble in the long run. We can find the min() of the array, and initialize with that. What do you think?"

I will look into it -- could you explain a bit more what you're thinking? Which array?

Thanks,
Mike

Hi Khalid,

Sorry for the long delay! I've been really busy and am just getting back to this...

How does this look? I believe it eliminates the need for magic values. If it looks good to you, I'll supply a new patch...

Mike

/**
* Public function to return the name of the highest * role a user has reached.
*
* @param $userpoints
*  current points for user
*
* @return
*  name of their role
*
*/
function userpoints_role_highest_role($uid = 0) {
  global $user;

  // If no user specified, then it is for the current user
  if (!$uid) {
    $uid = $user->uid;
  }

  // Get the points the user has
  $current_points = userpoints_get_current_points($uid);

  // Get a list of roles
  $role_points = _userpoints_load_role_points();
	
  $roles_within = array();
  foreach($role_points as $role) {
    //put all the roles user is in into an array.
    if ($current_points > $role['points']){
      $roles_within[ $role['points'] ] = $role['name'];
    }

    // also put each role into a placeholder array 
    // just in case user doesn't fit any roles.
    // ignore roles with points value = 0
    if ($role['points'] != 0){
      $roles[ $role['points'] ] = $role['name'];
    }
  }

  if (count($roles_within) > 0){
    // return the maximum role name
    // max() returns the value of the largest key
    return max($roles_within);
   }
   else {
     //return lowest role
     return min($roles);
   }
}