Add possibility to define the menu action in the menu class

[tsteur]

For a widget you can define the method to render the actual widget within the Widget class. For a task you can define the method to execute the actual task within the Tasks class. But for a menu item you have to create a controller and action in addition and it is not possible to define the method within the class.

Because of having to stay backwards compatible it is not that easy to make the Menu class work similar to Tasks and Widgets. After #6140 made it easy to generate URL's for controller actions (urlForAction(), urlForModuleAction, ...) we could add a method like urlForMethod($nameOfMethodWithnMenuClass) that makes it possible to define the related method within the Menu class.

Example

public function configureUserMenu(MenuUser $menu)
{
    $menu->addPlatformItem('UI Notifications', $this->urlForMethod('notifications'), $order = 3);
}

public function notifications() 
{
    return new RssRender()->render(); // return string to be displayed in menu page
}

We might have to adjust the router slightly to resolve such URL's similar to widgets.

[diosmosis]

Maybe there can be a simple base 'ControllerComponent' class or interface that components (ie, Menu/Widgets/whatever) can extend to signify to the routing logic that they act as controllers. FrontController would look for an action in the first object it found in a plugin that extended ControllerComponent. Could do the same for APIs.

What do you think?

[tsteur]

We should probably not extend such a class as extending a class is the strongest form of coupling. Especially since we cannot extend multiple classes. Interface could be possible but don't think it is designed for this (would not require anything?) and the developer would have to know that he has to implement this interface in order to make a method "routable". Would be nice if it worked automatically. Next problem could be that different components could implement the same method (this problem can probably happen with any solution). Also some classes would maybe have multiple public methods but only should be "routable".

I was rather thinking like generating a prefix for menu items like "menuNotifications". We could detect the prefix in the router and then perform a check whether there is a public method notifications() in the menu and whether it was explicitly exposed via $menu->addItem(). If not we would fall back to the controller. It works similar for widgets and reports.

But now that you are saying this I agree it is not a universal solution that can be used for anything. On the other side it is quite fast since we do not have to search for many classes and methods that are "routable".

There could be otherwise a method which is in the beginning not really exposed as an API but used internally. So when someone is calling $menu->urlForAction($methodName) or $widget->addWidget($methodName) those methods could in the background call a method like Router::addRoute($module, $classname, $action = $method) or Router::addRoute($module, $instance, $action = $method). This code could maybe moved to a router as well and maybe simplified: https://github.com/piwik/piwik/blob/master/core/FrontController.php#L107
Note: This solution would be often much slower since we would have to create an instance of all menus, all reports, all controllers and all widgets to build all the possible routes. In case of for instance an API call we would not need those components and would make them much slower. Maybe there is a solution that would be fast again. I haven't thought in detail about this. For instance if we make sure that a plugin can only add routes to itself then we would only have to check the controller, reports, widget and menus of the plugin which would be like > 30 times faster.

If a plugin wants to overwrite the action of another plugin it would have to listen to an event which is ok as it does not happen too often. So it can be a bit more complex to do something like this.

[diosmosis]

I got a request from @czolnowski during the meetup for an annotation that would allow you to split an API into separate classes, something like:

/**
 * @piwikApi
 */
class MyOtherApi
{
...
}

Since we don't use annotations, inheritance (of some form, base or interface) appears to be the only alternative. I think classes that are meant to handle routes are controllers so the is-a relationship is appropriate in my eyes.

Regarding speed, caching can be used to speed it up if speed's a problem. Or when DI is put in, we can look through the objects created for the plugin being routed through (and since php-di implements caching, we shouldn't have to worry about speed).

That said, this isn't that important to me, so consider my comments a suggestion. I don't need or require this.

[diosmosis]

Had another idea that might be useful or lead to something workable: maybe there can be a Routes component that allows plugins to specify where actions are? This isn't a full solution, but maybe it will lead to one. Or not, whatever ;)