Merge branch 'master' into nested_has_many_through

9 files changed, 22 insertions, 7 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/README.rdoc b/actionpack/README.rdoc
index a28d78f688..3661d27d51 100644
--- a/actionpack/README.rdoc
+++ b/actionpack/README.rdoc
@@ -323,7 +323,7 @@ The latest version of Action Pack can be installed with Rubygems:
 
 Source code can be downloaded as part of the Rails project on GitHub
 
-* http://github.com/rails/rails/tree/master/actionpack/
+* https://github.com/rails/rails/tree/master/actionpack/
 
 
 == License
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_dispatch/routing/route.rb b/actionpack/lib/action_dispatch/routing/route.rb
index 08a8408f25..eae9d4ea6d 100644
--- a/actionpack/lib/action_dispatch/routing/route.rb
+++ b/actionpack/lib/action_dispatch/routing/route.rb
@@ -1,3 +1,5 @@
+require 'active_support/core_ext/module/deprecation'
+
 module ActionDispatch
   module Routing
     class Route #:nodoc:
@@ -45,6 +47,7 @@ module ActionDispatch
       def to_a
         [@app, @conditions, @defaults, @name]
       end
+      deprecate :to_a
 
       def to_s
         @to_s ||= begin
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 61053d4464..b28f6c2297 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -333,7 +333,7 @@ module ActionDispatch
       def add_route(app, conditions = {}, requirements = {}, defaults = {}, name = nil, anchor = true)
         raise ArgumentError, "Invalid route name: '#{name}'" unless name.blank? || name.to_s.match(/^[_a-z]\w*$/i)
         route = Route.new(self, app, conditions, requirements, defaults, name, anchor)
-        @set.add_route(*route)
+        @set.add_route(route.app, route.conditions, route.defaults, route.name)
         named_routes[name] = route if name
         routes << route
         route
diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb
index 6df86ae65a..48abf119f1 100644
--- a/actionpack/lib/action_view/helpers/form_helper.rb
+++ b/actionpack/lib/action_view/helpers/form_helper.rb
@@ -266,8 +266,8 @@ module ActionView
       # === Removing hidden model id's
       #
       # The form_for method automatically includes the model id as a hidden field in the form.
-      # This is used to maintain the correlation between the form data and it's associated model.
-      # Some ORM systems do not use id's on nested models so in this case you want to be able
+      # This is used to maintain the correlation between the form data and its associated model.
+      # Some ORM systems do not use IDs on nested models so in this case you want to be able
       # to disable the hidden id.
       #
       # In the following example the Post model has many Comments stored within it in a NoSQL database,
diff --git a/actionpack/lib/action_view/helpers/prototype_helper.rb b/actionpack/lib/action_view/helpers/prototype_helper.rb
index f3a429fcce..18e303778c 100644
--- a/actionpack/lib/action_view/helpers/prototype_helper.rb
+++ b/actionpack/lib/action_view/helpers/prototype_helper.rb
@@ -103,7 +103,7 @@ module ActionView
                        :form, :with, :update, :script, :type ]).merge(CALLBACKS)
 
       # Returns the JavaScript needed for a remote function.
-      # See the link_to_remote documentation at http://github.com/rails/prototype_legacy_helper as it takes the same arguments.
+      # See the link_to_remote documentation at https://github.com/rails/prototype_legacy_helper as it takes the same arguments.
       #
       # Example:
       #   # Generates: <select id="options" onchange="new Ajax.Updater('options',
@@ -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/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb
index 2ce109ea99..3e2ddffa16 100644
--- a/actionpack/lib/action_view/test_case.rb
+++ b/actionpack/lib/action_view/test_case.rb
@@ -185,6 +185,7 @@ module ActionView
         @request
         @routes
         @templates
+        @options
         @test_passed
         @view
         @view_context_class
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