Removed deprecated parameters_for_method_reference concept (legacy from before named routes) [DHH] Added record identification with polymorphic routes for ActionController::Base#url_for and ActionView::Base#url_for [DHH]

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

15 files changed, 241 insertions, 39 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index fa72694ffe..b6907d831e 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,14 @@
 *SVN*
 
+* Added record identification with polymorphic routes for ActionController::Base#url_for and ActionView::Base#url_for [DHH]. Examples:
+
+    redirect_to(post)         # => redirect_to(posts_url(post))         => Location: http://example.com/posts/1
+    link_to(post.title, post) # => link_to(post.title, posts_url(post)) => <a href="/posts/1">Hello world</a>
+
+  Any method that calls url_for on its parameters will automatically benefit from this. 
+
+* Removed deprecated parameters_for_method_reference concept (legacy from before named routes) [DHH]
+
 * Add ActionController::Routing::Helpers, a module to contain common URL helpers such as polymorphic_url. [Nicholas Seckar]
 
 * Included the HttpAuthentication plugin as part of core (ActionController::HttpAuthentication::Basic) [DHH]
@@ -2694,7 +2703,7 @@ Default YAML web services were retired, ActionController::Base.param_parsers car
 
 * Fixed all helpers so that they use XHTML compliant double quotes for values instead of single quotes [htonl/bitsweat]
 
-* Added link_to_image(src, options = {}, html_options = {}, *parameters_for_method_reference). Documentation:
+* Added link_to_image(src, options = {}, html_options = {}). Documentation:
 
     Creates a link tag to the image residing at the +src+ using an URL created by the set of +options+. See the valid options in
     link:classes/ActionController/Base.html#M000021. It's also possible to pass a string instead of an options hash to
diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb
index 96192809e5..162669b79e 100755
--- a/actionpack/lib/action_controller.rb
+++ b/actionpack/lib/action_controller.rb
@@ -82,4 +82,4 @@ ActionController::Base.class_eval do
   include ActionController::RecordIdentifier
   include ActionController::Macros::AutoComplete
   include ActionController::Macros::InPlaceEditing
-end
+end
\ No newline at end of file
diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb
index c1c9e57342..a9d502e6f1 100755
--- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -560,7 +560,7 @@ module ActionController #:nodoc:
           when Hash
             @url.rewrite(rewrite_options(options))
           else
-            raise ArgumentError, "Unrecognized url_for options: #{options.inspect}"
+            polymorphic_url(options, self)
         end
       end
 
@@ -1015,7 +1015,7 @@ module ActionController #:nodoc:
       # When using <tt>redirect_to :back</tt>, if there is no referrer,
       # RedirectBackError will be raised. You may specify some fallback
       # behavior for this case by rescueing RedirectBackError.
