Groups whose creation process is abandoned become unjoinable

[boonebgorges]

After [13874] #9162, it's impossible to join a group if the invite_status groupmeta is not set. The intent in #9162 was primarily to prevent the joining of a (soon-to-be) non-public group between the time of its creation (end of step 1) and invite_status being set (end of step 2).

The placement of the groups_get_groupmeta( $group_id, 'invite_status' ) check inside groups_join_group(), as we see in [13874], fixes this problem, but it introduces new behavior that might qualify as a regression, or at least an inconvenience. Namely: If a group has been created without an invite_status, and then the group creation process is abandoned after step 1, the group is permanently unjoinable. Worse, the unjoinable status is not clear in the directories and elsewhere, since the 'Join Group' button appears (because the group is technically public) but joining will always fail silently.

This can be particularly problematic for BP-CLI, for the Dashboard admin panels, or for plugins and custom tools that use groups_join_group() directly. In these cases, it should be possible to bypass the invite_status check, but because of the placement of the check in the low-level groups_join_group(), it's not possible to do so.

I think there's a couple ways to address this. The less substantial way is to move the groupmeta check out of the low-level groups_join_group() and instead perform the check higher up the chain, namely in the places in BP where groups_join_group() is called. I'm going to attach a patch that suggests how this might be done. This move is important and valuable because it allows groups_join_group() to function more as a low-level CRUD function, which is how it ought to be seen by third-party plugins as well as BP-CLI, the Dashboard admin tools, etc.

At a deeper level, it seems like we should probably be ensuring that groups are not created without an invite_status to begin with. What if all groups were set to status=hidden and invite_status=admins at the time of creation (unless another value was provided explicitly via the function parameters), and then their status and invite_status changed after step 2 of group creation? This would ensure that abandoned (or in-the-process-of-being-created) groups remain unjoinable, but (a) they'd be hidden from all interfaces, and (b) their data structure would be more complete and intact. There may be backward compatibility concerns with this approach, but it feels more "correct" than what we currently do. (I feel like this was discussed at some point, but I can't find the reference here in Trac.)

Thanks in advance for considering.

[vapvarun]

added the similar issue earlier ​https://buddypress.trac.wordpress.org/ticket/9238

[imath]

Hi @boonebgorges

Thanks a lot for pointing us to the original ticket & for the great description you provided 👍. I agree your approach is better than the one I originally chose. Let's start with the patch you attached.

Thanks @vapvarun for your initial ticket, since Boone suggested a patch and identified the commit which caused the issue: I suggest to keep this ticket as reference and mark the one you shared as a duplicate to this one. If you could test & confirm the patch is fixing the issue for your usage, it would be awesome.

This PR adds a small adjustment inside Nouveau's Ajax Group's join handler to the ​patch shared by @boonebgorges directly into the Trac ticket.

Main difference compared to the patch is the fact the 'invite_status' meta check is done inside a previous conditional statement.

Trac ticket: ​https://buddypress.trac.wordpress.org/ticket/9241

[imath]

I just tested the patch. It mainly works great. There was just a glitch with the Nouveau template back: the "Join" button was wrongly updated to a "Leave group" one.

Here's the link to the edited part: ​https://github.com/buddypress/buddypress/pull/381/files#diff-d2a9d0c7c349e4912700b28092836d6fc2eae9cf2a3238c6ec3945644ee05d9dR171

[imath]

In 14043:

Move the invite_status group meta check out of groups_join_group()

[13874] introduced a regression where groups whose creation process is abandoned become unjoinable & or when using the WP BuddyPress CLI to create groups.

To avoid this, we now perform the check higher up the chain, namely in the places in BuddyPress where groups_join_group() is called.

Props boonebgorges, vapvarun.

See #9241 (trunk)
Fixes #9238
Closes ​https://github.com/buddypress/buddypress/pull/381

[imath]

In 14044:

Move the invite_status group meta check out of groups_join_group()

[13874] introduced a regression where groups whose creation process is abandoned become unjoinable & or when using the WP BuddyPress CLI to create groups.

To avoid this, we now perform the check higher up the chain, namely in the places in BuddyPress where groups_join_group() is called.

Props boonebgorges, vapvarun.

Fixes #9241 (branch 14.0)
See #9162

[imath]

Milestone renamed