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

[dcavins]

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.

[dcavins]

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

[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().

[dcavins]

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!

[boonebgorges]

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.

[dcavins]

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

[dcavins]

Use an intermediary array to collect valid types.

[dcavins]

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

[boonebgorges]

Seems good to me!

[dcavins]

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.

[espellcaste]

In 13879:

WPCS: miscellaneous fixes for the src/bp-core/admin/bp-core-admin-functions.php template.

See #7224
See #9164