Nested restful routes broken in edge?

Indeed, that's the change. It makes :id required for member routes. Otherwise, you can generate member URLs with :id => nil which comes out as a collection URL instead! And you can recognize PUT /collections as a member update action with nil id instead of a PUT on the collection itself.

What is the specific nested resource breakage? Please reopen the ticket with a failing test case if possible.

jeremy

In the new.rhtml for the comments form, they have

<% form_for :comment, :url => comment_url, :article_id => @article_id do >form> %>

What?? That should be comments_url. It's a bug that comment_url ever worked. That's what Jeremy was fixing. In your case, comments_url(@article_id). comment_url requires 2 params, like comment_url(@article_id, @comment_id).

Ok, I flipped to the rest section in AWDWR2, and the only new form example I saw was for the article form. They used articles_path for hte url, which is correct.

In the new.rhtml for the comments form, they have

<% form_for :comment, :url => comment_url, :article_id => @article_id do >form> %>

There are 2 reasons why this is incorrect. comment_url a :id parameter. So, use comments_url (or comments_path). However, since it's nested, it requires an article id. It looks like you tried to pass that, but instead it's being passed as a generic form_for argument and probably quietly being ignored in some dark hole somewhere.

The correct answer to this question is comments_url(@article_id), or comments_url(:article_id => @article_id).

comment_url failed to generate from {:article_id=>"1", :action=>"show", :controller=>"comments"}, expected: {:action=>"show", :controller=>"comments"}, diff: {:article_id=>"1"}

Still wrong, use comments_url. comment_url is for a single post.

No, that was expected. Allowing a singular route without an id was never supposed to work. It just encouraged bad habits like this :slight_smile: He added the restriction so you know right away that it's wrong. Sorry you got caught in the crossfire. He did merge it to 1.2, so rails 1.2.2 will behave this way.

Based on our discussion, I would think that both (1) and (2) would fail. Am I still off base here, or is this somehow slipping through the cracks and working even though it shouldn't?

Yup, that's what Jeremy was fixing. His fix even exposed some testing bugs in Mephisto. We all get affected, but I think it makes are code better in the long run.