Calling groups_get_group() with a non-existent group id returns a BP_Groups_Group object populated with the provided ID

[chriskeeble]

A number of BuddyPress plugins use checks for whether a group exists by ID using calls such as:

e.g. #1

if (!$group = groups_get_group(array('group_id' => $id))) {

e.g. #2

!groups_get_group(array('group_id' => $id))

If no group with the specified ID exists, should the BP_Groups_Group constructor set the ID property to null? It currently leaves it set to whatever value was passed in.

Ideally groups_get_group should return null (though this could be a breaking change?)

[DJPaul]

Good call. I am setting this to future release milestone, but if feedback from other developers think we could make a change in 1.6 for this, we'll do that.

I agree it should return null, but I think we need a full development cycle (1.7) to properly test and see if this breaks anything. I personally think we could do your first suggestion for 1.6, and the null change for 1.7.

[r-a-y]

I'd say this is important to fix for BP 1.6 because plugin devs are being encouraged to use groups_get_group() instead of new BP_Groups_Group() for the new object caching capability. See #3770.

[boonebgorges]

This is not an issue with groups_get_group() but with the BP_Groups_Group class itself. If you instantiate the class like this

$group = new BP_Groups_Group( $nonexistent_group_id );

it'll return an object with all properties null except for 'id', which will be $nonexistent_group_id. groups_get_group() is just a wrapper, and doesn't affect this aspect of the behavior.

So I agree with DJPaul that we can't just change it - it's likely that there are plugins relying on this odd behavior. If we wait until after 1.6, then we can make a breaking change and publicize it widely. As it stands, we are too close to release.

[r-a-y]

Gotcha! It's not good to post when you're sleep-deprived :)

[DJPaul]

What type of response should we return? An empty BP_Groups_Group seems a bit pointless. A WP_Error or a null response, maybe?

[johnjamesjacoby]

Looks like we missed the 1.7 early boat again. Punting to 1.8.

[boonebgorges]

I don't think we should be returning null or false. $foo = new BP_Groups_Group() should return a BP_Groups_Group object, regardless of whether there's a corresponding group. See the behavior of WP_Post and WP_User. I'm willing to be persuaded that the wrapper function groups_get_group() ought to return null for non-existent group ids, but that's not the subject of this ticket: the OP was referring to plugins that are checking against BP_Groups_Group itself. Those plugins are simply doing it wrong.

We should, however, probably be setting the id property to 0 in this case. I'm not persuaded that this'll really break much of anything - what is a scenario in which it's important for BP_Groups_Group::id to be set for a non-existent group? I'm just going to make the change, and we'll see if any plugins are doing anything strange during the beta period.

[boonebgorges]

In 7134:

BP_Groups_Group::id should be 0 for non-existent groups

Fixes #4185

Props chriskeeble