Move implicit nested call before options handling so that nested constraints work [#5513 state:resolved]

Signed-off-by: José Valim <jose.valim@gmail.com>

2 files changed, 25 insertions, 9 deletions

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index a3bd4771c2..a2570cb877 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -909,6 +909,11 @@ module ActionDispatch
               return true
             end
 
+            if resource_scope?
+              nested { send(method, resources.pop, options, &block) }
+              return true
+            end
+
             options.keys.each do |k|
               (options[:constraints] ||= {})[k] = options.delete(k) if options[k].is_a?(Regexp)
             end
@@ -925,13 +930,6 @@ module ActionDispatch
               options.merge!(scope_action_options) if scope_action_options?
             end
 
-            if resource_scope?
-              nested do
-                send(method, resources.pop, options, &block)
-              end
-              return true
-            end
-
             false
           end
 
@@ -1017,11 +1015,11 @@ module ActionDispatch
           end
 
           def id_constraint?
-            @scope[:id].is_a?(Regexp) || (@scope[:constraints] && @scope[:constraints][:id].is_a?(Regexp))
+            @scope[:constraints] && @scope[:constraints][:id].is_a?(Regexp)
           end
 
           def id_constraint
-            @scope[:id] || @scope[:constraints][:id]
+            @scope[:constraints][:id]
           end
 
           def canonical_action?(action, flag)
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index 0eee63e94f..c90c1041ed 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -448,6 +448,10 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
         :filename => /(.+)/,
         :as => :purchase
 
+      resources :lists, :id => /([A-Za-z0-9]{25})|default/ do
+        resources :todos, :id => /\d+/
+      end
+
       scope '/countries/:country', :constraints => lambda { |params, req| %[all France].include?(params[:country]) } do
         match '/',       :to => 'countries#index'
         match '/cities', :to => 'countries#cities'
@@ -2110,6 +2114,20 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     assert_equal '/purchases/315004be7e/Ruby_on_Rails_3.pdf', purchase_path(:token => '315004be7e', :filename => 'Ruby_on_Rails_3.pdf')
   end
 
+  def test_nested_resource_constraints
+    get '/lists/01234012340123401234fffff'
+    assert_equal 'lists#show', @response.body
+    assert_equal '/lists/01234012340123401234fffff', list_path(:id => '01234012340123401234fffff')
+
+    get '/lists/01234012340123401234fffff/todos/1'
+    assert_equal 'todos#show', @response.body
+    assert_equal '/lists/01234012340123401234fffff/todos/1', list_todo_path(:list_id => '01234012340123401234fffff', :id => '1')
+
+    get '/lists/2/todos/1'
+    assert_equal 'Not Found', @response.body
+    assert_raises(ActionController::RoutingError){ list_todo_path(:list_id => '2', :id => '1') }
+  end
+
 private
   def with_test_routes
     yield