-      def redirect_to(options = {}, *parameters_for_method_reference) #:doc:
+      def redirect_to(options = {}) #:doc:
         case options
           when %r{^\w+://.*}
             raise DoubleRenderError if performed?
@@ -1031,14 +1031,8 @@ module ActionController #:nodoc:
             request.env["HTTP_REFERER"] ? redirect_to(request.env["HTTP_REFERER"]) : raise(RedirectBackError)
 
           else
-            if parameters_for_method_reference.empty?
-              redirect_to(url_for(options))
-              response.redirected_to = options
-            else
-              # TOOD: Deprecate me!
-              redirect_to(url_for(options, *parameters_for_method_reference))
-              response.redirected_to, response.redirected_to_method_params = options, parameters_for_method_reference
-            end
+            redirect_to(url_for(options))
+            response.redirected_to = options
         end
       end
 
diff --git a/actionpack/lib/action_controller/polymorphic_routes.rb b/actionpack/lib/action_controller/polymorphic_routes.rb
new file mode 100644
index 0000000000..eeddc28d60
--- /dev/null
+++ b/actionpack/lib/action_controller/polymorphic_routes.rb
@@ -0,0 +1,52 @@
+module ActionController
+  module PolymorphicRoutes
+    extend self
+
+    def polymorphic_url(record_or_hash, url_writer, options = {})
+      record = extract_record(record_or_hash)
+
+      case
+      when options[:action] == "new"
+        url_writer.send(action_prefix(options) + RecordIdentifier.singular_class_name(record) + routing_type(options))
+
+      when record.respond_to?(:new_record?) && record.new_record?
+        url_writer.send(RecordIdentifier.plural_class_name(record) + routing_type(options))
+
+      else
+        url_writer.send(
+          action_prefix(options) + RecordIdentifier.singular_class_name(record) + routing_type(options), record_or_hash
+        )
+      end
+    end
+
+    def polymorphic_path(record_or_hash, url_writer)
+      polymorphic_url(record_or_hash, url_writer, :routing_type => :path)
+    end
+
+    %w( edit new formatted ).each do |action|
+      module_eval <<-EOT
+        def #{action}_polymorphic_url(record_or_hash, url_writer)
+          polymorphic_url(record_or_hash, url_writer, :action => "#{action}")
+        end
+
+        def #{action}_polymorphic_path(record_or_hash, url_writer)
+          polymorphic_url(record_or_hash, url_writer, :action => "#{action}", :routing_type => :path)
+        end
+      EOT
+    end
+
+
+    private
+      def action_prefix(options)
+        options[:action] ? "#{options[:action]}_" : ""
+      end
+      
+      def routing_type(options)
+        "_#{options[:routing_type] || "url"}"
+      end
+      
+      def extract_record(record_or_hash)
+        record_or_hash.is_a?(Hash) ? record_or_hash[:id] : record_or_hash
+      end
+  end
+end
\ No newline at end of file
diff --git a/actionpack/lib/action_controller/routing.rb b/actionpack/lib/action_controller/routing.rb
index 9bbb51cfbf..49661850ea 100644
--- a/actionpack/lib/action_controller/routing.rb
+++ b/actionpack/lib/action_controller/routing.rb
@@ -1,5 +1,6 @@
 require 'cgi'
 require 'uri'
+require 'action_controller/polymorphic_routes'
 
 class Object
   def to_param
@@ -255,6 +256,7 @@ module ActionController
     
     # A helper module to hold URL related helpers.
     module Helpers
+      include PolymorphicRoutes
     end
     
     class << self
diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb
index ca7e9872cc..2329fb4bc6 100644
--- a/actionpack/lib/action_view/helpers/url_helper.rb
+++ b/actionpack/lib/action_view/helpers/url_helper.rb
@@ -29,6 +29,13 @@ module ActionView
       # * <tt>:password</tt> -- Inline HTTP authentication (only plucked out if :user is also present)
       # * <tt>:escape</tt> -- Determines whether the returned URL will be HTML escaped or not (<tt>true</tt> by default)
       #
+      # ==== Relying on named routes
+      #
+      # If you instead of a hash pass a record (like an Active Record or Active Resource) as the options parameter,
+      # you'll trigger the named route for that record. The lookup will happen on the name of the class. So passing
+      # a Workshop object will attempt to use the workshop_path route. If you have a nested route, such as 
+      # admin_workshop_path you'll have to call that explicitly (it's impossible for url_for to guess that route). 
+      #
       # ==== Examples
       #   <%= url_for(:action => 'index') %>
       #   # => /blog/
@@ -47,15 +54,30 @@ module ActionView
       #
       #   <%= url_for(:action => 'checkout', :anchor => 'tax&ship', :escape => false) %>
       #   # => /testing/jump/#tax&ship
-      def url_for(options = {}, *parameters_for_method_reference)
-        if options.kind_of? Hash
+      #
+      #   <%= url_for(Workshop.new) %>
+      #   # relies on Workshop answering a new_record? call (and in this case returning true)
+      #   # => /workshops
+      #
+      #   <%= url_for(@workshop) %>
+      #   # calls @workshop.to_s
+      #   # => /workshops/5
+      def url_for(options = {})
+        case options
+        when Hash
           options = { :only_path => true }.update(options.symbolize_keys)
-          escape = options.key?(:escape) ? options.delete(:escape) : true
-        else
+          escape  = options.key?(:escape) ? options.delete(:escape) : true
+          url     = @controller.send(:url_for, options)
+        when String
           escape = true
+          url    = options
+        when NilClass
+          url = @controller.send(:url_for, nil)
+        else
+          escape = false
+          url    = polymorphic_path(options, self)
         end
 
-        url = @controller.send(:url_for, options, *parameters_for_method_reference)
         escape ? html_escape(url) : url
       end
 
@@ -104,7 +126,7 @@ module ActionView
       #        f.style.display = 'none'; this.parentNode.appendChild(f); f.method = 'POST'; f.action = this.href;
       #        var m = document.createElement('input'); m.setAttribute('type', 'hidden'); m.setAttribute('name', '_method'); 
       #        m.setAttribute('value', 'delete'); f.appendChild(m);f.submit(); };return false;">Delete Image</a>
-      def link_to(name, options = {}, html_options = nil, *parameters_for_method_reference)
+      def link_to(name, options = {}, html_options = nil)
         if html_options
           html_options = html_options.stringify_keys
           convert_options_to_javascript!(html_options)
@@ -113,7 +135,7 @@ module ActionView
           tag_options = nil
         end
 
-        url = options.is_a?(String) ? options : self.url_for(options, *parameters_for_method_reference)
+        url = options.is_a?(String) ? options : self.url_for(options)
         "<a href=\"#{url}\"#{tag_options}>#{name || url}</a>"
       end
 
@@ -222,8 +244,8 @@ module ActionView
       #           link_to("Go back", { :controller => 'posts', :action => 'index' }) 
       #        end 
       #     %>
-      def link_to_unless_current(name, options = {}, html_options = {}, *parameters_for_method_reference, &block)
-        link_to_unless current_page?(options), name, options, html_options, *parameters_for_method_reference, &block
+      def link_to_unless_current(name, options = {}, html_options = {}, &block)
+        link_to_unless current_page?(options), name, options, html_options, &block
       end
 
       # Creates a link tag of the given +name+ using a URL created by the set of
@@ -246,15 +268,15 @@ module ActionView
       #   # => <a href="/controller/reply/">Reply</a>
       #   # If not...
       #   # => <a href="/accounts/signup">Reply</a>
-      def link_to_unless(condition, name, options = {}, html_options = {}, *parameters_for_method_reference, &block)
+      def link_to_unless(condition, name, options = {}, html_options = {}, &block)
         if condition
           if block_given?
-            block.arity <= 1 ? yield(name) : yield(name, options, html_options, *parameters_for_method_reference)
+            block.arity <= 1 ? yield(name) : yield(name, options, html_options)
           else
             name
           end
         else
-          link_to(name, options, html_options, *parameters_for_method_reference)
+          link_to(name, options, html_options)
         end
       end
 
@@ -278,8 +300,8 @@ module ActionView
       #   # => <a href="/sessions/new/">Login</a>
       #   # If they are logged in...
       #   # => <a href="/accounts/show/3">my_username</a>
-      def link_to_if(condition, name, options = {}, html_options = {}, *parameters_for_method_reference, &block)
-        link_to_unless !condition, name, options, html_options, *parameters_for_method_reference, &block
+      def link_to_if(condition, name, options = {}, html_options = {}, &block)
+        link_to_unless !condition, name, options, html_options, &block
       end
 
       # Creates a mailto link tag to the specified +email_address+, which is
diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb
index df037ec28d..c7e4c19a0a 100755
--- a/actionpack/test/controller/redirect_test.rb
+++ b/actionpack/test/controller/redirect_test.rb
@@ -1,5 +1,24 @@
 require File.dirname(__FILE__) + '/../abstract_unit'
 
+class WorkshopsController < ActionController::Base
+end
+
+class Workshop
+  attr_accessor :id, :new_record
+
+  def initialize(id, new_record)
+    @id, @new_record = id, new_record
+  end
+  
+  def new_record?
+    @new_record
+  end
+  
+  def to_s
+    id.to_s
+  end
+end
+
 class RedirectController < ActionController::Base
   def simple_redirect
     redirect_to :action => "hello_world"
@@ -22,6 +41,14 @@ class RedirectController < ActionController::Base
     redirect_to :back
   end
 
+  def redirect_to_existing_record
+    redirect_to Workshop.new(5, false)
+  end
+
+  def redirect_to_new_record
+    redirect_to Workshop.new(5, true)
+  end
+
   def rescue_errors(e) raise e end
     
   def rescue_action(e) raise end
@@ -97,6 +124,19 @@ class RedirectTest < Test::Unit::TestCase
       get :redirect_to_back
     }
   end
