Skip to:
Content

BuddyPress.org

Opened 15 years ago

Closed 11 years ago

Last modified 9 years ago

#921 closed enhancement (fixed)

bp_group_has_members() - "order" parameter request

Reported by: r-a-y's profile r-a-y Owned by: boonebgorges's profile boonebgorges
Milestone: 2.0 Priority: normal
Severity: normal Version: 1.9.1
Component: Groups Keywords: has-patch needs-testing
Cc: math.viet@…

Description

Love the BP loops!

I would like to have a few additional parameters for the bp_group_has_members() loop.

Specifically, "orderby" and "order" arguments to sort by ascending, alphabetical order for example.

"order" could have the following options - 'alpha', 'joindate' (current default)
"orderby" could have the following - 'ASC', 'DESC'

The default list order is by whoever joined the group first.

Attachments (7)

921.diff (20.0 KB) - added by imath 11 years ago.
groupmembers-loop.php (3.9 KB) - added by imath 11 years ago.
921.02.diff (18.4 KB) - added by imath 11 years ago.
921.03.diff (13.8 KB) - added by boonebgorges 11 years ago.
921.04.diff (16.3 KB) - added by imath 11 years ago.
921.05.query.diff (8.8 KB) - added by boonebgorges 11 years ago.
921.05.template.diff (12.2 KB) - added by boonebgorges 11 years ago.

Download all attachments as: .zip

Change History (33)

#1 @apeatling
15 years ago

  • Milestone changed from 1.1 to 1.2

No time for this in 1.1, but will add for 1.2

#2 @DJPaul
15 years ago

  • Keywords reporter-feedback added
  • Milestone changed from 1.2 to 1.3

Still valid bearing in mind the numerous BP loop changes?

#3 @johnjamesjacoby
15 years ago

  • Component set to Core

Update on this:

Request is still valid, but the function names have changed.

Now this will apply to bp_has_members, and BP_Core_Members_Template.

#4 @DJPaul
14 years ago

  • Keywords needs-patch added
  • Priority changed from minor to normal

#5 @boonebgorges
14 years ago

  • Milestone changed from 1.3 to Future Release

Punting due to time constraints. If you can write a patch, we can consider rolling it back.

#6 @aaclayton
12 years ago

  • Component changed from Core to Groups
  • Severity set to normal

Seems like this ticket has been dead for a while, but I'd like to weigh in requesting that some arguments be added to bp_group_has_members();

Similar to the 'type' argument accepted by bp_has_members(), perhaps the group members loop could accept "newest, active, online, alphabetical, random"

Likewise, 'sort' => 'ASC' or 'type' -> 'oldest' would be great for many groups. Showing the newest members (the current default) at the top of a roster might be counterproductive to some groups organizational structure.

Version 0, edited 12 years ago by aaclayton (next)

#7 @imath
11 years ago

  • Cc math.viet@… added
  • Keywords has-patch needs-testing 2nd-opinion added; reporter-feedback needs-patch removed

Hi,

I find the idea very interesting, it can be very convenient to sort group members, so i've explored it and i'm suggesting the attached patch : 921.diff.

I'm sorry i had to create a new template file i've called groupmembers-loop.php in bp-template/bp-legacy/buddypress/groups/single/. I've also attached this template to this ticket if you want to test the patch.

The patch creates a new select box in the group members area and in the group admin (front) area. The different options are :

  • Newest (same as current default way of sorting group members 'last_modified')
  • Oldest (Newest with a ASC order instead of DESC one)
  • Alphabetical (using if xprofile enabled profile fields, else $wpdb->users display_name to sort by name ASC)
  • Active (to sort members by their latest update in the current group if activity component is active), this way it can also be a reply to #3430
  • Friends (to only show the friends of the loggedin user)

About the last option, i think it can be interesting before joining a group for a regular user to easily see if some of his friends are already in the group to eventually join them. And in the admin front part of the group, a group admin might want to find his friends to eventually promote them as mods or admins.

@imath
11 years ago

#8 @boonebgorges
11 years ago

  • Keywords early-2.0 added

imath - thanks for the patch. I would definitely like to get this in, but I do think that we'll want to think more about how some of it works.

The way you've reworked the handlers in buddypress-functions.php seems less than ideal to me (we can do better, I think, than having special cases just for 'groupmembers'). In particular, if we're going to require component info in bp_legacy_theme_object_template_loader(), we should be either (a) sending it along with the JS, or (b) getting it from some centralized place where loop templates are registered.

This might, in fact, be a good time to rethink how the AJAX works here more generally. What we have now is a nifty little system, but the hoops you jump through in your patch demonstrate that it's not very flexible. This could be done in conjuction with the new template pack slated for the next release.

Let's look at this for 2.0.

#9 @imath
11 years ago

Hi boonebgorges,

