Can we change the way hook_user() works (or remove it altogether)?
marc.groth - November 5, 2009 - 09:52
| Project: | U Create |
| Version: | 6.x-1.0-beta2 |
| Component: | ucreate.module |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Description
Hi there,
I've noticed that hook_user() doesn't seem to play nice with the content_profile_registration module. This is because with this module the user is forced to change their password once their account is created which messes up the functionality of content_profile_registration.
Could this functionality either be removed completely or (more likely) there be a setting in the admin where you can turn this feature on/off? That way, users who don't want to have to force their users to change their password can bypass this altogether, and those who do can keep it. I'd be quite happy to write the code for this if it has a chance of being adopted.
Thoughts?
#1
Thanks for breaking this out from the other issue :) We need to solve this issue.
The setting is a possibility. However, I'd like to avoid it because it always requires a user to *know* about the underlying problems that this setting can be used to avoid.
I'm hoping we can find a solution that keeps ucreate and content_profile from colliding in the first place. Can you explain in detail the problems that occur with the content_profile_registration and ucreate?
Thank you.
#2
No problem! :)
OK, well it turns out it's literally one line of code (two including the drupal_set_message()) which is causing the issue, so hopefully this is easy to fix...
The content_profile_registration module basically allows you to expose content types on the user registration form... And creates nodes for that content type upon successful user creation. Unfortunately, the drupal_goto() function (line 82 in ucreate.module) is intercepting this node save... And redirecting the user to their user edit page means the node doesn't get created successfully.
Do you know of any ways around being able to redirect a user instead of using drupal_goto()? Perhaps the $form['#redirect'] in the form itself? Have to admit I'm not entirely sure myself, but would that maybe work?...
#3
Thanks for describing the issue more in detail.
Only one way to find out... :)
#4
OK well I tried leaving the $form['#redirect'] in ucreate_user_form() and commented out the line of code that stops content_profile_registration module from working.
drupal_goto('user/'. $account->uid .'/edit', 'destination='. variable_get('site_frontpage', 'node'));It made the content_profile_registration module work, but then doesn't redirect the user to the edit page, obviously since it's been commented out.
The only way I can see around this at this moment in time is by adding an if statement to only show these lines of code if the content_profile_registration module isn't enabled:
drupal_set_message(t('Welcome to !site, !name - please change your password', array('!site' => variable_get('site_name', 'Drupal'), '!name' => $account->name)), 'welcome');drupal_goto('user/'. $account->uid .'/edit', 'destination='. variable_get('site_frontpage', 'node'));
So that code would become:
if (!module_exists('content_profile_registration') {drupal_set_message(t('Welcome to !site, !name - please change your password', array('!site' => variable_get('site_name', 'Drupal'), '!name' => $account->name)), 'welcome');
drupal_goto('user/'. $account->uid .'/edit', 'destination='. variable_get('site_frontpage', 'node'));
}
The only other way I can see around this is if there is some sort of setting in the admin section so the admin user can choose whether new users are redirected or not. Either way I'm not sure if it's best practice to force the user a redirect to their edit page when they log in, since other modules may want to do some execution after the user has been created, and using drupal_goto() prevents that (as we are seeing here). I think it's similar to the other bug, which was fixed by removing the drupal_goto() message altogether (#621160: Redirect preventing subsequent submit functions from running).
Thoughts?
#5
I just had a thought.
I noticed that in ucreate_user_form() there is a line of code $form['#redirect'] = $_GET['q']
What exactly is that value? Since you seem to want the user to ALWAYS redirect to user/%uid/edit page wouldn't it make sense to change that to:
$form['#redirect'] = 'user/'. $account->uid .'/edit';That way there is no need for drupal_goto() and any modules that it conflicts with can override the redirect value in hook_form_alter().
Thoughts? And if this can be done (and you're happy with it) shall I create a patch against HEAD?
Thanks :)
#6
#4: I am with you on doubts about whether this is best practice. Let's remove the hook_user() implementation altogether then. Thank you for the research. Being able to keep it would have been great.
#5 doesn't work because you would be redirecting the user that creates the account, not the user who just logged in for the first time.
#7
Ahh of course... You're right, sorry... I didn't really think that one through haha!
So shall I create a patch to remove the hook_user() function altogether? And should this be against HEAD?
Thanks for your help as well with finding a resolution :)
#8
Patch: yes please :) thanks!
#9
Attached is a patch which removes ucreate_user() altogether - against the 6.x-1.0-beta2 version available.
Thanks for all your help in this matter alex_b :)
#10
Setting to "needs review", as I think that's the status you wanted.
#11
Thanks rfay,
Yes that is probably correct. Because it's such a small patch I figured it could be implemented pretty much instantly. I just realised the "patch (to be ported)" status is for porting to a different version of Drupal, haha. My bad.
Thanks :)