wrong url construction with edge rails if params have the same names as symbols in routes

Thanks for the good description!

The specific error is in extra_keys. Instead of passing the (possibly merged) hash, we should just pass the options hash. Thus we can avoid passing the recalled parameters altogether. Jamis: Can you see anything wrong with this approach?

The above diff makes said change. (Although it needs a unit test before applying.)

Actually, nicholas, I think he's describing something different. He's describing a case where the resulting URL is wrong, independent of the query string.

That said, I can't seem to duplicate the error in a test case:

  def test_overriding_recalled_parameter_during_generation
    @rs.draw do |m|
      m.connect 'sprint/show/:sprint', :controller => "task", :action => "index"
      m.connect 'task/foo/:sprint', :controller => "task", :action => "foo"
    end
    given = { :action => "foo", :sprint => "1" }
    recall = { :controller => "task", :action => "index", :sprint => "205" }
    assert_equal "/task/foo/1", rs.generate(given, recall)
  end

After adding that test to the LegacyRouteSetTests suite in test/controller/routing_test.rb, all tests still pass. Is that test indicative of the problem the OP saw? Is there any way the OP could post their routes.rb file?

Also, I should note: the following construction is not supported:

  :params => { :sprint => 1 }

That _will_ give the result you describe. However, specifying :sprint => 1 directly ought to work, and if it is not, that is a bug. I cannot duplicate it, however.

(In [5709]) Fix a bug in Routing where a parameter taken from the path of the current request could not be used as a query parameter for the next. Closes #6752.

This patch did not make it into Rails 1.2.2..was it supposed to? Lack of this patch causes upgrading to Rails 1.2 to be excessively painful as one must make sure to go through all the routes in the application to see if they still work. Any chance of tagging this as 1.2.2 so that at least people who have externalized rails can get the update...?

The "1.2 regressions" milestone indicates that.

(I can't tell whether the fix made it to the 1-2-stable branch, though.)

Applied to stable in [6390]. Please try against the 1-2-stable branch and report success/failure.

I guess what was not clear to me is that I should be running 1-2-stable? My understanding is that if I wanted the latest rails I should be freezing TAG 1-2-2 because that was the last official announced release. I also assumed any critical errors such as the routing should be backpatched into that TAG?

Rails official blog announcements imply that those who want stable production releases should be running against the released tags. not the 'stable branch'...

1.2.2 is the latest release, skwp, and it's tagged as such. A tag is a snapshot in time; a branch is an ongoing line of development.

Backports don't go to tags, they go to branches, which are released by creating new tags.

This is a bugfix going into the next release, which will be tagged 1.2.3 from the 1-2-stable branch.

I don't understand what's confusing or misleading here. Perhaps it was technoweenie's suggestion that you preview the fix prior to 1.2.3 release by working with the 1-2-stable branch directly?

Ok I understand. Some people use tags as branches since svn really has no difference between a tag and a branch. For example when I release I typically push high priority fixes into the tags that have been released. Low priority fixes go into the next release branch.

I just assumed that something that seems to break most of routing should be released as soon as possible especially if it has been tested. Whether by patching the tags or pushing a 1.2.2.1 release or something like that...

I have tested 1-2-stable branch and it appears to work correctly with this patch. Thanks.

Committing to a tag is a really bad idea, but that's another topic.

This issue is important but the sensationalism is unnecessary. Constructively participate in development: raise issues on the mailing list, write tests, contribute patches.

Thanks for your report.