Issues

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

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

Added reproduction script and sample patch.

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.

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

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

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

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.

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?

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

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

Allows:

/users/new

/users/1/edit

/users/index/1/another/parameter

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.

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

fixed in revision 23420.

Merged r23420 to release branch 1.11.