Introduce query parameter for Navigation\Page\Mvc #4084

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

drdev commented Mar 20, 2013

Version 2.1.4 has deprecated Query routes, but not updated Navigation\Page\Mvc to conform new changes.

This patch introduces query parameter for Mvc pages and passes it accordingly into $router->assemble() method. Also uses fragment option to build URLs as introduced in 2.1.4.

drdev added some commits Mar 19, 2013

Pass `query` & `fragment` options to $route->assemble
Query parameter from the config below being passed to Page\Mvc is accounted now when assembling route

```php
    'participants/create'   => array(
        'label'     => 'Create',
        'route'     => 'admin/company',
        'params'    => array(
            'action' => 'edit',
            'id' => 'new',
            'query' => array(
                'a' => 'param1',
                'b' => 'param2',
            ),
        ),
        'fragment' => 'foo',
    ),
```
Contributor

drdev commented Mar 20, 2013

@DASPRiD, please review and comment

Member

froschdesign commented Mar 20, 2013

@hoochie-coochie
Please add unit tests.

Contributor

drdev commented Mar 20, 2013

@DASPRiD not sure if query part should be considered in Mvc::isActive method

library/Zend/Navigation/Page/Mvc.php
+ $options['fragment'] = $fragment;
+ }
+
+ if (($query = $this->getQuery()) != null) {
@DASPRiD

DASPRiD Mar 20, 2013

Member

Should probably be changed to if (null !== ($query = $this->getQuery()) { or as with the $fragment above

@drdev

drdev Mar 20, 2013

Contributor

In fact it's shameless copy-paste from the code above the line (see lines 195-201). I am with you on that, fixing

library/Zend/Navigation/Page/Mvc.php
+ public function setQuery($query)
+ {
+ $this->query = $query;
+ $this->hrefCache = null;
@DASPRiD

DASPRiD Mar 20, 2013

Member

Align assignment operators.

+ *
+ * @see getHref()
+ *
+ * @return array|string|null URL query part (as an array or string) or null
@DASPRiD

DASPRiD Mar 20, 2013

Member

Remove empty line between annotations.

library/Zend/Navigation/Page/Mvc.php
+ *
+ * @see getHref()
+ *
+ * @param array|string|null $query URL query part
@DASPRiD

DASPRiD Mar 20, 2013

Member

Remove empty line between annotations and align annotations properly.

@drdev

drdev Mar 20, 2013

Contributor

Different styles used in the file. I selected this one, as @see is not related to the function signature.

Where can I read guidelines for docblocks? Especially regarding aligning different parts of docblock.

Member

DASPRiD commented Mar 20, 2013

About your question; I wouldn't take the query into consideration in inActive().

weierophinney added a commit that referenced this pull request Mar 28, 2013

Merge pull request #4084 from hoochie-coochie/fix/pass-query-to-page-mvc
Introduce query parameter for Navigation\Page\Mvc

@ghost ghost assigned weierophinney Mar 28, 2013

weierophinney added a commit that referenced this pull request Mar 28, 2013

Owner

weierophinney commented Mar 28, 2013

Normally, new additions to the public API should be done against develop. However, since this PR is fixing a BC break, I'm applying it to master as well.

@hoochie-coochie Could you please update the documentation as well, detailing how to set the query parameters? Thanks!

Contributor

drdev commented Apr 12, 2013

@weierophinney I'll try, but promise me you'll fix my English ;)

Owner

weierophinney commented Apr 12, 2013

Absolutely!
On Apr 11, 2013 7:14 PM, "Alex Pogodin" notifications@github.com wrote:

@weierophinney https://github.com/weierophinney I'll try, but promise
me you'll fix my English ;)


Reply to this email directly or view it on GitHubhttps://github.com/zendframework/zf2/pull/4084#issuecomment-16269461
.

weierophinney added a commit to zendframework/zend-navigation that referenced this pull request May 15, 2015

Merge pull request zendframework/zendframework#4084 from hoochie-cooc…
…hie/fix/pass-query-to-page-mvc

Introduce query parameter for Navigation\Page\Mvc

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 join this conversation on GitHub. Already have an account? Sign in to comment