Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow a defining custom member field on resources #5581

Merged
merged 1 commit into from Mar 26, 2012

Conversation

jamie
Copy link
Contributor

@jamie jamie commented Mar 26, 2012

By default, resources routes are created with :resource/:id. A model
defining to_param can make prettier urls by using something more
readable than an integer ID when generating URLs, but since the route picks it up as :id you
wind up with awkward User.find_by_username(params[:id]) calls.

By overriding the key to be used in @request.params you can be more
obvious in your intent.

This override is basically as simple as it needs to be, just an extra :param
option on the resource(s) definition on the route. There's no work done to force
the url generators to use that field when passed a model, but since to_param
is already overridable I don't think it's necessary.

Patch includes tests for a plain resource as well as a nested resource
(generating user_login for the above example).

By default, resources routes are created with :resource/:id. A model
defining to_param can make prettier urls by using something more
readable than an integer ID, but since the route picks it up as :id you
wind up with awkward User.find_by_username(params[:id]) calls.

By overriding the key to be used in @request.params you can be more
obvious in your intent.
josevalim added a commit that referenced this pull request Mar 26, 2012
Allow a defining custom member field on resources
@josevalim josevalim merged commit adb802e into rails:master Mar 26, 2012
@TildeWill
Copy link
Contributor

Just realized I was working on a dupe of this issue (see #7116). Can I do a backport to rails 3.2 and add some docs?

pixeltrix added a commit that referenced this pull request Jul 20, 2012
The Mapper looks for a :id constraint in the scope to see whether it
should apply a constraint for nested resources. Since #5581 added support
for resource params other than :id, we need to check for a constraint on
the parent resource's param name and not assume it's :id.
pixeltrix added a commit that referenced this pull request Jul 20, 2012
Since #5581 added support for resources with custom params we should
not assume that it is :id when using shallow resource routing.
@rafaelfranca
Copy link
Member

@TildeWill I don't think we should backport this since it is adding feature. 3-2-stable is only to bug fixes now.

@pixeltrix
Copy link
Contributor

Just a note that a custom resource param is going to fail with passing a hash to polymorphic_url because of this. No real way to fix it because there's no way of knowing what it should be at this point - similar to #1769. Fortunately I don't think this is widely used pattern - it's usually a record or an array.

@kuraga
Copy link

kuraga commented Aug 23, 2012

But if I want to use both patterns - access resource by id AND by shortcut - will it be possible?

@pixeltrix
Copy link
Contributor

@kuraga the best solution for that is to use a url slug library like FriendlyId which overrides Model.find to search by slug or id and leave the param name as :id.

@kuraga
Copy link

kuraga commented Aug 23, 2012

@pixeltrix thanks!

@jamie @pixeltrix But I want to say that maybe it's a thin moment... If we right

resources :products, :only => :show
resources :products, :param => :slug, :only => :show

we get

scope '/products' do
  controller 'products' do
    get    '/:id/edit', :action => 'edit',  :as => :edit_product
  end
end

scope '/products' do
  controller 'products' do
    get    '/:slug/edit', :action => 'edit',  :as => :edit_product
  end
end

Am I right? If yes we get to gets with :as => :edit_product. Is it good?

@pixeltrix
Copy link
Contributor

@kuraga the problem is that edit_product_url will always point to the second route so edit_product_url :id => '1' will fail to generate.

@kuraga
Copy link

kuraga commented Aug 31, 2012

@pixeltrix Thank you! And i think that's a problem of this patch! If it's wrong to write two resources we have to highlight this in docs. Or to solve this issue...

@jamie
Copy link
Contributor Author

jamie commented Aug 31, 2012

@kuraga Why do you need two routes for Products#show?

resources :products, :only => :show
resources :products, :param => :slug, :only => :show

That's redundant. Pick one as the canonical URL scheme and stick with it.

zzak pushed a commit to zzak/rails that referenced this pull request May 29, 2014
zzak pushed a commit to zzak/rails that referenced this pull request May 29, 2014
zzak pushed a commit that referenced this pull request May 29, 2014
tenderlove added a commit that referenced this pull request May 29, 2014
* master:
  Update url to rake docs [ci skip]
  Name#model_name doesn't return a String object
  Result sets never override a model's column type
  [ci skip] Make last note show up in postgresql guide.
  Add missing `:param` option from the docs for Mapper#match [ci skip] Option discovered by @zackperdue in #14741, implemented in #5581.
  Add @senny's changed from #14741, including code font for `resources` options, and wrapped to 80 chars. [ci skip]
  Use github url for homepage of log4r [ci skip]
  Remove TODO.
  Ensure we always use instances of the adapter specific column class
  Fix indentation from 1b4b26f [ci skip]
  [ci skip] Improve form_helpers.md guide.
  Clear inflections after test.
  Remove unnecessary include for integration tests.
  Added documentation for the :param option for resourceful routing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants