Badges not removed on role change sometimes [#2508974]

I have some automatic addition/deletion of roles going on under Ubercart. When the role is removed by Ubercart I am not losing the associated badge. The UC code is using user_load, user_save and doing everything properly.

The issue is with user_badges reliance on the $account->badges_all variable. This is only set on the first pass of user_load and lost on a subsequent call of user_load due to the use of static variables.

Easy to reproduce from a command line script.

$stdout = fopen('php://stdout', 'w');

error_reporting(E_ERROR);

$_SERVER['REMOTE_ADDR'] = '127.0.0.1';
$_SERVER['SERVER_SOFTWARE'] = NULL;
$_SERVER['REQUEST_METHOD'] = 'GET';
$_SERVER['QUERY_STRING'] = '';
$_SERVER['HTTP_USER_AGENT'] = 'console';

// Bootstrap Drupal
chdir($_SERVER['PWD']);
include_once 'includes/bootstrap.inc';
drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL);

$uid = $argv[1];

$account = user_load($uid);
print "BADGES:";print_r($account->badges);
print "BADGES_ALL:";print_r($account->badges_all);
print "\n---";
$account = user_load($uid);
print "BADGES:";print_r($account->badges);
print "BADGES_ALL:";print_r($account->badges_all);
print "\n---";

This produces the following output when called with an existing uid (with no badges).


BADGES:Array
(
)
BADGES_ALL:Array
(
)
---
BADGES:Array
(
)
BADGES_ALL:
---

There's no value for the $account->badges_all variable.

I'll work on a patch now.

Comment	File	Size	Author
#4	changeuserprog-2508974-2.patch	6.8 KB	dunx

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

dunx CreditAttribution: dunx commented 19 June 2015 at 14:03

Issue summary:

View changes

Comment #2

dunx CreditAttribution: dunx at Webgreen Systems Limited commented 20 June 2015 at 14:14

The code does not do what it's supposed to at all when you add or remove roles programmatically. This is partly to do with its use of static values and partly with the parameters passed to hook_user where the code relies on role change that are not reflected in these parameters. I think I have a solution now. I just need to test a bit more thoroughly through the UI and my own use case to make sure I haven't broken anything.

Comment #3

dunx CreditAttribution: dunx at Webgreen Systems Limited commented 20 June 2015 at 14:14

Assigned:

Unassigned

» dunx

Comment #4

dunx CreditAttribution: dunx at Webgreen Systems Limited commented 21 June 2015 at 14:45

File	Size
changeuserprog-2508974-2.patch	6.8 KB

Patch provided. Let me walk you through it.

user_badges_user (hook_user) function
op - load
Made $badges_all static as it's relied upon elsewhere and not always set.
Change logic so existence of both $badges and $badges_all for the user are required.
Set both.

op - update
Major rework here. The $edit array passed to hook_user doesn't include what the code expects unless it comes from the UI. If you change a user programmatically, you can't tell whether any roles have been changed from any of the variables passed. So, instead I iterate through what roles they now have to make sure they have the badges they should and don't have the badges they shouldn't.

Another issue with static variables in function user_badges_get_badges meant I needed to pass another option to force the re-setting of the static variables.

In that same function there are some variables set that aren't ever needed for the 'select' case usage and that was also being cut short by badge limits, along with the 'all" use case, when they are both supposed to return all values. Corrected that too.

Testing:
Works as expected from the UI when a user is granted and revoked roles.
Works fine within other modules when a user has a new role granted or revoked; their badges are updated as expected. This was what wasn't working before.

Hope this is of use for anybody still on D6. Same issues may apply for D7... no idea.