Order of hook activation, esp. concerning roles

mfredrickson - December 2, 2005 - 16:53

Hello,

I am working on an idea I for Organic Groups I call "autogroups," where groups are created and populated automatically based on the roles in a system.

In looking to implement my solution, I want to act on user objects after updates to a user's role. It would seem the obvious hook to implement is hook_user.

The problem I am having is that my hook is called before the role permissions are updated in the user object. While I could steal the new roles out of the $edit array, I think this would be a bad idea as it forces my code to be dependent on someone else's code (where as the user object is more of an API).

Is there a better way to insert my code into the process after roles are updated? Or should I file a feature request/bug against core to get a hook put in (say hook_afterroleupdate, or similar).

Thanks
-Mark

ps. I am trying to package this as a separate module, so updating the og code is not really an option I'm considering.

» Login or register to post comments

When a user is updated,

coreyp_1 - December 4, 2005 - 00:55

After a user is updated, hook_load() is called. Could you use that?

hook_user with $type = load?

mfredrickson - December 5, 2005 - 19:34

I assume you mean the hook_user function (hook_load is for nodes).

This what I am currently working on but it is a hack, pure and simple. I have to run db queries on every user load (which is really frequent, btw). I cannot find a way to quickly determine if the roles have been updated, without running db queries. I'm not morally opposed to queries, but this seems like a bad use of them.

I think I will file a request against user.module to get a hook for this. My project is not the only one that would benefit from having real-time role update events available.

Thanks for the thoughts.

Oops.

coreyp_1 - December 5, 2005 - 23:55

Yes, I did mean hook_user() with $type = load.

That's the only option I see.

Perhaps a new hook would be beneficial. otherwise you are relegated to many unnecessary queries.

hook_user()

moshe weitzman - December 6, 2005 - 01:04

you want hook_user('insert') and hook_user('update'). see http://drupaldocs.org/hook_user

Close, but not quite

mfredrickson - December 7, 2005 - 23:00

I investigated both of these before I posted. And found them not to work. I subsequently see a way in the user_badges module:

http://drupal.org/project/userbadges

This module has to do something similar to what I want to do: every time roles are updated it has to add a 'badge' to a user.

It works by checking the $edit array and then making changes. I consider this a dangerous practice. What if the $edit values are bad? and are later rejected by some other validation routine? We've created side effects in this function that we can't easily undo. I think touching other parts of the $edit array is a bad idea. A module should only touch it's own additions to the $edit array and not count on other modules to keep those consistent.

Also, this function has to do processing for every update. Roles aren't updated that often. Wouldn't it better to add a flag for them?

I propose a hook_user type: role. The role type would signal that a user's roles have changed. It would be invoked from user.module from a new user_update_roles($user, $role_array) function.

This has the added benefit of standardizing how roles are changed. Currently, this code is duplicated at least 3 times in user.module. A function should be written.

Finally, I'm not sure that the code in user_badges gets invoked when a role is deleted. I would have to test this to be sure.

clarify

moshe weitzman - December 8, 2005 - 01:01

why does my suggestion not work? is something saving the role change without passing through hook_user(). please clarify. We don't need a special operation for a role change. Fixs the core bug if there is one. It is OK if you do a little processing on every user update. It is an infrequent action. Just avoid an INSERT/UPDATE query on every instance.

Ok

mfredrickson - December 8, 2005 - 02:17

I admit I was vague in my previous post. I'll write some code to demonstrate what I was talking about and post it tomorrow.

Sit tight. :-)

More information

mfredrickson - December 8, 2005 - 15:31

Ok,

I sat down last night a spent some rigorously investigating this "problem."

My conclusion: I was a bit hasty in calling for another hook, but I think there is still an underlying problem in the way roles are handled.

Process

I created a simple little module that implemented hook_user. The code was simply:

<?php
function testmod_user($type, &$account, &$edit, $category) {
drupal_set_message("For $account->uid, a $type operation was invoked.");
}
?>

This let me see what was going on. This helped me gather some basic data about what happens when 'update' is invoked, and when.

I later extended it with the devel modules's dprint_r function to print out the various arguments.

Results

Most of the time, 'update' will suffice to get role changes. The changes are in the $edit[role] array, and these can be used to update users.

However, deleting a role does not call 'update' on each user with that role.

Conclusion

First, I am an impatient jerk who would rather have someone else code a new hook than do the work himself. ;-)

That said, I think there is still a bug in user.module: roles can be deleted but modules are not alerted.

I can see two solutions to this problem:

Before deleting a role, user.module invokes hook_user on each user with that role.
Modules that need to know about roles, duplicate the role table and poll (via cron most likely) to see if any roles have disappeared.

I think solution one is clearly the better of the two. I would propose the following solution:

Pull out the role code into a separate function. It is duplicated in several places. Putting in it's own function may avoid similar problems in the future.
Recode the role deletion code to

Get an array of all users with that role.
Iterate over the array invoking hook_user('update') on each user with the new set of roles
Delete the role

I hope that makes things more clear. There is still a bug, but not where I thought it was originally.

good thinking

moshe weitzman - January 19, 2006 - 17:31

yes both of those points make sense. fyi - i have a role related patch in the queue, but it doesn't really help here: http://drupal.org/node/44379

Why I'm asking

mfredrickson - December 7, 2005 - 23:05

I thought I'd cross post the issue I'm working on:

http://drupal.org/node/28128