Skip to:
Content

BuddyPress.org

Opened 12 years ago

Closed 19 months ago

Last modified 17 months ago

#4954 closed enhancement (fixed)

Migrate BP's custom URI parser to use WP's Rewrite API

Reported by: boonebgorges's profile boonebgorges Owned by: imath's profile imath
Milestone: 12.0.0 Priority: high
Severity: major Version:
Component: Route Parser Keywords: has-patch has-unit-tests
Cc: david@…, vivek@…, sascha@…

Description

BP's custom URI parser (living mostly in bp_core_set_uri_globals()) is slow, error-prone, non-extensible, non-testable, and out of step with WP best practices. This ticket is a master ticket for the task of migrating from our current URI parser to the WordPress Rewrite API.

JJJ will be leading this task. Here's an outline of what will need to be covered, which I'm sure he and others will improve:

  • Port some of the basic rewrite architecture from somewhere like bbPress
  • Generalize some of the logic, to account for BP's system of current_component, current_action, action_variables
  • Ensure backward compatibility with plugins that add arbitrary sub-paths to BP addresses, whether it be via BP_Group_Extension, BP_Component, or manual detection of $bp->current_component etc
  • Add administation pages for the manual modification of slugs (see eg #2086)
  • Maintain backward compatibility with slugs set via constant (eg BP_GROUPS_SLUG)
  • Develop a tool for automatic migration from bp-pages to new slug system. In addition to the rewrite rules themselves, migration tool should be sensitive to Menus that include reference to bp-pages
  • Unit tests as appropriate

There is a fairly large number of tickets, both in and out of the 1.8 milestone, that will become irrelevant after the bp-pages schema has been abandoned. (Some will not necessarily be irrelevant, but they will require a totally different approach, and so should not be addressed until after the rewrite stuff is at least basically in place.) For the moment, I'm leaving those tickets open, in case the rewrite stuff were to take more than one dev cycle. An incomplete list from the 1.8 milestone: #2086 #4367 #4630 #4726 #4882 #4884 #4913 #4917 #3998

Attachments (4)

4954.ray.01.patch (54.8 KB) - added by r-a-y 11 years ago.
Patch created off r7859.
4954.ray.01b.ajax.patch (3.1 KB) - added by r-a-y 11 years ago.
Requires 4954.ray.01.patch.
4954.ray.01b.path.patch (511 bytes) - added by r-a-y 11 years ago.
Requires 4954.ray.01.patch - Fixes calculating path for backpat globals
4954.patch (132.5 KB) - added by imath 5 years ago.

Download all attachments as: .zip

Change History (108)

#1 @johnjamesjacoby
12 years ago

In 6954:

Introduce bp_add_rewrite_tags() and connect bp_generate_rewrite_rules() sub-action. See #4954.

#2 @boonebgorges
11 years ago

  • Milestone changed from 1.8 to 1.9

#3 @ddean
11 years ago

  • Cc david@… added

#4 @johnjamesjacoby
11 years ago

In 7367:

Rewrite Rules:

Add sub-actions for registering rewrite tags, rules, and permalink structures. See #4954.

#5 @johnjamesjacoby
11 years ago

In 7368:

Add rewrite rules and permalink structure methods to BP_Component class. See #4954.

#6 @johnjamesjacoby
11 years ago

In 7369:

Rewrite Rules:

Add parse_query() method to BP_Component class, allowing each component to have dedicated query parsing logic. See #4954.

#7 @johnjamesjacoby
11 years ago

In 7370:

Rewrite Rules:

Wrap $query param in array and pass byref in BP_Component::parse_query(). See #4954.

#8 @johnjamesjacoby
11 years ago

In 7371:

Rewrite Rules:

Hook bp_parse_query() to 'parse_query' allowing components to parse the main query if needed. See #4954.

#9 @sooskriszta
11 years ago

  • Cc vivek@… added

#10 @r-a-y
11 years ago

  • Milestone changed from 1.9 to 2.0

This ticket was mentioned in IRC in #buddypress-dev by jjj. View the logs.


11 years ago

#12 @boonebgorges
11 years ago

In 7809:

Store directory_pages data in the persistent cache.

This information is needed during every bootstrap, yet rarely changes, so there
should be significant performance benefit from caching it.

Invalidation takes place when one of the pages is updated, or when the bp-pages
option is changed.

Note that this change will likely be moot after migrating to the WP rewrite
API. See #4954.

See #5362

Props Denis-de-Bernardy

#13 @r-a-y
11 years ago

Attached is a first pass at rewrite rules.

This patch has no slug administration stuff and still uses our existing BP directory pages as I wanted to port our current URI functionality to work with rewrite rules at least for a first pass.

Note: When you apply the patch, make sure to manually flush the existing rewrite rules by going to "Settings > Permalinks" in the admin area.

Some dev notes:

  • Current URI properties are set on 'init' - Our old method of URI parsing is defined in the bp_core_set_uri_globals() function, which runs on 'init'. With rewrite rules, URI parsing starts on 'parse_query', which runs later than 'init'. This has the potential to break plugins looking for stuff like the current_X items, displayed_user or current_group objects on 'bp_init'. To workaround this, I had to unfortunately duplicate some logic. See the backpat_globals() method in the members and groups component loaders, which uses the new bp_core_get_from_uri() function. It's definitely not pretty, but perhaps we can leave this in for a few releases with the intent of removing this method entirely in a future release.
  • Some hooks are moved to 'bp_parse_query' - bp_setup_nav() and _bp_maybe_remove_redirect_canonical() now run on 'bp_parse_query' to account for the URI property changes above. This should rarely break anything.
  • AJAX and bp_current_X() checks - With rewrite rules, the 'parse_query' hook does not run during AJAX requests. That means any BP URI property checks (using a bp_is_current_X() function) during AJAX will fail. To workaround this, I decided to keep on using bp_core_set_uri_globals(), but run it on AJAX only. I don't see a better way around this without using some version of that function during AJAX. Might build upon the new bp_core_get_from_uri() function, which is lighter than the old bp_core_set_uri_globals(). Will look into this some more.
  • Shortlink canonical redirection - Try something like domain.com/?bp_user_id=1 or domain.com/?bp_group_id=1. This will redirect to the pretty permalink. Still a lot of work to do to our output link functions if we want to support non-fancy permalinks entirely.
  • bp-default compatibility - Yup, still works :) I've tweaked bp_core_load_template() to only run on bp-default set ups, while making sure the existing filters in that function will still work for plugins that use them. This isn't needed any more. I've patched this in a later version of BP. When repatching rewrite rules later on, ignore most of the stuff in bp_core_load_template() except for the stuff at the end as we'll want to remove the last else block.
  • BP directory page as front page - Briefly tested with the activity directory and this also works fine. Though I should point out that if we are getting rid of WP pages, setting a BP component as the front page will be a little harder to implement.
  • Page menu highlighting - Not sure how we're going to tackle this while keeping backward compatibility.
  • Existing routing unit tests pass - With some modifications to initialize and flush rewrite rules.
  • Root profiles doesn't work yet - Still need to look into this. First thoughts are this is going to be a PITA.
  • Search functionality doesn't work yet - Ran out of time for this patch. Need to probably write a specialized rewrite rule for BP_SEARCH_SLUG.

Feedback appreciated.

Last edited 10 years ago by r-a-y (previous) (diff)

@r-a-y
11 years ago

Patch created off r7859.

#14 @johnjamesjacoby
11 years ago

At a cursory, this is a really great first step.

In my imagination, the upgrade routine to 2.0.0 would look at the existing bp_pages setting, and create the appropriate rewrite rules for each directory. If no bp_page setting exists, we'd use a constant, or finally fall back on the default slug.

