Let link_to infer link name from Model#to_s by olivierlacan · Pull Request #4223...
source link: https://github.com/rails/rails/pull/42234
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
Conversation
Summary
Ever since I've started using Rails I've yearned to be able to link to models like this:
link_to @profile
And have the following rendered:
<a href="/profiles/1">Eileen</a>
This PR makes this a reality. And you'll no longer have to type the following if your model defines a to_s method:
link_to @profile, @profile
Or even:
link_to @profile, @profile.name
Other Information
I initially implemented this a Railtie plugin gem but the idea of touching the internals of url_for in a gem gives me the heebee jeebees as you might understand. I'd much rather this exist as an option, even if rarely used, within Action View than within a likely soon-to-be out-of-date gem.
I'm aware this amounts to me telling the Rails team to maintain this neato feature, but I'll gladly support maintenance.
@@ -146,6 +148,12 @@ def _filtered_referrer # :nodoc:
# link_to nil, "http://example.com"
# # => <a href="http://www.example.com">http://www.example.com</a>
#
# Pithier yet, when name is an ActiveRecord model that defines a
I'm not sure I understand the use of "pithier" here but perhaps my English skills are deteriorating 
Pithy means concise or forcefully expressive. Something concise and full of meaning, which IMO is exactly what a model is in relation to a URL helper. 
That said we can change it to "concise" to be more ESL friendly. I'll just point out I'm the ESL learner and you're the native English speaker. 
Mind you I think I just used the same adjective I found earlier in the docs:
@@ -146,6 +148,12 @@ def _filtered_referrer # :nodoc:
# link_to nil, "http://example.com"
# # => <a href="http://www.example.com">http://www.example.com</a>
#
# More concise yet, when +name+ is an ActiveRecord model that defines a
# +to_s+ method returning a default value or a model instance attribute
@zzak I changed this from:
+to_s+ method returning a name attribute or a default value
To this:
+to_s+ method returning a default value or a model instance attribute
I feel like the mention of a "name attribute" could have been misunderstood as the +name+ argument to link_to when really I meant a completely different model instance attribute called name. That was too confusing, the implementation of whatever custom #to_s is irrelevant.
In principle I'm 👍🏻 on the feature, but not keen on overloading to_s for this since models may be using it for other purposes already. We have to_param for the id used in a url so seems like we should come up with a similar convention - unsure what this should be of the top of my head so open to ideas.
@pixeltrix Good point, I'll change the docs to reference to_param.
Actually @pixeltrix, I don't think I agree. I read your response too fast.
to_s is the convention for implicitly converting a Ruby object into its String representation and it is already used in the tests as you can see here:
In my experience with Rails apps, most often there is no implementation of to_s on many models and people are instead verbosely telling link_to how to represent their model as a string by typing: link_to(@article, @article.title) over and over again, instead of a much more straightforward link_to(@article) where link_to can abstract away the fetching of the model route to url_for while we can rely on the model itself to find its string/display representation.
We could recommend alternatives but there's nothing in my implementation that calls to_s. I'm simply relying on implicit to_s behavior which is common throughout Rails and Ruby. In my opinion, alternatives would be breaking convention, which is not what we should do. If people have their own implementations of to_s that are specific, they can always opt out of this behavior by using the common explicit two-argument form of link_to.
If people have their own implementations of
to_sthat are specific, they can always opt out of this behavior by using the common explicit two-argument form oflink_to.
I don't think the problem is with link_to necessarily. The issue is that other parts of the system could depend on the implementation of to_s without even calling the to_s method. For example:
irb(main):001:0> class Foo; def to_s; "bar"; end; end
=> :to_s
irb(main):002:0> "foo #{Foo.new}"
=> "foo bar"
In this case, interpolation has a dependency on the to_s method and we can't really "see" it in the code. I think this is the type of implicit dependency that @pixeltrix (and I) am worried about.
Maybe we could add a link_text method? For example link_to(model) would be equivalent to link_to(model, model.link_text).
<%= my_model %>will already callto_s; having<%= link_to(my_model) %>produce a link with the same text seems entirely natural to me.
Ah, that's a good point and does seem very natural. I'm sold on to_s.
Ah, that's a good point and does seem very natural. I'm sold on to_s.
I guess that's a consensus then 
My other concern is that what's the out-of-the-box experience? Because currently they'll just get #<Post:0x00007f9e0e866470>, so do we need some kind of default implementation that looks for certain attribute names?
@pixeltrix to me the argument still holds: that's what you'd already get from <%= model %>.
I don't really see this as providing new deep link-label-guessing magic, and more just about making link_to(model) equivalent to existing link_to(model, model) -- if you're following the right conventions (defining a sensible to_s), then you can type a slightly-shorter, slightly-less-redundant, thing to get the same result.
My other concern is that what's the out-of-the-box experience? Because currently they'll just get
#<Post:0x00007f9e0e866470>, so do we need some kind of default implementation that looks for certain attribute names?
Speaking of good points. 
Maybe exposing a default to_s in ApplicationRecord could help make this feature discoverable. Otherwise if that feels a bit too intrusive, ActiveRecord::Base could have def to_s; "#{self} #{id}"; end. It'll print the class name and the ID attribute (if there's one, Post 1 or Post <uuid>). That would take care of the concern raised about people having their own to_s overload, since theirs would take precedence still.
It's not perfect for printing the identity of non-AR-models or models with no primary keys (or one not called id) but it feels like a slightly better default than printing the hex Ruby object ID. That said, @matthewd has me thinking it will be too backward incompatible for anyone who relied on the previous default to_s behavior.
It's almost like scaffolds if you think about it. The default behavior isn't great but you can easily improve it by bringing your own implementation.
Could you add a CHANGELOG entry?
@ghiculescu Sure thing.
force-pushed the actionview_inferred_model_url
branch
2 times, most recently
from
9df31a9
to
dbe7099
last month
@ghiculescu Pending CI but I think we're good to merge after this. Added the CHANGELOG entry at the top. Let me know if you think the entry is helpful:
* Allow `link_to` helper to infer link name from `Model#to_s` when it
is used with a single argument:
link_to @profile
#=> <a href="/profiles/1">Eileen</a>
Previously you had to supply a second argument even if the `Profile`
model implemented a `#to_s` method that called the `name` method.
link_to @profile, @profile.name
#=> <a href="/profiles/1">Eileen</a>
*Olivier Lacan*
Maybe the changelog should show how to_s is implemented on the model, to complete the picture.
Maybe the changelog should show how to_s is implemented on the model, to complete the picture.
Yeah I think that's helpful. Updated to:
* Allow `link_to` helper to infer link name from `Model#to_s` when it
is used with a single argument:
link_to @profile
#=> <a href="/profiles/1">Eileen</a>
This assumes the model class implements a `to_s` method like this:
class Profile < ApplicationRecord
# ...
def to_s
name
end
end
Previously you had to supply a second argument even if the `Profile`
model implemented a `#to_s` method that called the `name` method.
link_to @profile, @profile.name
#=> <a href="/profiles/1">Eileen</a>
*Olivier Lacan*
Thanks @rafaelfranca 
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
No one assigned
None yet
No milestone
Successfully merging this pull request may close these issues.
None yet
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK