Skip to:
Content

BuddyPress.org

Opened 10 years ago

Closed 10 years ago

#5836 closed defect (bug) (fixed)

Creating a new user from the user-new admin screen generates notices

Reported by: imath's profile imath Owned by: imath's profile imath
Milestone: 2.1 Priority: normal
Severity: normal Version:
Component: Extended Profile Keywords: has-patch commit
Cc:

Description

On a regular config (WP 4.0), when i create a user, the function xprofile_sync_bp_profile() is trying to get :

  • $user->ID
  • $user->display_name

Both are not yet set, in case of a user creation, as the wp_insert_user() happens a few lines after.

I imagine it can be interesting in case of an edited user, but i find it weird to use this hook as it's a way to figure out if there are errors and it's not used this way.

I suggest to include a check on these 2 variables.

Attachments (4)

5836.patch (561 bytes) - added by imath 10 years ago.
5836.02.patch (550 bytes) - added by imath 10 years ago.
5836.unit_test_only.patch (1.0 KB) - added by imath 10 years ago.
5836.unit_test_only.02.patch (1.1 KB) - added by imath 10 years ago.

Download all attachments as: .zip

Change History (13)

@imath
10 years ago

#1 @DJPaul
10 years ago

  • Milestone changed from Awaiting Review to 2.1

#2 follow-up: @DJPaul
10 years ago

Is this a regression in 2.1, or caused by a change in WP 4.0?

If it is something that needs to be fixed for 2.1, is there a better action we should use, or should we use your patch and file a ticket with WordPress core for WP 4.1?

#3 in reply to: ↑ 2 @imath
10 years ago

Replying to DJPaul:

Is this a regression in 2.1

Sorry, i should have checked. It is a regression introduced by r8810

I suggest 5836.02.patch to add the part that is in 2.0.2 and disappeared in 2.1. Tested it and it works fine.

@imath
10 years ago

#4 follow-up: @boonebgorges
10 years ago

*grumble* This is why we write unit tests when we change existing code *grumble*. imath, would you mind trying to write a simple test that demonstrates the error?

#5 in reply to: ↑ 4 @imath
10 years ago

Replying to boonebgorges:

imath, would you mind trying to write a simple test that demonstrates the error?

No at all, i suggest this test 5836.unit_test_only.patch. This time, i've also tested it running all the tests together ;)
both patch 5836.patch​ & 5836.02.patch are passing the test successfully

Last edited 10 years ago by imath (previous) (diff)

#6 follow-up: @boonebgorges
10 years ago

Thanks, imath! Looks mostly good. One change: can you make sure that you restore the $_POST global after you fake it?

$post_vars = $_POST;

$_POST = array( // ...

// ...

// clean up
$_POST = $post_vars;

Thanks!

#7 in reply to: ↑ 6 @imath
10 years ago

Replying to boonebgorges:

can you make sure that you restore the $_POST global after you fake it?

Of course, thanks for this tip :) it's done in 5836.unit_test_only.02.patch

#8 @boonebgorges
10 years ago

  • Keywords commit added

Looks good. Thanks!

#9 @imath
10 years ago

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

In 8987:

Make sure xprofile_sync_bp_profile() happens when a user is edited

Bring back checks on the $update var and WordPress error that were removed in r8810 so that the xProfile sync only happens when a user is edited. Add a unit test.

Props boonebgorges

Fixes #5836

Note: See TracTickets for help on using tickets.