Skip to:

Opened 10 months ago

Closed 2 months ago

#7529 closed enhancement (fixed)

Groups member loop missing bp_parse_args

Reported by: modemlooper Owned by: djpaul
Milestone: 3.0 Priority: normal
Severity: normal Version: 2.8.2
Component: Groups Keywords: has-patch 2nd-opinion


its still using wp_parse_args

No way to filter loop with.



$r = bp_parse_args( $args, array(
	'group_id'            => bp_get_current_group_id(),
	'page'                => 1,
	'per_page'            => 20,
	'max'                 => false,
	'exclude'             => false,
	'exclude_admins_mods' => $exclude_admins_mods,
	'exclude_banned'      => 1,
	'group_role'          => false,
	'search_terms'        => $search_terms_default,
	'type'                => 'last_joined',
), 'has_group_members' );

Attachments (2)

7529-groups-component.01.diff (15.1 KB) - added by David Cavins 5 months ago.
Use bp_parse_args() instead of wp_parse_args().
7529.02.diff (14.4 KB) - added by David Cavins 2 months ago.
Use bp_parse_args() instead of wp_parse_args() to take advantage of context-aware filter points.

Download all attachments as: .zip

Change History (12)

#1 @Paul Gibbs
8 months ago

  • Milestone changed from Awaiting Review to 3.0

Sure, it shouldn't take very long to change these out.

This ticket was mentioned in Slack in #buddypress by espellcaste. View the logs.

5 months ago

@David Cavins
5 months ago

Use bp_parse_args() instead of wp_parse_args().

#3 @David Cavins
5 months ago

I've added a patch that uses bp_parse_args() instead of wp_parse_args() in the groups component. I'm not sure that every instance needs a bp_parse_args, though. For instance, args are parsed in groups_get_groups(), then those args are passed to BP_Groups_Group::get() where they are parsed again. It seems like we should use one or the other, unless someone can come up with a use case where both would be needed. (And if we only use bp_parse_args() in one or the other, I'm assuming we'd add it to the "deep" function, BP_Groups_Group::get(), and add a note above the wp_parse_args() call in groups_get_groups().)

If we can come up with some general guidance for when we want to use bp_parse_args() generally, I'll refine this patch. Thanks!

#4 @espellcaste
5 months ago

Really good @dcavins.

I would argue to add on the "deep" function, as they are the ones doing the actual query. In this way, people using the deep level ones would be filtered correctly.

Adding on both places seems duplicate. Like you said, a use case would be good to evaluate it.

I also think this is a good opportunity to add to the other components to keep it consistent.

Last edited 5 months ago by espellcaste (previous) (diff)

#5 @espellcaste
5 months ago

  • Component changed from Members to Groups
  • Keywords has-patch 2nd-opinion added
  • Priority changed from high to normal
  • Type changed from defect (bug) to enhancement

#6 @Paul Gibbs
3 months ago

@dcavins Any interest updating this based on @espellcaste's feedback?

#7 @David Cavins
2 months ago

@DJPaul Sure thing. I'll make some changes and reupload.

#8 @David Cavins
2 months ago

I've updated the patch, and can at least imagine a use-case for every bp_parse_args() I've added. Thanks for your comments in advance.

@David Cavins
2 months ago

Use bp_parse_args() instead of wp_parse_args() to take advantage of context-aware filter points.

#9 @Paul Gibbs
2 months ago

Looks good

#10 @djpaul
2 months ago

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

In 11805:

Groups: use bp_parse_args() in low-level parts of the codebase.

These replace calls to wp_parse_args(). Our version supports a before/after filter, giving flexibility to third-party developers.

Fixes #7529

Props dcavins, espellcaste.

Note: See TracTickets for help on using tickets.