#921 closed enhancement (fixed)
bp_group_has_members() - "order" parameter request
Reported by: | r-a-y | Owned by: | 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)
Change History (33)
#2
@
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
@
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.
#5
@
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
@
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.
#7
@
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.
#8
@
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
@
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 :
This ticket was mentioned in IRC in #buddypress-dev by imathfromparis. View the logs.
11 years ago
#11
@
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).
#15
follow-up:
↓ 16
@
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 insetup_hooks()
. I don't totally remember the logic behind setting it insetup_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' tobp_ajax_querystring()
in groups/single/members.php? I'm not sure about theexclude_admins_mods
item - maybe we could set this in the JS somewhere (like you're doing withtemplate
)? 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.
#16
in reply to:
↑ 15
@
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:
↓ 19
@
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 byBP_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 insetup_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
@
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 byBP_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 :)
#21
@
11 years ago
- Owner set to boonebgorges
- Resolution set to fixed
- Status changed from new to closed
In 7949:
#22
@
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
@
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
#24
@
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.
#25
@
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
No time for this in 1.1, but will add for 1.2