namespacing breaks polymorphic_url

I’m currently using map.namespace to set the name_prefix to “admin/”. Doing this appears to break polymorphic_url (and thus form_for, url_for, etc) as it isn’t taking name_prefix into account. Quick hack would be to allow polymorphic_url to accept a namespace or name_prefix argument. Does anyone else have any better ideas to make this work?

Josh

Hi Josh,

I'm currently using map.namespace to set the name_prefix to "admin/". Doing this appears to break polymorphic_url (and thus form_for, url_for, etc) as it isn't taking name_prefix into account. Quick hack would be to allow polymorphic_url to accept a namespace or name_prefix argument. Does anyone else have any better ideas to make this work?

I'm not sure a namespace or name_prefix argument will work in all cases... consider that you could have namespaces and resources nested to any number of levels.

I was thinking that a syntax like the following might work though:

# routes.rb map.namespace :admin do |a|    a.resources :workshops do |w|      w.resources :sessions    end end

# views <%= url_for([:admin, @workshop, @session]) %> <%= link_to('Session', [:admin, @workshop, @session]) %> <% form_for([:admin, @workshop, @session] do |f| %> # ... <% end %>

The symbol would correspond to the namespace, and could be made to handle nested namespaces.

I'd apply a patch which would Dan's style work.

Please do investigate :slight_smile:

While I’m not a huge fan of the explicit array, it does fit with the current approach for handling nested resources. I got a version working last night which took a :name_prefix in the options, this should be easily adjusted to handle this case.

In the example above would it really be :admin, or would it need to be ‘admin_’ matching the appropriate namespace? More specifically, do you assume the ‘_’, or do you just use the literal string which would account for multiple nested namespaces?

Josh

It's really frustrating to see how much stuff is being bolted on here.

I've already written at length about how edge-cases (poly* resources) are being allowed to infect simpler cases:

http://rubyurl.com/jzN

And with this suggestion it just keeps getting worse.

By far the simplest and most common case is this:

form_for @something do ... end

That's easy. You haven't told form_for what the url is so it will best-guess it using @something.class and @something.new_record?

That doesn't work for you because you're doing something unusual with name_prefix or you've got a deeply nested resource? Sucks to be you for doing something complicated:

form_for @something, :url => polymorphic_path(@parent, @of, @something, :namespace => :something_arbitrary) do ... end

form_for *already* has a facility to supply a url that overrides any guesswork.

And did you see how polymorphic_path can have a richer interface?

Trying to ram a rich set of arguments down the throat of form_for by wrapping stuff up in an array is just plain ugly.

Take a look at this:

url_for([@parent, @child])

You know you want polymorphic_url to be called so you wrap up your arguments into an array so that they'll get passed through.

Then polymorphic_url tries to guess the url helper to be invoked. It builds a string based on the class of each argument because the auto-name_prefix convention demands it (otherwise it could just use the class of @child).

Then polymorphic_url calls the parent_child_url() helper, passing the arguments through.

parent_child_helper() unpacks the arguments into a single options hash and then calls... wait for it .... url_for().

Compare the two:

url_for([@parent, @child]) polymorphic_url(@parent, @child)

Seriously, what's clearer and easier to explain to a newcomer?

Save us all and stop trying to tunnel arguments to polymorphic_xxx through the method signature of url_for and form_for.

Trevor

It's really frustrating to see how much stuff is being bolted on here.

While you may find this particular situation frustrating, I think it's important to remember that everyone here is trying to achieve the same thing. A kickass web development framework.

I definitely think we need to be cautious about just how much we cram into these polymorphic situations, and perhaps we should take some out, but lets try and keep the tone of this list positive, and not assume malice or enterpriseyness on the part of a participant.

Koz,

I'm not assuming *any* malice or enterpriseyness here at all. I *know* that everyone has the best of intentions and that everyone involved in this is smart (in case that is being called into question but not being stated).

So in order to ensure the technical content of what I'm saying isn't derailed by my tone:

If I have offended anyone then I unequivocally aplogize; my email *was* harsh. It may help to know that I meant to hit "Save Now" and reread everything :frowning:

I would be very grateful if you could read beyond the overall tone and examine the technical merits of what I'm saying.

Trevor

I would be very grateful if you could read beyond the overall tone and examine the technical merits of what I'm saying.

I think that you may be right, we have to balance the 'neat' factor of having namespaces and polymorphic urls and the like magically working, with the need to have an understandable and 'obvious' api. Leaving aside implementation, what would a simpler api look like in your mind?

I've already written at length about how edge-cases (poly* resources) are being allowed to infect simpler cases:

One man's edge case is another's simple case. For me, polymorphic nested routes are common. I have stuff like /companies/1/tags and / people/1/tags as well as /people, /people/1, and /companies/6/people.

Trying to ram a rich set of arguments down the throat of form_for by wrapping stuff up in an array is just plain ugly.

I don't think it is. I think it's just using form_for as a delegate for url_for and ultimately polymorphic routes. I see nothing wrong with that. It's building on top of what we already have for the more involved cases while not complicating the simpler cases that already works today.

url_for([@parent, @child]) polymorphic_url(@parent, @child)

Seriously, what's clearer and easier to explain to a newcomer?

url_for is not intended to be used like this directly. It's just used as a gateway to polymorphic_url when called from higher-level helpers. So there will be no explanation needed.

Save us all and stop trying to tunnel arguments to polymorphic_xxx through the method signature of url_for and form_for.

I disagree. I think the majority of the hurt you're experiencing is coming from the fact that whatever application you've worked on didn't deal with multiple paths to the same models, like /people (all people) and /companies/5/people (just the people for company 6).

All the applications I've worked with deal with multiple paths to the same models. So that's what I've been involved with extracting. None of this stuff is about thought experiments, this is being extracted from real applications.

Naturally, none of this can fit all scenarios all the time. Which is why I really enjoy the layered approach we have. You're free to almost at any spot to do something different than what's baked into the framework. Even stuff like :has_many resources is just a thin wrapper that calls resource methods you can almost as easily call yourself.

In any case, it's great to hear about other use cases, but I would recommend (as Koz did earlier) that you turn down the tone of your arguments. The blog posting you link to is at least as inflammatory as your post to this group. Which is completely unnecessary. We're just interested in hearing about experiences.

The louder you shout, the lower we'll set the volume on our attention. And that would be a shame as you do seem to have some relevant inputs on this discussion. I'd hate to see them disregarded entirely because of your choice of words.

David,

The louder you shout, the lower we'll set the volume on our attention.

Thank you for taking the time to read what I've written despite my earlier tone.

> I've already written at length about how edge-cases (poly* resources) > are being allowed to infect simpler cases:

One man's edge case is another's simple case. For me, polymorphic nested routes are common. I have stuff like /companies/1/tags and / people/1/tags as well as /people, /people/1, and /companies/6/people.

There's a much better opportunity to explain my position on 'edge-case' below.

> Trying to ram a rich set of arguments down the throat of form_for by > wrapping stuff up in an array is just plain ugly.

I don't think it is. I think it's just using form_for as a delegate for url_for and ultimately polymorphic routes. I see nothing wrong with that. It's building on top of what we already have for the more involved cases while not complicating the simpler cases that already works today.

> url_for([@parent, @child])

<SNIP>

url_for is not intended to be used like this directly. It's just used as a gateway to polymorphic_url when called from higher-level helpers. So there will be no explanation needed.

I'd appreciate it if you would consider a couple of points:

* using url_for is already being described as a way to offset the changes introduced by auto-name_prefix (I link to 2 such articles in my blog post).

* form_for does not use url_for as a gateway as you stated - it (imho) *correctly* uses polymorphic_path() to generate the url

My feeling is that url_for should not delegate to polymorphic_url as it does now. If you want a polymorphic url or path, you should ask for one.

This would also help to concentrate any efforts to enrich form_for() in the right spot - polymorphic_xxx(). That way you would not have to try to overcome url_for's interface.

I believe these interface issues transcend any differences we may have (below) over polymorphics/polynymics and auto-name_prefix.

I disagree. I think the majority of the hurt you're experiencing is coming from the fact that whatever application you've worked on didn't deal with multiple paths to the same models, like /people (all people) and /companies/5/people (just the people for company 6).

