Small improvements to Member Types functions

[Offereins]

In BP 2.2-beta2, checking out the new Member Types API functionality, I encountered some bugs and redundant code:

• Modification of the type args $r without storing it in the type object
• Checking for type (array key) existence with empty() instead of isset()
• Declaring a $field variable without using it

A patch is attached for bp-members/bp-members-functions.php.

[Offereins]

Patch for bp-members-functions.php

[johnjamesjacoby]

The $output parameter is only used in user_admin_load(), but not for the names specifically anyways. We could eliminate it completely to simplify the function and remove the hardcoded names bit entirely IMO.

Looking at this more makes me think the bp_get_member_types() function is doing too much compared to other similar utility functions. ​6125.02.patch simplifies bp_get_member_types() and introduces another helper to filter those results if it ever becomes necessary.

[boonebgorges]

In 9360:

Parse arguments correctly in bp_register_member_type().

Custom labels were not being properly set due to a logic error in the
function.

Props Offereins.
See #6125.

[boonebgorges]

In 9361:

Fix incorrect variable name in docblock of bp_get_member_type_object().

Props johnjamesjacoby.
See #6125.

[boonebgorges]

Thanks, Offereins and johnjamesjacoby.

Checking for type (array key) existence with empty() instead of isset()

This is copied right from get_post_type_object(). There should be no natural case in which a member type key is set to an empty value (like an empty string). That being said, there's no harm in making the change to isset().

Looking at this more makes me think the bp_get_member_types() function is doing too much compared to other similar utility functions. 6125.02.patch simplifies bp_get_member_types() and introduces another helper to filter those results if it ever becomes necessary.

One of the goals here was to make the API look exactly like WP's post type API. So bp_get_member_types() has a function signature identical to get_post_types(). Unless there's a powerful argument to the contrary (like a bug that it introduces), I'd prefer not to break the filter logic out into a separate function.

[boonebgorges]

In 9362:

Remove copypasta from bp_get_member_types().

Props Offereins.
Fixes #6125.

[johnjamesjacoby]

Replying to boonebgorges:

One of the goals here was to make the API look exactly like WP's post type API. So bp_get_member_types() has a function signature identical to get_post_types(). Unless there's a powerful argument to the contrary (like a bug that it introduces), I'd prefer not to break the filter logic out into a separate function.

I figured it was something like this. My only concern with it how it is now, is the hardcoded limitation we put on names vs. any other possible field that someone may want to pluck by, and the odd position it puts the filter in. As it sits now, we've whitelisted names for no reason, as each usage overrides it to objects, which makes it look accidental or unfinished.

[boonebgorges]

My only concern with it how it is now, is the hardcoded limitation we put on names vs. any other possible field that someone may want to pluck by, and the odd position it puts the filter in. As it sits now, we've whitelisted names for no reason, as each usage overrides it to objects, which makes it look accidental or unfinished.

Not for no reason: because this is how get_post_types() does it. When pulling up a list of member types, what you generally want is either (a) to have a flat list of the member types (ie names), or (b) to have all available information about them (ie objects). I just happened to use wp_list_pluck() to make it happen. IMHO, it would add little value to support any member type key (like, say, 'labels') when it's just as simple for devs to pull the objects and wp_list_pluck() whatever they want themselves. By limiting what's officially supported in terms of parameter values (and, importantly, by making the documentation more limited - 'names' and 'objects', period), we make it clearer what the purpose of the function is. That said, if it bothers you a lot, I don't have a huge problem with just passing 'output' to wp_list_pluck() and having a special case for 'objects'.

The filter comes before the pluck so that filtering functions don't need to worry about checking to see if the values passed to the filter are objects or strings. If you think about the various ways you might use this filter in real life, I think that this setup makes more sense.