Directories filtered by member roles

[sooskriszta]

At the moment, BuddyPress has a monolithic member directory /members/

Now that we have Roles API, perhaps we should think about allowing visitors and members to filter the directory by Member Roles.

These "filtered" versions should each have their own unique URLs, e.g.
/members/moderators/
/members/dentists/
etc

[sooskriszta]

​Buddy Boss talking about the API in general, but examples apply to the ticket.

[boonebgorges]

Yes, absolutely. Thanks for opening the ticket.

Some initial considerations and questions:

• Should we prefer URLs like /members/foo/ or just /foo/? /foo/ is going to require a different kind of page-routing logic, because it's top-level. /members/foo/ is going to require that we check for username conflicts.
• If we go with /foo/, the natural thing would be to have user pages at /foo/username/, but this is going to raise all sorts of other problems. Maybe at first, best to stick with /members/username/ for individual user profiles.
• It may be easier, at a first pass, to implement with a querystring: /members/?member_type=foo. And in fact, in light of potential movement to rewrites, it might be best to do this under the hood in any case. That is, /members/foo/ would translate behind the scenes to /members/?member_type=foo - maybe we could even use a WP_Query query var for it, even though we're not using rewrite rules elsewhere.
• Presumably this will be toggleable with a bp_register_member_type() argument. WP has has_archive. We could do the same. Or perhaps has_directory? I suppose this would take a boolean false to disable, a boolean true to have the slug default to the member_type string, or a string to use a custom slug.
• What effect does this have on menu navigation? Perhaps, by default, we should have a dropdown/flyout underneath Members that shows each member type?
• Ideally we would have nice in-page navigation - maybe AJAX powered? - to filter by member type when looking at a directory. Perhaps this is best left for another ticket, though.

This would pair nicely with #5192 for 2.3. If anyone wants to contribute ideas or patches here, it'd help move us forward.

[sooskriszta]

implement with a querystring

/members/?member_type=foo makes sense.

However, if admin has opted for pretty/SEO/whatever permalinks, then this may seem like an oddity.

Personally, I would go with just /foo/

If we go with /foo/, the natural thing would be to have user pages at /foo/username/, but this is going to raise all sorts of other problems.

What sort of problems?
Problems such as /foo/username/ and /members/username/ wanting to point to the same user? Problems such as BuddyPress deciding to make Roles-Users a many-to-many relationship at some point?
I really like how ​OpenCart] deal with product URLs. Now, category-product is a many-to-many relationship. Product1 could belong to Catergory1 and Category5 and Category1 may have Product1 and Product3 in it.
So guess how URLs, Breadcrumbs, and page Titles work?
www.domain.com/category1/product1
www.domain.com/category5/product1
www.domain.com/product1
all lead to Product1's page, BUT
the page header indicates that www.domain.com/product1 is the canonical URL, AND
if we arrived at the product page via www.domain.com/category1/ then we reach www.domain.com/category1/product1 and breadcrumb shows that as Home > Category1 > Product1 and page Title shows as Product1 - Category1 - Store Name
I feel that's a very smart, and perhaps the right approach. Very user-centric (not to say that OpenCart as a whole is user-centric).

[boonebgorges]

What sort of problems?
Problems such as /foo/username/ and /members/username/ wanting to point to the same user? Problems such as BuddyPress deciding to make Roles-Users a many-to-many relationship at some point?

Yes, these are some issues. As you note, members can have more than one member type. Having the same content available at multiple URLs is not good practice, for SEO and for other reasons. So if user 'boone' had member types 'foo' and 'bar', you would want /foo/boone/ to resolve to /bar/boone/, or the other way around. But which? And what do you do when a user changes user type? Do you redirect or show 404s? If you redirect, you'll need a system for storing former member type associations. If, as you suggest, the canonical URL will be something member-type-agnostic - like /members/boone/ - then it seems like quite a bit of work to build the necessary system for routing, just to resolve to the URLs we've already got. So I'm going to suggest we do nothing with single user URLs for the moment.

