Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Zend\Navigation\Page\Mvc::getHref does not use RouteMatch parameters #4095

Conversation

adamlundrigan
Copy link
Contributor

The MVC page type in Navigation does not pull matched parameters from the provided RouteMatch object, which results in an error when using the breadcrumbs view helper:

Fatal error: Zend\Mvc\Router\Exception\InvalidArgumentException: Missing parameter "school" in <zf>/library/Zend/View/Helper/Navigation/AbstractHelper.php on line 466

My route hierarchy:

'router' => array(
    'routes' => array(
        'registration' => array(
            'type' => 'Literal',
            'options' => array(
                'route' => '/registration',
                'defaults' => array(
                    'controller' => 'CdliPortal\Controller\Registration\Dashboard',
                    'action' => 'index',
                ),
            ),
            'may_terminate' => true,
            'child_routes' => array(
                'school' => array(
                    'type' => 'Segment',
                    'options' => array(
                        'route' => '/school/:school',   
                        'constraints' => array(
                            'school' => '[0-9]+',
                        ),
                        'defaults' => array(
                            'controller' => 'CdliPortal\Controller\Registration\School\Dashboard',
                            'action'     => 'index',
                        ),
                    ),
                ),
            ),
        ),
    ),
),

The simplest fix i've found is to change Zend\Navigation\Page\Mvc::getHref to use the parameters from the RouteMatch object (if they are set) as the defaults and then merge the parameters set directly on the Zend\Navigation\Page\Mvc into that:

if ($this->getRouteMatch() !== null) {
    $params = array_merge($this->getRouteMatch()->getParams(), $this->getParams());
} else {
    $params = $this->getParams();
}

Though I am not 100% certain this is a comprehensive fix, or that it doesn't break / cause unexpected results with the other navigation view helpers (my application only uses bredcrumbs currently)

@ghost ghost assigned weierophinney Mar 28, 2013
weierophinney added a commit that referenced this pull request Mar 28, 2013
…utematch-parameters

Zend\Navigation\Page\Mvc::getHref does not use RouteMatch parameters
weierophinney added a commit that referenced this pull request Mar 28, 2013
@tranba
Copy link

tranba commented Apr 20, 2013

@adamlundrigan
@weierophinney
I'm consider this pull request has cause unexpected results with the menu view helpers. For example, with the Album module:

//module.config.php:
'router' => array(
        'routes' => array(
            'album' => array(
                'type'    => 'segment',
                'options' => array(
                    'route'    => '/album[/:action][/:id]',
                    'constraints' => array(
                        'action' => '[a-zA-Z][a-zA-Z0-9_-]*',
                        'id'     => '[0-9]+',
                    ),
                    'defaults' => array(
                        'controller' => 'Album\Controller\Album',
                        'action'     => 'index',
                    ),
                ),
            ),
        ),
    ),
//navigation:
'navigation' => array(
    'default' => array(
        array(
            'label' => 'Home',
            'route' => 'home',
        ),
        array(
            'label' => 'Album',
            'route' => 'album',
            'pages' => array(
                array(
                    'label' => 'Add',
                    'route' => 'album',
                    'action' => 'add',
                ),
                array(
                    'label' => 'Edit',
                    'route' => 'album',
                    'action' => 'edit',
                ),
                array(
                    'label' => 'Delete',
                    'route' => 'album',
                    'action' => 'delete',
                ),
            ),
        ),
    ),
),

//in view script:
<?php echo $this->navigation('navigation')->menu(); ?>

In normal:
- album menu link to: http://localhost/album
- add menu link to: http://localhost/album/add
- edit menu will link to: http://localhost/album/edit
-delete menu will link to: http://localhost/album/delete
But if we edit one album -> url: http://localhost/album/edit/1,
album menu will link to: http://localhost/album/edit/1
add menu will link to: http://localhost/album/add/1
edit menu will link to: http://localhost/album/edit/1
and delete menu will link to: http://localhost/album/delete/1

Sorry for my English!

@Slamdunk
Copy link
Contributor

@weierophinney : this PR introduce a big issue in navigation handling.

Navigation URLs must not pull params from active page, otherwise every navigation page with undefined params takes current page params, that is definitively wrong.

However @adamlundrigan is likely also right: current page breadcrumb has to have current page params.

I suggest to:

  1. restore default behaviour of Zend\Navigation\Page\Mvc
  2. create a specific flag into Zend\Navigation\Page\Mvc to pull current params on-the-fly
  3. use the switch in Zend\View\Helper\Navigation\Breadcrumbs and wherever it's needed

This way involves some test deletion and some new code in Zend\View\Helper\Navigation\*: I am not so deep into \Navigation stuff to make such a sensitive PR, so please jump here and speak out your thoughts.

A big note: at the time of writing, _release 2.1.5 Zend\View\Helper\Navigation\Menu is unusable_

@weierophinney
Copy link
Member

@Slamdunk Does #4282 fix this issue? (I think it likely does)

@weierophinney weierophinney reopened this Apr 23, 2013
@weierophinney weierophinney merged commit caf8e58 into zendframework:master Apr 23, 2013
weierophinney added a commit to zendframework/zend-navigation that referenced this pull request May 15, 2015
…n/feature/navigation-mvc-routematch-parameters

Zend\Navigation\Page\Mvc::getHref does not use RouteMatch parameters
weierophinney added a commit to zendframework/zend-navigation that referenced this pull request May 15, 2015
weierophinney added a commit to zendframework/zend-navigation that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants