Skip to:
Content

bbPress.org

Opened 19 months ago

Last modified 3 months ago

#3576 assigned defect (bug)

Prepare bbPress for BuddyPress 12.0.0

Reported by: imath's profile imath Owned by: johnjamesjacoby's profile johnjamesjacoby
Milestone: 2.7 Priority: highest omg sweet tea
Severity: normal Version: trunk
Component: Extend - BuddyPress Keywords: has-patch needs-testing needs-screenshots needs-refresh
Cc: boonebgorges, emaralive

Description

BuddyPress 12.0.0 will change how URL parsing is done / BP Links are built using the WordPress Rewrite API.

You'll find in the attached patch my suggestions to have bbPress ready for BuddyPress 12.0.0 and still back compatible with previous versions of BuddyPress.

Attachments (2)

3576.patch (27.2 KB) - added by imath 19 months ago.
3576.2.patch (28.1 KB) - added by imath 12 months ago.
Fixes broken pagination

Download all attachments as: .zip

Change History (27)

@imath
19 months ago

#1 @imath
19 months ago

  • Keywords has-patch added

This ticket was mentioned in Slack in #buddypress by imath. View the logs.


19 months ago

#3 @huzaifaalmesbah
19 months ago

  • Keywords needs-testing needs-screenshots added

This ticket was mentioned in Slack in #buddypress by imath. View the logs.


18 months ago

#5 @Robin W
17 months ago

@imath I'm not sure that the bbpress authors are actively engaged with bbpress at the moment. I help out on the support forum. Do you have a rough date when this version of buddypress will be released?

This ticket was mentioned in Slack in #bbpress by robinwilson. View the logs.


17 months ago

This ticket was mentioned in Slack in #bbpress by espellcaste. View the logs.


16 months ago

#8 @imath
15 months ago

@robin-w sorry, I'm replying so late. I wonder why I haven't noticed or received a notification about this. BuddyPress 12.0 should be released at the beginning of December 2023.

#9 @Robin W
15 months ago

thanks. Given that I suspect that no changes to the bbpress plugin will happen (unless buddypress has some influence), what is the impact to buddypress users if no change in bbpress?

#10 @imath
15 months ago

Sorry for my late reply @robin-w I don’t know why, I’m not receiving email notification when you comment this ticket. We’ve built BP Classic for such a case. It’s a BP backwards compatibility Add-on that is restoring the Legacy URL parser. So users can activate it and enjoy current bbPress version with BuddyPress 12.0.0 and they will be safe. They can already activate it and wait for BuddyPress upgrade being sure nothing will change for them. FYI, 12.0.0 will be released on December 11.

Last edited 15 months ago by imath (previous) (diff)

#11 @Robin W
15 months ago

ah great - thanks for that news

This ticket was mentioned in Slack in #bbpress by espellcaste. View the logs.


13 months ago

#13 @slaFFik
13 months ago

#3590 was marked as a duplicate.

This ticket was mentioned in Slack in #bbpress by slaffik. View the logs.


13 months ago

This ticket was mentioned in Slack in #buddypress by espellcaste. View the logs.


13 months ago

This ticket was mentioned in Slack in #bbpress by robinwilson. View the logs.


13 months ago

#17 @boonebgorges
13 months ago

Hi all - I've run into problems running bbPress on sites running BP 12.0, even with the bp-classic plugin running.

I tried using https://bbpress.trac.wordpress.org/attachment/ticket/3576/3576.patch but found that it caused some load-order problems that caused libraries to be loaded twice. It has something to do with the way that BBP_Forums_Component::includes() is ultimately hooked to 'bp_include', while imath's patch has certain libraries being loaded at bp_setup_components - things were getting fired twice. As a stopgap, I had to put some logic in place that prevents the includes() method from running twice. This is obviously not a proper solution, but I ran out of time to do proper debugging.

The other issue, which I think is missed in this ticket altogether, is that BP 12.0 introduces a rewrite-rule load order issue that breaks forum pagination within BP groups. Prior to BP 12.0, /page/ requests in group forums were caught by WP's default pagination rules. After BP 12.0, the following rule from BP has a higher priority than bbPress/WP's pagination rule:

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

As a result, the pagination data is getting dumped into BP's action_variables, and bbPress makes no attempt to turn this into proper pagination info. bbp_get_paged() always returns 1.

On my client sites, I will likely work around this by telling bbPress to set the paged global based on certain values of bp_action_variables(). However, this seems a bit backward when I consider the appropriate fix for bbPress. A more idiomatic approach is probably for bbPress's BuddyPress extension to register its own rewrite rules, which should take precedence over BP's. It would look 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

But of course, you can't hardcode 'groups', because these rewrite bases can be changed in BP 12.0. So the true logic will have to be more complicated. I'm not immediately familiar enough with how BP's new system works to make a recommendation about how/whether this would be implemented in bbPress.

For those who are facing a similar problem on a specific installation, here's the basic logic of my workaround: https://gist.github.com/boonebgorges/1295ed16c0bf85fece87c45649d96d7f

#18 @boonebgorges
13 months ago

  • Cc boonebgorges added

#19 @imath
12 months ago

  • Keywords needs-refresh added

Thanks for these new inputs @boonebgorges 😍.

About the fact you had troubles with BP Classic, this was about the /page/ thing, or were there other issues?

I'll test again bbPress + BP 12.0 + BP Classic and eventually include your /page/ fix.

@imath
12 months ago

Fixes broken pagination

#20 @imath
12 months ago

@boonebgorges I've updated the patch (.2.patch) to fix the pagination. I've tested with BP 12.0 (without BP Classic) and it seems to behave as expected. To do so, I chose to add a method to set the paged prop, see BBP_Forums_Component->setup_pagination().

I'm a bit surprised some libraries were loaded twice when you tested previous version of the patch because bbPress once the patch is applied doesn't hook to bp_include anymore, but bp_loaded a bit before bp_setup_components. I wasn't able to reproduce while working on updating the patch to fix the pagination as well as the Group's forum display tab.

Next step for me is to check what's going wrong with BP Classic. I'll keep subscribers of this ticket updated.

#21 @imath
12 months ago

I confirm BP Classic doesn't solve the pagination issue. It will in its next minor version (1.4.0) > https://github.com/buddypress/bp-classic/pull/44

#22 @slaFFik
12 months ago

#3592 was marked as a duplicate.

#23 @johnjamesjacoby
8 months ago

#3459 was marked as a duplicate.

#24 @johnjamesjacoby
8 months ago

  • Milestone changed from Awaiting Review to 2.7
  • Owner set to johnjamesjacoby
  • Priority changed from normal to highest omg sweet tea
  • Status changed from new to assigned

#25 @emaralive
3 months ago

  • Cc emaralive added
Note: See TracTickets for help on using tickets.