simply_helpful nested routes

rubyonrails-core
1

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)

2

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.

3

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.

4

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?

5

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

6

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?

7

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

8

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

9

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.

10

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

11

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.

12

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

13

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.

14

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

15

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: Parked at Loopia