boonebgorges / buddypress-docs

GNU General Public License v3.0
106 stars 44 forks source link

History: Fix improper redirection for group docs for BuddyPress 12 #755

Closed r-a-y closed 3 weeks ago

r-a-y commented 3 weeks ago

I've encountered an issue where group docs are throwing 404s in BuddyPress 12. The group doc URI is appended with edit.php?post_type=buddypress, which doesn't exist.

I traced the bug to the history addon.

tl;dr: The simple fix is to check if the revision's post parent is empty before doing anything else.


For the rundown, keep on reading. The issue is here:

https://github.com/boonebgorges/buddypress-docs/blob/c4f8c0ec33a63b69bce51c57364f2f9ff4414303/includes/addon-history.php#L183-L184

For a group doc, the $this->revision->post_parent is usually 0. The code is expecting get_post( $this->revision->post_parent ) to return a falsey value, but it is actually returning the WordPress post of the group directory: https://github.com/WordPress/WordPress/blob/a88967db6eb6a95da6d320d8511f78d00b1ffafd/wp-includes/post.php#L1092-L1095 . (I'm quite certain that BuddyPress 12 is causing the $GLOBALS['post'] variable to be populated with the group directory post, but I could be wrong. Some particulars about this BP install is BP Classic is not activated, which means BP is using rewrite rules for URI routing.)

Anyway, when the revision's post parent is empty and when there is an already-queried post in the $post global, the following occurs:

https://github.com/boonebgorges/buddypress-docs/blob/c4f8c0ec33a63b69bce51c57364f2f9ff4414303/includes/addon-history.php#L186-L190

https://github.com/boonebgorges/buddypress-docs/blob/c4f8c0ec33a63b69bce51c57364f2f9ff4414303/includes/addon-history.php#L205-L206

In the case of the group directory post, revisions and auto-save are not enabled, which causes the group doc to redirect to /groups/docs/EXAMPLE-DOC/edit.php?post_type=buddypress. This causes the 404.

boonebgorges commented 3 weeks ago

@r-a-y Thanks for the deep dive here.

I'm not able to reproduce the behavior on a vanilla instances of WP+BP, which is presumably why I didn't catch this in the past. On my vanilla installation, the value of $GLOBALS['post'] at the moment in question is the currently-requested Doc. So I think there's something happening that's specific to the Commons that's causing the 'bp-pages' directory page to be set as 'post'.

You're right that BP is responsible for setting it: https://github.com/buddypress/buddypress/blob/fc64b1964f5ddb65cf590d0dfa313df058024eb4/src/bp-groups/classes/class-bp-groups-component.php#L1188

The difference between my vanilla setup and the Commons is that the latter is running buddypress-docs-in-group, which is what causes BP to set up the 'groups' component. So the incompatibility is probably there. I'll spend some more time trying to track this down. But in the meantime, something like your fix looks good.

I think that more should probably be added to it - specifically, there's no reason to run much of the addon-history.php routing if we're not currently looking at the 'history' section. I'll add that after I've merged your fix.

boonebgorges commented 3 weeks ago

I just confirmed locally that it's the combination of BP 12 URL routing + buddypress-docs-in-group that's responsible for the behavior. I also confirmed the fixes in this PR.