So i put a new eye on the patch and choose option (a) : sending the template from js. I focused on the "display" screen of the group members (group admin screen hasn't changed). I've added 2 new options to filter the group member :

  • 'Oldest' which is the same than 'Newest' (last_modified) but with a ASC order
  • 'Alphabetical' which was used in Group single WordPress Administration screen but was not really working (see the comment in BP_Group_Member_Query->set_orderby() method).

I've also added a new search box as we can benefit from BP_User_Query.

Finally i've added a filter the way you suggested in #5356 to eventually let people add a custom option in the group members select box to change the order of the member_ids.

This is a screen capture of the result of 921.02.diff patch :

http://farm4.staticflickr.com/3798/12479282763_1b95721dd2_o.png

@imath
11 years ago

This ticket was mentioned in IRC in #buddypress-dev by imathfromparis. View the logs.


11 years ago

#11 @boonebgorges
11 years ago

I have some thoughts about how this ticket can be improved, but I'm first going to make a couple preliminary fixes (and then post an updated patch).

#12 @boonebgorges
11 years ago

In 7930:

Refactor bp_group_has_members() stack to use array-style parameters

See #3797, #921

#13 @boonebgorges
11 years ago

In 7931:

Ensure that 'page' value passed to BP_Groups_Group_Members_Template is respected

Previously, it was being ignored - the only value considered was
$_GETmlpage?, with a hardcoded fallback of 1.

See #921

Props imath

#14 @boonebgorges
11 years ago

In 7932:

Add support for 'search_terms' param in bp_group_has_members() stack

See #921

Props imath

@boonebgorges
11 years ago

#15 follow-up: @boonebgorges
11 years ago

  • Keywords 2nd-opinion removed

921.03.diff is imath's .02.diff, but updated for the current trunk. There were a couple of random bugfixes and improvements mixed up in the original patch, which I separated into different commits. Also, .02.diff had some changes to parameter order in various functions that would've broken backward compatibility, so I thought this would be a good time to convert to using the parameters-as-array format.

Specific thoughts on imath's approach:

  • Let's not pass parameters to BP_Group_Member_Query::get_group_member_ids(). Just get the value out of $this->query_vars.
  • I know we use the 'type' param (like type=last_modified) throughout BuddyPress, but I'm not a huge fan of it. It doesn't provide very much flexibility; 'order' and 'orderby' are much better, with 'type' being translated into 'order' and 'orderby' if necessary. Of course, now that I type this, I see that BP_User_Query doesn't support this syntax, so maybe it's an area for future enhancement.
  • Maybe instead of setting the $type in get_group_member_ids(), it'd be possible to do it in setup_hooks(). I don't totally remember the logic behind setting it in setup_hooks, but it looks like it might be reproducing (or reversing) some logic to do it later on as well. Play with it and see what you find.
  • The changes to BP_Group_Member_Query really need unit tests.
  • The special case for the Group Members page in bp_legacy_theme_ajax_querystring() is really ugly. Isn't there another way we can handle this? Why is 'object' not set properly by passing 'group_members' to bp_ajax_querystring() in groups/single/members.php? I'm not sure about the exclude_admins_mods item - maybe we could set this in the JS somewhere (like you're doing with template)? or via a hidden input in the template that's then slurped up by the JS? or maybe in the AJAX handler? bp_legacy_theme_ajax_querystring() is really meant to be a generic utility function, so I'd really like to keep special cases out of it if possible.

The rest of the approach looks good. The technique for determining member sort in BP_Group_Member_Query looks pretty elegant, and I like the change to bp_legacy_theme_ajax_querystring() and bp_filter_request() that let you pass a custom template. If we address some of the items I list above (especially unit tests! no messing with queries without them!) I think this'd be ready for 2.0.

@imath
11 years ago

#16 in reply to: ↑ 15 @imath
11 years ago

Replying to boonebgorges:

  • The special case for the Group Members page in bp_legacy_theme_ajax_querystring() is really ugly. Isn't there another way we can handle this? Why is 'object' not set properly

Thanks a lot boonebgorges for the commits and the "updated to the trunk" patch.

a) I've played with it and noticed that the filters were not working, so i've edited the arguments of groups_get_group_members() & BP_Groups_Group_Members_Template::__construct() to include a "type" one.
b) As i was checking theme backcompat i've found a specific case (group/single/home template updated but not groups/single/members) where the pagination were messed up with admin-ajax. So i suggest to use an absolute url for the pagination links, just in case ;)

Now the "ugly" check in bp_legacy_theme_ajax_querystring() :)

"once upon a time" of its birth :
In buddypress.js, the object is first defined by a parsing on the value of the id attribute of .item-list-tabs ul li.selected. in group members case, this value is "members" as the id is #members-groups-li. I was a bit afraid to modify this logic for this specific case, and chose to mimic the way friends case was handled by forcing the object to be set as "members" in case it is defined to "group_members" in bp_filter_request(). That's the reason, i use the ugly check.

