Skip to:
Content

BuddyPress.org

Opened 7 months ago

Last modified 3 months ago

#9019 new defect (bug)

`bp_is_group()` can return true on the group creation page

Reported by: r-a-y's profile r-a-y Owned by:
Milestone: Up Next Priority: normal
Severity: normal Version: 11.0.0
Component: Core Keywords: dev-feedback has-patch
Cc:

Description

Ran into an issue where bp_is_group() is returning true on the group creation page.

When a BuddyPress group is created, it sets the buddypress()->groups->current_group property to the new group's details: https://github.com/buddypress/buddypress/blob/7a2458117e98f4861f9a9b85806b712770fb5919/src/bp-groups/actions/create.php#L60-L62 . This code runs on 'bp_actions' at priority 10. So if you have any code running after 'bp_actions' at a priority on 10 or greater, then bp_is_group() will return true.

Example code:

<?php
add_action( 'bp_actions', function() {
  if ( bp_is_group_create() ) {
    var_dump( bp_is_group() );
  }
}, 11 );

Devs can workaround this by running their bp_is_group() checks before 'bp_actions' at priority 9 or lower. Or by adding a bp_is_group_create() check next to the bp_is_group() conditional.

I've attached a patch that does the latter.

Attachments (1)

9019.01.patch (469 bytes) - added by r-a-y 7 months ago.

Download all attachments as: .zip

Change History (7)

@r-a-y
7 months ago

#1 @imath
7 months ago

  • Milestone changed from Awaiting Review to Up Next
  • Version set to 11.0.0

Hi @r-a-y

Thanks for your report, I just checked it wasn't some kind of regression that was introduced in 12.0. It was already the case in 11.4 and I guess it has been the case for a while.

As we're getting close to 12.0 release, I'm wondering if we should delay your fix because I have no idea if it could have an impact on some parts of the group creation process or plugins.

@dcavins what's your opinion about it?

#2 @dcavins
7 months ago

As to whether it should go in 12, let me look at a couple of plugins to see how they're working with group creation. I think it should be OK, but I can also imagine devs thinking that the group creation process should be included in bp_is_group().

#3 @imath
7 months ago

I can also imagine devs thinking that the group creation process should be included in bp_is_group().

That's what worries me!

#4 @r-a-y
6 months ago

I can also imagine devs thinking that the group creation process should be included in bp_is_group().

If you look at the PHPDoc for bp_is_group(), bp_is_group() is meant to return true only when on a single group page.

I had some code that was doing checks for bp_is_group() and then doing some group nav changes, but since the group nav is only available when on a single group page, a fatal error occurred after group creation because the group nav wasn't available at that time. I guess I could have checked if the group nav was available and then I wouldn't have run into the fatal error, but the bp_is_group() behavior was odd enough that I wanted to create a ticket about it.

This patch isn't important to be in BP 12.

#5 @imath
5 months ago

  • Milestone changed from Up Next to 14.0.0

#6 @espellcaste
3 months ago

  • Milestone changed from 14.0.0 to Up Next
Note: See TracTickets for help on using tickets.