Better sorting for new bp_group_has_members() query

[aaclayton]

Since 1.8, the new bp_group_has_members() query is working great, but I perceive a limitation of the new approach. Previously, bp_group_has_members() sorted group members based on their join date (‘last_modified’ in the bp_groups_members table). After the update, the query is sorting based on ‘user_id’ instead.

It would be really great to include sorting arguments in bp_groups_has_members(). I think that this function could accept “type” and “sort” arguments, where “type” could take values of either ‘user_id’ or ‘last_modified’, and “sort” would be either ‘ASC’ or ‘DESC’.

From an organizational perspective, a group member’s join date is (typically) a more informative sorting variable, and depending on the use case, either ascending OR descending could make more sense. Giving BuddyPress users the ability to tweak their loop sorting in the function would be great.

Hope this is feedback that could be implemented. Thank you!

[boonebgorges]

Thanks for the report, aaclayton. I've confirmed the issue.

The changed behavior for bp_group_has_members() is a bug. It was intended that BP_Group_Member_Query would sort by last_modified. See ​http://buddypress.trac.wordpress.org/browser/tags/1.8/bp-groups/bp-groups-classes.php#L1100. And when these values were passed back to BP_User_Query, it was intended that the query would preserve this sorting. I'll look into why this isn't happening, but the fact that it's not is a regression, so I'm moving the ticket to 1.8.1.

As for the further request for better query params - I agree (see the inline todo!). Once I've fixed the regression in this ticket, I'll move this request to a separate enhancement ticket.

[boonebgorges]

There's a fundamental problem with the way BP_User_Query's fallback sort order works, or at least the way BP_Group_Member_Query relies on it. In MySQL, when you fail to provide an ORDER BY clause, the primary column is assumed, which in this case is the user id.

Because we want to maintain the sort order based on the order of the values passed to IN, we have to use a ORDER BY FIELD() clause. See 5106.patch. This is not elegant, and probably won't perform well when a group has many thousands of members, but this is a larger architectural problem that doesn't really need to be addressed in this ticket.

Posting here for review by another dev.

[DJPaul]

Haven't tested it just yet, but the patch looks good (ORDER BY FIELD is how WP_Query does its post__in ordering).

[boonebgorges]

Thanks, DJPaul - never even thought to look at how WP_Query does it, but the fact that they do it the same way is a good indicator that it's good enough for us too.

[boonebgorges]

In 7294:

Tell BP_Group_Member_Query queries to order by FIELD()

BP_Group_Member_Query works by first querying for a list of group members
that match the query params, sorted by date_modified. Then, it feeds those ids
to the BP_User_Query parent class. In order to maintain the proper
date_modified sort, we have to override BP_User_Query's ORDER BY param, and
replace it with ORDER BY FIELD(), using the user ids from the first query.

This fixes a regression in BP 1.8, where the default sort order for
bp_group_has_members() was inadvertantly changed to user_id DESC, rather than
the legacy date_modified that existed before the introduction of
BP_Group_Member_Query.

Fixes #5106

[boonebgorges]

Opened #5109 for the enhancement request.

[boonebgorges]

In 7296:

Tell BP_Group_Member_Query queries to order by FIELD()

BP_Group_Member_Query works by first querying for a list of group members
that match the query params, sorted by date_modified. Then, it feeds those ids
to the BP_User_Query parent class. In order to maintain the proper
date_modified sort, we have to override BP_User_Query's ORDER BY param, and
replace it with ORDER BY FIELD(), using the user ids from the first query.

This fixes a regression in BP 1.8, where the default sort order for
bp_group_has_members() was inadvertantly changed to user_id DESC, rather than
the legacy date_modified that existed before the introduction of
BP_Group_Member_Query.

Fixes #5106