I think root profiles won't be that bad. Existing code for bbPress's root forums handles this scenario, and I'm hoping that using rewrite rules will allow any component to exist in the root, barring the usual and obvious slug collision problem that exists in WordPress anyways.

This ticket was mentioned in IRC in #buddypress-dev by r-a-y. View the logs.


11 years ago

#16 @r-a-y
11 years ago

I think root profiles won't be that bad. Existing code for bbPress's root forums handles this scenario, and I'm hoping that using rewrite rules will allow any component to exist in the root

I'll have to take a look at how bbPress does root forums!


01b.ajax.patch is an alternative way to address the AJAX issue brought up in comment:13.

(Requires the earlier 01.patch)

This technique loads up the WordPress query with the wp() function during AJAX requests, which will allow 'parse_query' to run... well, almost! bp_parse_query() bails when in the admin area. There's probably a good reason why bp_parse_query() does this, so I left that function untouched and added a stub method to run our 'bp_parse_query' hook.

Hat-tip to Boone for pointing me in this direction.

The only other issue is the hardcoded PHP_SELF. I'm not sure if that value changes in different environments.

@r-a-y
11 years ago

Requires 4954.ray.01.patch.

#17 @johnjamesjacoby
11 years ago

bp_parse_query bails in wp-admin because it's only intended to be used to parse the query out into rewrite rule directives, and it's an unfortunate consequence of using WordPress's AJAX URL that this has to happen within admin_ajax.

The way bbPress gets around this is by introducing a dedicated set of theme-side AJAX handlers, which is a direction I support us going in eventually that should probably have a separate ticket.

@r-a-y
11 years ago

Requires 4954.ray.01.patch - Fixes calculating path for backpat globals

#18 follow-up: @johnjamesjacoby
11 years ago

One thought after applying this and stepping through it, I'm worried it's more invasive to the existing URI router than what I had imagined. I'm thinking we should be able to bolt the rewrite rules on top of the existing functionality, and de-prioritize the old URI router similar to the way we did with theme compatibility. That way (if I'm thinking about this correctly) any old third-party components without rewrite rules will still continue to function the same way they always have, with the URI router still catching them appropriately.

Thoughts on renaming bp_path to bp_action to retain the existing component/action/variables naming and relationships we're so used to now? Or, rather than having BuddyPress act as a central router again (with standard component/path/action vars) if maybe they should always be unique. Something like:

  • bp_directory=bp_members = Members Directory
  • bp_member_id=1 = Member Profile for User ID 1

We also should stick to using WordPress's wrappers instead of combining the rewrite rules array. Something like what bbPress does now would be best, though we may even want to have our own wrappers for easily registering component/action/variable style pages. See:

https://bbpress.trac.wordpress.org/browser/trunk/src/bbpress.php#L728

#19 in reply to: ↑ 18 @boonebgorges
11 years ago

Replying to johnjamesjacoby:

One thought after applying this and stepping through it, I'm worried it's more invasive to the existing URI router than what I had imagined. I'm thinking we should be able to bolt the rewrite rules on top of the existing functionality, and de-prioritize the old URI router similar to the way we did with theme compatibility. That way (if I'm thinking about this correctly) any old third-party components without rewrite rules will still continue to function the same way they always have, with the URI router still catching them appropriately.

r-a-y's patch works in part by setting up the current_component, current_action, and action_variables. My understanding is that in this way, plugins don't need to use rewrite rules if they don't want to. They can continue to use bp_is_current_component() and even reference the global properties directly if they want to.

Philosophically (if I may ;) ) I think that if we can do this in such a way as to complete remove reliance on bp_core_set_uri_globals(), we should do so. As I understand it, this is the whole impetus for moving to rewrite rules. The fact that r-a-y's patch allows this strikes me as a strength rather than a weakness.

Thoughts on renaming bp_path to bp_action to retain the existing component/action/variables naming and relationships we're so used to now?

The problem is that bp_path is an array of what we currently call current_action and action_variables.

Or, rather than having BuddyPress act as a central router again (with standard component/path/action vars) if maybe they should always be unique. Something like:

