Skip to:
Content

BuddyPress.org

Opened 10 years ago

Last modified 9 years ago

#5367 new defect (bug)

WP Admin BuddyPress profile when BuddyPress is not network activated

Reported by: imath's profile imath Owned by:
Milestone: Awaiting Contributions Priority: normal
Severity: normal Version:
Component: Administration Keywords: 2nd-opinion needs-patch
Cc:

Description

On a multisite config, it's possible to activate BuddyPress on a child blog instead of on the entire network. On this kind of configuration, the new WP Admin BuddyPress profile won't be available for all the community members, but only to the one that are users of the child blog.

Registration process

When a user is created from the WordPress Administration of the one of the blog of the network, he becomes a user of this blog having the role the administrator specified
When a user is created from the Users Network Administration screen, he doesn't belong to any blog.
When a user is created using the BuddyPress signup process, the user won't belong to any blog.

When BuddyPress is on non multisite config or activated on the network, i think we shouldn't change anything to this process, because people are used to :

  • find their users in their blog on non multisite config
  • find their members in users network administration on multisite config.
  • and everything works fine for the new WP Admin BuddyPress profile

As problem only appears on this particular config : BuddyPress is not activated on the network but on a child blog, i think we have 2 options to deal with it :

Option 1 : using the add_to_blog usermeta

I've attached the [ticketnumber]-add-to-blog.diff as a starting point of this option. Using it, any new activated signup will be created as a network member and as a user of the blog where BuddyPress is activated. The benefit is we don't have to touch to the WP Admin BuddyPress profile code, the disadvantage of this is that, the "old" members won't be in the blog's users list. So it will surely need a tool to "repare" this situation in order to batch attach BuddyPress members as users of the blog. Problem is "what" is a BuddyPress member? The only way i saw in next option is something that will be soon deprecated (great work @boone on #5128, i need to test it asap :) ) : the 'last_activity' user meta.

Option 2 : "trick" patch

In [ticketnumber]-trick.diff, i'm editing the WP Admin BuddyPress profile in order to list all the BuddyPress members (last_activity exists) without making them users of the blog. Using it, the Community administrator will be able to view all the members and not only the users of his blog. The benefit is we don't need to change the signup process and no specific tool will be necessary. The disadvantage is that it will quicky need a refresh to stop using last_activity meta.

What do you think ? Option 1, Option 2 or another way ?

Attachments (5)

5367-add-to-blog.diff (927 bytes) - added by imath 10 years ago.
5367-trick.diff (9.0 KB) - added by imath 10 years ago.
5367.02.diff (9.7 KB) - added by imath 10 years ago.
5367.required.diff (4.2 KB) - added by imath 10 years ago.
5367.03.diff (19.1 KB) - added by imath 10 years ago.

Download all attachments as: .zip

Change History (30)

@imath
10 years ago

#1 follow-up: @boonebgorges
10 years ago

Your last_activity trick seems like a huge amount of additional overhead to account for an edge case.

Is there some reason why we don't already add_user_to_blog() for new users, as you suggest in your add-to-blog.diff patch? Doesn't it make sense for us to be doing this in *all* cases, not just in the particular config here?

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

Thanks for your reply

Replying to boonebgorges:

Your last_activity trick seems like a huge amount of additional overhead to account for an edge case.

That's the other disadvantage of this way, i agree + needs a refresh to take in account #5128

Is there some reason why we don't already add_user_to_blog() for new users, as you suggest in your add-to-blog.diff patch? Doesn't it make sense for us to be doing this in *all* cases, not just in the particular config here?

Well, we could use the add_to_blog usermeta in registration process. But

  • only the new signups would be concerned not all the members, so we would need a tool to "repare", or repare progressively by using add_user_to_blog() if logged in user is not a member of the blog
  • having this usermeta for all cases makes sense, but it would confuse Administrator as they may be used to find their members in network admin users screen and there only.
  • the problem for the "Community Profile" only appears on this particular config as BuddyPress is not network activated.

We could, avoid the additional overhead by not trying to have an exhaustive 'All' tab, and by only adding the tab named "Members".

#3 follow-up: @boonebgorges
10 years ago

so we would need a tool to "repare"

Yes, of course.

it would confuse Administrator as they may be used to find their members in network admin users screen and there only.

Is this the only reason why we don't currently do it? I could swear we used to add users to the blog at the time of registration.

We could, avoid the additional overhead by not trying to have an exhaustive 'All' tab, and by only adding the tab named "Members".

I don't understand this suggestion. Tabs on what? users.php?

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

Replying to boonebgorges:

Is this the only reason why we don't currently do it? I could swear we used to add users to the blog at the time of registration.

At the beginning i thought too :) Then i went back on 1.2.7 to check the registration process, because i was remembering using the add_to_blog usermeta for one of my plugin and i thought i had this idea watching BuddyPress code. But no. It seems on a multisite config, users have never been added to a blog during registration process.
When i've found the trouble with Community Profile and the specific config "BuddyPress not activated on network but on a child blog" my first idea was to use add_to_blog usermeta for all config as it would ease a lot the signup patch i'm working on. But i thought if BuddyPress never used this, there might be a good reason, so i wasn't so sure at all that changing this to config where BuddyPress is network activated was a good idea.

  1. in this "regular multisite" config, profile fields are set in the network admin, the community profile is available there too, so does activity and group administration, and i'm afraid that having the members on the regular admin would confuse super administrators, because they would search for the Profile field submenu or the community profile and they would only find "Edit or remove-from-blog actions".
  2. Regular administrator (without the capacity of managing network users) may be surprised to see this change too like "wow how come all this members arrived in my administration ?", so they could batch remove users from the blog and that may be problematic...

