#15173 closed defect (bug) (fixed)
duplicate calls to 'add_new_user_to_blog'
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 3.6 | Priority: | normal |
Severity: | minor | Version: | |
Component: | Multisite | Keywords: | 3.2-early dev-feedback |
Focuses: | Cc: |
Description
It seems that there is a default filter in ms-default-filters:
add_action( 'wpmu_activate_user', 'add_new_user_to_blog', 10, 3 );
That calls the add user to blog on wpmu_activate_user action. That in itself is fine but wpmu_activate_signup() has this code:
add_new_user_to_blog( $user_id, $user_email, $meta );
do_action('wpmu_activate_user', $user_id, $password, $meta);
(line 831-832 ms-functions.php current trunk);
That means that the default behavior is to call add_new_user_to_blog twice. This triggers the actions attached to add_user_to_blog to fire twice.
While removing the action works for me, I question if this is the intended or expected behavior of most users and would offer that this behavior should be debated.
Thank you,
Stéphane
Attachments (2)
Change History (11)
#1
@
10 years ago
- Keywords has-patch added
- Milestone changed from Awaiting Review to 3.1
- Type changed from enhancement to defect (bug)
#3
in reply to:
↑ 2
@
10 years ago
Replying to scribu:
I think we should remove the direct call instead.
Also, the second parameter should be deprecated, since it's not used.
I agree. Removing the direct call allows the activation process to be altered by a plugin simply by removing the default hook.
#4
@
10 years ago
I spoke about Pete with this.. The issue is that we're currently passing the $user_email in one case and $password in the other, as the second parameter. Ideally we should be making that $user_email -- passing $password around is insane and useless. In that case, yes we can use the hook.
#5
@
10 years ago
Changing the hook may break a plugin that is using it but I agree with nacin that we should not be passing the password at all. Some things to keep in mind:
- Deprecate the email paramater for add_new_user_to_blog() since it is not used anymore?
- Update the hook to use email instead of password?
- Add new hook to use add_new_user_to_blog to avoid breaking a plugin?
#6
@
10 years ago
- Keywords 3.2-early dev-feedback added; has-patch removed
- Milestone changed from 3.1 to Future Release
This has been in MU since before 2.0. We can address this in the next release.
#7
@
8 years ago
- Milestone changed from Future Release to 3.6
It's actually safer to remove the direct call. In add_new_user_to_blog( $user_id, $email, $meta )
, $email
is unused. So, even though passing password sucks, it is ignored by the function when this is what is passed: add_new_user_to_blog( $user_id, $password, $meta )
#8
@
8 years ago
- Owner set to nacin
- Resolution set to fixed
- Status changed from new to closed
In 23535:
#9
@
8 years ago
Looking closer at this, the same three arguments (user_id, password, meta) are being passed to wpmu_welcome_user_notification(), passed to the wpmu_activate_user(), and then returned from wpmu_activate_signup(). So, not as alarming that $password is something actually used by a generic API function, which add_new_user_to_blog() is not. (Rather, there is add_user_to_blog() for that.)
If someone is removing that hook, they may have still been getting the user added anyway, but I'm not concerned about the change there. Since maybe_add_existing_user_to_blog() already goes through a hook ('init', blah), let's keep the consistency.
In all, this whole mess of signup code eventually needs cleanup one of these days. At that point, I imagine new functions and program flow.
I think we should remove the direct call instead.
Also, the second parameter should be deprecated, since it's not used.