Skip to:
Content

BuddyPress.org

Opened 6 years ago

Closed 6 years ago

#6007 closed defect (bug) (fixed)

Potential improvement to bp-members-classes.php and add_backcompat()

Reported by: Judder Owned by: djpaul
Milestone: 2.2 Priority: normal
Severity: normal Version: 2.1
Component: Core Keywords: good-first-bug commit
Cc:

Description

We noticed that error messages generated by the Wordpress core, for example when usernames already exist, weren't being displayed but instead a generic message was being shown.

It appears that bp-members-classes.php -> add_backcompat() is not re-using the $errors sent from wp_insert_user() but is instead creating it's own $errors object.

		if ( is_wp_error( $user_id ) || empty( $user_id ) ) {
			$errors->add( 'registerfail', sprintf( __( '<strong>ERROR</strong>: Couldn&#8217;t register you. Please contact the <a href="mailto:%s">webmaster</a>.', 'buddypress' ), bp_get_option( 'admin_email' ) ) );
			return $errors;
		}

We have changed our version to re-use the $user_id object instead, which, in the case where wp_insert_user() fails, contains the Wordpress error object

Could we suggest that this is added (or similar) going forwards as it is handy to have the correct error messages from Wordpress core returned?

Our minor change is:

		if ( is_wp_error( $user_id ) || empty( $user_id ) ) {
            $errors = $user_id;
			//$errors->add( 'registerfail', sprintf( __( '<strong>ERROR</strong>: Couldn&#8217;t register you. Please contact the <a href="mailto:%s">webmaster</a>.', 'buddypress' ), bp_get_option( 'admin_email' ) ) );
			return $errors;
		}

Thanks

[Buddy Press 2.1.1]

Attachments (1)

6007.diff (939 bytes) - added by Mamaduka 6 years ago.

Download all attachments as: .zip

Change History (8)

#1 @boonebgorges
6 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 2.2

Yes, I think this is a good suggestion - no reason for us to change the default error message here.

#2 @DJPaul
6 years ago

  • Keywords needs-patch good-first-bug added; has-patch removed

@Mamaduka
6 years ago

#3 @Mamaduka
6 years ago

  • Keywords has-patch added; needs-patch removed

#4 follow-up: @r-a-y
6 years ago

  • Keywords needs-patch added; has-patch removed

Mamaduka - thanks for the patch, but what is suggested above is we use WP's native registration errors instead of creating our own, custom error message.

#5 in reply to: ↑ 4 @boonebgorges
6 years ago

Replying to r-a-y:

Mamaduka - thanks for the patch, but what is suggested above is we use WP's native registration errors instead of creating our own, custom error message.

I think that this is what the patch does. $user_id in this case is the WP_Error object created by WP. We're just returning it untouched.

#6 @r-a-y
6 years ago

  • Keywords commit added; needs-patch removed

I think that this is what the patch does.

Apologies! Brainfart. Carry on! :)

#7 @djpaul
6 years ago

  • Owner set to djpaul
  • Resolution set to fixed
  • Status changed from new to closed

In 9200:

Members: when validating user signup, if wp_insert_user returns a WP_Error object, use it.

Prior to this change, we were replacing WP's helpful error message with a rather generic and unhelpful BP error message.
Fixes #6007, props Mamaduka

Note: See TracTickets for help on using tickets.