BuddyPress: Toggling group forum creates a duplicate forum

[r-a-y]

In BuddyPress, if you create a group with a forum attached to it, then navigate to the group's "Manage > Forum" page to disable the forum and save. Next, if you re-enable the forum and save again, another forum with the same name as the group will be created and will be used as the new group forum.

The previous forum still exists, but is now no longer attached to the group.

The issue is when removing and re-enabling the forum, bbPress doesn't record the previous forum ID and doesn't look for the previous forum.

I have attached a patch, which fixes this issue.

[r-a-y]

I should note that if #3014 is implemented, this patch will not work.

[johnjamesjacoby]

Hey @r-a-y! Thanks for the patch.

The approach you've taken works as intended for me, but...

I think that BBP_Forums_Group_Extension might make more sense, since it has methods for editing groups where a "trash/untrash" type functionality could be introduced, specifically disconnect_forum_from_group(), remove_forum(), and toggle_group_forum().

Then, bbPress can have new & separate functions similar to the others but specifically for interacting with the "previous" meta-data, and it can better target the edit action specifically instead of always assuming that bbp_remove_forum_id_from_group() handles it.

Bonus points for coding it in such a way that the hypothetical "multiple forums per group" functionality is not inhibited. I know it isn't fully implemented in the UI, but the current code is written with that in mind.

Going to bump this into the 2.7 milestone so there is a bit more time to iterate on the changes here.

[r-a-y]

Just getting to this.

Regarding:

I think that BBP_Forums_Group_Extension might make more sense, since it has methods for editing groups where a "trash/untrash" type functionality could be introduced, specifically disconnect_forum_from_group(), remove_forum(), and toggle_group_forum().

Then, bbPress can have new & separate functions similar to the others but specifically for interacting with the "previous" meta-data, and it can better target the edit action specifically instead of always assuming that bbp_remove_forum_id_from_group() handles it.

I think bbp_remove_forum_id_from_group() is the proper place to introduce the saving of the previous forum ID because it has all the validation logic.

If you want to move the previous forum ID saving logic back to the remove_forum() method, then that'd require duplicating the functionality from bbp_remove_forum_id_from_group().

Bonus points for coding it in such a way that the hypothetical "multiple forums per group" functionality is not inhibited

This would mean allowing multiple forum IDs to be passed to bbp_remove_forum_id_from_group() and also reworking this portion of the code for the edit_screen_save() method:
​https://bbpress.trac.wordpress.org/browser/trunk/src/includes/extend/buddypress/groups.php?marks=551-562#L543

So it will pass multiple forum IDs to the remove_forum() method. Likewise for the disconnect_forum_from_group() method. toggle_group_forum() doesn't look like it requires any changes.

If that's what you're thinking, let me know and I'll create a new patch.