simply_helpful nested routes

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
dgoodladgmail.com or davegoodlad.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-coregooglegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribegooglegroups.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 snag
is 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-coregooglegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribegooglegroups.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-coregooglegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribegooglegroups.com
For more options, visit this group at http:
//groups.google.com/group/rubyonrails-core
-~----------~----~----~----~------~----~------~--~---

On 10/18/06, DHH <david.heinemeiergmail.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-coregooglegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribegooglegroups.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-coregooglegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribegooglegroups.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-coregooglegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribegooglegroups.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.heinemeiergmail.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-coregooglegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribegooglegroups.com
For more options, visit this group at http:
//groups.google.com/group/rubyonrails-core
-~----------~----~----~----~------~----~------~--~---

On 10/18/06, Luke Redpath <contactlukeredpath.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
dgoodladgmail.com or davegoodlad.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-coregooglegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribegooglegroups.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 <dgoodladgmail.com> wrote:
>
> On 10/18/06, Luke Redpath <contactlukeredpath.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
> dgoodladgmail.com or davegoodlad.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-coregooglegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribegooglegroups.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.rub
yonrails.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-coregooglegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribegooglegroups.com
For more options, visit this group at http:
//groups.google.com/group/rubyonrails-core
-~----------~----~----~----~------~----~------~--~---