Make it easier to define URLs for Menu and remove an unnessary parameter #6140

Closed
tsteur opened this Issue Sep 6, 2014 · 0 comments

Projects

None yet

1 participant

@tsteur
Member
tsteur commented Sep 6, 2014

Already for a while I do not like the way we are defining the URL of a menu item as well as by the $displayForCurrentUser param of the MenuAbstract::add() method:

 $urlParams = array(
     'module' => 'Actions',
     'action' => 'menuGetPageUrls'
);
$menu->add('Menu', 'Submenu', $urlParams, $displayForCurrentUser = true, 15);

I just wanted to write a blog post about Controller + Menu and noticed this ones more so I am going to make this easier now. Therefore I will deprecate the add() method in favor of new addItem() method which no longer has the $displayForCurrentUser argument. The add() method will be removed in Piwik 3.0 I think. It is easy to stay backwards compatible here. Added a test for this.

One should use an if condition instead of the boolean parameter:

if (Piwik::isUserNotAnonymous()) {
   $menu->addItem('Menu', 'Submenu', $urlParams, 15);
}

To make the URL parameter easier I am going to introduce three new methods:

urlForDefaultAction($additionalParams = array())
urlForAction($controllerAction, $additionalParams = array())
urlForModuleAction($module, $controllerAction, $additionalParams = array())

The first two methods will detect the current module automatically so we remove a lot of duplicated code. The third method will make sure the module actually exist and is activated. The methods are also better readable than the array structure and allows us to change the generated array in the future without having to change the plugins.

Example:

$menu->addItem('Menu', 'Submenu', $this->urlForAction('menuGetPageUrls'), 15);

Note: A while ago I added also new methods to no longer having to know the translation keys of menu items so it is even shorter:

$menu->addManageItem('Title', $this->urlForAction('menuGetPageUrls'), 15);
$menu->addActionsItem('Title', $this->urlForAction('menuGetPageUrls'), 15);
...
@tsteur tsteur added a commit that referenced this issue Sep 6, 2014
@tsteur tsteur refs #6140 easier way to define URLs for menu items and introducing a…
… method to addItem without boolean parameter
eee6ed9
@tsteur tsteur added a commit that referenced this issue Sep 6, 2014
@tsteur tsteur refs #6140 forgot to replace this URL 10d8728
@tsteur tsteur added a commit that referenced this issue Sep 6, 2014
@tsteur tsteur refs #6140 fix links 68ed176
@tsteur tsteur added Enhancement Task and removed Enhancement labels Sep 6, 2014
@tsteur tsteur modified the milestone: Mid term, Piwik 2.7.0 Sep 6, 2014
@tsteur tsteur self-assigned this Sep 6, 2014
@tsteur tsteur closed this Sep 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment