Determine $bp->component->root_slug automatically

[boonebgorges]

One of the parameters for BP_Components::setup_globals() is root_slug. But there's no reason (that I can think of) why this should need to be provided manually. The root_slug should *always* match the path of the corresponding item in $bp->pages.

On that note, here are two different proposals:
1) Don't ask for a root_slug at all from component classes. Just take a moment in BP_Component::setup_globals() to get it out of $bp->pages.
2) Use the value out of $bp->pages as a default

I think that we can safely go with (1), but IMO the least we can do is (2). (FWIW, I think that (2) can - and should - be slipped in for BP 1.5, while there is not enough time for (1).)

Thoughts?

[boonebgorges]

3578-automatic.patch implements my idea (1), where we take the override option away from BP_Component extension classes (if they really need to change it - for what reason, I can't imagine - they can use the filter).

3578-defaults.patch takes the gentler route of providing a default value that can be overridden.

[johnjamesjacoby]

There are components that don't have root_slugs that never have them set. Friends, Messages, and Settings are good examples. I'd like to keep the root_slug argument around if/when we move components into custom post types, or if the directory-to-page idea doesn't work in practice.

The other reason was to keep decision making up to the individual component loaders and leave the lower level processing to the BP_Component class. In this instance it wasn't about DRY as much as developer understanding, using the BP core components as examples of how to use it.

The goal is ease of use and clarity of understanding. If you think it's too confusing now, I think 3578-defaults.patch is the best approach. Keep in mind this opens the door into moving each of the assignments towards having defaults in BP_Component::setup_globals() which I'd also like to avoid.

Point being, if the class isn't used 'correctly' is it responsible for making assumptions or does it do exactly what it's told.

[boonebgorges]

My "automatic" idea would tasse care of rootslugless components by assigning them an empty string. This is what's already happening anyway, just at the individual component level.

As for the broader question, I'm curious as to why you don't want to provide defaults for these values. It seems to me that the point of BP_Component is to make it as ready as possible to write new components. If we can help plugin authors by making intelligent guesses about values that they will never (or hardly ever) change, why shouldn't we?

[boonebgorges]

(Which is to say that I don't really find it *confusing*. I just want to make it as seamless as possible for others. Either way I don't feel ask that strongly about it, I just want to understand the philosophy behind providing a base class that requires more manual work by plugins)

[boonebgorges]

I still think we should be doing more to help plugin authors make good default decisions about this kind of stuff, but I'm willing to wait to continue the argument :)

[sooskriszta]

Have you gotten any feedback on this? Your option 1 seems quite reasonable to me, and I can't think of a reason why the patch (after testing) shouldn't be merged into 1.9

[boonebgorges]

At this point, I don't think we can do option 1 safely, because there may be plugins in the wild that are passing custom 'root_slug' parameters. Going with the 'automatic' patch would break them.

However, the 'defaults' patch would be a seamless addition. And, on further reflection, it's actually quite similar to what we already do with 'slug': we provide a default value ($this->id) that will make sense for most use cases. So I'm going to add it for 1.9 and call this ticket fixed.

[boonebgorges]

In 7465:

Default to the component directory root_slug in BP_Component::setup_globals()

This change will make BP_Component extensions a bit easier to build, as it will
no longer be necessary to fish the component root_slug out of bp_pages
manually (in the majority of standard cases).

Fixes #3578