A change made by bitsweat in edge apparently breaks nested routing.
AFAICT, the breakage is related to
http://dev.rubyonrails.org/ticket/7229. From what I can tell, a formerly
optional parameter is now required, but I can’t seem to determine how to
change my url references to make things work. Has anyone else seen this?
Is this a bug, and if not, how do you modify usage to make things work
again?
BTW, just as a check, I ran the restful2 example from AWDWRv2, and it
indeed breaks with the latest edge.
TIA,
Keith
On 1/29/07, Keith L. [email protected] wrote:
Keith L. wrote:
http://dev.rubyonrails.org/ticket/7229.
At least, I think that this is the change… backing off to version
6061 in edge fixes the problem, and the changes for this ticket are all
in the routes section…
Keith
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).
–
Rick O.
http://weblog.techno-weenie.net
http://mephistoblog.com
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).
–
Rick O.
http://weblog.techno-weenie.net
http://mephistoblog.com
Jeremy K. wrote:
On 1/29/07, Keith L. [email protected] wrote:
What is the specific nested resource breakage? Please reopen the
ticket with a failing test case if possible.
jeremy
Jeremy,
Having a bit of trouble getting trac registration, but in the mean
time…
The example in AWDWRv2 exhibits the behaviour. Its route setup is
ActionController::Routing::Routes.draw do |map|
map.resources :articles do |article|
article.resources :comments
end
end
In the new.rhtml for the comments form, they have
<% form_for :comment, :url => comment_url, :article_id => @article_id do
|form| %>
As written, this throws the following error:
comment_url failed to generate from {:action=>“show”,
:controller=>“comments”}, expected: {:action=>“show”,
:controller=>“comments”}, diff: {}
Changing the form line to :url => comment_url(@article.id) still results
in an error. I experimented and found that :url => comment_url(:id =>
@article_id) fixes the problem.
I assume this is not what was in mind with the fix, was it?
Keith
Rick O. wrote:
The correct answer to this question is comments_url(@article_id), or
comments_url(:article_id => @article_id).
–
Rick O.
http://weblog.techno-weenie.net
http://mephistoblog.com
Rick,
This is not my code - its from the book. The code snippit I used to
illustrate the issue was copied directly from the downloadable version
of the example in the book - the specific form is not actually included
in the chapter. If you note in my post above, I tried your solution and
still received the error —
comment_url failed to generate from {:article_id=>“1”, :action=>“show”,
:controller=>“comments”}, expected: {:action=>“show”,
:controller=>“comments”}, diff: {:article_id=>“1”}
Keith
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.
–
Rick O.
http://weblog.techno-weenie.net
http://mephistoblog.com
Rick O. wrote:
Still wrong, use comments_url. comment_url is for a single post.
Oops. Missed the plural.
Ok, so with Jeremy’s change, the example code (which worked previously)
will no longer work. I will post something to PragProg to get them to
change it. Jeremy’s change was to force the id to be required. Was it a
side effect in some way of that change that also caught the
singular/plural issue?
Keith
Keith L. wrote:
Rick O. wrote:
Still wrong, use comments_url. comment_url is for a single post.
Oops. Missed the plural.
Uh, nevermind. Got it. Will still post something to Prag about the
example code.
Thanks for the help,
Keith
On 1/29/07, Keith L. [email protected] wrote:
change it. Jeremy’s change was to force the id to be required. Was it a
side effect in some way of that change that also caught the
singular/plural issue?
No, that was expected. Allowing a singular route without an id was
never supposed to work. It just encouraged bad habits like this 
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.
–
Rick O.
http://weblog.techno-weenie.net
http://mephistoblog.com
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.
–
Rick O.
http://weblog.techno-weenie.net
http://mephistoblog.com
Rick O. wrote:
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.
Good, and agreed. I would note that my tests above were done on today’s
edge, so Jeremy’s fix may still have a couple of holes (since 1,2, and 3
all still work).
Keith
Rick O. wrote:
No, that was expected. Allowing a singular route without an id was
never supposed to work.
I played with the restful2 example a bit more using edge rails (with
Jeremy’s fix) and found some behaviour that I think will be confusing
(or, I am just still confused a bit).
In the example code from AWDWR2, the comment editing form looks like
this:
(1)
<% form_for :comment,
:url => comment_url,
:article_id => @article_id,
:html => {:method => :put} do |form| %>
This works just fine in edge rails. In fact, so does this:
(2)
<% form_for :comment,
:url => comment_url,
:html => {:method => :put} do |form| %>
And, so does this (which is the ‘correct’ way):
(3)
<% form_for :comment,
:url => comment_url(@article),
:html => {:method => :put} do |form| %>
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?
Keith