Notice while activating a member (with no role on any blogs) on a multisite config

[imath]

Multisite config with users & blogs registration opened

Steps to reproduce :

• register a dummy account by making sure you don't request a blog
• with a super admin account, go to the Manage Signups screen and activate the member.

Notice error in bp_blogs_add_user_to_blog() using array_keys() at line 862. I think as the member has no role on any blogs the check for the user meta $wpdb->get_blog_prefix( $blog_id ). 'capabilities' simply returns an empty string.

Casting this to an array is fixing the issue, see attached patch.

[imath]

It's also happening if a user registered a blog.

[boonebgorges]

This seems like a fine fix to me. A unit test would be nice.

[imath]

Replying to boonebgorges:

This seems like a fine fix to me. A unit test would be nice.

Thanks for your feedback 5834.unit_tests.patch is 5834.patch + 2 tests :

• one to test user accounts activation on regular/multisite configs
• one other to test user accounts + blogs activation on multisite config.

[boonebgorges]

Thanks, imath. Those tests look good and are nice to have, but they pass even without your patch to bp_blogs_add_user_to_blog(). Can you write a test just for this function that fails before your fix?

[imath]

Replying to boonebgorges:

but they pass even without your patch to bp_blogs_add_user_to_blog().

Oops my bad the last patch contains the previous one, i should have not include the fix in the patch sorry :(

If you checkout the changes to blogs functions file you should see tests failing

[imath]

Suddenly i have a doubt, checking !

[imath]

only the tests, previous was included the fix

[imath]

*including*

Just checked and added 5834.solo_tests.patch so it's easier to only apply the tests.

The problem only occurs on multisite configs actually so make sure to use the -c tests/phpunit/multisite.xml option to see the tests failing.

On regular configs each member has a role, so there's no problem.

[boonebgorges]

Thumbs up. Thanks, imath!

[imath]

Replying to boonebgorges:

Thumbs up. Thanks, imath!

Thanks, committing :)

[imath]

In 8965:

In {{{bp_blogs_add_user_to_blog
()}}} function make sure getting user roles returns an array

On multisite configs, when a member registers, he is not assigned to any blog and has no role.
When activating the user from the Manage signups screen, getting his role in bp_blogs_add_user_to_blog() was returning an empty string causing a notice error when trying to get the keys of the "capabilities" usermeta.
To avoid this, we now make sure this usermeta is an array.

Fixes #5834

[boonebgorges]

Looks like this broke another signup-related test ​https://travis-ci.org/buddypress/BuddyPress/builds/33634198 My bad, I should have run the whole suite before adding the 'commit' tag. imath, can you have another look?

[imath]

Checking right away

[imath]

Weird it's one of the tests i've added BP_Tests_BP_Signup::test_activate_user_accounts_with_blogs
but it's working fine for me ?

[boonebgorges]

Are you running all the tests together? There must be some global stuff bleeding through, or some other test not cleaning up properly.

[imath]

Replying to boonebgorges:

Are you running all the tests together? There must be some global stuff bleeding through, or some other test not cleaning up properly.

Just did, and saw there was a trouble :( Searching what is the trouble..

[imath]

The problem is i use path blogone, blogthree

and in WP_UnitTest_Factory_For_Blog it sets testpath1 for the first blog without taking in account the signup 'path' attribute. But for the 3rd blog it's taking in account the path attribute of the signup ?!?

i've tried many things like setting the domain, path, title to nothing for blogone, it keeps on creating a blog having a path set to testpath1

The only way i got it to work was to set blogone's path to 'testpath' knowing at the end the blogone's path would be 'testpath1'. Very strange :(

But then if only testing --group BP_Signup it dramatically fails :(

[imath]

Ok i think we cannot trust the first signup, so in 5834.cant-trust-first-signup.patch, i suggest to avoid taking it in account.

I've double checked by running all tests and --group BP_Signup twice and it's working.

Boone, can you check it ?

[imath]

wow cannot trust the first signup !

[boonebgorges]

and in WP_UnitTest_Factory_For_Blog it sets testpath1 for the first blog without taking in account the signup 'path' attribute. But for the 3rd blog it's taking in account the path attribute of the signup ?!?

I don't really follow this. Where in your BP_Signup tests are you actually creating blogs? And why would that blog creation happen with WP_UnitTest_Factory_For_Blog?

Worst case scenario, you should go back to the drawing board and make your test smaller. You're running a big long test with signups. The bug here, however, is in bp_blogs_add_user_to_blog() Write the smallest possible test that demonstrates the error:

$u = $this->factory->user->create();
$b = $this->factory->blog->create();

$result = bp_blogs_add_user_to_blog( $u, $b );

$this->assertSame( null, $result );

This test kinda stinks because bp_blogs_add_user_to_blog() doesn't return any value on success, but that's an item for a separate ticket. The point is that this tests something much closer to the problem, and skirts your signup issue.

[boonebgorges]

Tests pass for me. Go with it.

[imath]

Replying to boonebgorges:

Tests pass for me. Go with it.

Thanks doing it :)

[imath]

In 8966:

Edit the unit test test_activate_user_accounts_with_blogs()

Make sure the test is working while running all tests and only BP_Signup tests

Fixes #5834