Introduce functionality to query members by member_type where member_type NOT IN

[lakrisgubben]

It would be nice to have the ability to exclude certain member types from member listings. Attached patch introduces a
member_type__not_in syntax that follows how the member_type syntax works in bp_has_members(). Name is inspired by how WP Core usually names things (i.e. category__not_in, author__not_in etc).

[boonebgorges]

This is great, lakrisgubben - thanks for the patch. A couple thoughts:

• Gotta have unit tests for this.
• If we're going to introduce 'member_typenot_in', we should probably provide 'member_typein' as well. 'member_type' already allows for an array of types, but just for consistency. If someone passes both 'member_type' and 'member_typein', let's have 'member_type' take precedence.
• Can we do this without spinning up a separate WP_Tax_Query? Seems like we should be able to assemble arguments for a single tax query object related to bp_member_type. This might require that the SQL regex be reworked a bit.

[lakrisgubben]

thanks for the feedback boone! :) Latest patch makes it possible to use member_type__in instead of member_type by checking

if ( empty( $member_type ) && ! empty( $member_type__in ) ) {
	$member_type = $member_type__in;
}

Was it something along the lines of that that you'd thought of?

Patch also merges the two WP_Tax_Querys into one, but I'm not the best at writing regex so that could probably be handled nicer. :)

Regarding unit test, I'm not really familiar with writing them for BP, but it'd be nice to try and dip my toes into that water. Would be grateful though if you could give some pointers as to what exactly I should test for. :) After that I can probably manage to write them ;)

[boonebgorges]

@lakrisgubben - Cool, this is looking closer.

Here are some unit tests you can use as a starting point: ​https://buddypress.trac.wordpress.org/browser/tags/2.2.0/tests/phpunit/testcases/core/class-bp-user-query.php#L410 Tests needed: (a) show that 'member_type__in' works, (b) show that 'member_type' overrides 'member_type__in', (c) show that 'member_type__not_in' returns members both members from types other than those specified, as well as members that have no member type at all, (d) show that 'member_type__not_in' and 'member_type' work together in a sane way, (e) show that 'member_type__not_in' returns an empty set in the proper cases. :-D

I'm not sure I understand what's going on with the regex, but I'll take a look myself once there are some unit tests that make it a bit easier to test.

[lakrisgubben]

thanks @boone!

Latest patch adds unit tests for all the cases you mentioned above. I went with member_type__not_in takes precedence over member_type and member_type__in since that's how it seems that WP core handles those kinds of cases (looked at author__not_in vs author__in). Wasn't sure if I should test for all different cases of array, comma-separated etc. with member_type__not_in so if you'd like that please let me know and I'll add those as well. :)

[boonebgorges]

Thanks lakrisgubben! These unit tests look outstanding.

I've reworked the internals of the patch a bit so that we can avoid repetition. Because you've optend to follow WP_Query in ignoring member_type altogether when member_type__not_in is present, I've rolled back my original suggestion that you create a single tax_query that contains multiple clauses - it's not needed anymore. I also simplified the regex necessary to parse the NOT IN clause.

Looking more carefully at this, I am concerned that it's not possible to make a query for users who have no member type at all. Can you have a look at this? I wonder if we might set the default value of member_type__in to null, and then parse it when ! is_null( $member_type__in ), instead of ! empty(). This would allow devs to pass 'member_type' => array() to return only those users who have no member types. What do you think?

[lakrisgubben]

cool @boone, thanks!

I'm not sure that passing an empty array to member_type is the best way (might be wrong though), it seems a bit non-intuitive that an empty array would result in getting all members without a member_type. Looking at how WP_Query handles tags (which is the only default thing that can be empty, seeing as how a category and author is always set) it doesn't do anything at all (i.e. it ignores the tag parameter) if you pass an empty value to tag or tag__in

Wouldn't a query like member_type__not_in => bp_get_member_types() be an easier way to accomplish this?

[DJPaul]

@boonegorges I'm at WCEU and I've just discussed this ticket with @lakrisgubben. I agree that it's a very nice patch, and it's been well written.

After some consideration, I agree with the suggestion that using member_type__not_in => bp_get_member_types() to query for members without any member type is the best approach to resolve that problem. I think it just makes more sense to parse the requirement this way rather than rely on passing null (which strikes me as rather non-obvious). Fully support committing the latest version of the patch, but wanted to let you do that to give you a chance to communicate if you have a strong reason for preferring to do exclusion via the __in parameter.

[boonebgorges]

I think it just makes more sense to parse the requirement this way rather than rely on passing null (which strikes me as rather non-obvious).

Just to be clear: My proposal was that null be the *default* value. To fetch users who have no member types, you'd pass *an empty array*. This, to me, is fairly intuitive.

That being said, I don't feel strongly about it, and member_type__not_in => bp_get_member_types() is functionally identical. I'll go ahead with the patch as it currently stands.

[boonebgorges]

In 9976:

Add 'member_typein' and 'member_typenot_in' support to bp_has_members() stack.

Props lakrisgubben.
Fixes #6418.