Split direct method into two

Use a separate method called `resolve` for the custom polymorphic
mapping to clarify the API.

actionpack/CHANGELOG.md

@@ -5,23 +5,9 @@
 
     *Andrew White*
 
-*   Add the `direct` method to the routing DSL
+*   Add the `resolve` method to the routing DSL
 
-    This new method allows customization of the routing behavior in two ways:
+    This new method allows customization of the polymorphic mapping of models:
-
-    1.  Custom url helpers:
-
-        ``` ruby
-        direct(:apple) { "http://www.apple.com" }
-
-        >> apple_url
-        => "http://www.apple.com"
-        ```
-
-        This has the advantage of being available everywhere url helpers are available
-        unlike custom url helpers defined in helper modules, etc.
-
-    2.  Custom polymorphic mappings:
 
     ``` ruby
     resource :basket
@@ -37,13 +23,26 @@
     This generates the correct singular URL for the form instead of the default
     resources member url, e.g. `/basket` vs. `/basket/:id`.
 
-    Currently both forms of `direct` do not take anything from the current routing
-    scope so it's recommended to declare them outside of any `namespace` or `scope` block.
-
     Fixes #1769.
 
     *Andrew White*
 
+*   Add the `direct` method to the routing DSL
+
+    This new method allows creation of custom url helpers, e.g:
+
+    ``` ruby
+    direct(:apple) { "http://www.apple.com" }
+
+    >> apple_url
+    => "http://www.apple.com"
+    ```
+
+    This has the advantage of being available everywhere url helpers are available
+    unlike custom url helpers defined in helper modules, etc.
+
+    *Andrew White*
+
 *   Add `ActionDispatch::SystemTestCase` to Action Pack
 
     Adds Capybara integration directly into Rails through Action Pack!

actionpack/lib/action_dispatch/routing/mapper.rb

@@ -2020,8 +2020,8 @@ module ActionDispatch
         end
       end
 
-      module DirectUrls
+      module CustomUrls
-        # Define custom routing behavior that will be added to the application's
+        # Define custom url helpers that will be added to the application's
         # routes. This allows you override and/or replace the default behavior
         # of routing helpers, e.g:
         #
@@ -2037,32 +2037,6 @@ module ActionDispatch
         #     { controller: 'pages', action: 'index', subdomain: 'www' }
         #   end
         #
-        # The above example show how to define a custom url helper but it's also
-        # possible to alter the behavior of `polymorphic_url` and consequently the
-        # behavior of `link_to` and `form_for` when passed a model instance, e.g:
-        #
-        #   direct class: "Basket" do
-        #     [:basket]
-        #   end
-        #
-        # NOTE: This custom behavior only applies to simple polymorphic urls where
-        # a single model instance is passed and not more complicated forms, e.g:
-        #
-        #   # config/routes.rb
-        #   resource :profile
-        #   namespace :admin do
-        #     resources :users
-        #   end
-        #
-        #   direct(class: "User") { [:profile] }
-        #
-        #   # app/views/application/_menu.html.erb
-        #   link_to 'Profile', @current_user
-        #   link_to 'Profile', [:admin, @current_user]
-        #
-        # The first `link_to` will generate '/profile' but the second will generate
-        # the standard polymorphic url of '/admin/users/1'.
-        #
         # The return value from the block passed to `direct` must be a valid set of
         # arguments for `url_for` which will actually build the url string. This can
         # be one of the following:
@@ -2083,7 +2057,48 @@ module ActionDispatch
         #     [ :products, options.merge(params.permit(:page, :size)) ]
         #   end
         #
-        # You can pass options to a polymorphic mapping do - the arity for the block
+        # NOTE: The `direct` methodn can't be used inside of a scope block such as
+        # `namespace` or `scope` and will raise an error if it detects that it is.
+        def direct(name, options = {}, &block)
+          unless @scope.root?
+            raise RuntimeError, "The direct method can't be used inside a routes scope block"
+          end
+
+          @set.add_url_helper(name, options, &block)
+        end
+
+        # Define custom polymorphic mappings of models to urls. This alters the
+        # behavior of `polymorphic_url` and consequently the behavior of
+        # `link_to` and `form_for` when passed a model instance, e.g:
+        #
+        #   resource :basket
+        #
+        #   resolve "Basket" do
+        #     [:basket]
+        #   end
+        #
+        # This will now generate '/basket' when a `Basket` instance is passed to
+        # `link_to` or `form_for` instead of the standard '/baskets/:id'.
+        #
+        # NOTE: This custom behavior only applies to simple polymorphic urls where
+        # a single model instance is passed and not more complicated forms, e.g:
+        #
+        #   # config/routes.rb
+        #   resource :profile
+        #   namespace :admin do
+        #     resources :users
+        #   end
+        #
+        #   resolve("User") { [:profile] }
+        #
+        #   # app/views/application/_menu.html.erb
+        #   link_to 'Profile', @current_user
+        #   link_to 'Profile', [:admin, @current_user]
+        #
+        # The first `link_to` will generate '/profile' but the second will generate
+        # the standard polymorphic url of '/admin/users/1'.
+        #
+        # You can pass options to a polymorphic mapping - the arity for the block
         # needs to be two as the instance is passed as the first argument, e.g:
         #
         #   direct class: 'Basket', anchor: 'items' do |basket, options|
@@ -2094,20 +2109,18 @@ module ActionDispatch
         # array passed to `polymorphic_url` is a hash then it's treated as options
         # to the url helper that gets called.
         #
-        # NOTE: The `direct` methodn can't be used inside of a scope block such as
+        # NOTE: The `resolve` methodn can't be used inside of a scope block such as
         # `namespace` or `scope` and will raise an error if it detects that it is.
-        def direct(name_or_hash, options = nil, &block)
+        def resolve(*args, &block)
           unless @scope.root?
-            raise RuntimeError, "The direct method can't be used inside a routes scope block"
+            raise RuntimeError, "The resolve method can't be used inside a routes scope block"
           end
 
-          case name_or_hash
+          options = args.extract_options!
-          when Hash
+          args = args.flatten(1)
-            @set.add_polymorphic_mapping(name_or_hash, &block)
+
-          when String, Symbol
+          args.each do |klass|
-            @set.add_url_helper(name_or_hash, options, &block)
+            @set.add_polymorphic_mapping(klass, options, &block)
-          else
-            raise ArgumentError, "The direct method only accepts a hash, string or symbol"
           end
         end
       end
@@ -2213,7 +2226,7 @@ module ActionDispatch
       include Scoping
       include Concerns
       include Resources
-      include DirectUrls
+      include CustomUrls
     end
   end
 end

actionpack/lib/action_dispatch/routing/route_set.rb

@@ -160,7 +160,7 @@ module ActionDispatch
 
         def add_url_helper(name, defaults, &block)
           @custom_helpers << name
-          helper = DirectUrlHelper.new(name, defaults, &block)
+          helper = CustomUrlHelper.new(name, defaults, &block)
 
           @path_helpers_module.module_eval do
             define_method(:"#{name}_path") do |*args|
@@ -596,21 +596,15 @@ module ActionDispatch
         route
       end
 
-      def add_polymorphic_mapping(options, &block)
+      def add_polymorphic_mapping(klass, options, &block)
-        defaults = options.dup
+        @polymorphic_mappings[klass] = CustomUrlHelper.new(klass, options, &block)
-        klass = defaults.delete(:class)
-        if klass.nil?
-          raise ArgumentError, "Missing :class key from polymorphic mapping options"
-        end
-
-        @polymorphic_mappings[klass] = DirectUrlHelper.new(klass, defaults, &block)
       end
 
       def add_url_helper(name, options, &block)
         named_routes.add_url_helper(name, options, &block)
       end
 
-      class DirectUrlHelper
+      class CustomUrlHelper
         attr_reader :name, :defaults, :block
 
         def initialize(name, defaults, &block)

actionpack/test/dispatch/routing/custom_url_helpers_test.rb

@@ -1,6 +1,6 @@
 require "abstract_unit"
 
-class TestDirectUrlHelpers < ActionDispatch::IntegrationTest
+class TestCustomUrlHelpers < ActionDispatch::IntegrationTest
   class Linkable
     attr_reader :id
 
@@ -53,6 +53,21 @@ class TestDirectUrlHelpers < ActionDispatch::IntegrationTest
     end
   end
 
+  class Page
+    attr_reader :id
+
+    def self.name
+      super.demodulize
+    end
+
+    def initialize(id)
+      @id = id
+    end
+  end
+
+  class CategoryPage < Page; end
+  class ProductPage < Page; end
+
   Routes = ActionDispatch::Routing::RouteSet.new
   Routes.draw do
     default_url_options host: "www.example.com"
@@ -62,6 +77,7 @@ class TestDirectUrlHelpers < ActionDispatch::IntegrationTest
     get "/posts/:id", to: "posts#show", as: :post
     get "/profile", to: "users#profile", as: :profile
     get "/media/:id", to: "media#show", as: :media
+    get "/pages/:id", to: "pages#show", as: :page
 
     resources :categories, :collections, :products
 
@@ -80,10 +96,11 @@ class TestDirectUrlHelpers < ActionDispatch::IntegrationTest
     direct(:options)  { |options| [:products, options] }
     direct(:defaults, size: 10) { |options| [:products, options] }
 
-    direct(class: "Article") { |article| [:post, { id: article.id }] }
+    resolve("Article") { |article| [:post, { id: article.id }] }
-    direct(class: "Basket") { |basket| [:basket] }
+    resolve("Basket") { |basket| [:basket] }
-    direct(class: "User", anchor: "details") { |user, options| [:profile, options] }
+    resolve("User", anchor: "details") { |user, options| [:profile, options] }
-    direct(class: "Video") { |video| [:media, { id: video.id }] }
+    resolve("Video") { |video| [:media, { id: video.id }] }
+    resolve(%w[Page CategoryPage ProductPage]) { |page| [:page, { id: page.id }] }
   end
 
   APP = build_app Routes
@@ -102,6 +119,9 @@ class TestDirectUrlHelpers < ActionDispatch::IntegrationTest
     @user = User.new
     @video = Video.new("4")
     @article = Article.new("5")
+    @page = Page.new("6")
+    @category_page = CategoryPage.new("7")
+    @product_page = ProductPage.new("8")
     @path_params = { "controller" => "pages", "action" => "index" }
     @unsafe_params = ActionController::Parameters.new(@path_params)
     @safe_params = ActionController::Parameters.new(@path_params).permit(:controller, :action)
@@ -142,23 +162,6 @@ class TestDirectUrlHelpers < ActionDispatch::IntegrationTest
     assert_equal "/products?size=10", Routes.url_helpers.defaults_path
     assert_equal "/products?size=20", defaults_path(size: 20)
     assert_equal "/products?size=20", Routes.url_helpers.defaults_path(size: 20)
-
-    assert_equal "/basket", polymorphic_path(@basket)
-    assert_equal "/basket", Routes.url_helpers.polymorphic_path(@basket)
-
-    assert_equal "/profile#details", polymorphic_path(@user)
-    assert_equal "/profile#details", Routes.url_helpers.polymorphic_path(@user)
-
-    assert_equal "/profile#password", polymorphic_path(@user, anchor: "password")
-    assert_equal "/profile#password", Routes.url_helpers.polymorphic_path(@user, anchor: "password")
-
-    assert_equal "/media/4", polymorphic_path(@video)
-    assert_equal "/media/4", Routes.url_helpers.polymorphic_path(@video)
-    assert_equal "/media/4", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_model_call(self, @video)
-
-    assert_equal "/posts/5", polymorphic_path(@article)
-    assert_equal "/posts/5", Routes.url_helpers.polymorphic_path(@article)
-    assert_equal "/posts/5", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_model_call(self, @article)
   end
 
   def test_direct_urls
@@ -196,7 +199,40 @@ class TestDirectUrlHelpers < ActionDispatch::IntegrationTest
     assert_equal "http://www.example.com/products?size=10", Routes.url_helpers.defaults_url
     assert_equal "http://www.example.com/products?size=20", defaults_url(size: 20)
     assert_equal "http://www.example.com/products?size=20", Routes.url_helpers.defaults_url(size: 20)
+  end
 
+  def test_resolve_paths
+    assert_equal "/basket", polymorphic_path(@basket)
+    assert_equal "/basket", Routes.url_helpers.polymorphic_path(@basket)
+
+    assert_equal "/profile#details", polymorphic_path(@user)
+    assert_equal "/profile#details", Routes.url_helpers.polymorphic_path(@user)
+
+    assert_equal "/profile#password", polymorphic_path(@user, anchor: "password")
+    assert_equal "/profile#password", Routes.url_helpers.polymorphic_path(@user, anchor: "password")
+
+    assert_equal "/media/4", polymorphic_path(@video)
+    assert_equal "/media/4", Routes.url_helpers.polymorphic_path(@video)
+    assert_equal "/media/4", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_model_call(self, @video)
+
+    assert_equal "/posts/5", polymorphic_path(@article)
+    assert_equal "/posts/5", Routes.url_helpers.polymorphic_path(@article)
+    assert_equal "/posts/5", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_model_call(self, @article)
+
+    assert_equal "/pages/6", polymorphic_path(@page)
+    assert_equal "/pages/6", Routes.url_helpers.polymorphic_path(@page)
+    assert_equal "/pages/6", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_model_call(self, @page)
+
+    assert_equal "/pages/7", polymorphic_path(@category_page)
+    assert_equal "/pages/7", Routes.url_helpers.polymorphic_path(@category_page)
+    assert_equal "/pages/7", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_model_call(self, @category_page)
+
+    assert_equal "/pages/8", polymorphic_path(@product_page)
+    assert_equal "/pages/8", Routes.url_helpers.polymorphic_path(@product_page)
+    assert_equal "/pages/8", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_model_call(self, @product_page)
+  end
+
+  def test_resolve_urls
     assert_equal "http://www.example.com/basket", polymorphic_url(@basket)
     assert_equal "http://www.example.com/basket", Routes.url_helpers.polymorphic_url(@basket)
     assert_equal "http://www.example.com/basket", polymorphic_url(@basket)
@@ -215,29 +251,21 @@ class TestDirectUrlHelpers < ActionDispatch::IntegrationTest
     assert_equal "http://www.example.com/posts/5", polymorphic_url(@article)
     assert_equal "http://www.example.com/posts/5", Routes.url_helpers.polymorphic_url(@article)
     assert_equal "http://www.example.com/posts/5", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.url.handle_model_call(self, @article)
+
+    assert_equal "http://www.example.com/pages/6", polymorphic_url(@page)
+    assert_equal "http://www.example.com/pages/6", Routes.url_helpers.polymorphic_url(@page)
+    assert_equal "http://www.example.com/pages/6", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.url.handle_model_call(self, @page)
+
+    assert_equal "http://www.example.com/pages/7", polymorphic_url(@category_page)
+    assert_equal "http://www.example.com/pages/7", Routes.url_helpers.polymorphic_url(@category_page)
+    assert_equal "http://www.example.com/pages/7", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.url.handle_model_call(self, @category_page)
+
+    assert_equal "http://www.example.com/pages/8", polymorphic_url(@product_page)
+    assert_equal "http://www.example.com/pages/8", Routes.url_helpers.polymorphic_url(@product_page)
+    assert_equal "http://www.example.com/pages/8", ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.url.handle_model_call(self, @product_page)
   end
 
-  def test_raises_argument_error
+  def test_defining_direct_inside_a_scope_raises_runtime_error
-    routes = ActionDispatch::Routing::RouteSet.new
-
-    assert_raises ArgumentError do
-      routes.draw do
-        direct(1) { "http://www.rubyonrails.org" }
-      end
-    end
-  end
-
-  def test_missing_class_raises_argument_error
-    routes = ActionDispatch::Routing::RouteSet.new
-
-    assert_raises ArgumentError do
-      routes.draw do
-        direct(fragment: "core") { "http://www.rubyonrails.org" }
-      end
-    end
-  end
-
-  def test_defining_inside_a_scope_raises_runtime_error
     routes = ActionDispatch::Routing::RouteSet.new
 
     assert_raises RuntimeError do
@@ -248,4 +276,16 @@ class TestDirectUrlHelpers < ActionDispatch::IntegrationTest
       end
     end
   end
+
+  def test_defining_resolve_inside_a_scope_raises_runtime_error
+    routes = ActionDispatch::Routing::RouteSet.new
+
+    assert_raises RuntimeError do
+      routes.draw do
+        namespace :admin do
+          resolve("User") { "/profile" }
+        end
+      end
+    end
+  end
 end

railties/test/application/routing_test.rb

@@ -310,7 +310,7 @@ module ApplicationTests
             get 'mapping', to: 'foo#mapping'
 
             direct(:custom) { "http://www.microsoft.com" }
-            direct(class: "User") { "/profile" }
+            resolve("User") { "/profile" }
           end
         RUBY
 
@@ -332,7 +332,7 @@ module ApplicationTests
             get 'mapping', to: 'foo#mapping'
 
             direct(:custom) { "http://www.apple.com" }
-            direct(class: "User") { "/dashboard" }
+            resolve("User") { "/dashboard" }
           end
         RUBY
 
@@ -465,7 +465,7 @@ module ApplicationTests
           direct(:custom) { 'http://www.apple.com' }
 
           get 'mapping', to: 'foo#mapping'
-          direct(class: 'User') { '/profile' }
+          resolve('User') { '/profile' }
         end
       RUBY
 
@@ -557,7 +557,7 @@ module ApplicationTests
           get ':locale/foo', to: 'foo#index', as: 'foo'
           get 'users', to: 'foo#users', as: 'users'
           direct(:microsoft) { 'http://www.microsoft.com' }
-          direct(class: 'User') { '/profile' }
+          resolve('User') { '/profile' }
         end
       RUBY