Skip to:
Content

BuddyPress.org

Opened 3 years ago

Closed 3 years ago

#7224 closed enhancement (fixed)

bp_groups_get_group_type() probably shouldn't return unregistered group types.

Reported by: dcavins Owned by: dcavins
Milestone: 2.7 Priority: normal
Severity: normal Version: 2.6.0
Component: Groups Keywords: has-patch
Cc:

Description

The function bp_groups_register_group_type() adds items to buddypress()->groups->types and bp_groups_get_group_types() gets them from there. However, when you’re setting a type for a group, you’re using the new taxonomy to do so. So if a group type is no longer registered, bp_groups_get_group_type()—which is referring to the taxonomy relationship—could return currently unregistered types.

The attached patch checks that the found types via taxonomy are represented in the buddypress() global before including them in the return value.

Attachments (3)

7224.01.patch (1.5 KB) - added by dcavins 3 years ago.
Filter out unregistered group types and add a test showing the problem.
7224.02.patch (1.8 KB) - added by dcavins 3 years ago.
Use an intermediary array to collect valid types.
7224.03.patch (1.7 KB) - added by dcavins 3 years ago.
Remove the unneeded wp_list_pluck() since we're already looping through the array.

Download all attachments as: .zip

Change History (9)

@dcavins
3 years ago

Filter out unregistered group types and add a test showing the problem.

#1 follow-up: @boonebgorges
3 years ago

This looks good to me. One small thing: can you check the keys on the index after some items have been unset? I can't recall if it's numerically indexed; if so, running unset( $array[1] ) on [ 0 => 'foo', 1 => 'bar', 2 => 'baz' ] will leave you with an array with keys 0 and 2, and you should reset by passing through array_values().

#2 in reply to: ↑ 1 @dcavins
3 years ago

Replying to boonebgorges:

This looks good to me. One small thing: can you check the keys on the index after some items have been unset? I can't recall if it's numerically indexed; if so, running unset( $array[1] ) on [ 0 => 'foo', 1 => 'bar', 2 => 'baz' ] will leave you with an array with keys 0 and 2, and you should reset by passing through array_values().

Hi @boonebgorges -

Thanks for your comment. Yes, the array keys are no longer compact after the unset, and adding array_values() as you suggest fixes that issue.

I've read several things lately about avoiding direct modification of the active array in a foreach loop. Would it make more sense in a situation with small arrays like this to add the good terms to a validated array and pass that along, rather than removing bad terms from the array with an unset() which then should be followed up by an array_values()?

Thanks!

#3 @boonebgorges
3 years ago

Would it make more sense in a situation with small arrays like this to add the good terms to a validated array and pass that along, rather than removing bad terms from the array with an unset() which then should be followed up by an array_values()?

This sounds great to me.

#4 @dcavins
3 years ago

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

Here's a second patch that uses an intermediary array to collect the valid types.

@dcavins
3 years ago

Use an intermediary array to collect valid types.

@dcavins
3 years ago

Remove the unneeded wp_list_pluck() since we're already looping through the array.

#5 @boonebgorges
3 years ago

Seems good to me!

#6 @dcavins
3 years ago

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

In 11046:

Return active types only in bp_groups_get_group_type().

When compiling the array of types for the group in
bp_groups_get_group_type(), only include those types that are
currently registered.

Fixes #7224.

Note: See TracTickets for help on using tickets.