Rails core - Nov 2006 - 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.

--~--~---------~--~----~------------~-------~--~----~
 You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> 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

--~--~---------~--~----~------------~-------~--~----~
 You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On 11/24/06, Alain Ravet <alain.ravet@gmail.com>
wrote:
>
>    > 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.
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.

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
 You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

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

On 23-Nov-06, at 2:00 PM, Michael Koziarski wrote:
>
> On 11/24/06, Alain Ravet <alain.ravet@gmail.com> wrote:
>>
>>> 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.
>
> 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.
>
> -- 
> Cheers
>
> Koz
>
> 
--~--~---------~--~----~------------~-------~--~----~
 You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> 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.

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
 You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

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

On 23-Nov-06, at 3:33 PM, Michael Koziarski wrote:
>
>> 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.
>
> -- 
> Cheers
>
> Koz
>
> 
--~--~---------~--~----~------------~-------~--~----~
 You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> 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?
-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
 You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

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?



On 11/23/06, Michael Koziarski <michael@koziarski.com>
wrote:
>
>
> > 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?
> --
> Cheers
>
> Koz
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
 You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Fri, 2006-11-24 at 00:52 -0500, Nicholas Seckar
wrote:
> 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.
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.

--~--~---------~--~----~------------~-------~--~----~
 You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---