There are further potential issues with top-level directory URLs. The top-level page /members/ is currently handled by a WP page. To change that slug, you edit the page. Presumably we would not want to do this in the case of member types. But now we've introduced an asymmetry in the way top-level pages are managed by admins. In addition, we will have to cast a fairly broad net with regard to slug conflicts. Eg, if someone has a member type 'foo', what do we do if they also have a page 'foo', or a post called 'foo' with permalink structure /%pagename%/? These problems can be solved, of course, but I don't want to put in the work if people generally think that /members/foo/ is a better URL format. I would have to think about it a bit more to decide what I think is the best general solution.

[espellcaste]

Here is what I think:

Should we prefer URLs like /members/foo/ or just /foo/? /foo/ is going to require a different kind of page-routing logic, because it's top-level. /members/foo/ is going to require that we check for username conflicts.

I'm for /members/member_type. I believe is clear, less work and goes with the current schema of BuddyPress.

If we go with /foo/, the natural thing would be to have user pages at /foo/username/, but this is going to raise all sorts of other problems. Maybe at first, best to stick with /members/username/ for individual user profiles.

Agree!

It may be easier, at a first pass, to implement with a querystring: /members/?member_type=foo. And in fact, in light of potential movement to rewrites, it might be best to do this under the hood in any case. That is, /members/foo/ would translate behind the scenes to /members/?member_type=foo - maybe we could even use a WP_Query query var for it, even though we're not using rewrite rules elsewhere.

I'm for the /member/foo/ approach. I'm aware that some rewrite work is being done but if we consider the work of changing in the future, my guess is that there are other places where changes will be needed as well.

Presumably this will be toggleable with a bp_register_member_type() argument. WP has has_archive. We could do the same. Or perhaps has_directory? I suppose this would take a boolean false to disable, a boolean true to have the slug default to the member_type string, or a string to use a custom slug.

I think that's a great idea.

What effect does this have on menu navigation? Perhaps, by default, we should have a dropdown/flyout underneath Members that shows each member type?

Yes, a dropdown would be best here.

Ideally we would have nice in-page navigation - maybe AJAX powered? - to filter by member type when looking at a directory. Perhaps this is best left for another ticket, though.

Agree! But yes, something ajax powered.

[sooskriszta]

Replying to boonebgorges:

Having the same content available at multiple URLs is not good practice, for SEO

Actually declaring canonical URLs ensures that the multiple URLs create no issues at all for SEO.

So if user 'boone' had member types 'foo' and 'bar', you would want /foo/boone/ to resolve to /bar/boone/, or the other way around. But which?

Neither. I wouldn't want either to resolve to either. I's want both to show the same content (except for the page title, and breadcrumb, which would be different). Please refer to my comment above regarding how OpenCart handles this.

And what do you do when a user changes user type? Do you redirect or show 404s?

Again, neither. My idea is rather simplistic yet radical, so please do read fully and mull over it rather than reject out of hand. My idea is that /foo/boone/ and /bar/boone/ should always show the content, irrespective of whether boone is a member of foo or bar. It's just that boone will never show up in the directory of foo and bar if boone is not a member of these types.

what do we do if they also have a page 'foo', or a post called 'foo' with permalink structure /%pagename%/?

I guess for all permalink slugs we always check whether the slug exists for that post type. We'll probably need to extend that.

In WordPress, this issue actually exists in the form of tag/category/post name...there can be some confusion on occasion...

[boonebgorges]

#6333 has been opened to propose the use of member type slugs in single-member URLs. Let's use that ticket for further discussion of that specific feature, and use this one for the directory only.

[boonebgorges]

6286.patch​ is a first pass at adding member-type directories. Here's how it works:

• I've introduced the 'bp_register_member_type' hook. Member types must be registered here (instead of 'bp_init') in order to exist early enough to be detected during bp_core_set_uri_globals().
• bp_register_member_type() accepts a 'has_directory' param. Defaults to true. When true, the member type name is used as the slug. When a string value, that string value is used as the slug. When false, directories are disabled for the member type.
• URLs then look like example.com/members/foo, where 'foo' is the member type (or value of 'has_directory'). bp_core_set_uri_globals() has been reconfigured to make this work - this is the most fragile part of the patch, because bp_core_set_uri_globals() is such a beast. Unit tests are passing, but more will probably need to be written than what exist and are in this patch.
• The use of $_GET['member_type'] is also supported (example.com/members/?member_type=foo). In a fight between example.com/members/foo and example.com/members/?member_type=bar, foo wins.

I'd welcome feedback on the strategy here - in particular, the new 'bp_register_member_types' hook, the changes in bp_core_set_uri_globals(), and the 'has_directory' syntax I've chosen.

[DJPaul]

Will make time to review your patch later this week, but, I'm wondering how you are thinking we deal with URLs when a someone has their member-type changed? Redirects?

Also, do you think we need to block people from registering user names that match a member-type or not? Or would .com/moderators/moderators be acceptable?

[boonebgorges]

Thanks, DJPaul!

I'm wondering how you are thinking we deal with URLs when a someone has their member-type changed? Redirects?

I'm planning on not dealing with it at all - the current proposal is for directories only. Single member URLs are the subject of #6333. (The point about member type changes is, btw, yet another argument for a wontfix there.)

Also, do you think we need to block people from registering user names that match a member-type or not? Or would .com/moderators/moderators be acceptable?

This is a good point, but the issue is not .com/moderators/moderators, it's .com/members/moderator, where 'moderator' is a member type. In my inital patch, user pages will always win out in these types of clashes, and the member type directory is still available using ?member_type=moderator, but you're right that we should blacklist member-type directory slugs. I'll work on this for v2.

[r-a-y]

Is it possible to do this without manipulating the bp_core_set_uri_globals() function? Maybe as a hook to 'bp_screens'?

Something like:

function bp_members_set_member_type_dir_screen() {
	if ( ! bp_is_members_component() ) {
		return;
	}

	if ( bp_displayed_user_id() ) {
		return;
	}

	// logic to set the member type directory args
	// if ( bp_is_current_action( MATCHES_A_MEMBER_TYPE ) ) {
	//add_filter( 'bp_after_has_members_args', 'blah' );

	// load template
	bp_core_load_template( 'members/index' );
}
add_action( 'bp_screens', 'bp_members_set_member_type_dir_screen' );

[boonebgorges]

Replying to r-a-y:

Is it possible to do this without manipulating the bp_core_set_uri_globals() function? Maybe as a hook to 'bp_screens'?

Not really. If bp_core_set_uri_globals() detects that you're on the members component, it takes the next URL chunk and tries to identify a corresponding member, and if one is not found, it 404s. All this would have to be undone in a 'bp_screens' callback.

[r-a-y]

and if one is not found, it 404s. All this would have to be undone in a 'bp_screens' callback.

I guess we don't want to add any other hacks into the 'bp_screens' callback.

Patch looks good. What is the purpose of line 311?
} elseif ( empty( $bp->current_member_type ) ) {

Is this necessary?

This should be outside of this patch, but lines 307 and 312 look like they can be merged:
$bp_uri = array_merge( array(), array_slice( $bp_uri, $uri_offset + 2 ) );

[imath]

Hi Boone,

First i like very much the idea of filtering member types thanks to the url. I'm doing this kind of thing for the plugin ​BP Groups taxo hooking bp_actions at a 1 priority. For another ​plugin i wrote about member types i've chosen the Ajax road.

I guess the new has_directory argument is to use member types for admin management only purposes. Very nice idea :)

• changing from 'bp_init' to 'bp_register_member_type' hook (i'll need to update my plugin! and others will also need to do so), but i guess if it's still hooked to 'bp_init' then only the url feature will not be available ?
• About manipulating bp_core_set_uri_globals(), i've quickly checked something and surely did something wrong, but it seems bp_do_404 is not avoiding you to hook bp_actions: the ​quickdraft.patch allowed me to display the members directory as soon as a registered member type was in the url.

[boonebgorges]

Patch looks good. What is the purpose of line 311?
} elseif ( empty( $bp->current_member_type ) ) {

Oh, maybe I don't need that. Lemme play with it.

i guess if it's still hooked to 'bp_init' then only the url feature will not be available ?

Exactly.

the quickdraft.patch allowed me to display the members directory as soon as a registered member type was in the url.

Thanks for checking this out. I still would prefer to do this in bp_core_set_uri_globals(). Waiting until 'bp_actions' means that there's a decent portion of BP's bootstrap where the current member type is unset. More generally, doing URL routing in 'bp_actions' or 'bp_screens' seems wrong to me - it's always bothered me that this is how bp_is_directory() gets set. URL routing ought to take place early, and in a centralized place. The fact that bp_core_set_uri_globals() is scary and fragile is not really an argument against this :)

[imath]

@boonebgorges i know you find it ugly but i think there should be a type slug like members/type/teacher. Because once it will be possible to use this feature users will ask for editable slugs and we could arrive to the situation where registering with a teacher username would not be possible one day and possible another one as soon as the teacher member type slug will be edited or removed.
About i18n we can provide a filter till we have an UI in because inevitably there will be one :)
I'm also thinking of a potential groups taxonomy, if we don't use à slug prefix for member type then we will be a bit forced to not use one.

About the feature, i have another question once i arrived on the page filtered how i know i'm viewing members having the url member type without looking at the url ?

I think it's an important point because users might be confused as soon as they click on the "my friends" tab (or a tab added by a plugin). If they have 10 friends and only 3 are teachers and nothing is informing (else than the url) you are browsing members having the teacher type. I can imagine people screaming "i lost 7 friends" :)

[imath]

Well i guess the reply to my last point is: plugins using this feature will need to handle it :)

[boonebgorges]

Replying to imath:

@boonebgorges i know you find it ugly but i think there should be a type slug like members/type/teacher. Because once it will be possible to use this feature users will ask for editable slugs and we could arrive to the situation where registering with a teacher username would not be possible one day and possible another one as soon as the teacher member type slug will be edited or removed.

I think "editable slugs" (by which I guess you mean "editable user_login/nicename"?) is a red herring. But I'm starting to be convinced about the problem of user_nicename clashes. If someone registers with the name 'teacher', and then the admin activates a plugin that activates the member type 'teacher', there will be unpreventable problems. Though, to be fair, the same thing could happen in WP with a page called 'foo' and a plugin that registers a post_type 'foo'.

Then again, I think there's also an argument to be made that nearly all installations will want to prevent users from using member type names as user_login, URL issues aside - it's confusing, bordering on a security issue, for users to have names like 'teacher' if you also have a member_type called 'teacher'. And the implementation details of a 'type' slug are also very ugly: you suggest a filter, but I think that's a terrible piece of UX for something so public; a translatable string would be better, but causes its own problems.

Let me sleep on this. I really hate the idea of making the URLs hideous. example.com/members/type/foo implies a hierarchy that does not exist: "type" is not a type of member - "foo" is! But maybe there's no way around it.

About the feature, i have another question once i arrived on the page filtered how i know i'm viewing members having the url member type without looking at the url ?

Yeah, I forgot about this. I'll figure something out.

[espellcaste]

Agree that example.com/members/type/foo implies a hierarchy that does not exist. But in case it's decided to go in this way, would it be translatable or editable? The "type" part?

For example, in Brazilian Portuguese, "type" doesn't make much sense.

[boonebgorges]

Replying to espellcaste:

