Skip to:
Content

Opened 3 months ago

Closed 6 weeks ago

#7609 closed enhancement (fixed)

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

Reported by: dcavins Owned by: 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 3 months ago.
Add IDs only return format option.
7609.02.diff (6.6 KB) - added by dcavins 3 months ago.
Name the new parameter 'fields' in keeping with WP convention.

Download all attachments as: .zip

Change History (9)

@dcavins
3 months ago

Add IDs only return format option.

#1 @espellcaste
3 months 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 the only the ids, but the whole object is being fetched.

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

Version 0, edited 3 months ago by espellcaste (next)

#2 @boonebgorges
3 months 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
3 months 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
3 months 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
3 months ago

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

#5 @dcavins
3 months ago

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

#6 @DJPaul
7 weeks ago

  • Keywords commit added

I love it. Make it so!

#7 @djpaul
6 weeks 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.