Skip to:
Content

BuddyPress.org

Opened 7 years ago

Closed 7 years ago

#7609 closed enhancement (fixed)

Add "IDs only" return format option for BP_Groups_Group::get().

Reported by: dcavins's profile dcavins Owned by: dcavins's profile dcavins
Milestone: 3.0 Priority: normal
Severity: normal Version: 2.9.0
Component: Groups Keywords: has-patch commit
Cc: dcavins, contato@…

Description

In some cases, all I need back from groups_get_groups() is the IDs of the groups. It's possible to fetch the objects and then wp_list_pluck() out the IDs, but it's much more efficient to skip the group object creation step and just return the found IDs.

In the attached patch, I've added a new parameter, ids_only that skips creation of group objects. I also decided that if you wanted the IDs only, you probably don't want to prefetch the group meta and group admins and mods data. However, this is not required. The new parameter (whatever we want to call it) can be independent of update_meta_cache and update_admin_cache if that makes more sense in other use cases. Please offer your feedback about this approach and whether it should be independent or if it makes sense to assume that if you want the IDs only, you don't want extraneous data/queries.

Attachments (2)

7609.01.diff (6.5 KB) - added by dcavins 7 years ago.
Add IDs only return format option.
7609.02.diff (6.6 KB) - added by dcavins 7 years ago.
Name the new parameter 'fields' in keeping with WP convention.

Download all attachments as: .zip

Change History (9)

@dcavins
7 years ago

Add IDs only return format option.

#1 @espellcaste
7 years ago

  • Cc contato@… added

I find it really useful. On the wp-cli-buddypress, we are doing exactly how you described. Using wp_list_pluck to fetch only the ids, but the whole object is being fetched.

https://github.com/buddypress/wp-cli-buddypress/blob/dev/components/group.php#L410-L413

Last edited 7 years ago by espellcaste (previous) (diff)

#2 @boonebgorges
7 years ago

Yes, let's do it here and in other components as well.

The WordPress convention for this is 'fields' => 'ids', based on the prior convention of allowing different values for fields (like, say, 'name' or 'id=>parent'). It looks like we're already using this in BP_Activity_Activity, so maybe we can standardize on it?

#3 @dcavins
7 years ago

  • Owner set to dcavins
  • Status changed from new to accepted

That's a good point; I didn't call it fields because it seems that the WP_Query fields parameter is more robust than what I was thinking of (but it'd be better to plan ahead for future robustness, and we can add other options later).

Do you have an opinion about whether passing 'fields' => 'ids should also prevent the cache prefetch queries? Or should those be independent?

Thanks again @espellcaste and @boonebgorges for your feedback.

#4 @boonebgorges
7 years ago

That's a good point; I didn't call it fields because it seems that the WP_Query fields parameter is more robust than what I was thinking of (but it'd be better to plan ahead for future robustness, and we can add other options later).

IMO the main reason for going with fields=ids is standardization. It makes the API much more predictable, especially for folks with less BP experience.

Do you have an opinion about whether passing 'fields' => 'ids should also prevent the cache prefetch queries? Or should those be independent?

Again, looking at WP: object caches are *not* primed when fields=ids. This makes sense because people calling fields=ids know that they don't need the object data, and are looking to increase performance. So I'd say that we should follow this example and skip the cache priming.

@dcavins
7 years ago

Name the new parameter 'fields' in keeping with WP convention.

#5 @dcavins
7 years ago

Thanks for your feedback. I've updated the patch.

#6 @DJPaul
7 years ago

  • Keywords commit added

I love it. Make it so!

#7 @djpaul
7 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 11762:

Groups: add 'fields' parameter to BP_Groups_Group::get().

Allows return of only group IDs instead of entire objects when fields=ids is set, similiar to the fields parameter in WP_Query.

Fixes #7609

Props dcavins

Note: See TracTickets for help on using tickets.