Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 4 years 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
Cc:

Description

https://buddypress.trac.wordpress.org/browser/trunk/src/bp-groups/bp-groups-template.php#L3887

its still using wp_parse_args

No way to filter loop with.

bp_before_{filter}_parse_args

example:

$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 dcavins 4 years ago.
Use bp_parse_args() instead of wp_parse_args().
7529.02.diff (14.4 KB) - added by dcavins 4 years 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 @DJPaul
4 years 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.


4 years ago

@dcavins
4 years ago

Use bp_parse_args() instead of wp_parse_args().

#3 @dcavins
4 years 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
4 years 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 4 years ago by espellcaste (previous) (diff)

#5 @espellcaste
4 years 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 @DJPaul
4 years ago

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

#7 @dcavins
4 years ago

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

#8 @dcavins
4 years 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.

@dcavins
4 years ago

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

#9 @DJPaul
4 years ago

Looks good

#10 @djpaul
4 years 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.