[PATCH] Fix Array.to_param in url_for (including 1-2-stable branch)

Thanks Jerry! I successfully applied your patch and it fixed the issue for me.

I'm confused as to what the :path => Array construct does. I can't see that in the documentation. Hopefully Jamis or someone else can blog about it and explain it to me... :-)

Here is a hack which I had to put in my environment.rb file to overcome this bug, while still continuing to use Rails 1.1.2.

class Array
  def to_param
    self
  end
end

This patch isn't really adequate, because if we actually wanted to construct a url that passed a ruby array named 'path', it wouldn't work. If someone actually wanted to pass a string "a/b/c", the correct thing to do would be %w(a b c).join('/').

peter - I also cannot find anything in the rails documentation about :path having special meaning in routes. Jeremy, could you please elaborate on this?

The documentation for ActiveSupport::CoreExtensions::Array::Conversions.to_string says:

"When an array is given to url_for, it is converted to a slash separated string."

I think the bug here is that this is just the wrong thing to do, and ssinghi's solution is correct.

For the record, someone has a patch that adds proper support for nested arrays/hashes to url_for here. It also changes the array to_params method to return the object, rather than a joined string.

FireRabbit, you can use '/*foo' in routes to pull out a param spanning multiple '/', like a file path.

The url generator should distinguish path and query parameters and substitute :foo => %w(a b c) as a/b/c if present in the route or foo[]=a&foo[]=b&foo[]=c if not.

Also see the work in #7462, which I've pointed back here.

Could you guys get together and combine the best of these patches, and use the #to_query introduced in [6038]?

Thanks for looking at #7462

Use of #to_query makes the patch lighter, as it obsoletes recursive #add_query_values I introduced there. But please note, that I had to patch to_query by adding extra to_s call there. Thats because TrueClass has its to_param method returning self, and cgi-escaping on it with gsub fails.

I left my previous test cases unchanged, so hashes & recursion seems like working fine.

I just added a patch that adds a couple of small tweaks to bgipsy's work.

The unit test actionpack/test/controller/routing_test.rb was failing because the new build_query_string method uses to_query which quotes the query string keys, while the previous build_query_string did not. This test was expecting an unquoted key and failed. This patch fixes that.

The other change is that the query string parameters in build_query_string and Hash#to_query now sort the keys in the query string. The reason for that is that the keys in a Hash are not in any gauranteed order. This means there is a (slight) chance the query strings in your public URLs may not be consistent given the same key/value pairs. For browser and proxy caches you would want consistency in the URLs. An added bonus is that ordering the keys makes testing the generated query strings simpler.

Committed in [6343]. Thanks for pitching in, all.

Backport of [6038], [6039], [6148] and [6343] to branches/1-2-stable@7831.

I've attached a backport of the fixes in trunk for 1-2-stable branch. It passes all tests and works ok on my 1.2.4 freezed rails.

+1, tested against r7833.

(In [7834]) Backport: allow array and hash query parameters. Array route parameters are converted/to/a/path as before. Closes #7047.