That's why i've searched (again ;) ) a hardest way to avoid changing anything to the signup process.
On the other hand, i agree it would ease everything!! So i'm in favor but a bit "sur la pointe des pieds" ;)

I don't understand this suggestion. Tabs on what? users.php?

Sorry, actually the patch is adding a new view to the users.php WP List Table called members (that's what i've called a tab), the big overhead comes from the fact that i've tried to have in the "All" view : users of the blog + members.
So i was saying that only having the members view added would be enough if we prefer not to go in the "add_to_blog + repare" way. This view is only available to "can manage network users" admins and for the specific case where BuddyPress is not network activated.

#5 follow-up: @boonebgorges
10 years ago

Thanks for the additional research, imath!

I think just going with a Members tab sounds ideal for these purposes. We're dealing with a real edge case here, and I think that the dedicated Members tab will serve pretty well without mucking up the rest of the code.

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

  • Keywords needs-refresh added; needs-testing removed

Replying to boonebgorges:

Thanks for the additional research, math!

You're welcome :)

I think just going with a Members tab sounds ideal for these purposes. We're dealing with a real edge case here, and I think that the dedicated Members tab will serve pretty well without mucking up the rest of the code.

I agree, i'll refresh the patch asap, thanks a lot for your help

@imath
10 years ago

#7 @imath
10 years ago

Added 5367.02.diff it will need to be improved as soon as #5365 and #5128 are fixed.

This ticket was mentioned in IRC in #buddypress-dev by imathfromparis. View the logs.


10 years ago

#9 @imath
10 years ago

As i was working to bring the profile for members that are not users of the blog, i've noticed that before doing anything in this area, some bug were appearing if the user is a regular blog Administrator.

1/ A regular Administrator unlike a superadmin cannot edit users, so we need to disable the WordPress profile nav for them
2/ As a result, the edit row action is not set, and it causes a notice error for these regular administrators
3/ In BuddyPress front end: a regular administrator can edit a user's profile but not mark a user as a spammer. So we need to disable the spam / unspam actions of the wp-admin/profile (Community profile).

Before doing anything on bringing profile for members in this specific config, i really think we should first commit a patch to avoid the 3 points.
Here's my suggestion about them: 5367.required.diff

@imath
10 years ago

#10 @boonebgorges
10 years ago

Yeah, these checks seem good to me. Thanks, imath.

#11 @imath
10 years ago

In 8149:

In wp-admin/profile, be consistent with front-end when BuddyPress is not network activated

In this specific config, on front-end, the regular Admin :

  • can edit members profile,
  • cannot edit a Super Admin profile,
  • cannot mark as spam a user.

We need to make sure, this is also the case in the wp-admin/profile screens.

See #5367

#12 @boonebgorges
10 years ago

imath - Where does this stand?

#13 @imath
10 years ago

  • Keywords needs-refresh removed