On the contrary, I have *plenty* of situations like that (it's what I extracted resource_fu from).

I don't do it as a matter-of-course to satisfy a convention such as "only-1-layer-deep". For example, if 'all people' (/people) is not a meaningful concept for the application then I don't create a resource for it.

But when it has a place, I willingly use it.

My issue is not so much whether there are applications out there with multiple paths to the same models, it's the impact of trying to automatically accommodate them.

Polymorphic and Polynymic resources are by their very nature, DRY. So even though you have multiple paths to the same model, I'm guessing that you re-use many of the same actions/templates over and over.

With that in mind, consider this contrived example application:

map.resources :houses do |house|   house.resources :walls, :has_many => :damages   house.resources :windows, :has_many => :damages   house.resources :doors, :has_many => :damages   house.resources :damages end

The :damages resource appears four times, touching every other resource and the entire application seems to hinge on the concept Damage.

However, in terms of template and controller lines, Damage probably (simplistically) only accounts for about 20% the application total.

This is why I say that in my code poly* are an edge case and that I have far fewer poly* resources than non-poly* ones. It's an opinion borne out of counting relevant lines of code.

As such, I consider that anything the framework does to make Damage easier in this application isn't nearly as big a win as making Houses, Walls, Windows and Doors easier.

Of course I'm completely willing to accept that my experiences are unique.

Regards, Trevor

* using url_for is already being described as a way to offset the changes introduced by auto-name_prefix (I link to 2 such articles in my blog post).

People also used to talk about how to use with_scope in filters. Just because some folks get it wrong in the early days of an API doesn't mean the solution is a changed API.

* form_for does not use url_for as a gateway as you stated - it (imho) *correctly* uses polymorphic_path() to generate the url

My feeling is that url_for should not delegate to polymorphic_url as it does now. If you want a polymorphic url or path, you should ask for one.

I agree. You should never call url_for directly with the syntax. That form of calling url_for is for helpers. Such that all helpers that delegate to url_for can link to nested poly resources easily.

This would also help to concentrate any efforts to enrich form_for() in the right spot - polymorphic_xxx(). That way you would not have to try to overcome url_for's interface.

It's not really about url_for, it's about the delegation that goes through it. None of this is to enrich url_for for public consumption.

The :damages resource appears four times, touching every other resource and the entire application seems to hinge on the concept Damage.

However, in terms of template and controller lines, Damage probably (simplistically) only accounts for about 20% the application total.

First of all, I hate contrived examples. You can make a contrived example prove anything. I'm really only interested in discussion real code. Despite that, I still think that declaration is fine.

First of all, routes.rb shouldn't reflect the size of the underlying code base. It's about reflecting the resources exposed. Damages are exposed as 4 different base resources, walls for example only as one. Thus, it's fair for damages to take up more configuration space.

Second, we already have a fair number of techniques to deal with duplication like this. For example:

map.resources :houses do |house|   house.resources :damages   house.with_options(:has_many => :damages) do |fragile|     fragile.resources :walls     fragile.resources :windows     fragile.resources :doors   end end

But if you have a suggestion for an API that wraps this cleaner, but doesn't change the underlying generated routes, by all means shoot.

This is why I say that in my code poly* are an edge case and that I have far fewer poly* resources than non-poly* ones. It's an opinion borne out of counting relevant lines of code.

I believe that you're counting impact in the wrong place. Think of routes.rb as dealing with the impact of resources and URLs exposed and giving damage a premiere spot makes perfect sense (it's by far the most complex resource exposed).

(P.S.: The tone of this exchange was much improved. Thank you for paying attention to this.)

It's not really about url_for, it's about the delegation that goes through it. None of this is to enrich url_for for public consumption.

Okay, I understand and agree with that.

I haven't come across any spots where the framework "delegates through" url_for as we're describing. I can only find form_for() that delegates through polymorphic_path().

Would you consider a patch which removed this behavior (as un-needed) from url_for?

First of all, I hate contrived examples. You can make a contrived example prove anything. I'm really only interested in discussion real code. Despite that, I still think that declaration is fine.

I hate them too - but I'm not in a position to show any real code so I tried to put something together that reflected the ratios in my own code.

First of all, routes.rb shouldn't reflect the size of the underlying code base. It's about reflecting the resources exposed. Damages are exposed as 4 different base resources, walls for example only as one. Thus, it's fair for damages to take up more configuration space.

I definitely agree that it's fair for damages to take up more configuration space - that's actually my point.

But if you have a suggestion for an API that wraps this cleaner, but doesn't change the underlying generated routes, by all means shoot.

I'm not complaining that I have a way to make configuration easier or that routes.rb is not clean enough.

What I'm complaining about *is* the underlying routes (spec. their names).

In the example, I'm complaining about house_ - as in house_walls_path, house_windows_path, house_doors_path.

In my controllers and templates I type that stuff out *so* much more often than *damage_path yet :damages is the only route that actually needs disambiguation.

And I spend a lot less time configuring my routes.rb than I do coding controllers and templates so any routes.rb syntactic sugar like auto-name_prefix actually generates (and of course, maybe just for me) syntactic vinegar elsewhere.

I either have to explicitly switch it off with name_prefix => nil for every route not needing disambiguation or I have to refer to (imho) overly-verbose route names.

Either way though, given the constraints of limiting any solutions to ones that don't change underlying routes, it looks like there's not much I can offer.

Thanks for taking the time to discuss this.

(P.S.: The tone of this exchange was much improved. Thank you for paying attention to this.)

And thank-you for giving me the chance to approach this issue with the correct attitude.

Regards, Trevor

Didn’t realize my little thread would start so much discussion… That said, if I create a patch appending namespace to the front of the array is this still the approved direction?

I like it. Tobi mentioned doing just that when I first wrote the patch, but I wanted to get a basic solution out there and get some feedback going.

This is a bit off topic, but could anyone point me at some discussion about what the damages controller would look like in this situation. How much, and what code goes into determining what objects are to be setup?

For example. I have a situation where I have

:books, :has_many => :clips

/books/1/clips

This is fine, I can deal with this, but I would also like another part where I can look into others books

/public/username/books/1/clips

But I don’t really know what the controller should look like to set this up.

Any links would be great.

Cheers

http://dev.rubyonrails.org/ticket/8640

Daniel,

you're right in that it's slightly off-topic and probably a question better suited to the rubyonrails list (or IRC). That said, you've used my example to frame the question so I should probably give *some* sort of response.

Ultimately, the best advice I can give you is: solve the problem in front of you in a way that doesn't offend your taste and style. Here's what I mean:

Personally I like everything to be skinny (not just controllers) so I tend to have a lot of small and focussed objects that do only one job (but well, I hope).

I also favor named-route url helpers over generic "give me a url for these args, I have no idea what they are" because it's more expressive to me.

I don't automatically create top-level resources that mirror my nested ones - the idea being expressed has to actually exist (in a business sense) before it gets a url.

And finally, I find first-pass generalizations are often wrong. As in "hey, X is the same as Y" almost always turns into "well, they're *nearly* the same, I'll just add a few conditionals" which eventually becomes "oh, I forgot about these cases, I have to add more conditionals". After years of doing this I've realized that developers are very quick to refactor from the specific to the general, but are often reluctant to refactor in the opposite direction (google for "fearofaddingclasses" for related discussions).

So given that entire tug-of-war of my taste and style, the first thing I (often) reach for is "a specific controller for each route". I use a combination of inheritance, mixins and view re-use to avoid repetition. Once I have all the specifics nailed I check to make sure that my strategy isn't getting in the way. If it is, then it's a pretty simple matter to pull everything into a single controller to see how that looks.

You'll get completely different answers from other developers depending on their taste and style. Ask the question on the rails list to get more answers and take them out for a test-drive (thankfully we're not pouring concrete here).

HTH, Trevor

Thanx Trevor.

I didn’t even realise that this wasn’t the rails list. Forgot to look.

Thankyou for your response. It’s defintely thought provoking for me.

I’ll ask on the rails list to see if there is some kind of general consensus.

Cheers

Daniel

I’d apply a patch which would Dan’s style work.

Please do investigate :slight_smile:

http://dev.rubyonrails.org/ticket/8640

All this lively conversation has distracted us from the original intent of this thread, which is getting namespace support into polymorphic urls. Please take a look at the submitted patch and let me know if there are any changes requested.

I've put in some bug fixes and improvements to polymorphic routes, which all should integrate fine with the above namespaced poly routes patch (any conflicts should be trivial to resolve.)

1. added poly_url and poly_path method aliases for polymorphic_url and polymorphic_path, including action_ prefixed methods (new_poly_url, edit_poly_url, formatted_poly_url). Much easier to type this shorter form. http://dev.rubyonrails.org/ticket/8725

2. fixed polymorphic_path so that it accepts an options hash like polymorphic_url -- necessary when you need to feed in an action, e.g. polymorphic_path(@article, :action => 'new') http://dev.rubyonrails.org/ticket/8720

3. fixed polymorphic_url so that it generates :action => 'new' routes correctly with an array, e.g., calls the method "article_new_comment_url" instead of "new_article_comment_url" http://dev.rubyonrails.org/ticket/8719

4. Added handling of nil parent objects in an array argument to polymorphic url -- so, polymorphic_url([@parent, @child]) generates an url with the parent (/parents/1/children/5) unless @parent is nil, in which case it just generates the route for the child (/children/5). As a result, you can build views that handle any nesting situation without adding conditional logic -- <%= link_to @person.name, [@company, @person] %> can generate both "/companies/1/people/5" and "/ people/5", depending upon if @company is set in the controller or not. http://dev.rubyonrails.org/ticket/8705