CUNY Academic Commons

Activating the BP Classic plugin will help, but I think we would benefit from additional time and testing to ensure all our custom BuddyPress code will continue to work.

I'm probably overthinking this and perhaps this might not require a lot of time to get things working, but here are some quick thoughts:

• We are not loading BuddyPress deprecated code as of https://github.com/cuny-academic-commons/cac/commit/60ae93c883148058bdb16be102d3f7563b247f41 . BP 12 deprecated a bunch of functions not included with the BP Classic plugin. For example, bp_get_group_all_members_permalink(): https://github.com/buddypress/buddypress/blob/f27bac0209ecfc37192b6f41ac637704088e136d/src/bp-core/deprecated/12.0.php#L1470C10-L1470C44 . We should audit all BP 12 deprecated functions to see if our codebase is using any of them. If so, we can potentially add these functions back to the BP Classic plugin. Or for the short term, we might need to consider loading BP deprecated code again.

• In BP 12, the BP Default theme is removed from BuddyPress and is part of the BP Classic plugin. Over the past, few major releases, we have made large strides in removing bp-default from our bp-nelo theme, but there are still some hardcoded references to it. For example, https://github.com/cuny-academic-commons/cac/blob/2c52ce9f6e18dde2dfdca571d65da59817fd6dad/wp-content/themes/bp-nelo/functions.php#L205 . For this specific instance, we will need to switch the stylesheet URL to match the new BP Classic URL. We can remove this bp-default stylesheet call entirely once we have finished the Groups redesign.

• We should audit all our custom BuddyPress widgets to see whether we should remove them outright or not. See https://github.com/cuny-academic-commons/cac/blob/master/wp-content/plugins/cac-bp-custom-includes/widgets.php . This doesn't need to be done immediately.

• In the Group Library, clicking on a doc throws a 404.
• Pagination is not working on a group's /forum/ page or when in a single forum thread. There's a patch on bbPress Trac that hasn't been committed yet: https://bbpress.trac.wordpress.org/ticket/3576 . I haven't tested the patch yet.
• bbPress stylesheet is not enqueued on group forum pages. I've put a temporary fix in place here: https://github.com/cuny-academic-commons/cac/commit/05677c93c913947fc9bcb5eb5ca323bd24c0b44e . This can probably be reverted once bbPress releases a new version with the above patch.
• Some of our custom BuddyPress features need to run on the 'bp_parse_query' hook instead of 'init' or 'bp_init' due to the switch to use WP permalinks in BP 12. I've fixed up one instance in https://github.com/cuny-academic-commons/cac/commit/bf399847d075a68ac784e53735861012509f36d4 . There may be a few more places to check, but will need to do further testing.

Ray, thanks for having a look.

In the Group Library, clicking on a doc throws a 404.

I couldn't reproduce this, either on cdev or in my local environment. Can you share more details, or maybe a URL on cdev where I can test?

Pagination is not working on a group's /forum/ page or when in a single forum thread. There's a patch on bbPress Trac that hasn't been committed yet: https://bbpress.trac.wordpress.org/ticket/3576 . I haven't tested the patch yet.

I couldn't reproduce this either. See https://commons.gc.cuny.edu/groups/cac-community-team-project-planning/forum/ on cdev: https://commons.gc.cuny.edu/groups/cac-community-team-project-planning/forum/page/2/ My reading of that specific ticket is that bp-classic makes it a non-issue, since it reverts to the old URI-global system for page routing.

Some of our custom BuddyPress features need to run on the 'bp_parse_query' hook instead of 'init' or 'bp_init' due to the switch to use WP permalinks in BP 12. I've fixed up one instance in https://github.com/cuny-academic-commons/cac/commit/bf399847d075a68ac784e53735861012509f36d4 . There may be a few more places to check, but will need to do further testing.

Yeah, thanks for remembering this. I reviewed the following locations for items hooked to 'init' or 'bp_init': wp-content/plugins/cac-*, wp-content/mu-plugins, wp-content/themes/bp-nelo. I didn't find any additional places where callbacks made reference to BP's routing globals. But this is only a partial search - it's not possible for us to look at every 'init' callback in the entire codebase. So I agree that we probably just need more testing of common actions.

In the Group Library, clicking on a doc throws a 404.

