From d7c1e62c2cd2969b991bc4a1150b02b27f6d6e3f Mon Sep 17 00:00:00 2001 From: Andrew White Date: Tue, 21 Feb 2017 15:29:10 +0000 Subject: [PATCH] Split direct method into two Use a separate method called `resolve` for the custom polymorphic mapping to clarify the API. --- actionpack/CHANGELOG.md | 59 ++++---- .../lib/action_dispatch/routing/mapper.rb | 93 +++++++------ .../lib/action_dispatch/routing/route_set.rb | 14 +- ...ers_test.rb => custom_url_helpers_test.rb} | 126 ++++++++++++------ railties/test/application/routing_test.rb | 8 +- 5 files changed, 173 insertions(+), 127 deletions(-) rename actionpack/test/dispatch/routing/{direct_url_helpers_test.rb => custom_url_helpers_test.rb} (76%) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 869fc7d484..8226a5be47 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -5,45 +5,44 @@ *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 + resource :basket + direct(class: "Basket") { [:basket] } + ``` - ``` ruby - direct(:apple) { "http://www.apple.com" } + ``` erb + <%= form_for @basket do |form| %> + + <% end %> + ``` - >> 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 - direct(class: "Basket") { [:basket] } - ``` - - ``` erb - <%= form_for @basket do |form| %> - - <% end %> - ``` - - 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. + This generates the correct singular URL for the form instead of the default + resources member url, e.g. `/basket` vs. `/basket/:id`. 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! diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 4fc76e8591..8b4ce1ed6a 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -2020,8 +2020,8 @@ module ActionDispatch end end - module DirectUrls - # Define custom routing behavior that will be added to the application's + module CustomUrls + # 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 - when Hash - @set.add_polymorphic_mapping(name_or_hash, &block) - when String, Symbol - @set.add_url_helper(name_or_hash, options, &block) - else - raise ArgumentError, "The direct method only accepts a hash, string or symbol" + options = args.extract_options! + args = args.flatten(1) + + args.each do |klass| + @set.add_polymorphic_mapping(klass, options, &block) end end end @@ -2213,7 +2226,7 @@ module ActionDispatch include Scoping include Concerns include Resources - include DirectUrls + include CustomUrls end end end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index a2e5351697..84457c97de 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/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) - defaults = options.dup - 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) + def add_polymorphic_mapping(klass, options, &block) + @polymorphic_mappings[klass] = CustomUrlHelper.new(klass, options, &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) diff --git a/actionpack/test/dispatch/routing/direct_url_helpers_test.rb b/actionpack/test/dispatch/routing/custom_url_helpers_test.rb similarity index 76% rename from actionpack/test/dispatch/routing/direct_url_helpers_test.rb rename to actionpack/test/dispatch/routing/custom_url_helpers_test.rb index 9bdda60742..6d230a2557 100644 --- a/actionpack/test/dispatch/routing/direct_url_helpers_test.rb +++ b/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 }] } - direct(class: "Basket") { |basket| [:basket] } - direct(class: "User", anchor: "details") { |user, options| [:profile, options] } - direct(class: "Video") { |video| [:media, { id: video.id }] } + resolve("Article") { |article| [:post, { id: article.id }] } + resolve("Basket") { |basket| [:basket] } + resolve("User", anchor: "details") { |user, options| [:profile, options] } + 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 - 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 + def test_defining_direct_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 diff --git a/railties/test/application/routing_test.rb b/railties/test/application/routing_test.rb index 1132e7fb55..6742da20cc 100644 --- a/railties/test/application/routing_test.rb +++ b/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