patch 6677 - fixing URL helpers for nested resources

rubyonrails-core
1

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.

2

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

3

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.

4

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

5

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.

6

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

7

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?

8

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?

9

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.