Skip to content

Commit

Permalink
Filter params that return nil for to_param and allow through false va…
Browse files Browse the repository at this point in the history
…lues
  • Loading branch information
pixeltrix committed Mar 9, 2011
1 parent f41dd99 commit 03cbd96
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 2 deletions.
4 changes: 4 additions & 0 deletions actionpack/CHANGELOG
@@ -1,5 +1,9 @@
*Rails 3.1.0 (unreleased)* *Rails 3.1.0 (unreleased)*


* URL parameters which return false for to_param now appear in the query string (previously they were removed) [Andrew White]

* URL parameters which return nil for to_param are now removed from the query string [Andrew White]

* ActionDispatch::MiddlewareStack now uses composition over inheritance. It is * ActionDispatch::MiddlewareStack now uses composition over inheritance. It is
no longer an array which means there may be methods missing that were not no longer an array which means there may be methods missing that were not
tested. tested.
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_dispatch/http/url.rb
Expand Up @@ -41,7 +41,7 @@ def url_for(options = {})
path = options.delete(:path) || '' path = options.delete(:path) || ''


params = options[:params] || {} params = options[:params] || {}
params.reject! {|k,v| !v } params.reject! {|k,v| v.to_param.nil? }


rewritten_url << (options[:trailing_slash] ? path.sub(/\?|\z/) { "/" + $& } : path) rewritten_url << (options[:trailing_slash] ? path.sub(/\?|\z/) { "/" + $& } : path)
rewritten_url << "?#{params.to_query}" unless params.empty? rewritten_url << "?#{params.to_query}" unless params.empty?
Expand Down
1 change: 0 additions & 1 deletion actionpack/lib/action_view/helpers/prototype_helper.rb
Expand Up @@ -131,7 +131,6 @@ def remote_function(options)
"new Ajax.Updater(#{update}, " "new Ajax.Updater(#{update}, "


url_options = options[:url] url_options = options[:url]
url_options = url_options.merge(:escape => false) if url_options.is_a?(Hash)
function << "'#{ERB::Util.html_escape(escape_javascript(url_for(url_options)))}'" function << "'#{ERB::Util.html_escape(escape_javascript(url_for(url_options)))}'"
function << ", #{javascript_options})" function << ", #{javascript_options})"


Expand Down
8 changes: 8 additions & 0 deletions actionpack/test/controller/url_for_test.rb
Expand Up @@ -311,6 +311,14 @@ def test_with_hash_with_indifferent_access
assert_equal("/c/a", W.new.url_for(HashWithIndifferentAccess.new('controller' => 'c', 'action' => 'a', 'only_path' => true))) assert_equal("/c/a", W.new.url_for(HashWithIndifferentAccess.new('controller' => 'c', 'action' => 'a', 'only_path' => true)))
end end


def test_url_params_with_nil_to_param_are_not_in_url
assert_equal("/c/a", W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :id => Struct.new(:to_param).new(nil)))
end

def test_false_url_params_are_included_in_query
assert_equal("/c/a?show=false", W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :show => false))
end

private private
def extract_params(url) def extract_params(url)
url.split('?', 2).last.split('&').sort url.split('?', 2).last.split('&').sort
Expand Down

0 comments on commit 03cbd96

Please sign in to comment.