Allow Group Admins to Unban Members

[skyrie]

Right now, the only way to unban a member from a group is for the site admin to do it fron the backend. There is no way for a regular BuddyPress group admins to be able to unban members once they click the "Kick and Ban" option.

Group admins should be able to see banned members and unban them from the frontend group admin page.

See support thread here:
​https://buddypress.org/support/topic/unban-from-group/

[boonebgorges]

+1. This would be great.

[DJPaul]

I am working through this ticket with a first-time contributor.

[thebigA]

Patch

[thebigA]

When I looked at the source file for the template bp-templates/bp-legacy/buddypress/groups/single/admin.php I could see that there was already code to allow for a member being banned to be reinstated. This was never being processed so I looked to see what records the underlying dataset was working on.

The setting of the check for exclude_banned is in bp-groups-functions.php on line 544. This checks to see if the element passed through the parameters array is true. The original code had 'exclude_banned=false' which was being evaluated as 'false' being a string not a boolean so when it got to the test it was always evaluation as true since it was not an empty string. Changing the original code to 'exclude_banned=0" is evaluating as a boolean so now works as expected.

[r-a-y]

Good catch, thebigA.

The problem is with wp_parse_str(), which is used when we pass a querystring to convert into an array as in the case of bp_group_has_members() used in the group admin template.

When the querystring is converted over to an array, the 'true' or 'false' values are strings and need to be explicitly converted over to booleans.

I've attached a patch that fixes this by filtering 'wp_parse_str' and using PHP's filter_var() to convert the boolean over. This will fix any old string / boolean issues we've had with wp_parse_args() in the past.

I think this may be more applicable as an upstream patch to WP.

[DJPaul]

The filter fix would affect other code beyond BuddyPress; it could "fix" something that other people don't know that it's broken, causing a change in behaviour ("why does BuddyPress change this thing in X and Y plugins?"). I think I'd prefer to apply the original patch to BuddyPress, and, Ray, we take your patch upstream.

[DJPaul]

Any thoughts on this, r-a-y?

[r-a-y]

Regarding wp_parse_str(), I opened a ticket upstream:
​https://core.trac.wordpress.org/ticket/30753

I kind of agree that we shouldn't add my patch, so let's get thebigA's patch committed perhaps with a unit test.

Will try and get to this later today if no one beats me to it.

[r-a-y]

Okay, so I've had a chance to think about thebigA's fix a bit more.

His patch requires a template change and a similar change will need to go in bp-default as well (if we want to fix bp-default).

groups_get_group_members.patch is an alternate fix that occurs where we do our comparison checks to fetch group members. It explicitly casts the string 'false' value for 'exclude_banned' to a proper false value and doesn't require any template changes. This is a little ugly, but we've done similar things in the past (see r7574).

I'm fine with either patch being committed.

[boonebgorges]

Notwithstanding [7574], I think we should just make the template change suggested in 6013.1.patch​. The other options add layers of obfuscation or odd variable handling that I don't think will help us in the long run.

[r-a-y]

Let's commit thbigA's patch.

Unit test is already covered via source:/tags/2.1.1/tests/phpunit/testcases/groups/template.php#L367​

[r-a-y]

In 9292:

bp-legacy: Show banned members when on a group's Manage Members page.

Previously, we passed exclude_banned=false in a querystring. This gets
interpreted by wp_parse_args() as string false. Our exclude_banned`
check in groups_get_group_members() does not interpret string 'false'
values, which led to banned members being excluded instead of included.

This commit switches the querystring to exclude_banned=0, which
groups_get_group_members() will interpret as a proper false value
allowing banned members to be shown.

Props thebigA.

Fixes #6013.