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

ActiveRecord::Base.find([1,2,3]) should return mutiple records #184

Closed
Tim-Blokdijk opened this issue May 3, 2019 · 4 comments · Fixed by #221
Closed

ActiveRecord::Base.find([1,2,3]) should return mutiple records #184

Tim-Blokdijk opened this issue May 3, 2019 · 4 comments · Fixed by #221
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@Tim-Blokdijk
Copy link
Contributor

Currently ActiveRecord::Base.find([1,2,3]) returns a single record.
Should be an array of multiple records.
https://api.rubyonrails.org/classes/ActiveRecord/FinderMethods.html#method-i-find

@catmando
Copy link
Contributor

catmando commented May 4, 2019

Things I didn't know!

@catmando catmando added enhancement New feature or request good first issue Good for newcomers labels Jun 16, 2019
@catmando
Copy link
Contributor

Implementing should be straight forward:

def find(*args)
  args = args[0] if args[0].is_a? Array
  return args.collect { |id| find(id) } if args.count > 1
   # do whatever find does now with args[0]
   ...
end

note this will deal with find(1), find(1, 2, 3) and find([1, 2, 3]) all of which are legal.

@Tim-Blokdijk
Copy link
Contributor Author

@Tim-Blokdijk
Copy link
Contributor Author

Tim-Blokdijk commented Jul 18, 2019

Finally got around to this.

def find(*args)
    args = args[0] if args[0].is_a? Array
    return args.collect { |id| find(id) } if args.count > 1
    find_by(primary_key => args[0])
end

Seems to work. In Hyperstack ActiveRecord#find returns a nil if record is not found. In Rails it raises a ActiveRecord::RecordNotFound exception. I'm OK with this behavior, but we could also filter out the nil values when returning an array?

Still have to write a testcase, hope to push a pull request for this in the coming days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
2 participants