From 0ec967aa6655d62c92f72acb8d556a5b3f70762d Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Mon, 7 Nov 2016 00:01:53 +0900
Subject: [PATCH] Respect new records for `CollectionProxy#uniq`

Currently if `CollectionProxy` has more than one new record,
`CollectionProxy#uniq` result is incorrect.

And `CollectionProxy#uniq` was aliased to `distinct` in a1bb6c8b06db.
But the `uniq` method and the `SELECT DISTINCT` method are different
methods. The doc in `CollectionProxy` is for the `SELECT DISTINCT`
method, not for the `uniq` method.

Therefore, reverting the alias in `CollectionProxy` to fix the
inconsistency and to have the both methods.
---
 .../associations/collection_association.rb    |  7 -------
 .../associations/collection_proxy.rb          | 19 ++++++++++++++++---
 ...s_and_belongs_to_many_associations_test.rb |  2 +-
 .../has_many_associations_test.rb             |  1 +
 .../cases/associations/join_model_test.rb     |  2 +-
 activerecord/test/cases/relations_test.rb     |  1 -
 6 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index f8aac895d7..7cd25ea524 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -246,13 +246,6 @@ module ActiveRecord
         end
       end
 
-      def distinct
-        seen = {}
-        load_target.find_all do |record|
-          seen[record.id] = true unless seen.key?(record.id)
-        end
-      end
-
       # Replace this collection with +other_array+. This will perform a diff
       # and delete/add only records that have changed.
       def replace(other_array)
diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb
index 0800639c24..35a98d7090 100644
--- a/activerecord/lib/active_record/associations/collection_proxy.rb
+++ b/activerecord/lib/active_record/associations/collection_proxy.rb
@@ -718,6 +718,12 @@ module ActiveRecord
         @association.destroy(*records)
       end
 
+      ##
+      # :method: distinct
+      #
+      # :call-seq:
+      #   distinct(value = true)
+      #
       # Specifies whether the records should be unique or not.
       #
       #   class Person < ActiveRecord::Base
@@ -732,10 +738,17 @@ module ActiveRecord
       #
       #   person.pets.select(:name).distinct
       #   # => [#<Pet name: "Fancy-Fancy">]
-      def distinct
-        @association.distinct
+      #
+      #   person.pets.select(:name).distinct.distinct(false)
+      #   # => [
+      #   #      #<Pet name: "Fancy-Fancy">,
+      #   #      #<Pet name: "Fancy-Fancy">
+      #   #    ]
+
+      #--
+      def uniq
+        load_target.uniq
       end
-      alias uniq distinct
 
       def calculate(operation, column_name)
         null_scope? ? scope.calculate(operation, column_name) : super
diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
index 4902792eee..4b7ac594cf 100644
--- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -383,7 +383,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     dev.projects << projects(:active_record)
 
     assert_equal 3, dev.projects.size
-    assert_equal 1, dev.projects.distinct.size
+    assert_equal 1, dev.projects.uniq.size
   end
 
   def test_distinct_before_the_fact
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 3afd062950..d657be71cc 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -919,6 +919,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     company.clients_of_firm.build("name" => "Another Client")
     company.clients_of_firm.build("name" => "Yet Another Client")
     assert_equal 4, company.clients_of_firm.size
+    assert_equal 4, company.clients_of_firm.uniq.size
   end
 
   def test_collection_not_empty_after_building
diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb
index 15446c6dc7..a4345f3857 100644
--- a/activerecord/test/cases/associations/join_model_test.rb
+++ b/activerecord/test/cases/associations/join_model_test.rb
@@ -413,7 +413,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase
     author            = Author.includes(:taggings).find authors(:david).id
     expected_taggings = taggings(:welcome_general, :thinking_general)
     assert_no_queries do
-      assert_equal expected_taggings, author.taggings.distinct.sort_by(&:id)
+      assert_equal expected_taggings, author.taggings.uniq.sort_by(&:id)
     end
   end
 
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index 0501514ba9..96833ad428 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -785,7 +785,6 @@ class RelationTest < ActiveRecord::TestCase
     expected_taggings = taggings(:welcome_general, :thinking_general)
 
     assert_no_queries do
-      assert_equal expected_taggings, author.taggings.distinct.sort_by(&:id)
       assert_equal expected_taggings, author.taggings.uniq.sort_by(&:id)
     end