From 82b4d879bf31ebf409217e2c770cedfb7c79a44a Mon Sep 17 00:00:00 2001
From: Dan Langevin <dan.langevin@lifebooker.com>
Date: Mon, 5 May 2014 18:25:29 -0400
Subject: 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 ++++++++++++++++---------
 actionpack/test/dispatch/url_generation_test.rb | 18 +++++++++++++++++
 3 files changed, 40 insertions(+), 9 deletions(-)

(limited to 'actionpack')

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
 
-- 
cgit v1.2.3