fix form_for example in ActionController::Resources documentation. Closes #7362 [gnarg], Added enhanced docs to routing assertions. Closes #7359 [Rob Sanheim], improve error message for Routing for named routes. Closes #7346 [Rob Sanheim]

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

6 files changed, 77 insertions, 8 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index e4e8a444e6..6481bda79c 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,11 @@
 *SVN*
 
+* improve error message for Routing for named routes.  Closes #7346 [Rob Sanheim]
+
+* Added enhanced docs to routing assertions.  Closes #7359 [Rob Sanheim]
+
+* fix form_for example in ActionController::Resources documentation.  Closes #7362 [gnarg]
+
 * Make sure that the string returned by TextHelper#truncate is actually a string, not a char proxy -- that should only be used internally while working on a multibyte-safe way of truncating [DHH]
 
 * Added FormBuilder#submit as a delegate for FormTagHelper#submit_tag [DHH]
diff --git a/actionpack/lib/action_controller/assertions/routing_assertions.rb b/actionpack/lib/action_controller/assertions/routing_assertions.rb
index 54a6dfc5a7..11a649c49a 100644
--- a/actionpack/lib/action_controller/assertions/routing_assertions.rb
+++ b/actionpack/lib/action_controller/assertions/routing_assertions.rb
@@ -3,13 +3,23 @@ module ActionController
     module RoutingAssertions
       # Asserts that the routing of the given path was handled correctly and that the parsed options match.
       #
-      #   assert_recognizes({:controller => 'items', :action => 'index'}, 'items')
+      #   assert_recognizes({:controller => 'items', :action => 'index'}, 'items') # check the default action
+      #   assert_recognizes({:controller => 'items', :action => 'list'}, 'items/list') # check a specific action
+      #   assert_recognizes({:controller => 'items', :action => 'list', :id => '1'}, 'items/list/1') # check an action with a parameter
       #
       # Pass a hash in the second argument to specify the request method.  This is useful for routes
-      # requiring a specific method.
+      # requiring a specific HTTP method.  The hash should contain a :path with the incoming request path
+      # and a :method containing the required HTTP verb.
       #
+      #   # assert that POSTing to /items will call the create action on ItemsController
       #   assert_recognizes({:controller => 'items', :action => 'create'}, {:path => 'items', :method => :post})
       #
+      # You can also pass in "extras" with a hash containing URL parameters that would normally be in the query string.  This can be used
+      # to assert that values in the query string string will end up in the params hash correctly.  To test query strings you must use the
+      # extras argument, appending the query string on the path directly will not work.  For example: 
+      #
+      #   # assert that a path of '/items/list/1?view=print' returns the correct options
+      #   assert_recognizes({:controller => 'items', :action => 'list', :id => '1', :view => 'print'}, 'items/list/1', { :view => "print" }) 
       def assert_recognizes(expected_options, path, extras={}, message=nil)
         if path.is_a? Hash
           request_method = path[:method]
@@ -33,7 +43,12 @@ module ActionController
         end
       end
 
-      # Asserts that the provided options can be used to generate the provided path.
+      # Asserts that the provided options can be used to generate the provided path.  This is the inverse of assert_recognizes.
+      # For example:
+      #
+      #   assert_generates("/items", :controller => "items", :action => "index")
+      #   assert_generates("/items/list", :controller => "items", :action => "list")
+      #   assert_generates("/items/list/1", { :controller => "items", :action => "list", :id => "1" }) 
       def assert_generates(expected_path, options, defaults={}, extras = {}, message=nil)
         clean_backtrace do 
           expected_path = "/#{expected_path}" unless expected_path[0] == ?/
@@ -53,7 +68,8 @@ module ActionController
       end
 
       # Asserts that path and options match both ways; in other words, the URL generated from 
-      # options is the same as path, and also that the options recognized from path are the same as options
+      # options is the same as path, and also that the options recognized from path are the same as options.  This
+      # essentially combines assert_recognizes and assert_generates into one step.
       def assert_routing(path, options, defaults={}, extras={}, message=nil)
         assert_recognizes(options, path, extras, message)
         
diff --git a/actionpack/lib/action_controller/resources.rb b/actionpack/lib/action_controller/resources.rb
index 5e15bc8f3a..d55e7f02ce 100644
--- a/actionpack/lib/action_controller/resources.rb
+++ b/actionpack/lib/action_controller/resources.rb
@@ -158,7 +158,7 @@ module ActionController
     #   
     # or 
     #   
-    #   <% form_for :message, @message, message_path(@message), :html => {:method => :put} do |f| %>
+    #   <% form_for :message, @message, :url => message_path(@message), :html => {:method => :put} do |f| %>
     # 
     # The #resources method accepts various options, too, to customize the resulting
     # routes:
