Change behaviour of find
#195
Conversation
lib/dynamoid/finders.rb
Outdated
| expects_array ? Array(result) : result | ||
| else | ||
| find_all(ids) | ||
| result = find_all(ids) | ||
| raise Errors::RecordNotFound if result.size != ids.size |
There was a problem hiding this comment.
Do we want to provide the error message too?
ActiveRecord will say which id was passed and for an array of ids it'll say which ids passed and it'll produce "found X results, but was looking for N" kind of message so we could help report the difference.
| @@ -107,7 +107,7 @@ def build(attrs = {}) | |||
| def exists?(id_or_conditions = {}) | |||
| case id_or_conditions | |||
| when Hash then where(id_or_conditions).first.present? | |||
| else !! find(id_or_conditions) | |||
| else !! find_by_id(id_or_conditions) | |||
There was a problem hiding this comment.
Would like to see a spec for any behavior change we expect from switching to find_by_id. This looks like it might be a significant change.
There was a problem hiding this comment.
@pboling find uses find_by_id internally. The only difference between them - find supports passing multiple ids. As far as I see Document.exists? accepts either one id or condition.
So Document.exists?([1, 2, 3]) will not work. Do you think we should keep this behaviour?
There was a problem hiding this comment.
Ok, I think the change is fine then. Seems we did not have a spec for the old behavior of passing an array directly then.
It's common to get exception if one or more records can not be found for the requested ids
Both
MongoidandActiveRecordsupport this behaviour (here and here)Current behaviour -
findreturnsnilor smaller array.New behaviour - it raises
RecordNotFoundif one or more records can not be found for the requested ids