Mass user operations don't trigger hook_user including not immediately booted mass blocked users [#99426]

Introduction:

I need to trigger an operation on each user modifiaction. Specificly when a user has a role change, it checks for a specific permission and runs a DB query.

As far as single user operations my module is working perfectly. When a user is added to a role with the permission the script is run.

Problem:

There is a major usability problem if an admin decides to use mass-user-operations because the add-role/remove-role function totally ignores my triggers.

The function "user_multiple_role_edit($accounts, $operation, $rid)" does not trigger any user changes as hook_user would.

I need to be able to trigger on role changes. Even when they are done by mass-operations.

Solution:

Add a hook_multiple_role_edit when the changes are being made.

Would solve my problem.

Comment	File	Size	Author
#7	mass_0.patch	2.82 KB	RobRoy
#7
#6	mass.patch	2.81 KB	RobRoy
#6
#1	user_mass.patch	3.21 KB	RobRoy
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

RobRoy CreditAttribution: RobRoy commented 24 November 2006 at 10:20

Title:	user_user_operations hook request	» Mass user operations don't trigger hook_user including not immediately booted mass blocked users
Version:	5.0-beta1	» 5.x-dev
Category:	feature	» bug
Status:	Active	» Needs review

File	Size
user_mass.patch	3.21 KB

I think we just need to invoke the update hook on each user as we process them, or do a full user_load()/user_save(). I know that might be a performance hit, but mass user operations are only done by admins on rare occasions so we should go the cleanest route and not worry too much about performance. We definitely don't need a new hook.

It seems like since there is so much logic in user_save() regarding different changes including blocking users (immediately deleting their session), we should be calling that. So currently it looks like if we mass block currently logged in users, they aren't booted like they are when you do a single block on the user edit form.

This patch replaces all the nice/efficient mass updates with user_loads/saves which I KNOW is probably a little step back in performance, but this seems like the cleanest solution although it could use some optimization probably. I haven't put too much thought into it this, so am open to opinions.

Comment #2

imerlin CreditAttribution: imerlin commented 24 November 2006 at 12:46

Read through your patch and it looks good. I agree that this implementation is better than making a new hook for mass operations and the performance hit should be acceptable.

Nice job...

Comment #3

moshe weitzman CreditAttribution: moshe weitzman commented 28 November 2006 at 13:25

Status:

Needs review

» Needs work

definately the right approach.

- i don't think we can call node_load($account) like that. $account can't be trusted AFAICT. use an int() cast ahead of $account.
- personally, i don't need the same comment in each function stating that no menu clear is needed

Comment #4

pwolanin CreditAttribution: pwolanin commented 28 November 2006 at 16:07

@Moshe - I don't understand you comment, maybe you mean this code:

$account = user_load(array('uid' => $uid));

would be better as:

$account = user_load(array('uid' => (int)$uid));

Comment #5

moshe weitzman CreditAttribution: moshe weitzman commented 28 November 2006 at 22:12

pwolanin: yes, you understood me well.

Comment #6

RobRoy CreditAttribution: RobRoy commented 29 November 2006 at 09:38

Status:

Needs work

» Needs review

File	Size
mass.patch	2.81 KB

Implemented those two suggestions. Yeah, I left those comments in there knowing I was going to do a re-roll and just wanted to get some feedback. I just took them all out. Not sure why we can't trust $uid, but I casted em anyways. If that is the case then I added an $account !== FALSE in there to make sure it loaded alright.

Look better?

Comment #7

RobRoy CreditAttribution: RobRoy commented 29 November 2006 at 09:40

File	Size
mass_0.patch	2.82 KB

Missed one (int) cast.

Comment #8

moshe weitzman CreditAttribution: moshe weitzman commented 29 November 2006 at 15:05

Status:

Needs review

» Reviewed & tested by the community

tested and works as designed ... code looks good.

Comment #9

moshe weitzman CreditAttribution: moshe weitzman commented 29 November 2006 at 15:07

After this goes in, it would be helpful to email any contrib module maintainers who implement the user operations hook and ask them to emulate this new pattern.