Move the "to_param" logic out of ActiveRecord

rubyonrails-core
1

Hi everyone!

I’ve been using rails for a while (since rails 2) but never had the opportunity to contribute. I’d like to take this opportunity to thank all contributors to Rails :slight_smile:

Anyway, I’m running into some issue with to_param. Consider the case where you have a resource that you’d like to route using identifiers that are not the :id. Currently, the recommended way to achieve that is by overriding the #to_param method on the model. I think that’s questionable design.

Let’s consider a simple example : you want to expose the same resource at two different routes. For instance, on a blog, you’d want to route articles using a slug for SEO on public URLs, and using an id on the admin interface because you use like to use ids internally.

/articles/2019-05-my-breaking-news

/admin/articles/12

Problem : there’s no easy solution to achieve this, because overriding to_param is global.

Suggested solution : that’s actually a routing topic, so it should be solve in the routing. I suggest leveraging the “param” routing option to do this, and leave the Models alone :slight_smile:

routes.draw do

resources :articles, param: :slug

namespace :admin do

resources :articles

end

end

Currently, this changes the name of the parameter used in the route. That’s a good start. But it has no impact on the named route helpers.

Namely, url_for(@article) will still use the :id, even though “:slug” has been specified.

I suggest to also change the named route helpers to call “@article.#{param}” if a routing param has been specified.

I hope this suggestion makes sense, just let me know :slight_smile:

2

Problem : there’s no easy solution to achieve this, because overriding to_param is global.

Have you considered wrapping the object in a decorator that changes the to_param method to what you want?

url_for(@article)

=> /articles/2019-05-my-breaking-news

url_for(AdminArticleDecorator.new(@article))

=> /admin/articles/12

class AdminArticleDecorator

def initialize(article)

@artile = article

end

def to_param(*)

@article.id

end

end

3

Actually, you’re right, there are several ways to solve this. But I think all of them have their drawbacks.

  • The one you mentionned makes it mandatory to write form_for(AdminArticleDecorator.new(@article)) instead of the more standard and shorter form_for([:admin, @article])

  • One could also simply write a custom helper (e.g. my_admin_article_path(@article)). Problem, this doesn’t work well with form_for (especially if the record exists or not). And you still cannot do form_for([:admin, @article]).

  • Overriding the base helper, and don’t touch to_param

def article_path(@article)

super(@article.slug)

end

Problems: Overriding the helpers is a bit annoying because you have to keep loading the file they’re defined in (every time the routing helpers are normally automatically included).

And I still think this is a routing concern, and that ActiveRecord#to_param is a bit awkward.

Namely, I think one should be able to write form_for([:admin, @article]) and form_for(@article) and have those routed using different ids :slight_smile:

4

I’d use direct routes: Rails Routing from the Outside In — Ruby on Rails Guides

And form_for has been deprecated for a while. Use form_with url: my_admin_article_path(@article).

to_param is for the most common case. You use named/direct routes for the rest.

5

I’d use direct routes: https://guides.rubyonrails.org/routing.html#direct-routes

And form_for has been deprecated for a while. Use form_with url: my_admin_article_path(@article).

Yet the guide you reference uses form_for in the very next section:

https://guides.rubyonrails.org/routing.html#using-resolve

-Rob

6

I’d use direct routes: https://guides.rubyonrails.org/routing.html#direct-routes

Yes, I think that’d be another solution :slight_smile: One problem I have with direct routes is that they don’t seem to appear when doing “rake routes” though… Also, you lose the conveniency and conciseness of RESTful routes.

And form_for has been deprecated for a while. Use form_with url: my_admin_article_path(@article).

Allright, I will. But correct me if I’m wrong: this won’t handle the POST/PATCH method dispatch depending on wether @article is persisted, will it?

to_param is for the most common case. You use named/direct routes for the rest.

I guess that’s fair enough :slight_smile:

Still, I still think I had a point regarding the “param” option in restful routes. Let’s consider a simple example : resource :articles, param: :slug. This will generate routes such as “edit_article GET /articles/:slug/edit”.Now in this case, I really think edit_article_path(@article) should generate “/articles/#{@article.slug}/edit” instead of “/articles/#{@article.id}/edit”

Would you guys consider merging a PR that would do this? Or do you guys think I’m just wrong?