From 462d7cb3148e95c9a793d33fd882a99f0d9c57c2 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Sun, 9 Feb 2014 10:36:45 -0800 Subject: [PATCH] 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 | 8 ++ .../lib/action_dispatch/routing/mapper.rb | 13 +++- actionpack/test/dispatch/routing_test.rb | 75 +++++++++++++++++++ 3 files changed, 95 insertions(+), 1 deletion(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 642b847588..15541d58b5 100644 --- a/actionpack/CHANGELOG.md +++ b/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`. diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index d5eb770cb1..0b762aa9a4 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/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 diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 26821bdb56..1fa2cc6cf2 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/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)