This project is archived and is in readonly mode.
New router DSL doesn't propagate requirements to nested resources
Reported by Wincent Colaiuta | July 27th, 2010 @ 08:32 AM | in 3.0.2
Given a top-level resource like this:
resources :articles, :id => /[^\/]+/ , :path => 'wiki'
In other words, an ArticlesController
, available at
"/wiki", and with pretty much anything except slash allowed in the
:id
so that we can have wiki article URLs like
"/wiki/Rails_3.0_update_notes" (ie. without the period being
treated as a format separator).
Now let's add a nested resource. In beta 4 this worked:
resources :articles, :id => /[^\/]+/ , :path => 'wiki' do
resources :comments, :only => [ :create, :new ]
end
In the RC, any route involving a period in the article name yields a "No route matches" exception. e.g. "/wiki/some_article/comments/new" works fine, but "/wiki/Rails_3.0_update_notes/comments/new" raises the exception.
The workaround is to explicitly repeat the requirements in the nested resource like this:
resources :articles, :id => /[^\/]+/ , :path => 'wiki' do
resources :comments, :article_id => /[^\/]+/, :only => [ :create, :new ]
end
Wondering if this is an unintended regression? Back under Rails 2.3 and the old router DSL I also found that I had to repeat the requirements on the nested resource in order to prevent the period from being interpreted as a format separator:
map.resources :articles,
:as => :wiki,
:requirements => { :id => /[^\/]+/ } do |articles|
articles.resources :comments, :requirements => { :article_id => /[^\/]+/ }
end
When I switched to Rails 3 I noticed that I no longer needed to explicitly propagate those requirements, but now under the RC, I do again. So is this an unintended regression?
Comments and changes to this ticket
-
Wincent Colaiuta July 27th, 2010 @ 04:52 PM
Ok, according to
git bisect
this commit is the one that introduced the regression:commit c6843e23373c626ae49ad9fa253d4f7538d3434f Author: Andrew White <andyw@pixeltrix.co.uk> Date: Sun Jul 4 06:52:52 2010 +0100 Refactor resource options and scoping. Resource classes are now only responsible for controlling how they are named. All other options passed to resources are pushed out to the scope. Signed-off-by: José Valim <jose.valim@gmail.com>
-
Andrew White July 27th, 2010 @ 08:52 PM
- Milestone cleared.
- State changed from new to open
- Assigned user set to Andrew White
- Importance changed from to High
What's happening is that the id regexp is getting pulled into the options hash in the scope rather than the constraints hash. I'm guessing that the same will happen for the defaults hash as well. We probably need to scan the options hash passed to the resources call and extract appropriately before pushing them into the scope.
I can't get to it before the weekend as I've a major deadline to meet - if it can wait till then I'll work up a patch. However in the meantime here's a couple of nicer workarounds:
resources :articles, :constraints => { :id => /[^\/]+/ } , :path => 'wiki' do resources :comments, :only => [ :create, :new ] end
constraints(:id => /[^\/]+/) do resources :articles, :path => 'wiki' do resources :comments, :only => [ :create, :new ] end end
-
Wincent Colaiuta July 27th, 2010 @ 10:12 PM
Sure it can wait, especially seeing as there are workarounds. Thanks for the analysis. I don't really feel like I know the router well enough to be of much help in coding this up, but I can at least try to come up with a failing test case for it.
-
Andrew White August 18th, 2010 @ 12:23 PM
- Tag set to rails 3.0, constraints, patch, resources, routing
Sorry for the delay, but here's a patch that fixes the problem. Turns out that defaults work okay, it's just regexps that need moving to the constraints hash. Patch applies cleanly to both master and stable.
-
Andrew White August 19th, 2010 @ 02:44 PM
- Assigned user changed from Andrew White to José Valim
-
Repository August 19th, 2010 @ 07:08 PM
- State changed from open to resolved
(from [c019db8ca1c5639fdae80915cc7520eaad7dcd65]) Move regexps in options hash to :constraints hash so that they are pushed into the scope [#5208 state:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/c019db8ca1c5639fdae80915cc7520... -
Repository August 19th, 2010 @ 07:10 PM
(from [6b52a58f726955d5a245b2a9b0b6c5120e3f31d6]) Move regexps in options hash to :constraints hash so that they are pushed into the scope [#5208 state:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/6b52a58f726955d5a245b2a9b0b6c5... -
Wincent Colaiuta August 19th, 2010 @ 08:03 PM
- Assigned user changed from José Valim to Andrew White
Just tried it out and can confirm that this fixes the issue I originally reported. Thanks a lot Andrew.
-
Wincent Colaiuta August 19th, 2010 @ 08:04 PM
(Bah, stupid Lighthouse... yet another unintended user assignment change...)
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Referenced by
- 5208 New router DSL doesn't propagate requirements to nested resources (from [c019db8ca1c5639fdae80915cc7520eaad7dcd65]) Move re...
- 5208 New router DSL doesn't propagate requirements to nested resources (from [6b52a58f726955d5a245b2a9b0b6c5120e3f31d6]) Move re...
- 5994 routing and url_for doesn't accept dots in the custom parameters https://rails.lighthouseapp.com/projects/8994/tickets/53...