From 03cbd9672cfc142842c95aae0fb27b8eb27e6990 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Wed, 9 Mar 2011 14:44:25 +0000 Subject: [PATCH] Filter params that return nil for to_param and allow through false values --- actionpack/CHANGELOG | 4 ++++ actionpack/lib/action_dispatch/http/url.rb | 2 +- actionpack/lib/action_view/helpers/prototype_helper.rb | 1 - actionpack/test/controller/url_for_test.rb | 8 ++++++++ 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index fc3410ba6e..d9fe45d897 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,9 @@ *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 no longer an array which means there may be methods missing that were not tested. diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 535ff42b90..ac0fd9607d 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -41,7 +41,7 @@ module ActionDispatch path = options.delete(:path) || '' 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 << "?#{params.to_query}" unless params.empty? diff --git a/actionpack/lib/action_view/helpers/prototype_helper.rb b/actionpack/lib/action_view/helpers/prototype_helper.rb index b93a072abb..18e303778c 100644 --- a/actionpack/lib/action_view/helpers/prototype_helper.rb +++ b/actionpack/lib/action_view/helpers/prototype_helper.rb @@ -131,7 +131,6 @@ module ActionView "new Ajax.Updater(#{update}, " 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 << ", #{javascript_options})" diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb index 1c28da33bd..3f3d6dcc2f 100644 --- a/actionpack/test/controller/url_for_test.rb +++ b/actionpack/test/controller/url_for_test.rb @@ -311,6 +311,14 @@ module AbstractController assert_equal("/c/a", W.new.url_for(HashWithIndifferentAccess.new('controller' => 'c', 'action' => 'a', 'only_path' => true))) 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 def extract_params(url) url.split('?', 2).last.split('&').sort