Skip to content

Commit ce60672

Browse files
Correct the use of params options when given to url_for
Merge url for tests and add changelog entry for rails#8233.
1 parent e492c44 commit ce60672

File tree

3 files changed

+16
-10
lines changed

3 files changed

+16
-10
lines changed

actionpack/CHANGELOG.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,17 @@
11
## Rails 4.0.0 (unreleased) ##
22

3+
* Fix error when using a non-hash query argument named "params" in `url_for`.
4+
5+
Before:
6+
7+
url_for(params: "") # => undefined method `reject!' for "":String
8+
9+
After:
10+
11+
url_for(params: "") # => http://www.example.com?params=
12+
13+
*tumayun + Carlos Antonio da Silva*
14+
315
* Render every partial with a new `ActionView::PartialRenderer`. This resolves
416
issues when rendering nested partials.
517
Fix #8197

actionpack/lib/action_dispatch/http/url.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ def url_for(options = {})
2828
path = options.delete(:script_name).to_s.chomp("/")
2929
path << options.delete(:path).to_s
3030

31-
params = options[:params].is_a?(Hash) ? options[:params] : {}
31+
params = options[:params].is_a?(Hash) ? options[:params] : options.slice(:params)
3232
params.reject! { |_,v| v.to_param.nil? }
3333

3434
result = build_host_url(options)

actionpack/test/dispatch/request_test.rb

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
class RequestTest < ActiveSupport::TestCase
44

55
def url_for(options = {})
6-
options.reverse_merge!(:host => 'www.example.com')
6+
options = { host: 'www.example.com' }.merge!(options)
77
ActionDispatch::Http::URL.url_for(options)
88
end
99

@@ -25,6 +25,8 @@ def url_for(options = {})
2525
assert_equal 'http://www.example.com/', url_for(:trailing_slash => true)
2626
assert_equal 'http://dhh:[email protected]', url_for(:user => 'dhh', :password => 'supersecret')
2727
assert_equal 'http://www.example.com?search=books', url_for(:params => { :search => 'books' })
28+
assert_equal 'http://www.example.com?params=', url_for(:params => '')
29+
assert_equal 'http://www.example.com?params=1', url_for(:params => 1)
2830
end
2931

3032
test "remote ip" do
@@ -799,14 +801,6 @@ def url_for(options = {})
799801
end
800802
end
801803

802-
test "url_for options[:params]" do
803-
assert_equal 'http://www.example.com?params=', url_for(:params => '')
804-
assert_equal 'http://www.example.com?params=1', url_for(:params => 1)
805-
assert_equal 'http://www.example.com', url_for
806-
assert_equal 'http://www.example.com', url_for(:params => {})
807-
assert_equal 'http://www.example.com?name=tumayun', url_for(:params => { :name => 'tumayun' })
808-
end
809-
810804
protected
811805

812806
def stub_request(env = {})

0 commit comments

Comments
 (0)