Fixed the new_#{resource}_url route and added named route tests for Simply Restful. [Rick Olson]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4638 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 65 insertions, 14 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index 40ff1a5939..f45051314f 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Fixed the new_#{resource}_url route and added named route tests for Simply Restful.  [Rick Olson]
+
 * Added map.resources from the Simply Restful plugin [DHH]. Examples (the API has changed to use plurals!):
 
     map.resources :messages
diff --git a/actionpack/lib/action_controller/resources.rb b/actionpack/lib/action_controller/resources.rb
index fb27759004..b41cdf16e9 100644
--- a/actionpack/lib/action_controller/resources.rb
+++ b/actionpack/lib/action_controller/resources.rb
@@ -136,7 +136,7 @@ module ActionController
           route_options = requirements_for(method)
           actions.each do |action|
             path = action == :new ? resource.new_path : "#{resource.new_path};#{action}"
-            name = "new_#{resource.plural}"
+            name = "new_#{resource.singular}"
             name = "#{action}_#{name}" unless action == :new
 
             map.named_route("#{resource.name_prefix}#{name}", path, route_options.merge(:action => action.to_s))
diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb
index ff90f8d742..054c251d59 100644
--- a/actionpack/test/controller/resources_test.rb
+++ b/actionpack/test/controller/resources_test.rb
@@ -1,6 +1,7 @@
 require File.dirname(__FILE__) + '/../abstract_unit'
 
 class MessagesController < ActionController::Base
+  def index() render :nothing => true end
   def rescue_action(e) raise e end
 end
 
@@ -11,22 +12,26 @@ end
 class ResourcesTest < Test::Unit::TestCase
   def test_default_restful_routes
     with_restful_routing :messages do
-      assert_restful_routes_for :messages do
-        routing_options = {:controller => '/messages'}
-      end
+      assert_simply_restful_for :messages
     end
   end
 
   def test_with_path_prefix
     with_restful_routing :messages, :path_prefix => '/thread/:thread_id' do
-      assert_restful_routes_for :messages, :path_prefix => 'thread/5/', :options => { :thread_id => '5' }
+      assert_simply_restful_for :messages, :path_prefix => 'thread/5/', :options => { :thread_id => '5' }
     end
   end
 
   def test_with_collection_action
     with_restful_routing :messages, :collection => { :rss => :get } do
+      rss_options = {:action => 'rss'}
+      rss_path    = "/messages;rss"
       assert_restful_routes_for :messages do |options|
-        assert_routing "/messages;rss", options.merge(:action => 'rss')
+        assert_routing rss_path, options.merge(rss_options)
+      end
+      
+      assert_restful_named_routes_for :messages do |options|
+        assert_named_route rss_path, :rss_messages_path, rss_options
       end
     end
   end
@@ -34,10 +39,14 @@ class ResourcesTest < Test::Unit::TestCase
   def test_with_member_action
     [:put, :post].each do |method|
       with_restful_routing :messages, :member => { :mark => method } do
+        mark_options = {:action => 'mark', :id => '1'}
+        mark_path    = "/messages/1;mark"
         assert_restful_routes_for :messages do |options|
-          assert_recognizes(
-            options.merge(:action => 'mark', :id => '1'), 
-            {:path => "/messages/1;mark", :method => method})
+          assert_recognizes(options.merge(mark_options), :path => mark_path, :method => method)
+        end
+        
+        assert_restful_named_routes_for :messages do |options|
+          assert_named_route mark_path, :mark_message_path, mark_options
         end
       end
     end
@@ -45,10 +54,14 @@ class ResourcesTest < Test::Unit::TestCase
 
   def test_with_new_action
     with_restful_routing :messages, :new => { :preview => :post } do
+      preview_options = {:action => 'preview'}
+      preview_path    = "/messages/new;preview"
       assert_restful_routes_for :messages do |options|
-        assert_recognizes(
-          options.merge(:action => 'preview'), 
-          {:path => "/messages/new;preview", :method => :post})
+        assert_recognizes(options.merge(preview_options), :path => preview_path, :method => :post)
+      end
+      
+      assert_restful_named_routes_for :messages do |options|
+        assert_named_route preview_path, :preview_new_message_path, preview_options
       end
     end
   end
@@ -79,7 +92,13 @@ class ResourcesTest < Test::Unit::TestCase
         yield
       end
     end
-    
+
+    # runs assert_restful_routes_for and assert_restful_named_routes for on the controller_name and options, without passing a block.
+    def assert_simply_restful_for(controller_name, options = {})
+      assert_restful_routes_for       controller_name, options
+      assert_restful_named_routes_for controller_name, options
+    end
+
     def assert_restful_routes_for(controller_name, options = {})
       (options[:options] ||= {})[:controller] = controller_name.to_s
 
@@ -103,7 +122,37 @@ class ResourcesTest < Test::Unit::TestCase
       assert_recognizes(
         options[:options].merge(:action => 'destroy', :id => '1'),
         {:path => "/#{options[:path_prefix]}#{controller_name}/1", :method => :delete})
-      
+
+      yield options[:options] if block_given?
+    end
+    
+    # test named routes like foo_path and foos_path map to the correct options.
+    def assert_restful_named_routes_for(controller_name, singular_name = nil, options = {})
+      if singular_name.is_a?(Hash)
+        options       = singular_name
+        singular_name = nil
+      end
+      singular_name ||= controller_name.to_s.singularize
+      (options[:options] ||= {})[:controller] = controller_name.to_s
+      @controller = "#{controller_name.to_s.camelize}Controller".constantize.new
+      @request    = ActionController::TestRequest.new
+      @response   = ActionController::TestResponse.new
+      get :index, options[:options]
+      options[:options].delete :action
+
+      full_prefix = "/#{options[:path_prefix]}#{controller_name}"
+
+      assert_named_route "#{full_prefix}",        "#{controller_name}_path",           options[:options]
+      assert_named_route "#{full_prefix}.xml",    "formatted_#{controller_name}_path", options[:options].merge(:format => 'xml')
+      assert_named_route "#{full_prefix}/new",    "new_#{singular_name}_path",         options[:options]
+      assert_named_route "#{full_prefix}/1",      "#{singular_name}_path",             options[:options].merge(:id => '1')
+      assert_named_route "#{full_prefix}/1;edit", "edit_#{singular_name}_path",        options[:options].merge(:id => '1')
+      assert_named_route "#{full_prefix}/1.xml",  "formatted_#{singular_name}_path",   options[:options].merge(:format => 'xml', :id => '1')
       yield options[:options] if block_given?
     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
 end