WP 5.1 `wp_parse_id_list()` check breaks `parent_id` parsing

[boonebgorges]

In [11095], when hierarchical groups were introduced, we exploited the behavior of wp_parse_id_list() that converted false to 0. This no longer happens: ​https://core.trac.wordpress.org/ticket/43977 ​https://core.trac.wordpress.org/changeset/44546

The logic at ​https://buddypress.trac.wordpress.org/browser/tags/4.1.0/src/bp-groups/classes/class-bp-groups-group.php?marks=1206#L1204 needs changing.

[boonebgorges]

@dcavins - Since you originally wrote this code, could you please have a look at 8037.diff​ and check that I've captured the intended behavior of false here?

[dcavins]

Hi @boonebgorges. I'm pretty sure that "intended behavior" is a mild overstatement. :)

The logic as originally written was

• parent_id that is null => don't take the parent_id into account for the query.
• parent_id that's not null => pass the parameter to wp_parse_id_list() to sort it out.

The note in the code was just to remind users that passing parent_id === false would be equivalent (because of a quirk in wp_parse_id_list()) to passing a parent_id of 0 (and the query would return top-level groups only).

Your patch maintains the behavior as it was, quirks and all, but whether it's the best, most predictable behavior I can't say. The code note for the parameter says "Optional. Array or comma-separated list of group IDs. Results will be limited to children of the specified groups. Default: null." So we don't really make any promises about what false might do. Honestly, converting false to 0 seems to make less sense than treating false like null, except it would change BP_Groups_Group::get()'s behavior.

Thanks for catching this change!

[boonebgorges]

Thanks for looking, @dcavins!

I'm also not sure what the "best" behavior is, but I'd prefer to be conservative and stick with the current behavior.

[boonebgorges]

In 12326:

Improve parent_id parsing in BP_Groups_Group.

The previous logic depended on behavior in wp_parse_id_list() that
will change in WP 5.1.

Fixes #8037.

[boonebgorges]

In 12327:

Improve parent_id parsing in BP_Groups_Group.

The previous logic depended on behavior in wp_parse_id_list() that
will change in WP 5.1.

Merges [12327] to the 4.0 branch.

Fixes #8037.