In 921.04.diff, i've included a) & b) and suggest to move the check at the top of bp_group_has_members() function.

This ticket was mentioned in IRC in #buddypress-dev by imathfromparis. View the logs.


11 years ago

#18 follow-up: @boonebgorges
11 years ago

  • Keywords early-2.0 removed
  • Milestone changed from Future Release to 2.0

Cool, I think we're getting close!

This patch is getting a bit unwieldy, and really it does two different things (and so should be committed separately anyway), so I've split it into two patches:

921.05.query.diff

This contains the modifications to BP_Group_Member_Query that allow for $type sorting, as well as the mods to the bp_has_members() function stack that allow you to take advantage of $type. I've made a couple of modifications to 04.patch:

  • Changed 'last_modified' and 'first_modified' to 'last_joined' and 'first_joined'. This makes much more sense from the point of view of themers/developers.
  • Changed some of your logic in set_orderby(), so that it's possible to use *any* of the $type values supported by BP_User_Query (even though you're only using 'alphabetical' in your patch).
  • Added unit tests for the new $type parameter.
  • Reworked some documentation
  • Removed one instance of setting $this->query_vars['type']. This is already done in setup_hooks().

921.05.template.diff

This is the patch that implements the new features in query.diff within the context of single groups. I think the only thing I changed here was 'last_joined' and 'first_joined'. The rest of it looks fine to me.

Give it a look and see what you think.

#19 in reply to: ↑ 18 @imath
11 years ago

Replying to boonebgorges:

I've just tested and it works great!!

  • Changed 'last_modified' and 'first_modified' to 'last_joined' and 'first_joined'. This makes much more sense from the point of view of themers/developers.

I agree.

  • Changed some of your logic in set_orderby(), so that it's possible to use *any* of the $type values supported by BP_User_Query (even though you're only using 'alphabetical' in your patch).

Yes, it's a great idea. I've limited to alphabetical in my patches because i thought maybe at wrong ("peut-être à tort" ) that available orders in BP_User_Query was a bit too global (active in the whole community instead of in the group for instance). But online and popular can be interesting.

Give it a look and see what you think.

I think it's really great. This feature is really nice and i think it gives a new opportunity to theme/plugin developers to extend with new types of order using the action 'bp_groups_members_order_options' to add a customized order in the dropbox and the filter 'bp_group_member_query_group_member_ids' to build the corresponding order.
Bravo for the amazing work :)

#20 @boonebgorges
11 years ago

In 7948:

Introduce support for a 'type' sorting parameter in BP_Group_Member_Query

The 'type' parameter supports any of the arguments ('newest', 'active', etc)
supported by the parent class BP_User_Query. In addition, it's possible to
sort by type 'last_joined' and 'first_joined'.

This changeset also introduces the 'type' parameter through the
bp_group_has_members() function stack.

See #921

Props imath, boonebgorges

#21 @boonebgorges
11 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 7949:

Enable member sorting and pagination on group Members pages

Fixes #921

Props imath, boonebgorges

#22 @aaclayton
11 years ago

Hey guys, thanks so much for the awesome work on this, I'm really looking forward to incorporating some group member sorting into my theme. Thumbs up!

#23 @claenjoy
9 years ago

@boonebgorges

I can't find on buddypress default theme "order by" in the group, I have installed WP 4.2.2 with BP 2.3.2.1

there is "order by" on the tab "All members" but not for the single group.

Screenshot
https://www.evernote.com/shard/s403/sh/3867a8bb-613e-45fc-8d71-f252dfb6ea5b/08eacde75590aaac902476652cfcfb69

this is working on the WP Twenty 13,14,15 themes

Last edited 9 years ago by claenjoy (previous) (diff)

#24 @r-a-y
9 years ago

@claenjoy - The bp-default theme is in maintenance mode and isn't really accepting progressive feature enhancements any more. See:
https://bpdevel.wordpress.com/2013/11/13/the-future-of-the-bp-default-theme/

You'll have to apply the changes from:
https://buddypress.trac.wordpress.org/changeset/7949#file1

To the bp-default theme.

Last edited 9 years ago by r-a-y (previous) (diff)

#25 @claenjoy
9 years ago

  • Version set to 1.9.1

@r-a-y

thanks for your help, I tried for some reasons when I search in a group with only 3 members it show a members with the correct search but of an other group

"Order by" on any type of sort it lists the members only in alphabetical order, and also it show all the members of the others groups.

tested on WP Twenty 13,14,15 themes and it doesn't show other members only the groups members.

Can you please help to understand if I missed any code on the bp-default theme

thanks

#26 @claenjoy
9 years ago

managed to make the "search member" works

now when I use the "Order by" it just flash quick the members list and then appear the same list order, any suggestions ?

Note: See TracTickets for help on using tickets.