Don't overwrite unsaved updates when loading an association but preserve the order of the loaded records. [#4642 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>

3 files changed, 17 insertions, 3 deletions

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index d9903243ce..5b68ca2edb 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -388,7 +388,7 @@ module ActiveRecord
             begin
               if !loaded?
                 if @target.is_a?(Array) && @target.any?
-                  @target = find_target + @target.find_all {|t| t.new_record? }
+                  @target = find_target.map { |f| i = @target.index(f); i ? @target.delete_at(i) : f } + @target
                 else
                   @target = find_target
                 end
diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb
index 685b11cb03..a87683bd97 100644
--- a/activerecord/test/cases/nested_attributes_test.rb
+++ b/activerecord/test/cases/nested_attributes_test.rb
@@ -466,6 +466,20 @@ module NestedAttributesOnACollectionAssociationTests
     assert_equal 'Grace OMalley', @child_1.reload.name
   end
 
+  def test_should_not_overwrite_unsaved_updates_when_loading_association
+    @pirate.reload
+    @pirate.send(association_setter, [{ :id => @child_1.id, :name => 'Grace OMalley' }])
+    @pirate.send(@association_name).send(:load_target)
+    assert_equal 'Grace OMalley', @pirate.send(@association_name).target.find { |r| r.id == @child_1.id }.name
+  end
+
+  def test_should_preserve_order_when_not_overwriting_unsaved_updates
+    @pirate.reload
+    @pirate.send(association_setter, [{ :id => @child_1.id, :name => 'Grace OMalley' }])
+    @pirate.send(@association_name).send(:load_target)
+    assert_equal @pirate.send(@association_name).target.first.id, @child_1.id
+  end
+
   def test_should_take_a_hash_with_composite_id_keys_and_assign_the_attributes_to_the_associated_models
     @child_1.stubs(:id).returns('ABC1X')
     @child_2.stubs(:id).returns('ABC2X')
diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb
index f1dbe32c6e..d89c8cf381 100644
--- a/activerecord/test/models/pirate.rb
+++ b/activerecord/test/models/pirate.rb
@@ -1,7 +1,7 @@
 class Pirate < ActiveRecord::Base
   belongs_to :parrot, :validate => true
   belongs_to :non_validated_parrot, :class_name => 'Parrot'
-  has_and_belongs_to_many :parrots, :validate => true
+  has_and_belongs_to_many :parrots, :validate => true, :order => 'parrots.id ASC'
   has_and_belongs_to_many :non_validated_parrots, :class_name => 'Parrot'
   has_and_belongs_to_many :parrots_with_method_callbacks, :class_name => "Parrot",
     :before_add    => :log_before_add,
@@ -21,7 +21,7 @@ class Pirate < ActiveRecord::Base
   has_one :ship
   has_one :update_only_ship, :class_name => 'Ship'
   has_one :non_validated_ship, :class_name => 'Ship'
-  has_many :birds
+  has_many :birds, :order => 'birds.id ASC'
   has_many :birds_with_method_callbacks, :class_name => "Bird",
     :before_add    => :log_before_add,
     :after_add     => :log_after_add,