[buddypress-trac] [BuddyPress Trac] #4954: Migrate BP's custom URI parser to use WP's Rewrite API

[buddypress-trac]

#4954: Migrate BP's custom URI parser to use WP's Rewrite API
---------------------------+------------------------------
 Reporter:  boonebgorges   |       Owner:  johnjamesjacoby
     Type:  task           |      Status:  new
 Priority:  high           |   Milestone:  2.0
Component:  Rewrite Rules  |     Version:
 Severity:  major          |  Resolution:
 Keywords:                 |
---------------------------+------------------------------

Comment (by boonebgorges):

 Replying to [comment:18 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.

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/4954#comment:19>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac