Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

#routes_for method doesn't work at all

#3231

(in AR::Routing::RouteSet)

In ticket #3023 Removed unused routing method, #routes_for_controller_and_action is deprecated in favour of #routes_for. I am using #routes_for_controller_and_action, so am keen to change up.

Trouble is, #routes_for doesn't work at all. Here's the code form line 463 of route_set.rb

def routes_for(options, merged, expire_on)
  raise "Need controller and action!" unless controller && action
  controller = merged[:controller]
  merged = options if expire_on[:controller]
  action = merged[:action] || 'index'

  routes_by_controller[controller][action][merged.keys][1]
end

The method always dies on the first line, since controller and action are not in local scope. (and are not instance methods either).

There were no tests for this method, so I'm not sure of the intent. It looks like the raise should be relocated to line 5 of the method.

Reported by Ian White · September 18th, 2009 @ 08:49 AM

State: wontfix
Milestone: 2.3.6
Assigned to: josh josh
Importance: Medium

Activity

  1. josh
    josh
    • Milestone set to 2.3.6
    • State changed from new to open

    These methods are supposed to be "private" api. I'm willing to patch them if they are broken in 2.x but they will be gone in Rails 3.0.

    However, I'm interested in your use case for them. Maybe we can come up with a better api we can officially expose.

    September 18th, 2009 @ 03:22 PM

  2. Ian White
    Ian White

    My use case is for a plugin called resources_controller. This plugin enables controllers to load nesting resources by introspecting the route that invoked the controller (enabling stuff like one controller for a polymorphic assoc, instead of many controllers, one for each (parent,child) pair).

    Anyway, that aside, I originally solved this problem by patching Routing to store the the route that was recognised, and pass this along with the request to the controller. This worked, but I gave up on it, because I didn't want to rely on internals of Routing in the patch (And the plugin is supported on all rails 2.x versions)

    So I swapped this for relying of being able to re-recognise the route in the controller (this was waaaay back before a 'plugin API' was ever on the cards). I used #routes_by_controller_and_action to get a subset of routes to recognize, to make this process cheaper.

    Bottom line: I want to ask what route was recognised in a particular request. I'm not wedded to #routes_for, that's just a stepping stone.

    I think that being able to know what route invoked a controller is a nice bit of reflection, and it enables lots of cool stuff. If you agree, I'd happily write a patch for this (when I did it a year and a half ago it was a two liner + test).

    But back to this ticket, I think that either the #route_for method should be corrected, or obliterated, as it just doesn't work. If the latter, then the deprecation warning should be changed as well.

    September 18th, 2009 @ 06:42 PM

  3. josh
    josh

    Lets fix this in 2.3 for now. Does removing "raise "Need controller and action!" unless controller && action" fix the issue?

    September 18th, 2009 @ 07:12 PM

  4. Ian White
    Ian White

    @Josh, it doesn't really fix the issue as #routes_for and #routes_for_controller_and_action have different signatures, and return different things. I was checking out #routes_for only because of the deprecation warning that is issued when calling #routes_for_controller_and_action.

    In terms of my issue, I'm going to take a different route that avoids relying on the private API, so I'm not so worried about how this issue gets resolved.

    I just did a search, and #routes_for is never called, and is only referenced in the deprecation warning, so I think it might be a red herring that should go. Unless it's part of a refactoring effort that's not fully complete that is.

    September 19th, 2009 @ 08:38 AM

  5. josh
    josh
    • State changed from open to wontfix

    Yeah, those routes_for are just dead code. Should of been marked as private way back when.

    K, going close out this ticket for now. If you come up with a patch for 2.x, just let me know.

    September 19th, 2009 @ 04:27 PM