I couldn't reproduce this, either on cdev or in my local environment. Can you share more details, or maybe a URL on cdev where I can test?

Oops! Forgot to network-activate BP Classic! Group doc 404 is now resolved.

However, forum and thread pagination on group pages are still broken. For example, see https://commons.gc.cuny.edu/groups/cac-community-team-project-planning/forum/page/2/. The page is still reflecting the contents from page 1. I think we will need to hold off on deploying BuddyPress 12 until we decide to patch bbPress ourselves or until bbPress releases a new version.

I reviewed the following locations for items hooked to 'init' or 'bp_init': wp-content/plugins/cac-*, wp-content/mu-plugins, wp-content/themes/bp-nelo

Found one more instance in our bp-nelo theme. Addressed in https://github.com/cuny-academic-commons/cac/commit/4c609c1430253e4f1ef91304978dd3eca059b6b7

One thing I noticed on cdev is our BP directory pages in the nav menu are using the unpretty WordPress permalink (eg. /?post_type=buddypress&p=X). Might be something to be aware of when we deploy this to production. My local install isn't affected by this though.

I managed to reproduce the bbPress pagination issue while working on another project, and it is in fact due to rewrite rules. BP's new system registers a rule that matches group forum /topic/[topic-name]/page/ queries:

 groups/([^/]+)/([^/]+)/(.+?)/?$  => index.php?bp_groups=1&bp_group=$matches[1]&bp_group_action=$matches[2]&bp_group_action_variables=$matches[3] 

This rule is registered even when running bp-classic. And it's registered with a higher priority than the WP rule that handles /page/ pagination:

(.?.+?)/page/?([0-9]{1,})/?$ => index.php?pagename=$matches[1]&paged=$matches[2] 

As a result, the 'paged' query var is never set, and WP_Query (and therefore bbPress) thinks that we're always on page 1.

Here's a very hacky workaround that I put in place on the other project:

        // ...
    add_filter( 'bbp_get_global_object', array( $this, 'fix_rewrite_pagination' ), 10, 2 );
        // ...

    /**
     * Fixes rewrite pagination.
     *
     * Required for compatibility with BP 12.0, where the /groups/ rewrite rule will
     * be caught before bbPress's /page/ rule.
     */
    public function fix_rewrite_pagination( $object, $type ) {
        if ( 'wp_query' !== $type ) {
            return $object;
        }

        if ( ! bp_is_group() ) {
            return $object;
        }

        if ( ! bp_is_current_action( 'forum' ) ) {
            return $object;
        }

        $page_number = null;

        // Can't use bbp_is_single_topic() because it triggers a loop.
        $is_single_topic = bp_is_action_variable( 'topic', 0 );

        if ( $is_single_topic ) {
            if ( bp_is_action_variable( 'page', 2 ) ) {
                $page_number = bp_action_variable( 3 );
            }
        } else {
            if ( bp_is_action_variable( 'page', 0 ) ) {
                $page_number = bp_action_variable( 1 );
            }
        }

        if ( ! $page_number ) {
            return $object;
        }

        $object->set( 'paged', $page_number );

        return $object;
    }

I think a more correct fix is for bbPress's BP extension to register its own pagination rewrite rules for groups. Something like:

 groups/([^/]+)/forum/page/?([0-9]{1,})/?$  => index.php?bp_groups=1&bp_group=$matches[1]&bp_group_action=forum&paged=$matches[2]  // pagination for the single-forum topic listing
 groups/([^/]+)/forum/topic/([^/]+)/page/?([0-9]{1,})/?$  => index.php?bp_groups=1&bp_group=$matches[1]&bp_group_action=forum&paged=$matches[3]&bbp_topic=$matches[2]  // pagination for single-topic - I don't know how the bbp_topic part would work, this would need to be figured out in bbPress

Furthermore, when running bp-classic, I had to do some additional work to ensure that bbPress's BP compatibility layer is loaded in time, and is loaded only once. Like the approach above, it's pretty hacky (requiring a static variable to prevent double-loading) but it basically works. Here's the full patch: https://github.com/openlab-at-city-tech/openlab/commit/27f2a68354550295d85f0f3c5aaafc55004268f0

I don't think that imath's patch on https://bbpress.trac.wordpress.org/ticket/3576 handles either the bp-classic load-order issue or the pagination issue. I'll post a note on that ticket.