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

The comparison between `Relation` and `CollectionProxy` should be consistent. #14592

Conversation

@laurocaetano
Copy link
Contributor

laurocaetano commented Apr 4, 2014

Before this patch, it was behaving this way:

author.posts == Post.where(author_id: author.id)
# => true
Post.where(author_id: author.id) == author.posts
# => false

This patch checks if the object given to Relation#== is a CollectionProxy and then call to_a on it to do the comparison.

Fixes #13506

@egilburg
egilburg reviewed Apr 4, 2014
View changes
activerecord/lib/active_record/relation.rb Outdated
@@ -570,6 +570,8 @@ def uniq_value
# Compares two relations for equality.
def ==(other)
case other
when Associations::CollectionProxy
self == other.to_a

This comment has been minimized.

Copy link
@egilburg

egilburg Apr 4, 2014

Contributor

If

author.posts == Post.where(author_id: author.id)
# => true

Then would this also work as other == self, without having to load the array? Or is other == self doing that anyways?

This comment has been minimized.

@matthewd

This comment has been minimized.

Copy link
Member

matthewd commented Apr 4, 2014

Maybe loosely relevant: #14562

…sistent.

Example:

    author.posts == Post.where(author_id: author.id)
    # => true
      Post.where(author_id: author.id) == author.posts
    # => true

Fixes #13506
@laurocaetano

This comment has been minimized.

Copy link
Contributor Author

laurocaetano commented Apr 11, 2014

@rafaelfranca just updated and covered the AssociationRelation cases.

@thedarkone
thedarkone reviewed Apr 12, 2014
View changes
activerecord/lib/active_record/relation.rb Outdated
when Associations::CollectionProxy
self == other.to_a
when AssociationRelation
self == other.to_a

This comment has been minimized.

Copy link
@thedarkone

thedarkone Apr 12, 2014

Contributor

This could probably shorter as:

when Associations::CollectionProxy, AssociationRelation
  self == other.to_a

This comment has been minimized.

Copy link
@laurocaetano

laurocaetano Apr 12, 2014

Author Contributor

@thedarkone thanks! Updated 😄

@rafaelfranca rafaelfranca merged commit 783982a into rails:master Apr 13, 2014
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
rafaelfranca added a commit that referenced this pull request Apr 13, 2014
…_and_collection_proxy

The comparison between `Relation` and `CollectionProxy` should be consistent.

Conflicts:
	activerecord/CHANGELOG.md
rafaelfranca added a commit that referenced this pull request Apr 13, 2014
…_and_collection_proxy

The comparison between `Relation` and `CollectionProxy` should be consistent.

Conflicts:
	activerecord/CHANGELOG.md

Conflicts:
	activerecord/CHANGELOG.md
rafaelfranca added a commit that referenced this pull request Apr 13, 2014
…_and_collection_proxy

The comparison between `Relation` and `CollectionProxy` should be consistent.

Conflicts:
	activerecord/CHANGELOG.md

Conflicts:
	activerecord/CHANGELOG.md
	activerecord/lib/active_record/association_relation.rb
	activerecord/test/cases/base_test.rb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.