diff --git a/actionpack/lib/action_controller/routing.rb b/actionpack/lib/action_controller/routing.rb
index 5b888443d8..58207eed74 100644
--- a/actionpack/lib/action_controller/routing.rb
+++ b/actionpack/lib/action_controller/routing.rb
@@ -1240,8 +1240,11 @@ module ActionController
 
         if named_route
           path = named_route.generate(options, merged, expire_on)
-          raise RoutingError, "#{named_route_name}_url failed to generate from #{options.inspect}, expected: #{named_route.requirements.inspect}, diff: #{named_route.requirements.diff(options).inspect}" if path.nil?
-          return path
+          if path.nil? 
+            raise_named_route_error(options, named_route, named_route_name)
+          else
+            return path
+          end
         else
           merged[:action] ||= 'index'
           options[:action] ||= 'index'
@@ -1269,6 +1272,18 @@ module ActionController
     
         raise RoutingError, "No route matches #{options.inspect}"
       end
+      
+      # try to give a helpful error message when named route generation fails
+      def raise_named_route_error(options, named_route, named_route_name)
+        diff = named_route.requirements.diff(options)
+        unless diff.empty?
+          raise RoutingError, "#{named_route_name}_url failed to generate from #{options.inspect}, expected: #{named_route.requirements.inspect}, diff: #{named_route.requirements.diff(options).inspect}"
+        else
+          required_segments = named_route.segments.select {|seg| (!seg.optional?) && (!seg.is_a?(DividerSegment)) }
+          required_keys_or_values = required_segments.map { |seg| seg.key rescue seg.value } # we want either the key or the value from the segment
+          raise RoutingError, "#{named_route_name}_url failed to generate from #{options.inspect} - you may have ambiguous routes, or you may need to supply additional parameters for this route.  content_url has the following required parameters: #{required_keys_or_values.inspect} - are they all satisifed?"
+        end
+      end
   
       def recognize(request)
         params = recognize_path(request.path, extract_request_environment(request))
diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb
index 0c8628fbf7..f81d0393f0 100644
--- a/actionpack/test/controller/routing_test.rb
+++ b/actionpack/test/controller/routing_test.rb
@@ -242,7 +242,39 @@ class LegacyRouteSetTests < Test::Unit::TestCase
       map.connect ':controller/:action/:id'
     end
   end
+  
+  def test_should_list_options_diff_when_routing_requirements_dont_match
+    rs.draw do |map|
+      map.post 'post/:id', :controller=> 'post', :action=> 'show', :requirements => {:id => /\d+/}
+    end
+    exception = assert_raise(ActionController::RoutingError) { rs.generate(:controller => 'post', :action => 'show', :bad_param => "foo", :use_route => "post") }
+    assert_match /^post_url failed to generate/, exception.message
+    from_match = exception.message.match(/from \{[^\}]+\}/).to_s
+    assert_match /:bad_param=>"foo"/,   from_match
+    assert_match /:action=>"show"/,     from_match
+    assert_match /:controller=>"post"/, from_match
+    
+    expected_match = exception.message.match(/expected: \{[^\}]+\}/).to_s
+    assert_no_match /:bad_param=>"foo"/,   expected_match
+    assert_match    /:action=>"show"/,     expected_match
+    assert_match    /:controller=>"post"/, expected_match
 
+    diff_match = exception.message.match(/diff: \{[^\}]+\}/).to_s
+    assert_match    /:bad_param=>"foo"/,   diff_match
+    assert_no_match /:action=>"show"/,     diff_match
+    assert_no_match /:controller=>"post"/, diff_match
+  end
+
+  # this specifies the case where your formerly would get a very confusing error message with an empty diff
+  def test_should_have_better_error_message_when_options_diff_is_empty
+    rs.draw do |map|
+      map.content '/content/:query', :controller => 'content', :action => 'show'
+    end
+    exception = assert_raise(ActionController::RoutingError) { rs.generate(:controller => 'content', :action => 'show', :use_route => "content") }
+    expected_message = %[content_url failed to generate from {:action=>"show", :controller=>"content"} - you may have ambiguous routes, or you may need to supply additional parameters for this route.  content_url has the following required parameters: ["content", :query] - are they all satisifed?]
+    assert_equal expected_message, exception.message
+  end
+  
   def test_dynamic_path_allowed
     rs.draw do |map|
       map.connect '*path', :controller => 'content', :action => 'show_file'
diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb
index 88dc8aa1cd..c5e7505a36 100644
--- a/actionpack/test/template/form_helper_test.rb
+++ b/actionpack/test/template/form_helper_test.rb
@@ -239,7 +239,7 @@ class FormHelperTest < Test::Unit::TestCase
       _erbout.concat f.text_field(:title)
       _erbout.concat f.text_area(:body)
       _erbout.concat f.check_box(:secret)
-      _erbout.concat f.submit 'Create post'
+      _erbout.concat f.submit('Create post')
     end
 
     expected =