I like this. But to my mind, the strategy here is to do this as a progressive enhancement. We can set these items and use them internally (eg, as the internals of bp_is_user() or bp_is_directory(). But we should continue to set the old current_component, current_action, action_variables for backward compatibility reasons.

===

I think the general direction r-a-y's patch is good. However, it also terrifies me. It would be nice to draw up a plan for what needs to be tested, and how it's going to work. A couple preliminary thoughts:

  • Some scenarios: root profiles; multiblog mode; username compat mode; MS subdomains; MS subdirectories; WP installed in a subdirectory; page_on_front; nested bp-pages
  • Test compatibility with some popular plugins. It seems to me that the approach in r-a-y's patch is actually pretty conservative with respect to backward compatibility, but it would be helpful to pick maybe a dozen different kinds of plugins to manually test that this is actually the case.
  • We should automate the testing of as much of this stuff as possible. The routing stuff can be pretty icky to write unit tests for, but I think it will be worth our effort to do so.

#20 @r-a-y
11 years ago

Replying to johnjamesjacoby:

That way (if I'm thinking about this correctly) any old third-party components without rewrite rules will still continue to function the same way they always have, with the URI router still catching them appropriately.

If you're talking about any third-party root components, you'd be right that they might not work. By "not work", I mean the third-party component's single page might not load up. If anyone knows of any plugin that add a root component, please let me know. I think there is a premium events plugin out there.

Plugins that hook onto existing member and group components are the plugins we have to worry the most about and the ones I have tested so far work (bbPress, BP Follow and BP Docs). More testing is needed of course.

Thoughts on renaming bp_path to bp_action to retain the existing component/action/variables naming and relationships we're so used to now?

Our current_X items are inconsistently named across different pages.

For example, if you're on a single members component page - /members/admin/activity/groups/:

  • The directory page is 'members' also is used for bp_is_current_component() checks
  • $bp->current_item will return 'admin'
  • $bp->current_component will return 'activity'
  • $bp->current_action will return 'groups'
  • $bp->action_variables will return anything after 'groups' if set

In my patch, bp_path returns 'admin/activity/groups' and that is later broken down into the respective current_X items in the BP_Members_Component::parse_query() method.

If you're on a single groups component page - /groups/test-group/members/:

  • The directory page and $bp->current_component is 'groups'
  • $bp->current_item will return 'test-group'
  • $bp->current_action will return 'members'
  • $bp->action_variables will return anything after 'members' if set

In my patch, bp_path returns 'test-group/members' and that is later broken down into the respective current_X items in the BP_Groups_Component::parse_query() method.

If you're on a directory action page - /groups/create/:

  • The directory page and $bp->current_component is 'groups'
  • $bp->current_action will return 'create'
  • $bp->action_variables will return anything after 'create' if set

In my patch, bp_path returns 'create'. This is set in the main BP_Rewrite_::parse_query() method by default.

So my thoughts on literally renaming bp_path to bp_action in my patch doesn't really make sense. If we were following another approach, then, I'd be for it.

Because our old current_X system is pretty inconsistent, I ended up doing the current_X shuffling just like our old system for backpat. It's definitely not ideal, but I'd be interested in a better way to handle this.

Or, rather than having BuddyPress act as a central router again (with standard component/path/action vars) if maybe they should always be unique.

My patch allows the component to set their own query variables.

For example, your bp_member_id suggestion is already set as my bp_user_id in my patch during the BP_Members_Component::$query_vars array and parsed in the parse_query() method. But, I still use the old component/action method. I'd be interested to see what ideas you have for this.

I was thinking of renaming my bp_component query variable to bp_directory or bp_page.

We also should stick to using WordPress's wrappers instead of combining the rewrite rules array. Something like what bbPress does now would be best, though we may even want to have our own wrappers for easily registering component/action/variable style pages. See: https://bbpress.trac.wordpress.org/browser/trunk/src/bbpress.php#L728

Yeah, the bbPress method is better than combining the rewrite rules.

I can look into using add_rewrite_tag() and add_rewrite_rule() in a second patch, but I would still use bp_path to break down the current X items.

I'm definitely not set with the approach used in the first patch, so I'm interested in alternative ideas while keeping backwards compatibility with the old URI system.


Replying to boonebgorges:

Some scenarios: root profiles; multiblog mode; username compat mode; MS subdomains; MS subdirectories; WP installed in a subdirectory; page_on_front; nested bp-pages

I've tested username compat, MS subdirectories, WP installed in a subdirectory, page_on_front lightly and it works. Needs more thorough testing though.

Test compatibility with some popular plugins. It seems to me that the approach in r-a-y's patch is actually pretty conservative with respect to backward compatibility, but it would be helpful to pick maybe a dozen different kinds of plugins to manually test that this is actually the case.

As mentioned above, I've tested on bbPress, BP Follow and BP Docs.

We should automate the testing of as much of this stuff as possible. The routing stuff can be pretty icky to write unit tests for, but I think it will be worth our effort to do so.

For sure.

Last edited 11 years ago by r-a-y (previous) (diff)

#21 @boonebgorges
11 years ago

In 7892:

Add some basic unit tests for root_profiles routing. See #4954

#22 @landwire
11 years ago

  • Cc sascha@… added

#23 @r-a-y
11 years ago

  • Milestone changed from 2.0 to 2.1

As discussed in this week's dev chat, there's not enough time to address all the issues for 2.0.

Moving to 2.1.

#24 @r-a-y
10 years ago

  • Milestone changed from 2.1 to 2.2

JJJ stated that this wouldn't make 2.1 last week. Off to 2.2!

#26 @DJPaul
10 years ago

  • Milestone changed from 2.2 to Future Release

#27 @johnjamesjacoby
10 years ago

In 9466:

Replace all references to bp_get_root_domain() . '/' . bp_get_groups_root_slug() with the appropriate bp_get_groups_directory_permalink() function. This ensures all usages and filters are applied uniformly. See #4954 for the long-game here.

#28 @johnjamesjacoby
10 years ago

In 9467:

Replace all references to bp_get_root_domain() . '/' . bp_get_members_root_slug() with bp_get_members_directory_permalink(), ensuring all usages and filters are applied uniformly. See #4954.

#29 @johnjamesjacoby
10 years ago

In 9468:

Replace all references to bp_get_root_domain() . '/' . bp_get_blogs_root_slug() with bp_get_blogs_directory_permalink(), ensuring all usages and filters are applied uniformly. See #4954.

#30 @johnjamesjacoby
10 years ago

In 9472:

Update bp_blogs_creation_location() to use bp_get_blogs_directory_permalink(). See #4954.

#31 @DJPaul
8 years ago

  • Type changed from task to enhancement

#32 @DJPaul
8 years ago

  • Priority changed from high to strategic

#33 @DJPaul
8 years ago

  • Priority changed from strategic to high

#34 @DJPaul
7 years ago

  • Keywords trac-tidy-2018 added

We're closing this ticket because it has not received any contribution or comments for at least two years. We have decided that it is better to close tickets that are good ideas, which have not gotten (or are unlikely to get) contributions, rather than keep things open indefinitely. This will help us share a more realistic roadmap for BuddyPress with you.

Everyone very much appreciates the time and effort that you spent sharing your idea with us. On behalf of the entire BuddyPress team, thank you.

If you feel strongly that this enhancement should still be added to BuddyPress, and you are able to contribute effort towards it, we encourage you to re-open the ticket, or start a discussion about it in our Slack channel. Please consider that time has proven that good ideas without contributions do not get built.

For more information, see https://bpdevel.wordpress.com/2018/01/21/our-awaiting-contributions-milestone-contains/
or find us on Slack, in the #buddypress channel: https://make.wordpress.org/chat/

#35 @DJPaul
7 years ago

  • Milestone Awaiting Contributions deleted
  • Resolution set to maybelater
  • Status changed from new to closed

@imath
5 years ago

#36 @imath
5 years ago

  • Keywords has-patch dev-feedback added; trac-tidy-2018 removed
  • Milestone set to Up Next
  • Resolution maybelater deleted
  • Status changed from closed to reopened

Hi everyone!

I’m reopening this ticket to suggest a new approach about migrating from our legacy URL parser to the WordPress Rewrite API.

First, I’d like to thank @r-a-y for his work and the time he spent building the 3 patches. It helped me a lot to identify the difficulties and the potential impacts of such a change. And at the same time I’m telling him « sorry » because I chose another road to reach the goal of this ticket: I suggest to use the existing BP Component’s methods to add rewrite tags, rewrite rules, permalink structures and to parse the WP Query for each directory component.

My idea is to work on this change progressively because, as r-a-y explained:

  • a lot of BuddyPress globals such as the current component, item, action or action variables are set before the WP Query is parsed.
  • the Member or Group BP Navs are set before the WP Query is parsed
  • the “late included” files are included before the WP Query is parsed too.
  • Some specific features like adding a directory page as the site’s home page or making a directory page a sub-page of another WordPress page are pretty challenging to maintain.

I think it would be difficult to add this change without breaking some plugins or confusing some users habits with BuddyPress.

https://cldup.com/MYjIfSrbiy.png

That’s why I chose to use an “experimental” option to our options screen and to make the change reversible. I think this would leave the time to users to get used to it and to plugin authors to adapt to this new way of parsing URLs. Users could experiment Rewrites and eventually inform plugin authors they need to fix things and come back to the legacy URL parser.

https://cldup.com/9VNOxf-NEU.png

Once the user activate this experimental option, the “Pages” tab is replaced by a “URLs” one and it’s now possible to edit all directories slug and for this first step all the primary nav items of the displayed member as shown on the screen capture below.

https://cldup.com/XgBo4_-GTw.png

If the site’s were using BuddyPress directory pages into one or WP Menus, these pages are still there but they now have a different post type “BuddyPress directory”. The custom slugs are still saved within the post_name field of this post type for directory pages and for the navigation I chose to use a post meta.

https://cldup.com/ceGyTVeGSs.png

If you switch back to the legacy parser disabling the “Custom URLs” option: everything is brought back to their previous state.

https://cldup.com/GHTfg03t0O.png

If you are using Custom URLs, you can now choose Plain permalinks 🙂 Below is an example of the User’s profile page.

https://cldup.com/ZveIEMmL1f.png

I’ve tried to edit as less code as possible and chose to use filters to rebuild links (see src/bp-core/bp-core-rewrites.php. You’ll see that some parts are quite challenging (eg: the root profiles or Ajax). I made sure the few edits I had to make had no impact on the “legacy URL parser BP” (Unit tests are still successful).

There are still a lot of links to check and I think we can improve Ajax by only doing the needed “WP” parse for registered BuddyPress ajax actions. But you can already have a good preview of the result applying the 4954.patch.

What do you think ? What about doing a first step for the 6.0.0 milestone ? What about leaving it as an experimental option for 1 or 2 releases before deprecating the legacy URL parser ?

PS: if you want to see how I’ve built the patch step by step → https://github.com/imath/BuddyPress/commits/wp-rewrites

#37 @imath
5 years ago

  • Milestone changed from Up Next to 6.0.0

Move the first tickets to next major release.

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


5 years ago

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


5 years ago

#40 @imath
5 years ago

  • Milestone changed from 6.0.0 to Awaiting Contributions

We won't have time to discuss about it during 6.0.0 and I believe it's not easy to progress on such a change. Working on the patch made me realized we can work on this from a feature as a plugin. I will try to find the time soon to put this together.

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


4 years ago

#42 @imath
3 years ago

  • Keywords has-patch removed

Hi everyone,

Thanks for your contributions so far, the remaining work will happen in the BP Rewrites feature as a plugin.

Here's a detailed post about it: https://bpdevel.wordpress.com/2021/08/13/bp-rewrites-1-0-0-alpha/

#43 @teeboy4real
3 years ago

@imath is there any guarantee that BP Rewrites will be added to buddypress core or will it be a standalone plugin.

#44 @imath
3 years ago

Hi @teeboy4real

The goal is to have it merged into Core. Using a feature as a plugin will help us to have more people test it: a plugin is always easier to install than applying a patch. We believe for such a change, we need a lot of testing to make sure we can make it as smoothly as possible.

#45 @imath
22 months ago

  • Keywords needs-patch added; dev-feedback removed
  • Milestone changed from Awaiting Contributions to 12.0.0
  • Owner changed from johnjamesjacoby to imath
  • Status changed from reopened to assigned

BuddyPress will use the WP Rewrite API although we didn't get enough BP Rewrites testers.

As discussed into various BP Slack discussions and announced into the 11.0.0 Hello Screen (modal displayed when you activated BuddyPress) or into the 11.0.0 release post, we'll switch our current URL parser in favor of using the WP Rewrite API.

This will be our main focus for the 12.0.0 release.

This ticket was mentioned in PR #65 on buddypress/buddypress by @imath.


22 months ago
#46

  • Keywords has-patch has-unit-tests added; needs-patch removed

Create the bp-core-rewrites.php file to put BP Rewrites function inside it & edit the BP_Component class to generate rewrites when a component has a directory.

Compared to the BP Rewrites plugin, I've put some new code to make it easier for plugin developers to create BP Rewrites for their custom component. Most of the job can be done by setting the rewrite_ids values from this component eg: $component->rewrite_ids = array( 'directory' => 'custom_rewrite_id' ).

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#47 @imath
22 months ago

In 13422:

Improve BP_Component methods relative to setting BP Rewrites

So far BP_Component::add_rewrite_tags(),
BP_Component::add_rewrite_rules() & BP_Component::add_permastructs()
were just firing hooks. Thanks to the BP Rewrites plugin we were able to
test how we can use them to actually enjoy the WP Rewrite API in
BuddyPress. Compared to what was tested in this feature as plugin, some
improvements has been brought so that it's easier to benefit from the BP
Rewrites API for custom components (analyzing the included PHPUnit tests
can give BP plugin authors a preview about how to process).

Introduce the BP_Component::pre_query() method to avoid superfluous Post
queries when a BuddyPress page is displayed.

The src/bp-core/bp-rewrites.php file has also been inited, this is the
place where all Core component rewrites function will be placed.

Props r-a-y, johnjamesjacoby, boonebgorges

See #4954

@imath commented on PR #65:


22 months ago
#48

To be continued in future PRs...

This ticket was mentioned in PR #67 on buddypress/buddypress by @imath.


22 months ago
#49

The custom post type name is buddypress. Why?

  • It should be a unique name no other plugins chose
  • Using buddypress should help us enjoy WP template hierarchy

Bump raw db version to the first commit number we made about BP Rewrites.
Add an upgrade routine to edit existing directory pages post type as well as nav menu items.
Stop using get_page_by_path() in favor of bp_core_get_directory_page_id().

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#50 @imath
22 months ago

In 13431:

Use a custom post type instead of the page post type for directories

  • The buddypress post type is now the one used for the BP component

directory page.

  • Introduce the bp_core_set_unique_directory_page_slug() function to

make sure there is no conflict with an existing WP page when setting the
BP Component directory page post_name.

  • Introduce the bp_restricted custom status. We'll be able to use it for

future visibility features.

  • Deprecate the Admin Slugs screen and corresponding functions
  • Bump raw_db_version to 13422 (the first commit about merging BP

Rewrites).

  • Add an update function to update the post type of the existing BP

component directory pages and potential WP Nav Menus including these.

Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/67
See #4954

This ticket was mentioned in PR #69 on buddypress/buddypress by @imath.


22 months ago
#51

It introduces the bp_rewrites_get_url() function. Its role is to build every BuddyPress URL using the BP Rewrites API.

It also deprecates softly some key functions like bp_get_root_domain() to let us review all BP links during 12.0 development cycle and make them use the bp_rewrites_get_url() function or a wrapper of it.

It also deprecates slug constant as we will be able to customize every slugs from the future "URL" tab of BuddyPress settings page.

It puts the Components $rewrite_ids in place and deprecates the $root_domain BP global.

This PR will soon be completed with Unit tests for the bp_rewrites_get_url() function.

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#52 @imath
21 months ago

In 13432:

BP Rewrites: introduce the bp_rewrites_get_url() function

Its role is to build every BuddyPress URL using the BP Rewrites API.

This commit also deprecates softly some key functions like bp_get_root_domain() to let us review (thanks to deprecated notices) all BuddyPress links during 12.0 development cycle and make them use the introduced bp_rewrites_get_url() function or a wrapper of it. Once all replacements achieved, we'll need to fully deprecate:

  • bp_get_root_domain()
  • bp_root_domain()
  • bp_core_get_root_domain()

Slug constants have also been completely deprecated as we will be able to customize every slugs from the future "URL" tab of the BuddyPress settings page.

The $bp->root_domain BuddyPress global has been deprecated in favor of $bp->root_url.

Finally, the Components $rewrite_ids properties are now in place and corresponding rewrite rules are successfully generated.

Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/69
See #4954

This ticket was mentioned in PR #70 on buddypress/buddypress by @imath.


21 months ago
#53

bp_members_get_user_url() is a wrapper of bp_rewrites_get_url(). It replaces bp_core_get_user_domain() to build Member URLs using BP Rewrites.

As many URLs are built concatenating bp_core_get_user_domain() with URL chunks, the safer way to make sure Developers update the way they build their BuddyPress links is to deprecate this function.
bp_core_get_username() has also been deprecated in favor of bp_members_get_user_slug().

This PR also updates some PHPUnit tests!

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#54 @imath
21 months ago

In 13433:

BP Rewrites: Introduce the bp_members_get_user_url() function

As many member URLs are built concatenating bp_core_get_user_domain()
with URL chunks, the safer way to make sure developers update the way they
build their member URLs in favor of using BP Rewrites is:

  1. to deprecate this function
  2. create a new function bp_members_get_user_url() which is a wrapper of

bp_rewrites_get_url()

  1. replace all bp_core_get_user_domain() occurrences by

bp_members_get_user_url()

This commit also deprecates bp_core_get_username() in favor of the new
bp_members_get_user_slug() function and updates PHPUnit tests.

Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/70
See #4954

This ticket was mentioned in PR #73 on buddypress/buddypress by @imath.


21 months ago
#55

  • Improve the Members component adding permastructs and custom rewrite rules for registration and activation pages.
  • Improve the Blogs component adding custom rewrite rule to handle the Blogs create page.
  • Make a huge progress about replacing all occurrences of bp_get_root_domain() (45 replacements were performed). There might be some last ones inside PHPUnit tests.
  • Deprecate bp_displayed_user_domain(), bp_loggedin_user_domain(), bp_groups_directory_permalink(), bp_get_groups_directory_permalink(), bp_blogs_directory_permalink() & bp_get_blogs_directory_permalink() and replace them with these new functions: bp_displayed_user_url(), bp_loggedin_user_url(), bp_groups_directory_url(), bp_get_groups_directory_url(), bp_blogs_directory_url() & bp_get_blogs_directory_url(). These deprecations are required because these functions were used to build some BuddyPress URLs concatenating URL chunks.
  • Start replacing bp_loggedin_user_domain() & bp_displayed_user_domain() with the new functions.

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

sbrajesh commented on PR #73:


21 months ago
#56

This is a real bad idea. This is going to create havoc as the functions you are trying to deprecate are used very commonly.
Why not change the internal code of the functions to use current method for generating url instead of deprecating? what are the cons of doing that?
If the current deprecation spree keeps going on with the core functions, this will kill the whole BuddyPress eco system(not all themes and plugins are going to update their code for the trivial reason of name change).
Please reconsider it.

@imath commented on PR #73:


21 months ago
#57

Hi,

Thanks for your feedback. It's nice to read from you, it's been a while. If you can reply with a solution to this question I'm very open to reconsider it.

How can you know when a plugin author uses this code: bp_blogs_directory_permalink() . 'create/'? I need to make sure to convert it to an URL built using the BP Rewrites so that it can use the customized slug and be reachable using pretty or plain links ?

sbrajesh commented on PR #73:


21 months ago
#58

Hi,
Thank you for your work on the BuddyPress core!
It is nice to participate again.

As far as I am aware BuddyPress has limited number of end point which allow create actions(group,blogs).
If we look at these 2, we only have to worry about these 2 end points with hard coded action. A simple solution will be to add a canonical redirection from old url to new and have a filter to toggle this behaviour for these two.

For everything else, there are proper functions available in BuddyPress that points to the url. My main concerns were bp_loggedin_user_domain, bp_displayed_user_domain which are pervasive. These were marked deprecated in this commit
https://github.com/buddypress/buddypress/pull/73/commits/5da133d58cf97310ce697471bafd892713220cac
I am looking forward to spend time with the rewrite changes this week and have a better grasp to provide any feedback/assistance from next week.

@imath commented on PR #73:


21 months ago
#59

I wish it there was "proper functions available in BuddyPress that points to the URL".

I've spent 2 years on the BP Rewrites plugin and I can tell you it's not the case. Here are some examples from our code base:

  • ​​bp_displayed_user_domain() . bp_get_profile_slug() . '/change-avatar/delete-avatar
  • bp_loggedin_user_domain() . bp_get_members_invitations_slug() . '/send-invites'
  • ​​bp_displayed_user_domain() . bp_get_messages_slug() . '/' . bp_current_action()

There are a lot more.

So you're suggesting to redirect all hardcoded URLs?

sbrajesh commented on PR #73:


21 months ago
#60

Great points.
In my opinion, a function should be deprecated if the stop doing what they are supposed to do.
How a function is used by others may not be a ground for it. Even the newly introduced functions can be misused and hard coded by some people.

I will put one example:-
bp_loggedin_user_domain() - The purpose of this function is to provide url of the logged user. Has that changed? is that not achievable anymore by internally modifying it? If the purpose did not change, there is no rationale behind deprecating it.
Same goes for others.
Core can not track all the use cases and can not stop the misuse but the core can update the function to reflect the current approach and also replace the hard coded instance in the core itself(if there are some).
The goal should be to allow users minimum friction to update.

Even with the new rewrite API, the end points are not going to change automatically unless the admin does it manually. Making them aware that they test their change for url and report to the breaking plugin/theme will save a lot of time and resentment from community.
Also, when I said redirect, my intention was for group/blog creation only. I don't see any harm in doing it from other end points as part of a back compat plugin if enough people demand it.

Thank you for your attention.

@imath commented on PR #73:


21 months ago
#61

@sbrajesh

Thanks for your feedback. I'm more in line with this definition of deprecation. These 2 functions will not be used anymore in BuddyPress once BP Rewrites will be fully merged. As we used to use them the wrong way, concatenating URL chunks, plugin authors did the same and it's fine. We need to stop doing so in favor of new functions making sure it's possible to customize slugs, be compatible with plain links and comply with a WordPress standard: the WP Rewrite API. I believe our role is to tell plugin developers they need to update their plugin for community benefits. When a user will have an issue he won't look for the code responsible for the fail, he will write a topic into our support forum giving more work to people contributing to support.

We agreed on this scenario:

  • move to BP Rewrites
  • build a BP Classic plugin for plugin who won't update their code.

My plan is to deprecate the problematic functions to be sure thanks to notices I replaced these functions from our code base.

Then I can move these functions into the BP Classic plugin. If this plugin is not used the deprecation notices will help us to inform people are using a plugin requiring the BP Classic plugin to be compatible with BuddyPress.

If I don't have this deprecation notices, I'm losing this possibility.

As @dcavins and you seem very concerned about the two functions (logged in & displayed domains). I'm going to leave them the way they are for now (although I think it will end up generating issues). It will make my work more difficult. Then we'll discuss again about these 2 functions. So far your arguments haven't convinced me, but I need to move on.

I'll commit this PR soon.

sbrajesh commented on PR #73:


21 months ago
#62

Hi @imath
Thank you.
I am sure I know a bit or two about deprecation. My concern is your reason behind deprecation.
In order to deprecate something, you have to think about its impact and the benefits. is it absolutely necessary?

When you allow 3rd parties to use your API, you need to consider it even more deeply than if you were using your own APIs.
I am sure you have good intention but the direction and rationale is plainly incorrect. It is like knowingly breaking every existing BuddyPress theme and plugin(yes, throwing notice is same as breaking).
With the current shrinking market that BuddyPress has I doubt that there will be a recovery from this move. It will be much prudent for developers to work with platforms which provide stability like WordPress does instead of working with a volatile API.

It is your decision as you have put a lot of work in BuddyPress but I will suggest that you re-asses the way WordPress and the APIs WordPress eco system works. When did you remember a major API break in WordPress? There was a time around 10 -12 years ago when a change in $wpdb->query had a wide impact but other than that, it has always been trivial change for existing applications.
This is how a software that has a large eco system/dependent plugin systems work. APIs are like contract and you don't break because you got a new one. you break it when it can't be maintained.

Anyway, I am not a big fan of breaking existing things for the sake of changing names and what you are doing is going outside the scope of the core to force others to update. Not sure if that will help or be harmful for the BP.

With that I will leave it for you to take the final decision.

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


21 months ago

#64 @imath
21 months ago

In 13436:

Make more BuddyPress generated links ready for BP Rewrites

  • Improve the Members component adding permastructs and custom rewrite

rules for registration and activation pages.

  • Improve the Blogs component adding custom rewrite rule to handle the

Blogs create page.

  • Make a huge progress about replacing all occurrences of

bp_get_root_domain() (45 replacements were performed).

  • Deprecate bp_groups_directory_permalink(),

bp_get_groups_directory_permalink(), bp_blogs_directory_permalink() &
bp_get_blogs_directory_permalink() and replace them with these new
functions: bp_groups_directory_url(), bp_get_groups_directory_url(),
bp_blogs_directory_url() & bp_get_blogs_directory_url().

  • Although bp_loggedin_user_domain() & bp_displayed_user_domain()

should also be deprecated, we're leaving them as aliases of the right
functions to use. Plugin authors shouldn't use them to build other links
than member's profile home url.

NB: these deprecations are required because these functions were used
to build BuddyPress URLs concatenating URL chunks. Once the BP Classic
plugin will be built we will adapt the code to remove these deprecation
notices.

Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/73
See #4954

This ticket was mentioned in PR #75 on buddypress/buddypress by @imath.


21 months ago
#65

  • Add rewrite tags/rules for the groups directory type and the create URLs
  • Introduce bp_get_group_url()/bp_group_url() & to retrieve/output a Groups single item URL using BP Rewrites.
  • Introduce bp_get_groups_directory_url()/bp_groups_directory_url() to retrieve/output the Groups directory URL using BP Rewrites.
  • Introduce bp_get_group_restricted_screens(), bp_get_group_extension_screens() & bp_get_group_screens() to get information about the Groups screens (in particular each screen rewrite ID).
  • Perform easiest replacements for bp_get_group_permalink()/bp_group_permalink() & bp_get_groups_directory_permalink()/bp_groups_directory_permalink().
  • Improve code formatting & properly escape single group URLs into templates.
  • Update impacted Unit Tests

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#66 @imath
21 months ago

In 13437:

BP Rewrites: start the migration process for the Groups component

  • Add rewrite tags & rules for the Groups directory type and the create URLs.
  • Introduce bp_get_group_url()/bp_group_url() & to retrieve/output a Groups single item URL using BP Rewrites.
  • Introduce bp_get_group_restricted_screens(), bp_get_group_extension_screens() & bp_get_group_screens() to get information about the Groups screens (in particular each screen rewrite ID). These functions will ease slug customizations from the BuddyPress URL settings tab.
  • Improve the Group creation process making sure it's using BP Rewrites to build URLs.
  • Perform easiest replacements for bp_get_group_permalink()/bp_group_permalink() & bp_get_groups_directory_permalink()/bp_groups_directory_permalink() in favor of bp_get_group_url()/bp_group_url() & bp_get_groups_directory_url()/bp_groups_directory_url()`.
  • Improve code formatting & properly escape single group URLs into templates.
  • Update impacted Unit Tests.

Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/75
See #4954

This ticket was mentioned in PR #77 on buddypress/buddypress by @imath.


21 months ago
#67

BP_Core_Nav is the class we use to set up BuddyPress single item navigations. It now only needs slugs to build URLs.

  • Remove all components parent_url attributes when setting sub nav items.
  • When a plugins will specify a link for the main nav item or a parent_url for the sub nav item, it will use this link to preserve backward compatibility.
  • The PR also migrates the Community search feature so that it uses BP Rewrites.
  • Update some PHPUnit tests.

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#68 @imath
21 months ago

In 13441:

Make BP_Core_Nav generate "BP Rewrites ready" navigation links

  • Remove all components $parent_url attributes when setting sub nav items.
  • Only use the bp_core_create_nav_link() $link attribute argument & the bp_core_create_subnav_link() $parent_url attribute argument if specified to preserve backward compatibility.
  • Migrates the Community search feature so that it uses BP Rewrites.
  • Perform some bp_loggedin_user_domain() in favor of bp_loggedin_user_url().
  • Update some PHPUnit tests.

Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/77
See #4954

This ticket was mentioned in PR #78 on buddypress/buddypress by @imath.


21 months ago
#69

  • Introduce a function to get all component navigations. bp_get_component_navigations() will be used to make it easier to get all customizable slugs within the BuddyPress settings area.
  • Perform all remaining bp_loggedin_user_domain() replacements in favor of bp_loggedin_user_url().

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#70 @imath
21 months ago

In 13442:

Improve Components Member's single item navigation generation

  • Edit the BP_Component Class so that it globalize navigation items before registering it.
  • Introduce bp_get_component_navigations(), a new function that will be used to get Member's single navigation customizable slugs within the BuddyPress settings area.
  • Perform all remaining bp_loggedin_user_domain() replacements (55) in favor of the bp_loggedin_user_url() function which uses BP Rewrites to build URLs.
  • Improve bp_loggedin_user_link() by adding a new $chunks array of arguments to output escaped URL in templates.
  • Adapt some PHPUnit testcases.

Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/78
See #4954

This ticket was mentioned in PR #79 on buddypress/buddypress by @imath.


20 months ago
#71

  • Replace all remaining bp_displayed_user_domain() usage in favor of bp_displayed_user_url().
  • Introduce the bp_members_get_path_chunks() function to quickly build BP Rewrites argument for member's URL using an array of slugs.
  • Deprecate bp_activities_member_rss_link(), bp_blogs_blog_tabs() & bp_groups_header_tabs(),
  • Improve bp_displayed_user_link() so that it's possible to pass an array of slugs to output an escaped BP Rewrites ready URL.

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#72 @imath
20 months ago

In 13443:

Make sure all displayed user URLs are built using the BP Rewrites API

Replace all remaining bp_displayed_user_domain() usage in favor of
bp_displayed_user_url().
Introduce the bp_members_get_path_chunks() function to quickly build BP
Rewrites argument for member's URL using an array of slugs.
Deprecate bp_activities_member_rss_link(), bp_blogs_blog_tabs() &
bp_groups_header_tabs().
Improve bp_displayed_user_link() so that it's possible to pass an array
of slugs to output an escaped BP Rewrites ready URL.

Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/79
See #4954

This ticket was mentioned in PR #81 on buddypress/buddypress by @imath.


20 months ago
#73

  • Introduce the bp_groups_get_path_chunks() function to build BP Rewrites arguments using a regular array.
  • Improve bp_members_get_path_chunks() so that it avoids looking for a custom slug when the chunk is a numeric value.
  • Introduce the bp_get_group_manage_url() function to build Group's front-end Admin URLs using BP Rewrites.
  • Deprecate bp_group_admin_permalink() & bp_get_group_admin_permalink() in favor of bp_group_manage_url() & bp_get_group_manage_url().
  • Replace all remaining usage of bp_get_group_permalink() in favor of bp_get_group_url() or bp_get_group_manage_url().

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#74 @imath
20 months ago

In 13446:

Use the BP Rewrites API to generate single Groups item links

  • Introduce the bp_groups_get_path_chunks() function to build BP Rewrites arguments using a regular array.
  • Improve bp_members_get_path_chunks() so that it avoids looking for a custom slug when the chunk is a numeric value.
  • Introduce the bp_get_group_manage_url() function to build Group's front-end Admin URLs using BP Rewrites.
  • Deprecate bp_group_admin_permalink() & bp_get_group_admin_permalink() in favor of bp_group_manage_url() & bp_get_group_manage_url().
  • Replace all remaining usage of bp_get_group_permalink() in favor of bp_get_group_url() or bp_get_group_manage_url().

Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/81
See #4954

This ticket was mentioned in PR #83 on buddypress/buddypress by @imath.


20 months ago
#75

  • Start moving deprecated functions inside a function_exists( 'bp_classic' ) check, corresponding functions were added inside the BP Classic backcompat plugin.
  • Replace all remaining deprecated functions usage.
  • Improve bp_groups_get_path_chunks() to deal with front-end group admin URLs and the groups create URLs.
  • Adjust some routing unit tests

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#76 @imath
20 months ago

In 13449:

Make sure all Groups component URLs are built using BP rewrites API

  • Introduce the bp_groups_get_create_url() to ease Groups create URLs generation.
  • Improve bp_groups_get_path_chunks() to deal with front-end group admin URLs and the Groups create URLs.
  • Deprecate bp_get_groups_directory_permalink() in favor of bp_get_groups_directory_url().
  • Replace all remaining deprecated functions usage.
  • Start putting deprecated functions behind a function_exists( 'bp_classic' ) check, corresponding functions were added inside the BP Classic backcompat plugin.
  • Adjust some Groups routing unit tests.

Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/83
See #4954

This ticket was mentioned in PR #84 on buddypress/buddypress by @imath.


20 months ago
#77

Compared to commit 13442, change the logic of Components user navigation generation by introducing a BP_Component::register_nav() function to globalize the nav items and make them available for the added URLs WP Admin settings screen. After a second thought, using BP_Component::setup_nav() to play the registration role can be problematic considering backward compatibility. We are maintaining BP_Component::setup_nav() to play the role of generating the user navigation, Third party plugins wishing their slugs to be customizable will need to "opt-in" BP Rewrites using BP_Component::register_nav().

This first version of the URLs WP Admin settings screen does not handle slug customization yet, it's used to make sure all BP Component user navigation slugs can be customized.

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#78 @imath
20 months ago

In 13450:

Administration: add a new settings tab to manage slugs customization

Compared to [13442], change the logic of Components user navigation
generation by introducing a BP_Component::register_nav() method to
globalize the nav items early (ie: the registration step) and make them
available for the new settings tab to manage slugs customization.

After a second thought, the BP_Component::setup_nav() should remain the
navigation generation step instead of playing the registration role. This
will maximize backward compatibility & third party plugins wishing their
slugs to be customizable will need to "opt-in" for BP Rewrites using the
BP_Component::register_nav() method.

This first version of the URLs settings tab does not handle slugs
customization yet, its first usage is to make sure all BP Components user
navigation slugs were registered & to put the Accordion UI in place.

Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/84
See #4954

This ticket was mentioned in PR #85 on buddypress/buddypress by @imath.


20 months ago
#79

  • Edit BP_Groups_Component::setup_nav() so that it uses bp_get_group_screens() to generate the single group's navigation items.
  • bp_get_group_screens() is making it possible to get all single group's nav item slugs from the BP Urls settings tab, use it to add these slugs into the Accordion UI.

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#80 @imath
20 months ago

In 13451:

Administration: add Group slugs management into the URLs settings tab

  • Add a new $built_in argument to bp_get_group_screens() to only get BP built in single Groups item screens.
  • Edit the BP_Groups_Component::setup_nav() method so that it uses bp_get_group_screens() to generate the single group's navigation items.
  • bp_get_group_screens() is making it easier to manage all single group's nav item slugs from the BP Urls settings tab.

Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/85
See #4954

This ticket was mentioned in PR #86 on buddypress/buddypress by @imath.


20 months ago
#81

Add bp_rewrites_pre_get_slug filter to bypass slug customization. This filter will let the BP Classic plugin apply a classic BuddyPress experience (without BP Rewrites improvements).

Improve the BP_Group_Extension to support BP Rewrites and do some code formatting cleanup.

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#82 @imath
20 months ago

In 13452:

Improve the BP_Group_Extension to support the BP Rewrites API

  • Do the needed adaptations so that slugs used by plugins extending the

BP_Group_Extension can be customizable from the BP URls WP Admin
settings screen.

  • Do some code formatting cleanup in various class methods.

This commit also introduces a new filter bp_rewrites_pre_get_slug the BP
Classic plugin will be able to use to skip any slug customizations.

Props Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/86
See #4954

#83 @imath
20 months ago

In 13454:

Fix a variable name typo in bp_members_get_user_slug()

See #4954

This ticket was mentioned in PR #87 on buddypress/buddypress by @imath.


20 months ago
#84

  • Start implementing the parse_query() methods for the Activity/Blogs and Members components.
  • Remove the notice used to force pretty permalinks
  • Use plain links to test parse_query() methods are rightly setting BuddyPress URI globals
  • Introduce some helper functions used during the BP Rewrites API parsing process :
    • bp_is_directory_homepage() is checking if a BP Directory is used as the site's homepage.
    • bp_rewrites_get_custom_slug_rewrite_id() finds a Rewrite ID out of a custom slug.
    • bp_rewrites_get_member_data() returns the field to use to get the user by.
    • bp_reset_query() makes it possible to parse again a request in specific cases (eg: root profiles)

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#85 @imath
20 months ago

In 13455:

Start implementing parse_query() methods for directory components

First concerned commponents are: Activity, Blogs & Members.

  • Remove the notice used to force pretty permalinks
  • Use plain links to test parse_query() methods are rightly setting BuddyPress URI globals
  • Introduce some helper functions used during the BP Rewrites API parsing process :
    • bp_is_directory_homepage() is checking if a BP Directory is used as the site's homepage
    • bp_rewrites_get_custom_slug_rewrite_id() finds a Rewrite ID out of a custom slug.
    • bp_rewrites_get_member_data() returns the field to use to get the user by.
    • bp_reset_query() makes it possible to parse again a request in specific cases (eg: root profiles)

Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/87
See #4954

This ticket was mentioned in PR #88 on buddypress/buddypress by @imath.


20 months ago
#86

Implement parse_query() method for Groups component
Create a method to set the current group and be able to use it inside the parse_query() method of the Groups component

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#87 @imath
20 months ago

In 13456:

Implement a parse_query() method for the Groups component

Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/88
See #4954

This ticket was mentioned in PR #91 on buddypress/buddypress by @imath.


20 months ago
#88

  • Introduces bp_core_setup_query_parser() to decide when to postpone some key hooks firing to bp_parse_query.
  • Introduces bp_core_get_query_parser() to get the query parser in use. It contains a filter BP Classic will be able to use to force the Legacy URL parser. So far it uses legacy for pretty links and rewrites for plain links.
  • Edit bp_redirect_canonical() & bp_get_canonical_url() so that they use the BP Rewrites API.
  • Introduces bp_core_set_ajax_uri_globals() to set the BuddyPress URI globales using the BP Rewrites API inside the Ajax context thanks to the updated bp_reset_query() function.
  • To avoid using WP() at each Ajax call, introduces a simple Ajax actions registration process thanks to the new bp_ajax_register_action().
  • Update Legacy & Nouveau template packs so that they use this logic. As BP Default will require the legacy URL parser and will be moved inside BP Classic, the theme can stay the way it is.
  • Improve the bp_rewrites_pre_get_slug filter logic making it depends on the bp_core_get_query_parser() function.
  • Make sure to reset the Members navigation to a new BP_Core_Nav based on the displayed user once the user is available in the components parse_query() method.

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#89 @imath
20 months ago

In 13461:

Make canonical redirection & Ajax requesting BP Rewrites ready

  • Introduces bp_core_setup_query_parser() to decide when to postpone some key hooks firing to bp_parse_query.
  • Introduces bp_core_get_query_parser() to get the query parser in use. It contains a filter BP Classic will be able to use to force the Legacy URL parser. So far it uses legacy for pretty links and rewrites for plain links.
  • Edit bp_redirect_canonical() & bp_get_canonical_url() so that they use the BP Rewrites API.
  • Introduces bp_core_set_ajax_uri_globals() to set the BuddyPress URI globales using the BP Rewrites API inside the Ajax context thanks to the updated bp_reset_query() function.
  • To avoid using WP() at each Ajax call, introduces a simple Ajax actions registration process thanks to the new bp_ajax_register_action().
  • Update Legacy & Nouveau template packs so that they use this logic. As BP Default will require the legacy URL parser and will be moved inside BP Classic, the theme can stay the way it is.
  • Improve the bp_rewrites_pre_get_slug filter logic making it depends on the bp_core_get_query_parser() function.
  • Make sure to reset the Members navigation to a new BP_Core_Nav based on the displayed user once the user is available in the component's parse_query() method.

Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/91
See #4954

This ticket was mentioned in PR #94 on buddypress/buddypress by @imath.


19 months ago
#90

  • Deprecate bp_core_set_uri_globals() && completely stop using the BP Legacy URL parser in favor of BP Rewrites 🙌. This function will be moved inside the BP Classic compatibility plugin see https://github.com/buddypress/bp-classic/pull/3
  • Introduce a new hook bp_register_nav to hook to when globalizing Members single item navigations in BP_Component.
  • Improve bp_get_component_navigations() so that Avatar/Cover images nav items are moved inside the Profile subnav if Extended profile is active
  • Improve BP_Core_Nav so that any Action Variable slugs can be customized.
  • Improve Members & Groups component canonical redirections.
  • Handle slugs customization persistency using directory pages post metas
  • Introduce a new repair tool to reset all slugs to BuddyPress default one.

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#91 @imath
19 months ago

In 13468:

Stop using BP Legacy URL parser in favor of the new BP Rewrites API

  • Deprecate bp_core_set_uri_globals(). This function is moved inside the BP Classic compatibility plugin.
  • Introduce the new bp_register_nav action to hook to when globalizing Members single item navigations from the BP_Component class.
  • Improve bp_get_component_navigations() so that Avatar/Cover images navigation items are moved inside the Profile sub nav if the Extended profile component is active.
  • Register Avatar/Cover images Ajax actions so that these actions trigger our new URL Parser inside Ajax context.
  • Improve the BP_Core_Nav::add_nav() method so that any BP action variable slugs can be customized.
  • Improve Members & Groups component canonical redirections.
  • Handle slugs customization persistency using directory pages post metas.
  • Introduce a new repair tool to reset all slugs to BuddyPress default one.
  • Adapt some PHPUnit tests to better handle our new URL parser.

Props Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/94
See #4954

This ticket was mentioned in PR #95 on buddypress/buddypress by @imath.


19 months ago
#92

  • Make sure BP Activity Embeds are supported when using the BP Rewrites API
  • Deprecate bp_admin_display_directory_states() and move the BP Page directory states feature to BP Classic, see https://github.com/buddypress/bp-classic/pull/4
  • Remove the Page settings Admin help tab to move it in BP Classic
  • Introduce the bp_core_include_directory_on_front() in order to maintain the possibility to use a BP Directory page as the site's home page.

PS: this will be the last step of the BP Rewrites merge process. All new issues about the BP Rewrites API will be managed in new tickets.

Trac ticket: https://buddypress.trac.wordpress.org/ticket/4954

#93 @imath
19 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 13471:

BP Rewrites merge process, final touches

This commit is closing the main ticket about migrating our Legacy URL parser to using the WP Rewrite API. Please open next possible issues in new tickets.

The final touches:

  • Make sure BP Activity Embeds are fully supported when using the BP Rewrites API.
  • Make sure BuddyPress directories are fetched into the Site Editor's Navigation block.
  • Deprecate bp_admin_display_directory_states() and move the BP Page directory states feature into the BP Classic backcompat plugin.
  • Remove the Page settings Admin help tab & move it into BP Classic
  • Introduce the bp_core_include_directory_on_front() in order to maintain the possibility to use a BP Directory page as the site's home page.

Props r-a-y, johnjamesjacoby, boonebgorges

Closes https://github.com/buddypress/buddypress/pull/95
Fixes #4954

#94 @imath
18 months ago

In 13492:

Make sure BP_Component::parse_query() always run

Only running it when BP Rewrites are on is not the right way to go as some plugins might have been using the WP Rewrites API for a while and might have extended this method from their component's class.

Instead of only running this method if BP Rewrites are on, we're adding a URL parser check inside each component having a directory to figure out whether it's needed to set BP URI globals.

See #4954
Fixes #8908
Closes https://github.com/buddypress/buddypress/pull/109

#95 @imath
18 months ago

In 13495:

Prevent potential issues with late_includes() in Component classes

Use require_once instead of require to load action & screen files.

See #4954
Fixes #8910
Closes https://github.com/buddypress/buddypress/pull/112

#96 @imath
18 months ago

In 13502:

buddypress post type dynamic creation for has_directory components

12.0 introduces a buddypress post type which is used to store the components directory pages into the database & replace the page post type that was used so far. It also replaced the Page association WP Admin screen with another WP Admin screen to customize all BP URLs.

In case a custom component is using a directory page and is not taking care of generating the corresponding buddypress post type entry, instead of asking for a page association in an admin notice, this entry is dynamically generated. If users are not happy with the title or slug of this entry, they can use the BP URLs settings tab to customize these afterwards.

Fixes #8918
See #4954
Closes https://github.com/buddypress/buddypress/pull/116

#97 @imath
18 months ago

In 13503:

BP Rewrites: optimize the code used to build URLs

The bp_members_get_path_chunks() & bp_groups_get_path_chunks() functions are making sure URL chunks are customized according to the slugs settings. Instead of redoing at many places all or some of the operations performed by these functions, update the code to build URLs so that it uses these functions.

See #4954
Fixes #8923
Closes https://github.com/buddypress/buddypress/pull/117

#98 @imath
17 months ago

In 13512:

Check the requested Member’s page matches a valid nav screen function

The BP_Members_Component::check_parsed_query() method happens once the query is parsed & once the single displayed Member's navigation is set. It checks the current action matches a navigation slug & whether the corresponding screen function exists and is callable. If it's not the case a 404 is displayed, just like it's currently the case when the BP Legacy URL parser is in use.

See #4954
Fixes #8932
Closes https://github.com/buddypress/buddypress/pull/125

#99 @imath
17 months ago

In 13518:

Make sure directory rewrite rules match exact directory slugs

Before this change slugs beginning like directory slugs were wrongly consider as matches. Example: membership which begins like members was parsed as the Members directory.

See #4954
Fixes #8938
Closes https://github.com/buddypress/buddypress/pull/132

#100 @imath
17 months ago

In 13519:

Make sure bp_core_new_nav_default() is ready for the BP Rewrites API

  • In case plain permalinks are used, use the $GLOBALS['wp']->query_string to set the buddypress()->unfiltered_uri global.
  • Do not edit sub nav links when calling bp_core_new_nav_default() if the URL parser is set to 'rewrites'.
  • Avoid a PHP notice if the new sub nav object misses the slug and/or the link properties.

Props corzel

See #4954
Fixes #8936
Closes https://github.com/buddypress/buddypress/pull/130

#101 @imath
17 months ago

In 13520:

Use BP Rewrites API functions instead of home_url() to build links

See #4954
Fixes #8937
Closes https://github.com/buddypress/buddypress/pull/131

#102 @imath
17 months ago

In 13521:

Ensure the way the single group canonical stack is set is consistent

If the manage context does not need the current action to get the customized slugs for action variables, the read context does.

Moreover, as the current group is not set during the creation process, we don't need to get customized action variables for the create context.

Fixes #8940
See #4954
Closes https://github.com/buddypress/buddypress/pull/134

#103 @imath
17 months ago

In 13522:

Do the homepage directory check only when the requested URL is home

Fixes #8939
See #4954
Closes https://github.com/buddypress/buddypress/pull/133

#104 @imath
17 months ago

In 13526:

Add BP Rewrites API support to Activities auto-refreshing feature

Do register the heatbeat Ajax action into the buddypress()->ajax_actions global. This registration was missed during the BP Rewrites migrating process.

See #4954
Fixes #8954
Closes https://github.com/buddypress/buddypress/pull/139

Note: See TracTickets for help on using tickets.