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

Add find_by_id method #19

Open
theblang opened this issue Jan 9, 2018 · 4 comments
Open

Add find_by_id method #19

theblang opened this issue Jan 9, 2018 · 4 comments
Labels

Comments

@theblang
Copy link
Contributor

theblang commented Jan 9, 2018

What are your thoughts on a find_by_id method that would mimic ActiveRecord in that it wouldn't throw an exception if the record is not found?

@sirupsen
Copy link
Owner

sirupsen commented Jan 9, 2018

Good with me :)

@cbortz
Copy link

cbortz commented Feb 12, 2018

Nice suggestion. I'd opt to implement something more along the lines of find_by! that would take any attribute and throw an exception if the record isn't found.

@chrisfrank
Copy link
Collaborator

@sirupsen I'll have some time over the next week to work on this. How far down the rabbit hole do you think we should go? I'm tempted to add at least these methods:

#find_by("Field" => "Value") => a single matching record, nil on no match
#find_by!("Field" => "Value") => a single matching record, raises on no match
#where("Field" => "Value", "Other Field" => "Etc") => an array of matching records

And I think it wouldn't be a huge lift to get from there to ActiveRecord-style query chains, like Tea.where("Type" => "Black").order("Name" => "asc").limit(10)

What do you think? Would also love to hear from @Meekohi, @theblang and @cbortz.

@sirupsen
Copy link
Owner

I'm happy to go the AR-route. I'm wondering if we can lean on some of the AR-tooling, instead of re-inventing it from the ground up? I'm not super familiar with the APIs there, but e.g. being able to support validations and callbacks from those libraries would be neat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants