CollectionProxy#include? returns true and false as documented.
#10987
Conversation
|
@fxn can you take a look? |
|
Looks good to me. Do we really need a CHANGELOG? If someone reads the CHANGELOG he may think this is something new, if we are fixing a regression introduced sometime in the 4 branch I believe it is not needed. |
|
@fxn agreed, entry is removed. |
|
Awesome :). |
…s_true `CollectionProxy#include?` returns `true` and `false` as documented.
| @@ -830,7 +830,7 @@ def many?(&block) | |||
| # person.pets.include?(Pet.find(20)) # => true | |||
| # person.pets.include?(Pet.find(21)) # => false | |||
| def include?(record) | |||
| @association.include?(record) | |||
| !!@association.include?(record) | |||
There was a problem hiding this comment.
Just wondering why would underlying @association's .include? not return a boolean in the first place? If it behaved like Ruby array/enum, it would be boolean whether true or false. Should the change instead live in @association?
There was a problem hiding this comment.
Not really in my view. The wrapped predicate is not even documented, and there is no reason it should return anything in particular, you only need to know it behaves as a predicate. On the other hand the method patched here is public, he is the one responsible for his contract.
Closes #10979
The
CollectionProxy#include?method is documented to returntruebut it can also return1andnil. I adjusted the tests and the implementation to make sure it returns booleans.