Permalink
Browse files

Only use shallow nested scope when depth is > 1

By tracking the depth of resource nesting we can push the need for nested
shallow scoping to only those routes that are nested more than one deep.
This allows us to keep the fix for #12498 and fix the regression in #14224.

Fixes #14224.
  • Loading branch information...
1 parent ed0fb4a commit dcc91a04a177466516dac12abc358c590743fa71 @pixeltrix pixeltrix committed Mar 2, 2014
Showing with 83 additions and 20 deletions.
  1. +23 −20 actionpack/lib/action_dispatch/routing/mapper.rb
  2. +60 −0 actionpack/test/dispatch/routing_test.rb
@@ -1323,8 +1323,10 @@ def member
end
with_scope_level(:member) do
- scope(parent_resource.member_scope) do
- yield
+ if shallow?
+ shallow_scope(parent_resource.member_scope) { yield }
+ else
+ scope(parent_resource.member_scope) { yield }
end
end
end
@@ -1347,16 +1349,8 @@ def nested
end
with_scope_level(:nested) do
- if shallow?
- with_exclusive_scope do
- if @scope[:shallow_path].blank?
- scope(parent_resource.nested_scope, nested_options) { yield }
- else
- scope(@scope[:shallow_path], :as => @scope[:shallow_prefix]) do
- scope(parent_resource.nested_scope, nested_options) { yield }
- end
- end
- end
+ if shallow? && nesting_depth > 1
+ shallow_scope(parent_resource.nested_scope, nested_options) { yield }
else
scope(parent_resource.nested_scope, nested_options) { yield }
end
@@ -1567,11 +1561,13 @@ def with_scope_level(kind)
def resource_scope(kind, resource) #:nodoc:
old_resource, @scope[:scope_level_resource] = @scope[:scope_level_resource], resource
+ @nesting.push(resource)
with_scope_level(kind) do
scope(parent_resource.resource_scope) { yield }
end
ensure
+ @nesting.pop
@scope[:scope_level_resource] = old_resource
end
@@ -1584,6 +1580,10 @@ def nested_options #:nodoc:
options
end
+ def nesting_depth #:nodoc:
+ @nesting.size
+ end
+
def param_constraint? #:nodoc:
@scope[:constraints] && @scope[:constraints][parent_resource.param].is_a?(Regexp)
end
@@ -1596,18 +1596,20 @@ def canonical_action?(action, flag) #:nodoc:
flag && resource_method_scope? && CANONICAL_ACTIONS.include?(action.to_s)
end
- def shallow_scoping? #:nodoc:
- shallow? && @scope[:scope_level] == :member
+ def shallow_scope(path, options = {}) #:nodoc:
+ old_name_prefix, old_path = @scope[:as], @scope[:path]
+ @scope[:as], @scope[:path] = @scope[:shallow_prefix], @scope[:shallow_path]
+
+ scope(path, options) { yield }
+ ensure
+ @scope[:as], @scope[:path] = old_name_prefix, old_path
end
def path_for_action(action, path) #:nodoc:
- prefix = shallow_scoping? ?
- "#{@scope[:shallow_path]}/#{parent_resource.shallow_scope}" : @scope[:path]
-
if canonical_action?(action, path.blank?)
- prefix.to_s
+ @scope[:path].to_s
else
- "#{prefix}/#{action_path(action, path)}"
+ "#{@scope[:path]}/#{action_path(action, path)}"
end
end
@@ -1645,7 +1647,7 @@ def name_for_action(as, action) #:nodoc:
when :new
[prefix, :new, name_prefix, member_name]
when :member
- [prefix, shallow_scoping? ? @scope[:shallow_prefix] : name_prefix, member_name]
+ [prefix, name_prefix, member_name]
when :root
[name_prefix, collection_name, prefix]
else
@@ -1786,6 +1788,7 @@ def initialize(set) #:nodoc:
@set = set
@scope = { :path_names => @set.resources_path_names }
@concerns = {}
+ @nesting = []
end
include Base
@@ -3033,6 +3033,66 @@ def test_shallow_path_inside_namespace_is_not_added_twice
assert_equal '/admin/posts/1/comments', admin_post_comments_path('1')
end
+ def test_shallow_path_and_prefix_are_not_added_to_non_shallow_routes
+ draw do
+ scope shallow_path: 'projects', shallow_prefix: 'project' do
+ resources :projects do
+ resources :files, controller: 'project_files', shallow: true
+ end
+ end
+ end
+
+ get '/projects'
+ assert_equal 'projects#index', @response.body
+ assert_equal '/projects', projects_path
+
+ get '/projects/new'
+ assert_equal 'projects#new', @response.body
+ assert_equal '/projects/new', new_project_path
+
+ post '/projects'
+ assert_equal 'projects#create', @response.body
+
+ get '/projects/1'
+ assert_equal 'projects#show', @response.body
+ assert_equal '/projects/1', project_path('1')
+
+ get '/projects/1/edit'
+ assert_equal 'projects#edit', @response.body
+ assert_equal '/projects/1/edit', edit_project_path('1')
+
+ patch '/projects/1'
+ assert_equal 'projects#update', @response.body
+
+ delete '/projects/1'
+ assert_equal 'projects#destroy', @response.body
+
+ get '/projects/1/files'
+ assert_equal 'project_files#index', @response.body
+ assert_equal '/projects/1/files', project_files_path('1')
+
+ get '/projects/1/files/new'
+ assert_equal 'project_files#new', @response.body
+ assert_equal '/projects/1/files/new', new_project_file_path('1')
+
+ post '/projects/1/files'
+ assert_equal 'project_files#create', @response.body
+
+ get '/projects/files/2'
+ assert_equal 'project_files#show', @response.body
+ assert_equal '/projects/files/2', project_file_path('2')
+
+ get '/projects/files/2/edit'
+ assert_equal 'project_files#edit', @response.body
+ assert_equal '/projects/files/2/edit', edit_project_file_path('2')
+
+ patch '/projects/files/2'
+ assert_equal 'project_files#update', @response.body
+
+ delete '/projects/files/2'
+ assert_equal 'project_files#destroy', @response.body
+ end
+
private
def draw(&block)

0 comments on commit dcc91a0

Please sign in to comment.