bp_group_is_user_banned() doesn't reference the group loop's "is_banned" value properly

[r-a-y]

More optimization tweaks!

If you're logged in with regular user rights (not a BP moderator or admin) and you're on the group directory, a check is done to see if you are banned from each group in the loop using bp_group_is_user_banned() function when rendering the group join button:
​https://buddypress.trac.wordpress.org/browser/tags/2.0/bp-groups/bp-groups-template.php#L1773

This function does not check the already-queried data from BP_Groups_Group::get_group_extras() properly. Some attempts were made in the past to address this, but did not take effect.

This could lead to 20 additional DB queries on the group directory page if you are a regular user.

Attached patch removes these queries.

Some gotchas:

• groups_is_user_banned() returns a string of the integer when a result is matched and null when no match is found. I found this out when writing unit tests. However, it is possible for bp_group_is_user_banned() to return a boolean.
• My question is do we want to force bp_group_is_user_banned() to return a boolean or a string of the integer at all times? What about those that do strict conditional type checking?

[boonebgorges]

Patch looks good to me.

My question is do we want to force bp_group_is_user_banned() to return a boolean or a string of the integer at all times? What about those that do strict conditional type checking?

It really seems like this function, and others like it, should be returning booleans. I would suggest making that change. It's highly unlikely anyone's doing strict checking with this function (if you tried, you'd end up discovering how weird the return values are, and you'd go back to loose checks). May be worth noting in the inline docs that return values were changed, on the off chance it does break something and a dev does a stack trace to debug.

[r-a-y]

It really seems like this function, and others like it, should be returning booleans. I would suggest making that change.

For sure, will do this. Just wanted a sanity check from another dev!

[r-a-y]

In 8531:

Check groups loop data when using bp_group_is_user_banned().

Previously, `bp_group_is_user_banned() did not reference the data already
queried in the groups loop via BP_Groups_Group::get_group_extras(). As a
result, if a user without the 'bp_moderate' cap was logged in and viewing
the group directory, this added twenty additional DB queries to the page.

Commit also standardizes the return value of bp_group_is_user_banned() to
a boolean. Previously, it was possible for the function to return either a
string of the integer, null or a boolean.

Fixes #5705.