From 82b4d879bf31ebf409217e2c770cedfb7c79a44a Mon Sep 17 00:00:00 2001 From: Dan Langevin Date: Mon, 5 May 2014 18:25:29 -0400 Subject: [PATCH] Fixes URL generation with trailing_slash: true URL generation with trailing_slash: true was adding a trailing slash after .:format Routes.draw do resources :bars end bars_url(trailing_slash: true, format: 'json') # => /bars.json/ This commit removes that extra trailing slash --- actionpack/CHANGELOG.md | 5 ++++ actionpack/lib/action_dispatch/http/url.rb | 26 ++++++++++++------- .../test/dispatch/url_generation_test.rb | 18 +++++++++++++ 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 7427fba5e2..b5a09bca15 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -94,4 +94,9 @@ *Tony Wooster* +* Fix URL generation with :trailing_slash such that it does not add + a trailing slash after `.:format` + + *Dan Langevin* + Please check [4-1-stable](https://github.com/rails/rails/blob/4-1-stable/actionpack/CHANGELOG.md) for previous changes. diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 6f5a52c568..ffc9fc7301 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -34,19 +34,15 @@ module ActionDispatch path = options.delete(:script_name).to_s.chomp("/") path << options.delete(:path).to_s + add_trailing_slash(path) if options[:trailing_slash] + params = options[:params].is_a?(Hash) ? options[:params] : options.slice(:params) params.reject! { |_,v| v.to_param.nil? } result = build_host_url(options) - if options[:trailing_slash] - if path.include?('?') - result << path.sub(/\?/, '/\&') - else - result << path.sub(/[^\/]\z|\A\z/, '\&/') - end - else - result << path - end + + result << path + result << "?#{params.to_query}" unless params.empty? result << "##{Journey::Router::Utils.escape_fragment(options[:anchor].to_param.to_s)}" if options[:anchor] result @@ -54,6 +50,18 @@ module ActionDispatch private + def add_trailing_slash(path) + # includes querysting + if path.include?('?') + path.sub!(/\?/, '/\&') + # does not have a .format + elsif !path.include?(".") + path.sub!(/[^\/]\z|\A\z/, '\&/') + end + + path + end + def build_host_url(options) if options[:host].blank? && options[:only_path].blank? raise ArgumentError, 'Missing host to link to! Please provide the :host parameter, set default_url_options[:host], or set :only_path to true' diff --git a/actionpack/test/dispatch/url_generation_test.rb b/actionpack/test/dispatch/url_generation_test.rb index fdea27e2d2..445eb50b68 100644 --- a/actionpack/test/dispatch/url_generation_test.rb +++ b/actionpack/test/dispatch/url_generation_test.rb @@ -15,6 +15,8 @@ module TestUrlGeneration Routes.draw do get "/foo", :to => "my_route_generating#index", :as => :foo + resources :bars + mount MyRouteGeneratingController.action(:index), at: '/bar' end @@ -97,6 +99,22 @@ module TestUrlGeneration test "omit subdomain when key is blank" do assert_equal "http://example.com/foo", foo_url(subdomain: "") end + + test "generating URLs with trailing slashes" do + assert_equal "/bars.json", bars_path( + trailing_slash: true, + format: 'json' + ) + end + + test "generating URLS with querystring and trailing slashes" do + assert_equal "/bars.json?a=b", bars_path( + trailing_slash: true, + a: 'b', + format: 'json' + ) + end + end end