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
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.
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:
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.
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.