Agree that example.com/members/type/foo implies a hierarchy that does not exist. But in case it's decided to go in this way, would it be translatable or editable? The "type" part?

For example, in Brazilian Portuguese, "type" doesn't make much sense.

Yes, we'll do something - this was the "l18n" point discussed above. Either a constant, or a filter, or a translatable string, or some combination of these. The parallel case in WP is taxonomy bases - like 'category' in example.com/category/foo/ - which are editable on Dashboard > Permalinks. We don't currently have a UI for editing any slugs, and I imagine that once we do, these slugs will be editable there. See #2086, #4954.

[boonebgorges]

6286.2.patch​ includes the following improvements to the first patch:

• Changed URL structure from example.com/members/foo/ to example.com/members/type/foo/. The string 'type' is translatable - so that it can be included in language packs - and also run through a filter. When/if we have a UI for editing slugs, support for fetching from the database can be added.
• Added a current-member-type-message to the top of member directories, when being filtered: "Viewing members of the type: Teacher". Suggestions for improved wording/placement/styling are welcome.

[boonebgorges]

In 9723:

Introduce member-type-specific Members directories.

The new 'has_directory' argument for bp_register_member_type() allows
developers to specify whether a list of members matching a given type 'foo'
should be available at http://example.com/members/type/foo/. A slug can be
passed to 'has_directory' to customize the URL used for the member type's
directory.

Note that plugins registering member types must do so at the new hook
'bp_register_member_types' in order to be able to customize the 'has_directory'
value (from its default of true).

bp_has_members() automatically detects the presence of a member type in a
URL. When no member type of the form example.com/members/type/foo/ is found,
URLs of the form example.com/members/?member_type=foo will be detected.

See #6286.

[boonebgorges]

First pass is in [9723]. I'll leave this ticket open for additional feedback and fixes leading up to beta.

[johnjamesjacoby]

This all looks really good, save for maybe one bit:

    $member_type = bp_get_current_member_type();
    if ( ! $member_type && ! empty( $_GET['member_type'] ) ) {
        if ( is_array( $_GET['member_type'] ) ) {
            $member_type = $_GET['member_type'];
        } else {
            // Can be a comma-separated list.
            $member_type = explode( ',', $_GET['member_type'] );
        }
    }

Possible we can pluck or parse this against known registered types, to ensure we're dealing with proper member types and not Little Bobby;\" DROP TABLES?

[boonebgorges]

We're protected further upstream, in multiple ways:

• ​https://buddypress.trac.wordpress.org/browser/trunk/src/bp-core/classes/class-bp-user-query.php?marks=430,431,432#L421 ensures that we're only checking against registered member types
• The fact that we're running member types through WP_Tax_Query ​https://buddypress.trac.wordpress.org/browser/trunk/src/bp-core/classes/class-bp-user-query.php?marks=438#L421 means that we get the SQL injection protection there.

The only $_GET-specific sanitization that might be appropriate here is URL decoding, but I left that out because member type names can't have urlencoded characters in them anyway ​https://buddypress.trac.wordpress.org/browser/trunk/src/bp-members/bp-members-functions.php?marks=2479#L2467

[Bowromir]

This looks really good and everything seems to work great on my test install. I have one feature request/idea:

I think it could be very useful to add an additional option to restrict viewing of a directory based on member type or user role.

has_directory = true,
access_role = "admin,editor"
access_type = "student, teacher"

This feature would be useful if you want to utilise the awesomeness of the Member Directors for quick searching/editing members from the front end without making that directory public.

Restricting by member type would be cool to say "Only teachers can view all of the students". Some communities might not have a public member directory but still need a front end searching/listing for a certain member type.

[boonebgorges]

Thanks for the feedback, Bowromir!

I like your suggestion, and I think it's worth having a separate ticket for it. We don't really do a very good job throughout BP with controlling access to content, so this might be a good test case for us to build a more generalizable system.

[boonebgorges]

I think we're done here.