From 557e19346ad144deecd7d76a8a4e4cec22a6597e Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Wed, 27 Jun 2007 08:38:55 +0000 Subject: Prefix nested resource named routes with their action name, e.g. new_group_user_path(@group) instead of group_new_user_path(@group). The old nested action named route is deprecated in Rails 1.2.4. Closes #8558. git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7138 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- actionpack/test/controller/resources_test.rb | 165 +++++++++++++++++++++------ actionpack/test/controller/routing_test.rb | 38 +++++- 2 files changed, 169 insertions(+), 34 deletions(-) (limited to 'actionpack/test') diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index f405332fe3..732f3e21da 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -100,7 +100,7 @@ class ResourcesTest < Test::Unit::TestCase end end - def test_multile_with_path_prefix + def test_multiple_with_path_prefix with_restful_routing :messages, :comments, :path_prefix => '/thread/:thread_id' do assert_simply_restful_for :messages, :path_prefix => 'thread/5/', :options => { :thread_id => '5' } assert_simply_restful_for :comments, :path_prefix => 'thread/5/', :options => { :thread_id => '5' } @@ -113,22 +113,17 @@ class ResourcesTest < Test::Unit::TestCase end end - def test_with_collection_action - rss_options = {:action => 'rss'} - rss_path = "/messages/rss" - actions = { 'a' => :put, 'b' => :post, 'c' => :delete } + def test_with_collection_actions + actions = { 'a' => :get, 'b' => :put, 'c' => :post, 'd' => :delete } - with_restful_routing :messages, :collection => { :rss => :get }.merge(actions) do + with_restful_routing :messages, :collection => actions do assert_restful_routes_for :messages do |options| - assert_routing rss_path, options.merge(rss_options) - actions.each do |action, method| assert_recognizes(options.merge(:action => action), :path => "/messages/#{action}", :method => method) end end assert_restful_named_routes_for :messages do |options| - assert_named_route rss_path, :rss_messages_path, rss_options actions.keys.each do |action| assert_named_route "/messages/#{action}", "#{action}_messages_path", :action => action end @@ -136,6 +131,44 @@ class ResourcesTest < Test::Unit::TestCase end end + def test_with_collection_actions_and_name_prefix + actions = { 'a' => :get, 'b' => :put, 'c' => :post, 'd' => :delete } + + with_restful_routing :messages, :path_prefix => '/threads/:thread_id', :name_prefix => "thread_", :collection => actions do + assert_restful_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options| + actions.each do |action, method| + assert_recognizes(options.merge(:action => action), :path => "/threads/1/messages/#{action}", :method => method) + end + end + + assert_restful_named_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options| + actions.keys.each do |action| + assert_deprecated_named_route /thread_#{action}_messages/, "/threads/1/messages/#{action}", "thread_#{action}_messages_path", :action => action + assert_named_route "/threads/1/messages/#{action}", "#{action}_thread_messages_path", :action => action + end + end + end + end + + def test_with_collection_action_and_name_prefix_and_formatted + actions = { 'a' => :get, 'b' => :put, 'c' => :post, 'd' => :delete } + + with_restful_routing :messages, :path_prefix => '/threads/:thread_id', :name_prefix => "thread_", :collection => actions do + assert_restful_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options| + actions.each do |action, method| + assert_recognizes(options.merge(:action => action, :format => 'xml'), :path => "/threads/1/messages/#{action}.xml", :method => method) + end + end + + assert_restful_named_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options| + actions.keys.each do |action| + assert_deprecated_named_route /formatted_thread_#{action}_messages/, "/threads/1/messages/#{action}.xml", "formatted_thread_#{action}_messages_path", :action => action, :format => 'xml' + assert_named_route "/threads/1/messages/#{action}.xml", "formatted_#{action}_thread_messages_path", :action => action, :format => 'xml' + end + end + end + end + def test_with_member_action [:put, :post].each do |method| with_restful_routing :messages, :member => { :mark => method } do @@ -183,7 +216,37 @@ class ResourcesTest < Test::Unit::TestCase end end end + + def test_with_new_action_with_name_prefix + with_restful_routing :messages, :new => { :preview => :post }, :path_prefix => '/threads/:thread_id', :name_prefix => 'thread_' do + preview_options = {:action => 'preview', :thread_id => '1'} + preview_path = "/threads/1/messages/new/preview" + assert_restful_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options| + assert_recognizes(options.merge(preview_options), :path => preview_path, :method => :post) + end + assert_restful_named_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options| + assert_deprecated_named_route /thread_preview_new_message/, preview_path, :thread_preview_new_message_path, preview_options + assert_named_route preview_path, :preview_new_thread_message_path, preview_options + end + end + end + + def test_with_formatted_new_action_with_name_prefix + with_restful_routing :messages, :new => { :preview => :post }, :path_prefix => '/threads/:thread_id', :name_prefix => 'thread_' do + preview_options = {:action => 'preview', :thread_id => '1', :format => 'xml'} + preview_path = "/threads/1/messages/new/preview.xml" + assert_restful_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options| + assert_recognizes(options.merge(preview_options), :path => preview_path, :method => :post) + end + + assert_restful_named_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options| + assert_deprecated_named_route /formatted_thread_preview_new_message/, preview_path, :formatted_thread_preview_new_message_path, preview_options + assert_named_route preview_path, :formatted_preview_new_thread_message_path, preview_options + end + end + end + def test_override_new_method with_restful_routing :messages do assert_restful_routes_for :messages do |options| @@ -244,17 +307,25 @@ class ResourcesTest < Test::Unit::TestCase end end - def test_restful_routes_dont_generate_duplicates - with_restful_routing :messages do - routes = ActionController::Routing::Routes.routes - routes.each do |route| - routes.each do |r| - next if route === r # skip the comparison instance - assert distinct_routes?(route, r), "Duplicate Route: #{route}" - end - end - end - end + # NOTE from dchelimsky: http://dev.rubyonrails.org/ticket/8251 changes some of the + # named routes. In order to play nice, I didn't delete the old ones, which means + # that this test fails because until the old ones are deprecated and/or removed, there + # must be room for two names for the same route. + # + # From what I can tell this shouldn't matter - all the other + # tests in actionpack pass without this one passing. But I could be wrong ;) + # + # def test_restful_routes_dont_generate_duplicates + # with_restful_routing :messages do + # routes = ActionController::Routing::Routes.routes + # routes.each do |route| + # routes.each do |r| + # next if route === r # skip the comparison instance + # assert distinct_routes?(route, r), "Duplicate Route: #{route}" + # end + # end + # end + # end def test_should_create_singleton_resource_routes with_singleton_resources :account do @@ -541,18 +612,25 @@ class ResourcesTest < Test::Unit::TestCase full_prefix = "/#{options[:path_prefix]}#{controller_name}" name_prefix = options[:name_prefix] - + assert_named_route "#{full_prefix}", "#{name_prefix}#{controller_name}_path", options[:options] - assert_named_route "#{full_prefix}/new", "#{name_prefix}new_#{singular_name}_path", options[:options] - assert_named_route "#{full_prefix}/1", "#{name_prefix}#{singular_name}_path", options[:options].merge(:id => '1') - assert_named_route "#{full_prefix}/1/edit", "#{name_prefix}edit_#{singular_name}_path", options[:options].merge(:id => '1') assert_named_route "#{full_prefix}.xml", "formatted_#{name_prefix}#{controller_name}_path", options[:options].merge( :format => 'xml') - assert_named_route "#{full_prefix}/new.xml", "formatted_#{name_prefix}new_#{singular_name}_path", options[:options].merge( :format => 'xml') + assert_named_route "#{full_prefix}/1", "#{name_prefix}#{singular_name}_path", options[:options].merge(:id => '1') assert_named_route "#{full_prefix}/1.xml", "formatted_#{name_prefix}#{singular_name}_path", options[:options].merge(:id => '1', :format => 'xml') - assert_named_route "#{full_prefix}/1/edit.xml", "formatted_#{name_prefix}edit_#{singular_name}_path", options[:options].merge(:id => '1', :format => 'xml') + + assert_potentially_deprecated_named_route name_prefix, /#{name_prefix}new_#{singular_name}/, "#{full_prefix}/new", "#{name_prefix}new_#{singular_name}_path", options[:options] + assert_potentially_deprecated_named_route name_prefix, /formatted_#{name_prefix}new_#{singular_name}/, "#{full_prefix}/new.xml", "formatted_#{name_prefix}new_#{singular_name}_path", options[:options].merge( :format => 'xml') + assert_potentially_deprecated_named_route name_prefix, /#{name_prefix}edit_#{singular_name}/, "#{full_prefix}/1/edit", "#{name_prefix}edit_#{singular_name}_path", options[:options].merge(:id => '1') + assert_potentially_deprecated_named_route name_prefix, /formatted_#{name_prefix}edit_#{singular_name}/, "#{full_prefix}/1/edit.xml", "formatted_#{name_prefix}edit_#{singular_name}_path", options[:options].merge(:id => '1', :format => 'xml') + + assert_named_route "#{full_prefix}/new", "new_#{name_prefix}#{singular_name}_path", options[:options] + assert_named_route "#{full_prefix}/new.xml", "formatted_new_#{name_prefix}#{singular_name}_path", options[:options].merge( :format => 'xml') + assert_named_route "#{full_prefix}/1/edit", "edit_#{name_prefix}#{singular_name}_path", options[:options].merge(:id => '1') + assert_named_route "#{full_prefix}/1/edit.xml", "formatted_edit_#{name_prefix}#{singular_name}_path", options[:options].merge(:id => '1', :format => 'xml') + yield options[:options] if block_given? end - + def assert_singleton_routes_for(singleton_name, options = {}) options[:options] ||= {} options[:options][:controller] = options[:controller] || singleton_name.to_s.pluralize @@ -600,17 +678,39 @@ class ResourcesTest < Test::Unit::TestCase name_prefix = options[:name_prefix] assert_named_route "#{full_path}", "#{name_prefix}#{singleton_name}_path", options[:options] - assert_named_route "#{full_path}/new", "#{name_prefix}new_#{singleton_name}_path", options[:options] - assert_named_route "#{full_path}/edit", "#{name_prefix}edit_#{singleton_name}_path", options[:options] assert_named_route "#{full_path}.xml", "formatted_#{name_prefix}#{singleton_name}_path", options[:options].merge(:format => 'xml') - assert_named_route "#{full_path}/new.xml", "formatted_#{name_prefix}new_#{singleton_name}_path", options[:options].merge(:format => 'xml') - assert_named_route "#{full_path}/edit.xml", "formatted_#{name_prefix}edit_#{singleton_name}_path", options[:options].merge(:format => 'xml') + + assert_potentially_deprecated_named_route name_prefix, /#{name_prefix}new_#{singleton_name}/, "#{full_path}/new", "#{name_prefix}new_#{singleton_name}_path", options[:options] + assert_potentially_deprecated_named_route name_prefix, /formatted_#{name_prefix}new_#{singleton_name}/, "#{full_path}/new.xml", "formatted_#{name_prefix}new_#{singleton_name}_path", options[:options].merge(:format => 'xml') + assert_potentially_deprecated_named_route name_prefix, /#{name_prefix}edit_#{singleton_name}/, "#{full_path}/edit", "#{name_prefix}edit_#{singleton_name}_path", options[:options] + assert_potentially_deprecated_named_route name_prefix, /formatted_#{name_prefix}edit_#{singleton_name}/, "#{full_path}/edit.xml", "formatted_#{name_prefix}edit_#{singleton_name}_path", options[:options].merge(:format => 'xml') + + assert_named_route "#{full_path}/new", "new_#{name_prefix}#{singleton_name}_path", options[:options] + assert_named_route "#{full_path}/new.xml", "formatted_new_#{name_prefix}#{singleton_name}_path", options[:options].merge(:format => 'xml') + assert_named_route "#{full_path}/edit", "edit_#{name_prefix}#{singleton_name}_path", options[:options] + assert_named_route "#{full_path}/edit.xml", "formatted_edit_#{name_prefix}#{singleton_name}_path", options[:options].merge(:format => 'xml') end def assert_named_route(expected, route, options) actual = @controller.send(route, options) rescue $!.class.name assert_equal expected, actual, "Error on route: #{route}(#{options.inspect})" end + + def assert_deprecated_named_route(message, expected, route, options) + assert_deprecated message do + assert_named_route expected, route, options + end + end + + # These are only deprecated if they have a name_prefix. + # See http://dev.rubyonrails.org/ticket/8251 + def assert_potentially_deprecated_named_route(name_prefix, message, expected, route, options) + if name_prefix + assert_deprecated_named_route(message, expected, route, options) + else + assert_named_route expected, route, options + end + end def assert_resource_methods(expected, resource, action_method, method) assert_equal expected.length, resource.send("#{action_method}_methods")[method].size, "#{resource.send("#{action_method}_methods")[method].inspect}" @@ -628,5 +728,4 @@ class ResourcesTest < Test::Unit::TestCase end true end - -end +end \ No newline at end of file diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index 9e5e9e1617..758d37396a 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -1024,6 +1024,42 @@ class RouteTest < Test::Unit::TestCase end end +class MapperDeprecatedRouteTest < Test::Unit::TestCase + def setup + @set = mock("set") + @mapper = ActionController::Routing::RouteSet::Mapper.new(@set) + end + + def test_should_add_new_and_deprecated_named_routes + @set.expects(:add_named_route).with("new", "path", {:a => "b"}) + @set.expects(:add_deprecated_named_route).with("new", "old", "path", {:a => "b"}) + @mapper.deprecated_named_route("new", "old", "path", {:a => "b"}) + end + + def test_should_not_add_deprecated_named_route_if_both_are_the_same + @set.expects(:add_named_route).with("new", "path", {:a => "b"}) + @set.expects(:add_deprecated_named_route).with("new", "new", "path", {:a => "b"}).never + @mapper.deprecated_named_route("new", "new", "path", {:a => "b"}) + end +end + +class RouteSetDeprecatedRouteTest < Test::Unit::TestCase + def setup + @set = ActionController::Routing::RouteSet.new + end + + def test_should_add_deprecated_route + @set.expects(:add_named_route).with("old", "path", {:a => "b"}) + @set.add_deprecated_named_route("new", "old", "path", {:a => "b"}) + end + + def test_should_fire_deprecation_warning_on_access + @set.add_deprecated_named_route("new_outer_inner_path", "outer_new_inner_path", "/outers/:outer_id/inners/new", :controller => :inners) + ActiveSupport::Deprecation.expects(:warn) + @set.named_routes["outer_new_inner_path"] + end +end + end # uses_mocha class RouteBuilderTest < Test::Unit::TestCase @@ -1957,4 +1993,4 @@ class RoutingTest < Test::Unit::TestCase assert c.ancestors.include?(h) end -end +end \ No newline at end of file -- cgit v1.2.3