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 | 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)
Change History (9)
#1
@
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
#2
@
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
@
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
@
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.
Add IDs only return format option.