This project is archived and is in readonly mode.
Path to extend named routes support for an object's to_params() method
-
Philip Hallstrom
As it currently exists this patch doesn't work for all cases. I'm looking into it now. Seems routes get generated a couple of different ways not always through the method I patched.
-
Philip Hallstrom
Figured out the problem. There is a route optimization method that runs prior to the rest of the routing code. The conditions were such that if there was a single argument which was not a hash it would optimize the route and not get a chance to call to_params on it. I've updated the attached patch to add an additional check to not optimize routes where the single argument responds_to?(:to_params).
-
Michael Koziarski
This, or something like it, could be a good fit for 2.3. But for now with 2.2 so close I'm going to hold off on reviewing it.
Have you released it as a plugin so others can try it?
-
Philip Hallstrom
I haven't. Thought about it, but it modifies two methods deep in Rails routing and it seemed any attempt at a plugin would be pretty fragile...
I'm actively using it now on a project and will update this ticket if I come across any issues...
-
Philip Hallstrom
One thing to keep in mind that I've just come across is that a model that has to_params() defined will cause some havoc with mapped resources and form_for. I'm not sure if there's a way to intercept that and not use to_params() even if defined if the route is a resource.
-
Daniel Schierbeck
Just an idea: since you've defined your route as
/schools/:state/:city/:id/:name, wouldn't it make sense forschool_path(obj)to simply inject the return values of callingobj.state.to_param,obj.city.to_param, ...? -
Philip Hallstrom
"Just an idea: since you've defined your route as /schools/:state/:city/:id/:name, wouldn't it make sense for school_path(obj) to simply inject the return values of calling obj.state.to_param, obj.city.to_param, ...?"
Not in every case. In the real world usage of these, the segments can't be mapped as easily as you've outlined.
You also run into the problem where you may have obj has_one state and state's to_param would return "123-washington". And for obj's url I only want "washington".
-
Philip Hallstrom
Michael - Now that 2.2 is out, is it a good time to take another look at this patch?
-
Michael Koziarski
I'm just not sold on the API to be honest. It just feels a little too coupled. f.ex what's the to_params return for a Photo when you have
post/1/photos/2 photos/2 user/4/photos/2
?
-
Philip Hallstrom
I agree about nested routes. I mentioned earlier that maybe the best thing to do is not call to_params() for nested routes since that doesn't make much sense in any case since the whole point of to_params is to generate a extremely generous url for SEO reasons.
I haven't looked to see if it's possible to skip over to_params() if the route is restful/nested. If someone knows if it's possible that seems like a reasonable thing to do.
-
Ryan Bates
That is a neat idea, but I don't know if it's completely necessary. You could create a helper method to handle this:
# helpers/paths_helper.rb def school_path(school) school_in_city_path(school.state.full_name, school.city.name, school.id, school.name.downcase) endThis way the route nesting isn't tied to the model, and you can handle route logic which spans multiple models.
However, there are a couple downsides to this. One is it's not available in controllers. To solve this, I usually make a PathsHelper module and include it in ApplicationController. The other problem is there's no _url variation. I'm not sure of the best way to handle that problem though.
-
Michael Koziarski
- State changed from new to wontfix
I agree with ryan here, the right approach here is to use a custom helper rather than modify the routing code base.
