Closed (fixed)
Project:
Auto Assign Role
Version:
5.x-1.1
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Reporter:
Created:
26 Feb 2008 at 02:02 UTC
Updated:
7 Sep 2008 at 07:23 UTC
Jump to comment: Most recent file
I have been using the auto assign role module for a while.
Recently, I have enabled Login Toboggan which works great.
However, there is a problem: auto assign role give the new role during the Toboggan pahse 1 ( REGISTRATION), and not during phase 2 (CONFIRMATION). It means that before confirmation, my users have got 2 roles: the toboggan minor role and the full power role given bu the auto assign role module....
So my request: could loggin toboggan module implement a new feature? once the users have confirm their application, could they be assign to a specific role ( the same way they are upon registration).
Many thanks for a wonderful module :)
Expat9
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | autoassignrole.module.patch | 3.27 KB | rmpel |
Comments
Comment #1
hunmonk commentedi have no interest in coding this right now, but i would be willing to review a patch for it if it shows up in this thread, and commit it to the module if i think the implementation is quality.
Comment #2
hunmonk commentedComment #3
Delta Bridges commentedOk thanks for your prompt reply.
I don't have the ability to create a patch myself, but if anyone post one here, I will surely test it and report feedback.
Thanks again,
Expat9
Comment #4
fletch11 commentedI have found this same issue. It seems like logintoboggan may be even more powerful if it integrated the auto assign role function into the registration confirmation phase (step 2). Alternatively, if there was a way to hook the selected user role, per auto assign role, into the users role that would be assigned upon confirmation that would also be great. However, it really seems like these modules should be combined in some fashion.
Thanks for another great contribution to the Drupal community.
Comment #5
hunmonk commentedfrom
function logintoboggan_validate_email():this functionality is in both 5.x and 6.x -- it's been around awhile. what that means is that any other module can look for
$account->logintoboggan_email_validatedin the 'update' op of hook_user(), and determine if the account has just been validated.if other modules want to extend functionality to the validation process, they should leverage this method. i'm not going to add special cases to the module when this method exists.
reassigning to Auto Assign Role, as the request is more appropriate for that queue.
Comment #6
meeotch commentedHoly *crap*, it took me forever to track this issue down to this particular post! Tons of threads out there, none of them with a working solution. Anyway, of course the fix turns out to be two lines of code... in function autoassignrole_user(), I added an "update" case and a conditional break to the "op" switch:
I don't know if this causes other issues. I don't know if making the autoassignrole module aware of logintoboggan is any better than the reverse (an idea that has been shot down in other threads). I don't even know how to roll this into a proper patch, being a bit of a drupal noob. However, I do know that the two modules are now working for me, so I'm happy.
EDIT: not sure why drupal seems to be inserting newlines into the code above.
Comment #7
Delta Bridges commentedThanks for this contribution.... I will test it on my site and report here the result :)
Expat9
Comment #8
meeotch commentedUpdate to my fix in #6 above: note that this only seems to work for "AUTO" role assignment, not "USER" role assignment. The problem is that user's role choice happens on the registration page (hook_user 'insert'), but the 'update' call to hook_user doesn't happen until the logintoboggan email verification. At that point, the info from the AUTOASSIGNROLE_ROLE_USER form is gone.
I think what's needed is for autoassignrole to save the AUTOASSIGNROLE_ROLE_USER form info into a table during registration (insert), then pull it back out again on update. Or - logintoboggan needs to save any pre-assigned roles during the 'insert' phase, and re-apply them upon verification.
Still trying to figure out how to implement this. Suggestions or tips welcome.
Comment #9
meeotch commentedFurther update. I've got a fix that works for me - though I feel that it may not be "correct" implementation. Basically, I discovered that the AUTOASSIGNROLE_ROLE_USER form value was getting stuffed into the user table "data" row. Therefore, changing references to
$edit['AUTOASSIGNROLE_ROLE_USER']in autoassignrole_user to
$user->AUTOASSIGNROLE_ROLE_USERseems to do the trick. During the 'update' phase, the form values that were saved during registration are pulled out & the users_roles table is updated with the correct role.
This all smells sort of hacky to me - particularly since I'm too new at this to know why the registration form values are saved to the user object, or if that's a "legal" thing to do. There seems to be at least one place in the core user_register_submit() function where this is explicitly avoided: http://api.drupal.org/api/function/user_register_submit/5 (search for "unset"). So any words of wisdom from someone who knows better are welcome.
To summarize - in autoassignrole_module:
1) add "update" case, and logintoboggan_email_validated check
2) change $edit['AUTOASSIGNROLE_ROLE_USER'] to $user->AUTOASSIGNROLE_ROLE_USER
This has only been tested with *single* user roles. Don't know if it works with multiple roles. I've got to catch a plane in about six hours, or I'd spend more time testing this & turning it into a patch.
Comment #10
bjzimmeron commentedThanks for posting this fix. However, I can't get it to work for me. I suspect I'm doing something wrong.
First, do you apply the latest patch (from here: http://drupal.org/node/235707) to the module before making this change?
Second, I'm a bit thrown because the syntax I see in the module doesn't match your final line above. Are you leaving out all the code for the insert case above, or should the modification go above it?
Please take a look at this and tell me where I went wrong.
It would be great if you added a few lines above and below your code so it was clear to coding novices like me exactly what is being replaced and where.
Thanks,
Ben
Comment #11
meeotch commentedAs far as the patch you reference - there's a note in that thread indicating that the patch in question is superseded by the one in this thread: http://drupal.org/node/228774#comment-781025 I believe that I did apply the latter patch.
Your "update" case seems correct, except that, as you noted, the syntax of the next couple of lines is different. I have:
This could be a result of the patch mentioned above - I'm not sure. However, note that you can open up a patch file and read it as text to find out what changed. (Sorry if that's obvious. I didn't know it myself at first.)
All of the above pertains to the first "if" clause in autoassignrole_user(). That's the case that handles "AUTO" role assignment (i.e., non-user-selected roles).
With respect to the "$edit" change I listed in my most recent post: There are two instances of "$edit['AUTOASSIGNROLE_ROLE_USER']" - both are in the second "if" clause of that same function, which is the case that handles "USER" role selection. The first one relates to the case where there are multiple roles, and the second one relates to the case where there's just one role selected. I have only tested the second case. My entire function looks like this:
Note two things: 1) I changed the "$user" variable to "$account", because this seemed to be consistent with other uses I've seen elsewhere, and I think there's a global $user variable, so I thought this was clearer. 2) All of this could be a complete hack - I'm not really familiar enough with the way things are "supposed" to work to say.
Comment #12
Cory Goodwin commentedsubscribing
Comment #13
rmpel commentedHello all.
I'm subscribing to this issue, but at the same time offering a solution to this problem
The attached patch does a few things, and I say upfront that this is NOT the total solution, but it's a working one.
First, it implements the hook_user as described in post #11.
Second, it bugfixes that implementation; During the registration process, there is no $account->AUTOASSIGNROLE_ROLE_USER. There is a test for $edit['blabla'] and that one should be used. (so in my opinion, the original code is correct in that part)
Third, and as I see it the biggest problem, is that the roles are placed in an array, given to the form_api (as they should be), trouble is, there is a problem with the contents of that array.
To function with this code
the delivered value must be a role-id (rid).
To get that, the array for the forms-api must be in the form
rid => 'role description'Unfortunately the function
_autoassignrole_intersectcreates a non-associative array (the keys are lost) so where the example role 'Site tester' used to have role-id 4, it now has role-id 0 (if its the first in line). I traced the problem to the use of thearray_multisortcall.I took the fastest and most logical way to solve this;
array_multisortfor sorting a single array? no need for that, useasortandarsort. Also I replaced aforeachloop to cleanup the array with a call toarray_filter. (I used to do it with aforeachmyself until I discoverredarray_filterand found that it's much faster :) )I hope you don't mind me altering your code, the only reason that I do is; I like the module and I like for it to get as optimal as possible.
Remon
p.s.
I took the liberty of including a few lines of comment for the function
_autoassignrole_array_intersect_keyso that people reading the code don't have to figure out what this function does :PComment #14
meeotch commented@rmpel - I tried your patch (against the stock 5.x-1.1 release from the main page), and it doesn't seem to fix the original issue. That is, when using "user role assignment" (user-selected roles at registration), when the user verifies his account (via LoginToboggan verification), his selected roles aren't being set. At least, it's not working for me.
You noted that during registration, there is no $account->AUTOASSIGNROLE_ROLE_USER, and therefore $edit['AUTOASSIGNROLE_ROLE_USER'] should be used... If my memory is correct, when I looked into it I discovered that during loginToboggan verification, there is no form information in $edit['AUTOASSIGNROLE_ROLE_USER']. (It's not the registration form - it's the verification form that is calling hook_user.) Could this be the reason I haven't gotten your patch to work?
Comment #15
rmpel commentedHmmmm, perhaps we have been talking about similar but different problems.
I had the problem that roles weren't set correctly and my patch fixes that, but, I must admit, I'm not using LoginToboggan. (I assumed this module would pass the information to the hooks correctly)
Your remark suggests that LoginToboggan sends the info to the hook in a different format than should be. (difference between $account-> and $edit[])
According to drupal code; $account should be used for identifying the user to edit and $edit should hold the altered information.
As I'm not using LoginToboggan, I can only guess here, but I think the rest of this problem is caused by a bug in LoginToboggan.
I'm kinda low on spare time atm, so I cant look into the details atm (of LoginToboggan), I hope someone else will take that upon him/her.
I'll check back soon and hope to have more time next month, in case there is no fix yet.
Comment #16
meeotch commentedI think the issue is that at the time LoginToboggan calls hook_user(), there's no information for it to pass, through either mechanism. I believe LT hijacks the registration page & user roles, giving new users its own "pending" role until they use the link in their verification mail to verify their account. It calls hook_user() with type "update" during verification, but doesn't pass any info (because the user-selected roles are never saved during registration). My fix above was taking advantage of the fact that these selected roles seemed to be ending up in $account, and so could be retrieved from there - but I admit that this feels like a hack.
That's definitely an arguable position. Anyway, as you can see from the first post, this incompatibility is what this bug refers to. The "correct" way to fix it would probably be either:
1) autoassignrole stuffs the selected roles into a table somewhere on registration (hook_user("insert")), then retrieves them at verification (hook_user("update")). Or...
2) logintoboggan calls hook_user() at registration to allow the roles to be set, then removes them and stores them somewhere, then retrieves them and sets them again at verification time.
#2 feels better to me - but if you search the database, there are a number of other bug entries related to this, and the maintainer of Logintoboggan has basically refused to "make a special case" for autoassignrole. He might be willing to change the code if someone wrote the patch for him, and made an argument that it's the "correct" way for LT to behave. Unfortunately, I don't think I'm knowledgeable enough to be that person.
Comment #17
rmpel commentedA thought just struck
As i see it this is the requested functionality;
Open site:
1. User registers on site
2. User is activated with roles of choice
Semi-open (admin must approve)
1. User registers on site
2. user is stored ' inactive ' with roles of choice
3. Admin activates user
User-activated (and correct me if im misinterpreting)
1. User registers on site
2. user is stored ' inactive ' with roles of choice
3. User activates himself by clicking a link
(from here on it's just thinking on virtual-paper)
Now, the logintoboggan module tries to implement this.
If I were to make such a module, i would let the registration play out completely with one alteration; set the user->active to 0 (a.k.a. inactive user)
As I'm typing im grabbing the latest version of LT to see what it's doing.
[moment of silence]
/gasp
A lot of code for a seamingly simple task.
LT is hijacking form submit functions (as well as doing a lot of other stuff) of (among others) the user_register form.
at line 282 the user is saved the same way the user_register_submit function does ( user_save(empty variable in place of object, array with values to save)
after clicking the link the user info is loaded and stored again correctly (function logintoboggan_validate_email)
All seems ok here
the module invokes the hook_user with $op = update, $edit being an empty array and $account being a valid user object.
And then it confuses me;
Why request other modules to perform the operation 'update' but not saving the user afterwards?
I would add a user_save($account, $edit); here, making the code:
Perhaps that would solve all your troubles.
Perhaps the line
user_module_invoke('update', $edit, $account);shouldn't even be here...Again, just thinking aloud here ...
Comment #18
meeotch commentedTried the change that you suggested, but it didn't seem to work for me. In debugging it, it appears that the "INSERT INTO" query in autoassignrole is getting called at registration time, as it should be - but nothing ever shows up in my "user_roles" table. Is it possible that LT (or some other piece of code) is DELETEing the roles immediately afterwards? (Sadly, I don't have access to mysql logs on my host in order to check this.)
Comment #19
xsyn commentedHas ay further progress been made around meeotch's issue? I've had the same problem.
Comment #20
meeotch commentedThe workarounds I posted are working for me. However, you should note that they probably aren't "correct". It feels like the "correct" fix would be for LoginToboggan to remember the roles that other modules assign at registration time, and re-apply them at confirmation time.
Comment #21
ckidowIt works fine with autoassignrole and logintoboggan... big THX!
Comment #22
cyberswat commentedI think this has been fixed by http://drupal.org/cvs?commit=135899 ... let me know if it's still a problem.
Comment #23
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.