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

simply_helpful_nested_params.diff (956 Bytes)

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.

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?

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

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?

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

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-

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.

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

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.

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

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.

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

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