Groups Bugs

[ken_k]

1. When you are an admin in a Group, you are able to demote yourself which results in a 404 error, and a 404 when you leave the membership in the group.

2. In addition to above, this makes the group “hidden” if the admin was the sole member. So there is a group count for a group that does not exist.

[johnjamesjacoby]

We used to have sanity checks that prevented this. Moving to 1.5.1 for review.

[djpaul]

(In [5203]) Prevent sole admins leaving groups. See #3631

[djpaul]

(In [5204]) Prevent sole admins leaving groups. See #3631

[DJPaul]

Remaining issue is how to deal with existing groups that have no admins; the log dumps errors like:

[30-Sep-2011 21:34:23] WordPress database error You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ')' at line 1 for query SELECT COUNT(DISTINCT ID) FROM wp_users WHERE user_status = 0 AND ID IN (  )  made by require, wp, WP->main, do_action_ref_array, call_user_func_array, bp_screens, do_action, call_user_func_array, groups_screen_group_admin_manage_members, bp_core_load_template, load_template, require_once, locate_template, load_template, require_once, bp_has_members, BP_Core_Members_Template->__construct, BP_Core_User->get_specific_users

$include in BP_Core_User->get_specific_users() is an empty string, causing the false check to be handled incorrectly (line 184ish, bp-members-template.php). I see 3 options to fix: 1) Update this query logic, 2) Update the 'group admin manage members' to check we don't pass an empty string to get_specific_users(), or 3) don't do anything as this is an edge case caused by a (now fixed) bug.

I think option 3 is appropriate but appreciate input from other devs.

[boonebgorges]

I would be OK with option 3, given that this particular instance is not a regression. Otherwise, an easy fix is to bail at the beginning of BP_Core_User::get_specific_users() if !$user_ids (or rather, not to bail, but to return array( 'users' => array(), 'total' => array() ) ). At least, it's worth testing this solution; if it causes a bunch of PHP notices downstream, then I say we leave it as it is.

[boonebgorges]

On further reflection, I think that for 1.5.1 we should probably call this good enough. If we want to cover the SQL issue, which will take some non-trivial mucking about in the query class, we'll do it for 1.6 (and it should get another ticket, which I will open now #3663).