Hi all I discussed this with Rick this morning. I''ve written a quick patch to simply_helpful that will allow you to define a to_params class method on your model to allow it to change what parameters are passed to your named routes. This is useful when you have nested resources like this: map.resources :projects do |project| project.resources :things end When you use form_for to edit an existing Thing, it used to only pass the Thing record to the named route, generating the wrong route of course. Instead, it needs to pass thing.id and thing.project_id like so: thing_url(:project_id => thing.project_id, :id => thing.id) I''ve attached the patch to this email for others to have a quick look. To use it, you''d just do something like this: class Thing < ActiveRecord::Base def to_params; [:project_id, :id]; end end I''ve left it like this to avoid touching activerecord or the routing code for now, as Rick suggested. What do you guys think? Dave -- Dave Goodlad dgoodlad@gmail.com or dave@goodlad.ca http://david.goodlad.ca/ --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On Mon, 2006-10-16 at 08:46 -0700, David Goodlad wrote:> Hi all > > I discussed this with Rick this morning. I''ve written a quick patch > to simply_helpful that will allow you to define a to_params class > method on your model to allow it to change what parameters are passed > to your named routes. This is useful when you have nested resources > like this: > > map.resources :projects do |project| > project.resources :things > end > > [...] > > What do you guys think?I''ve been attacking the same problem myself over the last few days, but I went for a different approach. Given that you can already use something like ''edit_thing_path(@project, @thing)'' I thought it would be consistent to use the same kind of approach in form_for, so I''ve changed it so you can use ''form_for @project, @thing do ...''.>From a few days experience of working with this approach the main snagis probably that you really need to have loaded records for each of the resources in your hierarchy (i.e. projects and things in our example). Generally that''s not bad practice anyway as you''ll probably want to traverse the model to ensure the combination of primary keys is valid, but there might be reasons not to. In theory you could use ''form_for @thing.project_id, @thing do ...'', but if there''s a possibility that the result of @thing.project_id is a String it becomes impossible to distinguish this usage of form_for from the original ''form_for "project", @project, :url => ... do ...''. FWIW, adding the to_params method to the model feels like the wrong place to put it to me. The contents of array that you''re returning are determined by the controllers and the routing configuration, so it feels like we''re pushing knowledge into the model that doesn''t belong there. I''d also wonder how it would handle cases where you have the same model object exposed as a resource with two different nested routes (e.g. the tags model used as an example in the documentation for the :name_prefix option). Hmm; an alternative solution that crosses my mind would be to allow an array parameter to form_for: form_for [ @thing.project_id, @thing ] do ... 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 -~----------~----~----~----~------~----~------~--~---
> Hmm; an alternative solution that crosses my mind would be to allow an > array parameter to form_for: > > form_for [ @thing.project_id, @thing ] do ... > > Thoughts?I like this a lot better. I don''t think the model should know anything about the routing requirements either. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 10/18/06, DHH <david.heinemeier@gmail.com> wrote:> > > Hmm; an alternative solution that crosses my mind would be to allow an > > array parameter to form_for: > > > > form_for [ @thing.project_id, @thing ] do ... > > > > Thoughts? > > I like this a lot better. I don''t think the model should know anything > about the routing requirements either.Does it need too? For this: map.resources :projects do |projects| projects.resources :tasks end map.resources should be able to store the necessary metadata to generate: task_url(task.project, task) Or am I missing something? -- 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 -~----------~----~----~----~------~----~------~--~---
On 18/10/2006, at 8:52 AM, Michael Koziarski wrote:> For this: > > map.resources :projects do |projects| > projects.resources :tasks > end > > map.resources should be able to store the necessary metadata to > generate: > > task_url(task.project, task) > > Or am I missing something?As long as we can handle non-trivial resource routes, otherwise it''s pretty much useless in a production app. I think we''d at least need some type of support for name_prefix in the helpers to distinguish between similarly named resources (see :tags below): ActionController::Routing::Routes.draw do |map| map.resources :people, :member => { :photo => :any, :password => :put, :personal_details => :put, :as_array => :get, :message => :any } do |people| people.resources :tags, :controller => ''taggings'', :name_prefix => ''person_'' people.resources :relationships, :name_prefix => ''person_'' end map.resources :tags, :member => { :people => :get }, :collection => { :messages => :get } do |tags| tags.resources :messages, :controller => ''tag_messages'', :name_prefix => ''tag_'' end # ...snip... end -- tim --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> As long as we can handle non-trivial resource routes, otherwise it''s > pretty much useless in a production app. I think we''d at least need > some type of support for name_prefix in the helpers to distinguish > between similarly named resources (see :tags below):I''m not sure I follow, shouldn''t there be *one* url per resource? It''s Uniform Location? -- 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 -~----------~----~----~----~------~----~------~--~---
Im with koz on this - I love using nested resources and the named routes, but its always struck me as odd that you have to pass in all of the objects back up the hierarchy for a nested named route - it would be much nicer if Rails could work this out for you, so instead of: task_url(task.project, task) All you need is: task_url(task) It should be able to get the project from the task (task.project). Cheers Luke On 17 Oct 2006, at 23:52, Michael Koziarski wrote:> > On 10/18/06, DHH <david.heinemeier@gmail.com> wrote: >> >>> Hmm; an alternative solution that crosses my mind would be to >>> allow an >>> array parameter to form_for: >>> >>> form_for [ @thing.project_id, @thing ] do ... >>> >>> Thoughts? >> >> I like this a lot better. I don''t think the model should know >> anything >> about the routing requirements either. > > Does it need too? > > For this: > > map.resources :projects do |projects| > projects.resources :tasks > end > > map.resources should be able to store the necessary metadata to > generate: > > task_url(task.project, task) > > Or am I missing something? > > -- > 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 -~----------~----~----~----~------~----~------~--~---
On 10/18/06, Luke Redpath <contact@lukeredpath.co.uk> wrote:> > Im with koz on this - I love using nested resources and the named > routes, but its always struck me as odd that you have to pass in all > of the objects back up the hierarchy for a nested named route - it > would be much nicer if Rails could work this out for you, so instead of: > > task_url(task.project, task) > > All you need is: > > task_url(task) > > It should be able to get the project from the task (task.project).So, instead of all this hackery to push the right params to the named route method, we''d need to modify routing to determine what method to call on the object automatically. I agree, that''s the cleanest way in terms of usage. It might get a little messy when you start dealing with some more complex routes. On the other hand, almost anything would be better than the default right now, which puts the item''s id in the wrong place in the url (ie: task_url(task) generates /projects/#{task.id}/tasks/ iirc.). Might as well continue rewarding people for following conventions, but make sure there''s the ability override them. I may or may not have any time during the rest of the week to try and write up a patch for the routes to do this, but hopefully I will this weekend. Dave -- Dave Goodlad dgoodlad@gmail.com or dave@goodlad.ca http://david.goodlad.ca/ --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Dave- I am absolutely with you on this. We have been doing all of our internal projects with nested REST routes, and have found this to be really, really frustrating. So much repetition! Especially when routes should be able to see the params nearby. For a User/Photo, we definitely know the User by several methods (params[:user_id] and @photo.user_id), but the path seems to ignore it. -hampton. On 10/18/06, David Goodlad <dgoodlad@gmail.com> wrote:> > On 10/18/06, Luke Redpath <contact@lukeredpath.co.uk> wrote: > > > > Im with koz on this - I love using nested resources and the named > > routes, but its always struck me as odd that you have to pass in all > > of the objects back up the hierarchy for a nested named route - it > > would be much nicer if Rails could work this out for you, so instead of: > > > > task_url(task.project, task) > > > > All you need is: > > > > task_url(task) > > > > It should be able to get the project from the task (task.project). > > So, instead of all this hackery to push the right params to the named > route method, we''d need to modify routing to determine what method to > call on the object automatically. I agree, that''s the cleanest way in > terms of usage. > > It might get a little messy when you start dealing with some more > complex routes. On the other hand, almost anything would be better > than the default right now, which puts the item''s id in the wrong > place in the url (ie: task_url(task) generates > /projects/#{task.id}/tasks/ iirc.). Might as well continue rewarding > people for following conventions, but make sure there''s the ability > override them. > > I may or may not have any time during the rest of the week to try and > write up a patch for the routes to do this, but hopefully I will this > weekend. > > Dave > > -- > Dave Goodlad > dgoodlad@gmail.com or dave@goodlad.ca > http://david.goodlad.ca/ > > > >--~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
One thing I''ve been thinking about is having non-trivial named routes
take a block. The block would take the object that is passed into the
generated method my_named_route_for(some_object). The block would
then construct a url hash appropriate for that object. This seems to
be a little more DRY to me, it would allow us to remove code from the
model or helpers that seem to have too much intimate knowledge of
what''s in routes.rb.
Below is the test I wrote that demonstrates how it might work:
def test_named_route_with_block
rs.add_named_route(:page, ''page/:title/:chapter'',
:controller =>
''content'', :action => ''show_page'') do
|page|
{ :title => page.title, :chapter => page.chapter }
end
x = setup_for_named_route.new
test_page = Struct.new(:title, :chapter).new
test_page.title = ''new stuff''
test_page.chapter = ''chapter stuff''
assert_equal({:controller => ''content'', :action =>
''show_page'',
:title => test_page.title, :chapter => test_page.chapter, :use_route
=> :page},
x.send(:page_url_for, test_page))
end
-Lee
On 10/18/06, David Goodlad <dgoodlad@gmail.com>
wrote:>
> On 10/18/06, Luke Redpath <contact@lukeredpath.co.uk> wrote:
> >
> > Im with koz on this - I love using nested resources and the named
> > routes, but its always struck me as odd that you have to pass in all
> > of the objects back up the hierarchy for a nested named route - it
> > would be much nicer if Rails could work this out for you, so instead
of:
> >
> > task_url(task.project, task)
> >
> > All you need is:
> >
> > task_url(task)
> >
> > It should be able to get the project from the task (task.project).
>
> So, instead of all this hackery to push the right params to the named
> route method, we''d need to modify routing to determine what method
to
> call on the object automatically. I agree, that''s the cleanest
way in
> terms of usage.
>
> It might get a little messy when you start dealing with some more
> complex routes. On the other hand, almost anything would be better
> than the default right now, which puts the item''s id in the wrong
> place in the url (ie: task_url(task) generates
> /projects/#{task.id}/tasks/ iirc.). Might as well continue rewarding
> people for following conventions, but make sure there''s the
ability
> override them.
>
> I may or may not have any time during the rest of the week to try and
> write up a patch for the routes to do this, but hopefully I will this
> weekend.
>
> Dave
>
> --
> Dave Goodlad
> dgoodlad@gmail.com or dave@goodlad.ca
> http://david.goodlad.ca/
>
> >
>
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
On Tue, 2006-10-17 at 14:38 -0700, DHH wrote:> > Hmm; an alternative solution that crosses my mind would be to allow an > > array parameter to form_for: > > > > form_for [ @thing.project_id, @thing ] do ... > > > > Thoughts? > > I like this a lot better. I don''t think the model should know anything > about the routing requirements either.Here''s a patch: http://dev.rubyonrails.org/ticket/6432 I dropped support for passing the list of records as arguments (i.e. ''form_for @project, @thing do ...'') as the implementation I had wasn''t particularly nice. Using an array is much tidier. -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 -~----------~----~----~----~------~----~------~--~---
On 18/10/2006, at 11:58 AM, Michael Koziarski wrote:> >> As long as we can handle non-trivial resource routes, otherwise it''s >> pretty much useless in a production app. I think we''d at least need >> some type of support for name_prefix in the helpers to distinguish >> between similarly named resources (see :tags below): > > I''m not sure I follow, shouldn''t there be *one* url per resource? > It''s Uniform Location?How would it deal with polymorphic associations that needed different controllers? E.g. /post/34/taggings /photo/16/taggings map.resources :posts do |posts| posts.resources :taggings, :controller => ''post_taggings'', :name_prefix => ''post_'' end map.resources :photos do |photos| photos.resources :taggings, :controller => ''photo_taggings'', :name_prefix => ''photo_'' end What will form_for do for <% form_for @tagging %> ? -- tim --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 10/19/06, Tim Lucas <t.lucas@toolmantim.com> wrote:> > On 18/10/2006, at 11:58 AM, Michael Koziarski wrote: > > > > >> As long as we can handle non-trivial resource routes, otherwise it''s > >> pretty much useless in a production app. I think we''d at least need > >> some type of support for name_prefix in the helpers to distinguish > >> between similarly named resources (see :tags below): > > > > I''m not sure I follow, shouldn''t there be *one* url per resource? > > It''s Uniform Location? > > How would it deal with polymorphic associations that needed different > controllers?For updating or creating, why not have a top level taggings controller? I can see why the urls like that (/post/345/taggings) can make sense when viewing the taggings for a photo, but not when creating them or viewing an individual ''tagging'' instance. -- 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 -~----------~----~----~----~------~----~------~--~---
Actually it makes perfect sense to me, and is part of why I love the nested resources. Whilst a Tag is a single entity in our domain model, a post tag and photo tag are two different conceptual resources - hence being able to nest resources with prefixes to distinguish the different resources. If you want to create a tag for a photo, then it seems that the most sensible thing would be to send a POST request to photo/xx/taggings. Of course, there would already be a TaggingsController and photo/xx/ taggings will go to the TaggingsController with photo and xx available as params, making it easy to create taggings for different things just be extracting the first part of the URL. Having a top-level route for taggings just for creating/updating doesn''t make any sense and certainly isn''t very DRY. Not only that, but you''d have to send extra parameters for tagging type and tagging type id as well, whereas you get that for free when you use the nested routes. Cheers Luke On 19 Oct 2006, at 00:03, Michael Koziarski wrote:> > > For updating or creating, why not have a top level taggings > controller? I can see why the urls like that (/post/345/taggings) > can make sense when viewing the taggings for a photo, but not when > creating them or viewing an individual ''tagging'' instance. > > -- > 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 -~----------~----~----~----~------~----~------~--~---
I have an quick and dirty (no tests) implementation working that uses the list of records as arguments. I think the interface for form_for should be the same as for the named url methods. I see the Array implementation as a leaky abstraction that presents an interface counter to what is expected given the other nested route helpers. It would be nice if only the record (parents not included) were required. This seems like a bigger change, so for now it would be nice for all of the methods based on nested named routes to have a similar interface (No Array) Patch: http://pastie.caboo.se/19940 --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---