Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fix map.resources to always generate named routes if they're needed

Signed-off-by: Michael Koziarski <michael@koziarski.com>
  • Loading branch information...
commit 4c0921024471c0463d67f8b8fb6a115a94d343aa 1 parent 57d795b
@tomstuart tomstuart authored NZKoz committed
View
13 actionpack/lib/action_controller/resources.rb
@@ -597,11 +597,11 @@ def map_default_collection_actions(map, resource)
end
map_resource_routes(map, resource, :index, resource.path, index_route_name)
- map_resource_routes(map, resource, :create, resource.path)
+ map_resource_routes(map, resource, :create, resource.path, index_route_name)
end
def map_default_singleton_actions(map, resource)
- map_resource_routes(map, resource, :create, resource.path)
+ map_resource_routes(map, resource, :create, resource.path, "#{resource.shallow_name_prefix}#{resource.singular}")
end
def map_new_actions(map, resource)
@@ -632,9 +632,10 @@ def map_member_actions(map, resource)
end
end
- map_resource_routes(map, resource, :show, resource.member_path, "#{resource.shallow_name_prefix}#{resource.singular}")
- map_resource_routes(map, resource, :update, resource.member_path)
- map_resource_routes(map, resource, :destroy, resource.member_path)
+ route_path = "#{resource.shallow_name_prefix}#{resource.singular}"
+ map_resource_routes(map, resource, :show, resource.member_path, route_path)
+ map_resource_routes(map, resource, :update, resource.member_path, route_path)
+ map_resource_routes(map, resource, :destroy, resource.member_path, route_path)
end
def map_resource_routes(map, resource, action, route_path, route_name = nil, method = nil)
@@ -642,7 +643,7 @@ def map_resource_routes(map, resource, action, route_path, route_name = nil, met
action_options = action_options_for(action, resource, method)
formatted_route_path = "#{route_path}.:format"
- if route_name
+ if route_name && @set.named_routes[route_name.to_sym].nil?

This causes issues with singleton resources whereby the named resource route refers to the create action and not the show action, there is a fix in LH #1400

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
map.named_route(route_name, route_path, action_options)
map.named_route("formatted_#{route_name}", formatted_route_path, action_options)
else
View
78 actionpack/test/controller/resources_test.rb
@@ -822,6 +822,84 @@ def test_singleton_resource_does_not_have_destroy_action
end
end
+ def test_resource_has_only_create_action_and_named_route
+ with_routing do |set|
+ set.draw do |map|
+ map.resources :products, :only => :create
+ end
+
+ assert_resource_allowed_routes('products', {}, { :id => '1' }, :create, [:index, :new, :show, :edit, :update, :destroy])
+ assert_resource_allowed_routes('products', { :format => 'xml' }, { :id => '1' }, :create, [:index, :new, :show, :edit, :update, :destroy])
+
+ assert_not_nil set.named_routes[:products]
+ end
+ end
+
+ def test_resource_has_only_update_action_and_named_route
+ with_routing do |set|
+ set.draw do |map|
+ map.resources :products, :only => :update
+ end
+
+ assert_resource_allowed_routes('products', {}, { :id => '1' }, :update, [:index, :new, :create, :show, :edit, :destroy])
+ assert_resource_allowed_routes('products', { :format => 'xml' }, { :id => '1' }, :update, [:index, :new, :create, :show, :edit, :destroy])
+
+ assert_not_nil set.named_routes[:product]
+ end
+ end
+
+ def test_resource_has_only_destroy_action_and_named_route
+ with_routing do |set|
+ set.draw do |map|
+ map.resources :products, :only => :destroy
+ end
+
+ assert_resource_allowed_routes('products', {}, { :id => '1' }, :destroy, [:index, :new, :create, :show, :edit, :update])
+ assert_resource_allowed_routes('products', { :format => 'xml' }, { :id => '1' }, :destroy, [:index, :new, :create, :show, :edit, :update])
+
+ assert_not_nil set.named_routes[:product]
+ end
+ end
+
+ def test_singleton_resource_has_only_create_action_and_named_route
+ with_routing do |set|
+ set.draw do |map|
+ map.resource :account, :only => :create
+ end
+
+ assert_singleton_resource_allowed_routes('accounts', {}, :create, [:new, :show, :edit, :update, :destroy])
+ assert_singleton_resource_allowed_routes('accounts', { :format => 'xml' }, :create, [:new, :show, :edit, :update, :destroy])
+
+ assert_not_nil set.named_routes[:account]
+ end
+ end
+
+ def test_singleton_resource_has_only_update_action_and_named_route
+ with_routing do |set|
+ set.draw do |map|
+ map.resource :account, :only => :update
+ end
+
+ assert_singleton_resource_allowed_routes('accounts', {}, :update, [:new, :create, :show, :edit, :destroy])
+ assert_singleton_resource_allowed_routes('accounts', { :format => 'xml' }, :update, [:new, :create, :show, :edit, :destroy])
+
+ assert_not_nil set.named_routes[:account]
+ end
+ end
+
+ def test_singleton_resource_has_only_destroy_action_and_named_route
+ with_routing do |set|
+ set.draw do |map|
+ map.resource :account, :only => :destroy
+ end
+
+ assert_singleton_resource_allowed_routes('accounts', {}, :destroy, [:new, :create, :show, :edit, :update])
+ assert_singleton_resource_allowed_routes('accounts', { :format => 'xml' }, :destroy, [:new, :create, :show, :edit, :update])
+
+ assert_not_nil set.named_routes[:account]
+ end
+ end
+
def test_resource_has_only_collection_action
with_routing do |set|
set.draw do |map|

1 comment on commit 4c09210

@NZKoz
Owner

Thanks geoff, applied.

Please sign in to comment.
Something went wrong with that request. Please try again.