Skip to:
Content

BuddyPress.org

Opened 3 months ago

Closed 3 months ago

#8558 closed defect (bug) (fixed)

Adapt Group extensions registration to BP Rewrites

Reported by: imath Owned by: imath
Milestone: 10.0.0 Priority: normal
Severity: normal Version: 1.1
Component: Groups Keywords: has-patch
Cc:

Description

I suggest to move the bp_register_group_extension() function from the BP_Group_Extension class file to the src/bp-groups/bp-groups-functions.php file.

Using an anonymous function inside the bp_register_group_extension() function to init Group Extensions at bp_init is problematic.

  1. I suggest to only use this function to register Group Extension classnames into a specific global (for now! I plan to extend this from the BP Rewrites plugin to manage slug customizations).
  2. And use a new function (bp_init_group_extensions()) to init the Group Extensions by looping into this specific global.

It seems to have no regression effects according to my manual tests, and PHPUnit tests are not failing.

Unless one of us feels strongly about this change, I'm pretty confident about it so I'll commit it soon to carry on progressing on BP Rewrites.

Attachments (2)

8558.patch (3.4 KB) - added by imath 3 months ago.
8558.2.patch (3.7 KB) - added by imath 3 months ago.

Download all attachments as: .zip

Change History (6)

@imath
3 months ago

#1 @boonebgorges
3 months ago

Thanks for working on this, @imath ! The change described here seems fine to me at a glance.

You mentioned in the corresponding GitHub ticket https://github.com/buddypress/bp-rewrites/issues/9#issuecomment-907216220 that you were considering moving the setup of BP_Group_Extension classes to bp_parse_query. I would be very cautious when considering this change. I've written a number of plugins and customizations that are sensitive to the specific load-order of BP_Group_Extension at bp_init, and moving it all the way to parse_query would almost certainly result in widespread breakage.

#2 @imath
3 months ago

Hi @boonebgorges,

Thanks a lot for your feedback and the tests you did with some of your plugins with BP Rewrites.

About moving Group extensions registration down to bp_parse_query: I must admit I'm considering it because it seems the easiest way ☺️ for BP Rewrites.

But, I'm fine with leaving the Group Extension registration to happen at bp_init. I'll start checking each method of the BP_Group_Extension API first to see if they are trying to get something that can't be set with BP Rewrites on. For example the get_group_id() method shouldn't be used before bp_parse_query (the origin of the issue reported by @shanebp), and at the same time I can understand the need to adapt the Group Extension behavior according to the displayed Group, so I think we should provide guidances/new methods/functions to do that once the Group can be set.

Our goal is to make this change without breaking any plugins (there might be a few exceptions, e.g. when no tests were made about these few plugins with the BP Rewrites one).

BP Rewrites won't be merged in BP 10.0.0 or even in 11.0.0, I believe. The goal is to release a stable version of BP Rewrites on the WP.org plugins directory to ease testing. And take the needed time, from there, to test as much BuddyPress plugins as possible. We'll improve the BP Rewrites plugin often and until we reach the required level of confidence we need to do the big switch to the WP Rewrite API safely into BuddyPress core.

We still have a quite long road 😅.

@imath
3 months ago

#3 @imath
3 months ago

.2.patch improves first patch making it possible to override the BP Group Extension class from (the BP Rewrites) plugin.

#4 @imath
3 months ago

  • Owner set to imath
  • Resolution set to fixed
  • Status changed from new to closed

In 13101:

Improve the Group Extension API extensibility

  • Use a new global to store the registered Group extensions of the Groups component.
  • Edit the behavior of the bp_register_group_extension() function so that it only registers custom group extensions into the new global.
  • Introduce a new function bp_init_group_extensions() to init the registered Group extensions.

These improvements will make it possible for the BP Rewrites feature as a plugin to allow slug customization of the registered Group extensions.

Props boonebgorges

Fixes #8558

Note: See TracTickets for help on using tickets.