Make WordPress Core

Opened 10 years ago

Closed 10 days ago

#31166 closed defect (bug) (worksforme)

wpmu_signup_user_notification filter is incorrect

Reported by: johnrom's profile johnrom Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: Login and Registration Keywords: dev-feedback
Focuses: multisite Cc:

Description

Simple ticket here,

The wpmu_signup_user_notification filter seems to be filtering the wrong option

	if ( ! apply_filters( 'wpmu_signup_user_notification', $user, $user_email, $key, $meta ) )
		return false;

If I'm thinking correctly, the filter should be filtering a boolean. If two filters are added to this and the first returns false, there is no way for the second filter to recover the $user variable.

This is how I see it working

WP4.1, /wp-includes/ms-functions.php line 919

	if ( ! apply_filters( 'wpmu_signup_user_notification', true, $user, $user_email, $key, $meta ) )
		return false;

Change History (6)

#1 @DrewAPicture
10 years ago

  • Focuses multisite added

#2 @earnjam
10 years ago

  • Version changed from 4.1 to 3.0

Looks like it's been this way since MU.

#3 @SergeyBiryukov
10 years ago

Same for wpmu_signup_blog_notification.

Technically these filters can still be used, they just have to return a truthy or falsy value.

Fixing this would break backwards compatibility. We could probably introduce new filters and deprecate these ones, otherwise this seems like a wontfix.

#4 @johnrom
10 years ago

Since it is only used during user creation, I don't think a second filter will hurt, performance-wise. Deprecation would be fine. For forward-compatibility, another plugin could set the username to false and break mission-critical functionality. If I'm not mistaken, these are the only places the username is exposed on a filter between user signup and the redirection when creating a user in multisite.

#5 @jeremyfelt
10 years ago

  • Keywords dev-feedback added

It could be tough to really deprecate the filter as you'd always need to account for it.

A new filter would likely go immediately after the current filter. Then if using the new filter, you would need to add_filter( 'wpmu_signup_blog_notification', '__return_true', 999 ); to try and overrule any previous use before hooking in to new_wpmu_signup_blog_notification.

Part of me wants to wontfix. While somewhat annoying, if a plugin has a notification that really needs to fire, a if ( ! $user ) { // find user by email or key from signups table } could do the trick.

I could go either way though. :)

#6 @callumbw95
10 days ago

  • Resolution set to worksforme
  • Status changed from new to closed

Hi All,

I have just been familiarising myself with the history on this ticket and the code surrounding this issue, and it looks like this has been resolved now. Due to the age of this ticket and the fix that has been in place for almost as long as this ticket, I am going to mark this ticket as worksforme however please feel free to reopen the ticket if you wish to continue the discussion further. 😃

Note: See TracTickets for help on using tickets.