+  
+  def test_redirect_to_record
+    ActionController::Routing::Routes.draw do |map|
+      map.resources :workshops
+      map.connect ':controller/:action/:id'
+    end
+    
+    get :redirect_to_existing_record
+    assert_equal "http://test.host/workshops/5", redirect_to_url
+
+    get :redirect_to_new_record
+    assert_equal "http://test.host/workshops", redirect_to_url
+  end
 end
 
 module ModuleTest
diff --git a/actionpack/test/template/active_record_helper_test.rb b/actionpack/test/template/active_record_helper_test.rb
index c1dc73990d..5eab1c8525 100644
--- a/actionpack/test/template/active_record_helper_test.rb
+++ b/actionpack/test/template/active_record_helper_test.rb
@@ -83,7 +83,7 @@ class ActiveRecordHelperTest < Test::Unit::TestCase
     setup_user
 
     @controller = Object.new
-    def @controller.url_for(options, *parameters_for_method_reference)
+    def @controller.url_for(options)
       options = options.symbolize_keys
 
       [options[:action], options[:id].to_param].compact.join('/')
diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb
index 71378584fa..cd4b56a2a0 100644
--- a/actionpack/test/template/asset_tag_helper_test.rb
+++ b/actionpack/test/template/asset_tag_helper_test.rb
@@ -303,7 +303,7 @@ class AssetTagHelperNonVhostTest < Test::Unit::TestCase
     @controller = Class.new do
       attr_accessor :request
 
