Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

New router DSL doesn't propagate requirements to nested resources

#5208

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?

Reported by Wincent Colaiuta · July 27th, 2010 @ 08:32 AM

State: resolved
Milestone: 3.0.2
Assigned to: Andrew White Andrew White
Importance: High

Activity

  1. Wincent Colaiuta
    Wincent Colaiuta

    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>
    

    July 27th, 2010 @ 04:52 PM

  2. Andrew White
    Andrew White
    • 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
    

    July 27th, 2010 @ 08:52 PM

  3. Wincent Colaiuta
    Wincent Colaiuta

    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.

    July 27th, 2010 @ 10:12 PM

  4. Andrew White
    Andrew White
    • 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.

    August 18th, 2010 @ 12:23 PM

  5. Andrew White
    Andrew White
    • Assigned user changed from Andrew White to José Valim

    August 19th, 2010 @ 02:44 PM

  6. Repository
    Repository
    • 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...

    August 19th, 2010 @ 07:08 PM

  7. Repository
    Repository

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

    August 19th, 2010 @ 07:10 PM

  8. Wincent Colaiuta
    Wincent Colaiuta
    • 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.

    August 19th, 2010 @ 08:03 PM

  9. Wincent Colaiuta
    Wincent Colaiuta

    (Bah, stupid Lighthouse... yet another unintended user assignment change...)

    August 19th, 2010 @ 08:04 PM

  10. Jeremy Kemper