Permalink
Browse files

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.

(cherry picked from commit 462d7cb)

Conflicts:
	actionpack/CHANGELOG.md
	actionpack/test/dispatch/routing_test.rb
  • Loading branch information...
pixeltrix committed Feb 9, 2014
1 parent 9337f64 commit ce671be9a15a644a661bbb50d02e31f9c8e83752
View
@@ -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*
+
* Do not discard query parameters that form a hash with the same root key as
the `wrapper_key` for a request using `wrap_parameters`.
@@ -697,6 +697,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))
@@ -1359,7 +1363,7 @@ def namespace(path, options = {})
end
def shallow
- scope(:shallow => true, :shallow_path => @scope[:path]) do
+ scope(:shallow => true) do
yield
end
end
@@ -1479,6 +1483,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
@@ -1876,6 +1876,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
@@ -2869,6 +2928,22 @@ def test_multiple_positional_args_with_the_same_name
assert_equal '/downloads/1/1.tar', download_path('1', '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)

0 comments on commit ce671be

Please sign in to comment.