-      def url_for(options, *parameters_for_method_reference)
+      def url_for(options)
         "http://www.example.com/collaboration/hieraki"
       end
     end.new
diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb
index c5e7505a36..a7336c5b08 100644
--- a/actionpack/test/template/form_helper_test.rb
+++ b/actionpack/test/template/form_helper_test.rb
@@ -38,7 +38,7 @@ class FormHelperTest < Test::Unit::TestCase
 
     @controller = Class.new do
       attr_reader :url_for_options
-      def url_for(options, *parameters_for_method_reference)
+      def url_for(options)
         @url_for_options = options
         "http://www.example.com"
       end
@@ -528,7 +528,7 @@ class FormHelperTest < Test::Unit::TestCase
 
     form_for(:post, @post, :url => 'http://www.otherdomain.com') do |f| end
 
-    assert_equal 'http://www.otherdomain.com', @controller.url_for_options
+    assert_equal '<form action="http://www.otherdomain.com" method="post"></form>', _erbout
   end
 
   def test_form_for_with_hash_url_option
@@ -540,6 +540,14 @@ class FormHelperTest < Test::Unit::TestCase
     assert_equal 'action', @controller.url_for_options[:action]
   end
   
+  def test_form_for_with_record_url_option
+    _erbout = ''
+
+    form_for(:post, @post, :url => @post) do |f| end
+
+    expected = "<form action=\"/posts/123\" method=\"post\"></form>"
+  end
+  
   def test_remote_form_for_with_html_options_adds_options_to_form_tag
     self.extend ActionView::Helpers::PrototypeHelper
     _erbout = ''
@@ -549,4 +557,10 @@ class FormHelperTest < Test::Unit::TestCase
     
     assert_dom_equal expected, _erbout
   end
-end
+
+
+  protected
+    def polymorphic_path(record, url_writer)
+      "/posts/#{record.id}"
+    end
+end
\ No newline at end of file
diff --git a/actionpack/test/template/form_tag_helper_test.rb b/actionpack/test/template/form_tag_helper_test.rb
index 8b5d09d2a6..2390edff86 100644
--- a/actionpack/test/template/form_tag_helper_test.rb
+++ b/actionpack/test/template/form_tag_helper_test.rb
@@ -9,7 +9,7 @@ class FormTagHelperTest < Test::Unit::TestCase
 
   def setup
     @controller = Class.new do
-      def url_for(options, *parameters_for_method_reference)
+      def url_for(options)
         "http://www.example.com"
       end
     end
@@ -44,7 +44,7 @@ class FormTagHelperTest < Test::Unit::TestCase
     _erbout = ''
     form_tag("http://example.com") { _erbout.concat "Hello world!" }
 
-    expected = %(<form action="http://www.example.com" method="post">Hello world!</form>)
+    expected = %(<form action="http://example.com" method="post">Hello world!</form>)
     assert_dom_equal expected, _erbout
   end
 
@@ -52,7 +52,7 @@ class FormTagHelperTest < Test::Unit::TestCase
     _erbout = ''
     form_tag("http://example.com", :method => :put) { _erbout.concat "Hello world!" }
 
-    expected = %(<form action="http://www.example.com" method="post"><div style='margin:0;padding:0'><input type="hidden" name="_method" value="put" /></div>Hello world!</form>)
+    expected = %(<form action="http://example.com" method="post"><div style='margin:0;padding:0'><input type="hidden" name="_method" value="put" /></div>Hello world!</form>)
     assert_dom_equal expected, _erbout
   end
 
diff --git a/actionpack/test/template/java_script_macros_helper_test.rb b/actionpack/test/template/java_script_macros_helper_test.rb
index 10d77558f7..13318958c8 100644
--- a/actionpack/test/template/java_script_macros_helper_test.rb
+++ b/actionpack/test/template/java_script_macros_helper_test.rb
@@ -12,7 +12,7 @@ class JavaScriptMacrosHelperTest < Test::Unit::TestCase
   
   def setup
     @controller = Class.new do
-      def url_for(options, *parameters_for_method_reference)
+      def url_for(options)
         url =  "http://www.example.com/"
         url << options[:action].to_s if options and options[:action]
         url