Well, i'm afraid it's still very complex. In 5367.03.diff i'm using a new WP_Liste_Table to list the members that have no role on the blog. To do so i get an id list of blog users with a role that i use as an exclude argument to bp_core_get_users(). I think the id list should not be that huge as the problem is precisely that registered members are not attached to the blog. But as in Administration i need the spam users, i have to hook to 'bp_pre_user_query' in order to include them..

I've tested the patch on this particular config (BuddyPress not network activated) and it's working. Then i've tested it on regular config and multisite one where BuddyPress is network activated to check for an eventual regression and didn't find it (which is nice :) )

I'm really hesitating about it, as it remains a complex patch for just making sure this config will be able to use the wp-admin/profile for all members.. What do you think ?

@imath
10 years ago

#14 @boonebgorges
10 years ago

  • Milestone changed from 2.0 to 2.1

I'm really hesitating about it, as it remains a complex patch for just making sure this config will be able to use the wp-admin/profile for all members.. What do you think ?

I'm hesitating for the same reason. Since this is a new feature anyway, if we don't include it for 2.0, we won't be doing any damage to any existing sites. Let's leave it out for the moment, and come back to it with fresh minds for 2.1. Thanks for your work on it.

#15 @imath
10 years ago

You're welcome :) and i agree

This ticket was mentioned in IRC in #buddypress-dev by imathfromparis. View the logs.


10 years ago

#17 @DJPaul
10 years ago

This is likely to move to 2.2 unless someone wants to pick it up and get it done in the next two weeks before beta. I'm not sure where things stand with this -- is the probably the amount of code or hackery we're doing to add such a feature, and that it just needs someone else to look to see if there's further improvements we could make? Or is this feature only to the benefit of a small number of sites, and so we're not sure if it's worth spending time on?

#18 @DJPaul
10 years ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from 2.1 to 2.2
  • Priority changed from high to normal

This ticket was mentioned in Slack in #buddypress by imath. View the logs.


9 years ago

#20 @johnjamesjacoby
9 years ago

These edge cases are clearly tough, but it's important we have them covered. Because BuddyPress is such a large library, activating it only where it's needed is not unusual (we do this on profiles.wordpress.org.)

The members list table idea makes sense, but lets not allow them to edit other users quite yet. We should come up with a map of activation-types vs. desired results, and work slowly on each.

  • single site, single network
  • single site, global user tables, many networks
  • multisite, active on one site
  • multisite, active on one network
  • multisite, active on some networks
  • multisite, active on all networks
  • multisite with multiblog on all of the above
  • probably other configs

#21 follow-up: @johnjamesjacoby
9 years ago

Took a cursory look at the patch. I'd like us to avoid hard coded checks for 1 installation type, and instead consider the several permutations and come up with relatively simple rules for what happens and what we allow users to perform.

WordPress is choosing to go a similar route as this patch with wp_is_trusted_network(), which has already proven to not be flexible enough for our use case. We need either a global variable in the BuddyPress singleton for installation_type or a white-listed array of types to hint at the application what kind of approach should be enforced regardless of what environment variables tell us.

This is to say, we can make several assumptions based on constants and activation types, but there is no guarantee our assumptions are accurate. Just because MULTISITE is true, and BuddyPress is network activated, doesn't mean there aren't other networks, doesn't mean network admins can edit all users outside of their networks, etc… We should audit common uses, ensure we are providing sane default experiences, and make sure we aren't escalating privileges or allowing members to access to areas and functionality they wouldn't othherwise have in WorsPress, unless we explicitly design it to for some greater purpose (as there are always exceptions to these rules.)

Version 0, edited 9 years ago by johnjamesjacoby (next)

#22 in reply to: ↑ 21 @imath
9 years ago

Replying to johnjamesjacoby:

Took a cursory look at the patch.

Thanks a lot johnjamesjacoby. I'm sorry, i realize that maybe i wasn't clear in my message in slack. I think #5977 can also resolve this ticket. Could you have a look at it ?

#23 @imath
9 years ago

  • Milestone changed from 2.2 to 2.3

See #5977 for an explanation of why i'm delaying this ticket to next dev-cycle.

#24 @imath
9 years ago

  • Milestone changed from 2.3 to 2.4

#25 @imath
9 years ago

  • Milestone changed from 2.4 to Future Release
Note: See TracTickets for help on using tickets.