Set the :shallow_path as each scope is generated

If we set :shallow_path when shallow is called it can result in incorrect
paths if the resource is inside a namespace because namespace itself sets
the :shallow_path option to the namespace path.

We fix this by removing the :shallow_path option from shallow as that should
only be turning shallow routes on and not otherwise affecting the scope.
To do this we need to treat the :shallow option to resources differently to
other scope options and move it to before the nested block is called.

This change also has the positive side effect of making the behavior of the
:shallow option consistent with the shallow method.

Fixes #12498.

actionpack/CHANGELOG.md

@@ -1,3 +1,11 @@
+*   Set the `:shallow_path` scope option as each scope is generated rather than
+    waiting until the `shallow` option is set. Also make the behavior of the
+    `:shallow` resource option consistent with the behavior of the `shallow` method.
+
+    Fixes #12498.
+
+    *Andrew White*, *Aleksi Aalto*
+
 *   Properly require `action_view` in `AbstractController::Rendering` to prevent
     uninitialized constant error for `ENCODING_FLAG`.
 

actionpack/lib/action_dispatch/routing/mapper.rb

@@ -707,6 +707,10 @@ def scope(*args)
           options[:path] = args.flatten.join('/') if args.any?
           options[:constraints] ||= {}
 
+          unless shallow?
+            options[:shallow_path] = options[:path] if args.any?
+          end
+
           if options[:constraints].is_a?(Hash)
             defaults = options[:constraints].select do
               |k, v| URL_OPTIONS.include?(k) && (v.is_a?(String) || v.is_a?(Fixnum))
@@ -1369,7 +1373,7 @@ def namespace(path, options = {})
         end
 
         def shallow
-          scope(:shallow => true, :shallow_path => @scope[:path]) do
+          scope(:shallow => true) do
             yield
           end
         end
@@ -1490,6 +1494,13 @@ def apply_common_behavior_for(method, resources, options, &block) #:nodoc:
               return true
             end
 
+            if options.delete(:shallow)
+              shallow do
+                send(method, resources.pop, options, &block)
+              end
+              return true
+            end
+
             if resource_scope?
               nested { send(method, resources.pop, options, &block) }
               return true

actionpack/test/dispatch/routing_test.rb

@@ -1889,6 +1889,65 @@ def test_shallow_nested_resources_within_scope
     assert_equal 'notes#destroy', @response.body
   end
 
+  def test_shallow_option_nested_resources_within_scope
+    draw do
+      scope '/hello' do
+        resources :notes, :shallow => true do
+          resources :trackbacks
+        end
+      end
+    end
+
+    get '/hello/notes/1/trackbacks'
+    assert_equal 'trackbacks#index', @response.body
+    assert_equal '/hello/notes/1/trackbacks', note_trackbacks_path(:note_id => 1)
+
+    get '/hello/notes/1/edit'
+    assert_equal 'notes#edit', @response.body
+    assert_equal '/hello/notes/1/edit', edit_note_path(:id => '1')
+
+    get '/hello/notes/1/trackbacks/new'
+    assert_equal 'trackbacks#new', @response.body
+    assert_equal '/hello/notes/1/trackbacks/new', new_note_trackback_path(:note_id => 1)
+
+    get '/hello/trackbacks/1'
+    assert_equal 'trackbacks#show', @response.body
+    assert_equal '/hello/trackbacks/1', trackback_path(:id => '1')
+
+    get '/hello/trackbacks/1/edit'
+    assert_equal 'trackbacks#edit', @response.body
+    assert_equal '/hello/trackbacks/1/edit', edit_trackback_path(:id => '1')
+
+    put '/hello/trackbacks/1'
+    assert_equal 'trackbacks#update', @response.body
+
+    post '/hello/notes/1/trackbacks'
+    assert_equal 'trackbacks#create', @response.body
+
+    delete '/hello/trackbacks/1'
+    assert_equal 'trackbacks#destroy', @response.body
+
+    get '/hello/notes'
+    assert_equal 'notes#index', @response.body
+
+    post '/hello/notes'
+    assert_equal 'notes#create', @response.body
+
+    get '/hello/notes/new'
+    assert_equal 'notes#new', @response.body
+    assert_equal '/hello/notes/new', new_note_path
+
+    get '/hello/notes/1'
+    assert_equal 'notes#show', @response.body
+    assert_equal '/hello/notes/1', note_path(:id => 1)
+
+    put '/hello/notes/1'
+    assert_equal 'notes#update', @response.body
+
+    delete '/hello/notes/1'
+    assert_equal 'notes#destroy', @response.body
+  end
+
   def test_custom_resource_routes_are_scoped
     draw do
       resources :customers do
@@ -2958,6 +3017,22 @@ def test_resource_routes_with_dashes_in_path
     assert_equal '/photos/1', photo_path('1')
   end
 
+  def test_shallow_path_inside_namespace_is_not_added_twice
+    draw do
+      namespace :admin do
+        shallow do
+          resources :posts do
+            resources :comments
+          end
+        end
+      end
+    end
+
+    get '/admin/posts/1/comments'
+    assert_equal 'admin/comments#index', @response.body
+    assert_equal '/admin/posts/1/comments', admin_post_comments_path('1')
+  end
+
 private
 
   def draw(&block)