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

Updated ActiveRecord::Base.find_or_create_by for 2 attributes when 1 is n #1276

Closed
wants to merge 1 commit into from
Closed

Updated ActiveRecord::Base.find_or_create_by for 2 attributes when 1 is n #1276

wants to merge 1 commit into from

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented May 24, 2011

Updated ActiveRecord::Base.find_or_create_by for 2 attributes when 1 is nil

Updated the find_or_create_by_foo_and_bar('hello') to process the find portion
similar to find_by_foo_and_bar('hello'). In the original
find_by_foo_and_bar('hello'), the resulting query was something like
"foo = 'hello' AND bar IS NULL". In the original
find_or_create_by_foo_and_bar('hello'), the resulting query was something like
"foo = 'hello'"

This patch aligns those two methods expected behaviors on the "find" portion
of the method call.

…is nil

Updated the find_or_create_by_foo_and_bar('hello') to process the find portion
similar to find_by_foo_and_bar('hello').  In the original
find_by_foo_and_bar('hello'), the resulting query was something like
"foo = 'hello' AND bar IS NULL".  In the original
find_or_create_by_foo_and_bar('hello'), the resulting query was something like
"foo = 'hello'"

This patch aligns those two methods expected behaviors on the "find" portion
of the method call.
@sikachu
Copy link
Member

sikachu commented Jul 11, 2011

Interesting. I feel like it should instead raise an error if you're doing find_by_foo_and_bar('hello') and passing only one argument. I think if you're using _and_, you actually have to pass two arguments to it.

/cc @tenderlove

@jeremyf
Copy link
Contributor Author

jeremyf commented Jul 11, 2011

Given that find_by_foo_and_bar will not raise an error if only one param is passed, I'd expect find_or_create_by_foo_and_bar to behave the same. Having these two related functions behave differently on missing parameters is a big no no for me. And altering the find_by_foo_and_bar to raise an error on a missing parameter will likely break existing applications.

@isaacsanders
Copy link
Contributor

@jeremyf Is this still an issue?

@rafaelfranca
Copy link
Member

Yeah. I don't remember but seems like @jonleighton want to remove the dynamic finders in the next version. If so, please close this one @jonleighton

@jeremyf
Copy link
Contributor Author

jeremyf commented Apr 28, 2012

If we are removing dynamic finders, then I'll close it.

@jeremyf jeremyf closed this Apr 28, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants