default_url_options is being ignored by named route optimisation

Here's a patch that checks for the existence of a non-blank #default_url_options and bypasses named route optimization: http://github.com/chuyeow/rails/...

Attaching a patch instead - gonna delete my branch.

+1 applies cleanly on 6f20efd (Wed Apr 30 23:30:50 2008 -0500)

I've been bitten by this one, thanks for the patch

+1 looks good, better than my local solution to the referenced ticket :) Patch applied fine to 74436d (Thu May 1 10:21:46 2008 +0100) after removing the empty line changes in action_controller/base.rb

For the next major release we will probably deprecate / remove default_url_options. But in the meantime nice patch.

Hmm, pity to see it go. I found it useful in cases where you want to add some parameters to the urls based on if some methods on a controller return a value or return nil.

E.g. an url may need a section_id to identify the area it's accessed from, but this is not needed always and can change dynamically. In this case you can add { :section_id => SomeController.section_id } to the default_url_options and don't have to worry about it anymore in each generated url.

Is there are better way to archive this?

Daniel

Thanks for applying the patch.

I agree with Daniel. I found the default_url_options quite useful for my app where I had to redirect from a subdomain to the main domain name for some pages. Without default_url_options, I would have to insert a hash with hostname and port into every url call.

It would be better to fix the optimisation of routes to take into account values from the default_url_options hash.

We definitely need something else which is like default_url_options, which is why we haven't removed it yet :).

Since this commit, all of my named_routes are failing with

ArgumentError: wrong number of arguments (1 for 0)
        from (eval):2:in `default_url_options'

Example: (On a clean rails app, with rails at 6a6b439)

  # (in routes.rb)
  map.resources :things

$ script/console

>> include ActionController::UrlWriter
=> Object
>> things_path
ArgumentError: wrong number of arguments (1 for 0)
        from (eval):2:in `default_url_options'
        from (eval):2:in `things_path'
        from (irb):3

I'm investigating a patch

Ack I see why that's a problem (and why it wasn't covered by existing tests nor my test in the applied patch) - default_url_options takes a single argument in AC::B but is simply a (hash) attribute in ActionController::UrlWriter.

Ian thanks for spotting this!

I'm attaching a patch that is a quick fix for this. Basically it gives AC::B#default_url_options a default argument. I didn't want to remove it since I imagine that would involve deprecation.

Btw, now I'm more convinced that default_url_options does need some cleaning up/re-thinking!

Here's a testcase that fails on 6a6b43 but passes on the previous commit 437f918

(you have to paste the test into 437f918- the diff doesn't apply)

No worries, uploaded my test patch without seeing Cheah Chu Yeow's updates updates on this ticket, you can just ignore it

Does this need to re-opened as a new ticket? I notice that it's status is 'resolved'.

apostrophe man commin to get me... I wish I could edit lighthouse comments

Actually I see that Cheah Chu Yeow's patch doesn't include a regression test, so it might be useful to use the diff I attached for that.

Too slow! I got it...