Merge pull request #35174 from ypresto/fix-concat-retval

Fix `CollectionProxy#concat` to return self by alias it to `#<<`

4 files changed, 10 insertions, 35 deletions

diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 6f5df807fe..68f53d5c1c 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -109,9 +109,8 @@ module ActiveRecord
         end
       end
 
-      # Add +records+ to this association. Returns +self+ so method calls may
-      # be chained. Since << flattens its argument list and inserts each record,
-      # +push+ and +concat+ behave identically.
+      # Add +records+ to this association. Since +<<+ flattens its argument list
+      # and inserts each record, +push+ and +concat+ behave identically.
       def concat(*records)
         records = records.flatten
         if owner.new_record?
diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb
index 4fbbc713e4..78993cee8a 100644
--- a/activerecord/lib/active_record/associations/collection_proxy.rb
+++ b/activerecord/lib/active_record/associations/collection_proxy.rb
@@ -366,34 +366,6 @@ module ActiveRecord
         @association.create!(attributes, &block)
       end
 
-      # Add one or more records to the collection by setting their foreign keys
-      # to the association's primary key. Since #<< flattens its argument list and
-      # inserts each record, +push+ and #concat behave identically. Returns +self+
-      # so method calls may be chained.
-      #
-      #   class Person < ActiveRecord::Base
-      #     has_many :pets
-      #   end
-      #
-      #   person.pets.size # => 0
-      #   person.pets.concat(Pet.new(name: 'Fancy-Fancy'))
-      #   person.pets.concat(Pet.new(name: 'Spook'), Pet.new(name: 'Choo-Choo'))
-      #   person.pets.size # => 3
-      #
-      #   person.id # => 1
-      #   person.pets
-      #   # => [
-      #   #       #<Pet id: 1, name: "Fancy-Fancy", person_id: 1>,
-      #   #       #<Pet id: 2, name: "Spook", person_id: 1>,
-      #   #       #<Pet id: 3, name: "Choo-Choo", person_id: 1>
-      #   #    ]
-      #
-      #   person.pets.concat([Pet.new(name: 'Brain'), Pet.new(name: 'Benny')])
-      #   person.pets.size # => 5
-      def concat(*records)
-        @association.concat(*records)
-      end
-
       # Replaces this collection with +other_array+. This will perform a diff
       # and delete/add only records that have changed.
       #
@@ -1033,8 +1005,9 @@ module ActiveRecord
       end
 
       # Adds one or more +records+ to the collection by setting their foreign keys
-      # to the association's primary key. Returns +self+, so several appends may be
-      # chained together.
+      # to the association's primary key. Since +<<+ flattens its argument list and
+      # inserts each record, +push+ and +concat+ behave identically. Returns +self+
+      # so several appends may be chained together.
       #
       #   class Person < ActiveRecord::Base
       #     has_many :pets
@@ -1057,6 +1030,7 @@ module ActiveRecord
       end
       alias_method :push, :<<
       alias_method :append, :<<
+      alias_method :concat, :<<
 
       def prepend(*args)
         raise NoMethodError, "prepend on association is not defined. Please use <<, push or append"
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 4c9e4d0ad2..5fdc5a92fc 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -996,9 +996,10 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
     assert_predicate companies(:first_firm).clients_of_firm, :loaded?
 
-    companies(:first_firm).clients_of_firm.concat([Client.new("name" => "Natural Company"), Client.new("name" => "Apple")])
+    result = companies(:first_firm).clients_of_firm.concat([Client.new("name" => "Natural Company"), Client.new("name" => "Apple")])
     assert_equal 4, companies(:first_firm).clients_of_firm.size
     assert_equal 4, companies(:first_firm).clients_of_firm.reload.size
+    assert_equal companies(:first_firm).clients_of_firm, result
   end
 
   def test_transactions_when_adding_to_persisted
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 0133beccec..0ac56c6168 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -242,9 +242,10 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
   def test_concat
     person = people(:david)
     post   = posts(:thinking)
-    post.people.concat [person]
+    result = post.people.concat [person]
     assert_equal 1, post.people.size
     assert_equal 1, post.people.reload.size
+    assert_equal post.people, result
   end
 
   def test_associate_existing_record_twice_should_add_to_target_twice