Issues

ZF-9823: Zend_Rest_Route#assemble does not properly build new|edit urls

Issue Type: Bug Created: 2010-05-10T14:13:05.000+0000 Last Updated: 2010-11-21T02:07:23.000+0000 Status: Resolved Fix version(s): - 1.11.1 (30/Nov/10)

Reporter: Ross Tuck (rosstuck) Assignee: Wil Moore III (wilmoore) (wilmoore) Tags: - Zend_Rest_Route

Related issues: - ZF-10117

Attachments:

Description

The assemble() method in Zend_Rest_Route can not create a url for the "new" action. Instead, it provides a link to the index action ("/products" instead of "/products/new"). This is not the same as the match() method which makes allowances for the "index" and "new" actions.

This may also be the root issue behind ZF-7753. Included patch is against trunk.

The fix is small, but it may create a (very small) BC-break/bug if the $params['index'] is also set as this also tries to generate a url for the index action. That said, the $params['index'] behavior may be a bug that should instead become $params['action] == 'index'. Please let me know if I should file this under another ticket.

Comments

Posted by Ross Tuck (rosstuck) on 2010-05-10T14:15:57.000+0000

Added reproduction script and sample patch.

Posted by Wil Moore III (wilmoore) (wilmoore) on 2010-07-07T00:23:44.000+0000

I have verified this as an issue. A co-worker reported this issue to me yesterday and after a quick search, I found this issue.

Unit tests verifying the issue are attached.

Posted by Wil Moore III (wilmoore) (wilmoore) on 2010-07-07T00:33:53.000+0000

Changed to major as not being able to consistently build urls is likely a deal-breaker for many users wanting to use this component.

Posted by Wil Moore III (wilmoore) (wilmoore) on 2010-07-07T12:53:31.000+0000

>> BC-break/bug if the $params['index'] is also set as this also tries to generate a url for the index action.

There is no problem here because 'index' is checked for as a key (see example):

$this->url(array('id' => 1, 'key' => 'value', 'action' => 'edit', 'index' => true));

The TRUE constant is only needed so that some positive (evaluates to true) value is given. The Zend_Rest_Route#assemble method will find 'index' to be set and it's value evaluates to true (so 1 or '1' or true would work equally well as it doesn't check for === TRUE). The 'index' value is this removed from the parameters array and the remaining parameters (id, key) are added to the resulting URL string.

'action' has already been removed from the parameters array by this point so it will not accidentally be added to the string.

Having 'index' => true also negates having 'action' => 'edit' or 'action' => 'new'. In other words, if 'index' => true, then 'action' is ignored.

Posted by Wil Moore III (wilmoore) (wilmoore) on 2010-07-07T12:55:56.000+0000

FYI, removed old diffs after getting the original poster's permission to do so.

Posted by Ross Tuck (rosstuck) on 2010-07-07T13:34:05.000+0000

The BC-break only occured (as I recall) with the now removed patches. Your newer patch fixes a few more things and does not suffer from the same problem. All good. :)

However, my main question about the index key was exactly what you point out. Why does it search for "index" => true, and not "action" => "index" like all other routes do? If I pass in array("action" => "edit", "index" => true), I expect it not to ignore action but instead treat index as a named parameter.

Posted by Wil Moore III (wilmoore) (wilmoore) on 2010-07-07T13:45:52.000+0000

If I pass in array("action" => "edit", "index" => true), I expect it not to ignore action but instead treat index as a named parameter.

I see what you mean. Good point. So, if you did this:

$this->url(array('id' => 1, 'action' => 'index', 'another' => 'parameter'));

would you want it to generate this:

/index/1/another/parameter

or this:

/index/id/1/another/parameter

or something completely different?

Posted by Ross Tuck (rosstuck) on 2010-07-07T13:53:12.000+0000

Personally I would prefer the former. ID is a special param in Zend_Rest_Route, it makes sense for it to remain invisible.

Posted by Wil Moore III (wilmoore) (wilmoore) on 2010-07-08T00:44:18.000+0000

Diff updated to include three new unit tests which are now passing.

Allows:

/users/new

/users/1/edit

/users/index/1/another/parameter

Posted by Wil Moore III (wilmoore) (wilmoore) on 2010-08-16T12:08:44.000+0000

Assigning this issue to myself. If I don't hear anything negative about the patch within the next few days, I'm going to commit it so it gets into the next release.

Posted by Grayson Koonce (merrix) on 2010-10-30T19:05:12.000+0000

did you commit this patch? im having this issue with 1.10.8 still.

Posted by Wil Moore III (wilmoore) (wilmoore) on 2010-11-21T01:44:41.000+0000

fixed in revision 23420.

Posted by Wil Moore III (wilmoore) (wilmoore) on 2010-11-21T02:05:34.000+0000

Merged r23420 to release branch 1.11.

Have you found an issue?

See the Overview section for more details.

Copyright

© 2006-2016 by Zend, a Rogue Wave Company. Made with by awesome contributors.

This website is built using zend-expressive and it runs on PHP 7.

Contacts