patch 6677 - fixing URL helpers for nested resources

Could someone have a look at my patch for ticket #6677:
http://dev.rubyonrails.org/ticket/6677

It's a simple fix to the behaviour of URL helpers when passed bare
objects as parameters: by pairing objects with dynamic route segments
starting at the end of the list instead of the beginning we can fall
back on the classic URL generation behaviour of filling missing segments
from the request parameters.

The practical upshot is that this small change allows the
scaffold_resource generated code to work unchanged when nested within
another resource - at the moment much of the scaffold route generation
calls need altering if you want to use nested resources.

It also provides a simple solution to get simply_helpful's form_for
extension working with nested routes.

I think this should be in 1.2. Thoughts?

-Mark.

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

   > It's a simple fix to the behaviour of URL helpers when passed bare

I agree: it's so unlike Rails to be forced to write
     book_url(article.department.store, article.department, article)
when
     book_url(article)
should do.
David Black has filled that hole with his inferred_routes plugin, but
I'm sure a great lot of upgraders would be surprised by the current
core behaviour, and that could translate in unnecessary list traffic
and bug reports.

Alain Ravet

The boat has sailed on the featureset for 1.2, so it won't be in that.

Also, I'm not so sure it's that cut and dried. When discussed on
this list, there were a number of commenters who came up with
situations (like taggings) where you could have them as resources
nested under multiple parents.

I think perhaps such functionality would be better suited to simply helpful.

Hey,

regarding resources nested under multiple parents - in that situation
don't you need to have a :name_prefix so that the defined helper
methods don't get overwritten?

To put it another way, there can be only one book_url and it has its
own unique position in its own unique nested location, regardless of
whether the books controller is used in a nested resource elsewhere.
As such, book_url can safely make assumptions about how to fill in
any unspecified arguments.

Or have I completely missed something?

Regards,
Trevor

regarding resources nested under multiple parents - in that situation
don't you need to have a :name_prefix so that the defined helper
methods don't get overwritten?

That's a good point, the context we'd discussed it was within
simply_helpful where it's link_to :name, book.

The problem is, I really don't like the idea of having to modify
routing to know about active record associations. So lets start this
stuff in simply helpful, and see where we can go from there.

Hey again,

the patch - http://dev.rubyonrails.org/ticket/6677

has nothing to do with active record associations (are you confusing
it with David Black's plugin?), and it doesn't modify routing as
such. It just changes the way that the generated url helpers pick-
off and process positional arguments.

Trev

Hey again,

the patch - http://dev.rubyonrails.org/ticket/6677

has nothing to do with active record associations (are you confusing
it with David Black's plugin?), and it doesn't modify routing as
such. It just changes the way that the generated url helpers pick-
off and process positional arguments.

Yeah, that discussion drifted miles from your original point :).

Perhaps nicholas can chime in here?

Echoing comment on ticket:

We need to decide if reversing the order is a special case, or if all
positional parameters ought to be in reversed order.

I suspect the former is more the case. If so, then we can add support for

an explicit order to be provided. Then we can update the resources call to
specify whatever order we feel makes sense for simply restful.

Jamis – does this sound good?

Just to be clear, the patch doesn't reverse the order that you need to
supply positional parameters to the URL helper methods. It also doesn't
change the behaviour of these methods in the case where you pass the
same number of positional parameters are there are dynamic segments in
the corresponding named route. And it isn't specific to resource-style
routes, it's just that resources have made product_path(@product) a much
more common idiom.

It does change the behaviour when you pass fewer positional parameters
than there are dynamic segments - at the moment this case will typically
give you either a URL that is meaningless (because the positional
parameter is used to fill in the wrong dynamic segment) or raise an
exception because there are insufficient parameters. By pairing
parameters with segments starting at the end of both lists the patch
makes the URL helper do what feels like the right thing: falling back on
the request parameters to supply the missing dynamic segments as per
standard URL generation behaviour.

The calls to 'reverse' in the patch are just because that's the simplest
way of implementing the behaviour: reverse both the parameter list and
the segment keys list, then pair up corresponding elements. The order
doesn't actually matter as the pairs are stuffed into a hash after that
step anyway.

-Mark.