Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
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.

(cherry picked from commit dcc91a0)
  • Loading branch information...
commit 7de1a6819032ffff9694bb0d214dbc323f526c7f 1 parent 0919ecb
@pixeltrix pixeltrix authored
View
43 actionpack/lib/action_dispatch/routing/mapper.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
View
60 actionpack/test/dispatch/routing_test.rb
@@ -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)
Please sign in to comment.
Something went wrong with that request. Please try again.