norman / friendly_id

FriendlyId is the “Swiss Army bulldozer” of slugging and permalink plugins for ActiveRecord. It allows you to create pretty URL’s and work with human-friendly strings as if they were numeric ids for ActiveRecord models.
http://norman.github.io/friendly_id/
MIT License
6.15k stars 591 forks source link

Can we have a find_by, returning nil when not found #937

Closed fidalgo closed 4 years ago

fidalgo commented 4 years ago

Instead of having a find that will throw an exception, it would help to have a find_by method, that would have the same behaviour as find_by in rails, where it returns the first match or nil?

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

parndt commented 4 years ago

@fidalgo does find_by(slug: params[:id]) work?

fidalgo commented 4 years ago

@parndt find_by(slug: id) always return nil, when the record exists or not.

I'm using Model.friendly.find_by

parndt commented 4 years ago

Does the below work, without the friendly filter applied?

Model.find_by(slug: params[:id])

My reasoning is that slug should just be a column on your database table so bypassing this library should give the behaviour that you're after. I tried this with an app that I have running friendly_id and it works as expected.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

fidalgo commented 4 years ago

I'm sorry, @parndt but I guess we're having a communication issue here.

With friendly_id gem, I can use the method find with either slug or id, but then the record does not exist, it will raise a record not found an exception.

So my request is to add another method that will mimic the find_by in Rails https://api.rubyonrails.org/classes/ActiveRecord/FinderMethods.html#method-i-find_by

so instead of raising an exception is will return nil.

When I run Model.find_by(slug: params[:id]) it works as expected, otherwise, it would be a bug on Rails, what I would like would be to be able to use Model.friendly.find_by(slug: params[:id]) that would return the record or nil.

Please let me know if I'm missing something!

parndt commented 4 years ago

@fidalgo understood, I think. I'm mostly wondering why we need to add that when Rails does it already? 😄 it seems like FriendlyId doesn't need a method for doing what Rails does already. That is why we put our functionality behind .friendly so that you still have Rails' methods available.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

joemsak commented 2 years ago

I found this issue (sup, @parndt how you been) looking for something similar:

For me the idea would be that you don't know if you have primary key ID or slug friendly ID and still want a way to use them interchangeably and just get a nil instead of a raised error:

MyModel.friendly.find_safe(id_or_slug) 
# not saying the method has to be called `find_safe()` but hopefully you get the idea

Does that make sense?

parndt commented 2 years ago

@joemsak I think the idea does make sense in principle, and we could probably support it using something like MyModel.friendly.find(id_or_slug, raise: false)

Then this implementation https://github.com/norman/friendly_id/blob/a33d246a712a3ffcda15a17adff8c2fdd358a4c7/lib/friendly_id/finder_methods.rb#L16-L23 could change to raise_not_found_exception(id) if raise.

We would need to formalise the args though away from def find(*args) to def find(id, raise_on_missing: true) or something. I'm not immediately sure what the implication of this would be. I'm not sure what other args find might be expecting.

Super nice to hear from you too, Joe! ❤️

joemsak commented 2 years ago

Yeah, to be sure, my present needs for it almost certainly result from deeper code design issues where the id_or_slug should not be nil in the first place, but it does also seem nice to have a friendly "find by" behavior where you expect a nil when the ID is nil or the record isn't found

joemsak commented 2 years ago

BTW @parndt if you are interested in this idea, would you like me to try a go at a PR?

parndt commented 2 years ago

yes please, I think it'd be good to see if it's feasible, and the idea makes sense. 😄