This project is archived and is in readonly mode.
Nested resources inherit requirements on parent ids
Reported by Matt Powell | May 22nd, 2009 @ 03:21 AM | in 3.0.2
The problem
Consider the following resource routing code for a site where product ids are of the form ABCD.123:
map.resources :products, :requirements => { :id => /[A-Z]{4}\.[0-9]{3}/ } do |products|
products.resources :reviews
end
You'd expect product_reviews_path("ABCD.123") to return
"/products/ABCD.123/reviews", right? But actually, you get a
routing error, because the nested resource (reviews) doesn't know
that the product_id
parameter is allowed to have
periods in it.
What you end up needing is something like:
map.resources :products, :requirements => { :id => /[A-Z]{4}\.[0-9]{3}/ } do |products|
products.resources :reviews, :requirements => { :product_id => /[A-Z]{4}\.[0-9]{3}/ }
end
...which isn't very DRY.
What this patch does
This patch changes resource generation subtly so that
requirements on :id on the parent resource get passed to the child
as, for example, requirements[:product_id]
.
Notes
- I'm aware that some people may take issue with using periods in
an ID parameter in the first place. I'm not disagreeing, but
sometimes these requests come from higher up the chain and aren't
easily dismissed ;)
- I could perhaps have made this more general, but I'm not aware
of any other widely-used parameters that need this kind of
treatment. Perhaps this could be a configuration option.
- I think cascading the ID requirement in this way is a sensible default, and in any event it can be overridden in the nested resource's requirements. However, if this is the sort of thing that should be off by default, again, I'm happy to create a configuration option.
Thoughts? Questions? Comments? This is my first attempt to patch Rails officially, so I'm completely open to discussion.
Comments and changes to this ticket
-
David Jones May 22nd, 2009 @ 03:54 AM
+1 this makes a lot of sense. Since the child resource inherits the parent into their URL surely you'd expect the requirements to come across with it.
-
Andrew White June 27th, 2010 @ 09:33 PM
- Assigned user set to Andrew White
- State changed from new to open
- Milestone cleared.
- Importance changed from to Medium
-
Andrew White June 28th, 2010 @ 12:13 PM
This almost works with the new router now:
# Works resources :movies, :id => /\d+/ do resources :reviews end # Works resources :movies, :constraints => { :id => /\d+/ } do resources :reviews end # Works scope :id => /\d+/ do resources :movies do resources :reviews end end # Fails scope :constraints => { :id => /\d+/ } do resources :movies do resources :reviews end end # Fails constraints :id => /\d+/ do resources :movies do resources :reviews end end
This is because :constraints is a separate hash in the scope. The attached patch merges the constraints from the scope into the resource options.
-
Repository June 28th, 2010 @ 01:41 PM
- State changed from open to resolved
(from [e717631a8481935e8ac1eeb2445da341bdd4c868]) Merge :constraints from scope into resource options [#2694 state:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/e717631a8481935e8ac1eeb2445da3... -
Anthony Green August 29th, 2010 @ 10:22 PM
I'm still seeing this bug in Actionpack (3.0.0.rc2) but it has been marked as resolved ?
-
Andrew White August 30th, 2010 @ 05:58 AM
You are using the new router dsl, right? I'm asking because your example is using the old syntax (and it doesn't work using the old syntax). If you're using the old syntax then nothing will have changed as that's been kept the same deliberately to ease the migration of old apps.
-
Anthony Green August 30th, 2010 @ 09:04 AM
Now I'm not sure if this is the same issue or a slight variation:
resources :foo, :constraints => { :id => /\d+/ } do
resources :bar endrake routes:
edit_foo_bar GET /foo/:foo_id/bar/:id/edit(.:format) {:id=>/\d+/, :foo_id=>/\d+/, ...} foo_bar GET /foo/:foo_id/bar/:id(.:format) {:id=>/\d+/, :foo_id=>/\d+/, ...} foo_bar PUT /foo/:foo_id/bar/:id(.:format) {:id=>/\d+/, :foo_id=>/\d+/, ...} foo_bar DELETE /foo/:foo_id/bar/:id(.:format) {:id=>/\d+/, :foo_id=>/\d+/, ...}
here you can see both :foo_id and :id inherit the constraint. It should only be :foo_id
-
Anthony Green August 30th, 2010 @ 09:11 AM
Moments later I spot the typo:
the nested resource should be declared singularly so resource not resources
resources :foo, :constraints => { :id => /\d+/ } do
resource :bar endis there a use case for
resources :foo do
resources :bar endor should it throw an error ?
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Referenced by
- 2694 Nested resources inherit requirements on parent ids (from [e717631a8481935e8ac1eeb2445da341bdd4c868]) Merge :...
- 5994 routing and url_for doesn't accept dots in the custom parameters https://rails.lighthouseapp.com/projects/8994/tickets/53...