diff --git a/actionpack/test/template/prototype_helper_test.rb b/actionpack/test/template/prototype_helper_test.rb
index ef658517ff..cd098f2a47 100644
--- a/actionpack/test/template/prototype_helper_test.rb
+++ b/actionpack/test/template/prototype_helper_test.rb
@@ -17,7 +17,7 @@ module BaseTest
   def setup
     @template = nil
     @controller = Class.new do
-      def url_for(options, *parameters_for_method_reference)
+      def url_for(options)
         if options.is_a?(String)
           options
         else
diff --git a/actionpack/test/template/scriptaculous_helper_test.rb b/actionpack/test/template/scriptaculous_helper_test.rb
index cb1171a640..e40debc0f9 100644
--- a/actionpack/test/template/scriptaculous_helper_test.rb
+++ b/actionpack/test/template/scriptaculous_helper_test.rb
@@ -13,7 +13,7 @@ class ScriptaculousHelperTest < Test::Unit::TestCase
   
   def setup
     @controller = Class.new do
-      def url_for(options, *parameters_for_method_reference)
+      def url_for(options)
         url =  "http://www.example.com/"
         url << options[:action].to_s if options and options[:action]
         url
diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb
index 9ecbc77a4e..1762015d9d 100644
--- a/actionpack/test/template/url_helper_test.rb
+++ b/actionpack/test/template/url_helper_test.rb
@@ -10,7 +10,7 @@ class UrlHelperTest < Test::Unit::TestCase
   def setup
     @controller = Class.new do
       attr_accessor :url, :request
-      def url_for(options, *parameters_for_method_reference)
+      def url_for(options)
         url
       end
     end
@@ -168,7 +168,7 @@ class UrlHelperTest < Test::Unit::TestCase
     assert_equal "Showing", link_to_unless(true, "Showing", :action => "show", :controller => "weblog")
     assert_dom_equal "<a href=\"http://www.example.com\">Listing</a>", link_to_unless(false, "Listing", :action => "list", :controller => "weblog")
     assert_equal "Showing", link_to_unless(true, "Showing", :action => "show", :controller => "weblog", :id => 1)
-    assert_equal "<strong>Showing</strong>", link_to_unless(true, "Showing", :action => "show", :controller => "weblog", :id => 1) { |name, options, html_options, *parameters_for_method_reference|
+    assert_equal "<strong>Showing</strong>", link_to_unless(true, "Showing", :action => "show", :controller => "weblog", :id => 1) { |name, options, html_options|
       "<strong>#{name}</strong>"
     }
     assert_equal "<strong>Showing</strong>", link_to_unless(true, "Showing", :action => "show", :controller => "weblog", :id => 1) { |name|
@@ -352,3 +352,72 @@ class LinkToUnlessCurrentWithControllerTest < Test::Unit::TestCase
       end
     end
 end
+
+
+class Workshop
+  attr_accessor :id, :new_record
+
+  def initialize(id, new_record)
+    @id, @new_record = id, new_record
+  end
+  
+  def new_record?
+    @new_record
+  end
+  
+  def to_s
+    id.to_s
+  end
+end
+
+class PolymorphicControllerTest < Test::Unit::TestCase
+  class WorkshopsController < ActionController::Base
+    self.view_paths = ["#{File.dirname(__FILE__)}/../fixtures/"]
+
+    def self.controller_path; 'workshops' end
+
+    def index
+      @workshop = Workshop.new(1, true)
+      render :inline => "<%= url_for(@workshop) %>\n<%= link_to('Workshop', @workshop) %>"
+    end
+
+    def show
+      @workshop = Workshop.new(params[:id], false)
+      render :inline => "<%= url_for(@workshop) %>\n<%= link_to('Workshop', @workshop) %>"
+    end
+
+    def rescue_action(e) raise e end
+  end
+
+  include ActionView::Helpers::UrlHelper
+
+  def setup
+    @request    = ActionController::TestRequest.new
+    @response   = ActionController::TestResponse.new
+    @controller = WorkshopsController.new
+  end
+
+  def test_new_resource
+    with_restful_routing do
+      get :index
+      assert_equal "/workshops\n<a href=\"/workshops\">Workshop</a>", @response.body
+    end
+  end
+
+  def test_existing_resource
+    with_restful_routing do
+      get :show, :id => 1
+      assert_equal "/workshops/1\n<a href=\"/workshops/1\">Workshop</a>", @response.body
+    end
+  end
+
+  protected
+    def with_restful_routing
+      with_routing do |set|
+        set.draw do |map|
+          map.resources :workshops
+        end
+        yield
+      end
+    end
+end
\ No newline at end of file