Pushing a record onto a has_many :through sets the association's foreign key to the associate's primary key. Closes #5815.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4790 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 21 insertions, 10 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 3f24f91ca1..357f8a4d92 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Pushing a record onto a has_many :through sets the association's foreign key to the associate's primary key.  #5815 [josh@hasmanythrough.com]
+
 * PostgreSQL: simplify index introspection query.  #5819 [stephen_purcell@yahoo.com]
 
 * Add records to has_many :through using <<, push, and concat by creating the association record. Raise if base or associate are new records since both ids are required to create the association. #build raises since you can't associate an unsaved record. #create! takes an attributes hash and creates the associated record and its association in a transaction. [Jeremy Kemper]
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index 7662e31914..b5feeff1d2 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -53,7 +53,7 @@ module ActiveRecord
         klass.transaction do
           args.each do |associate|
             raise ActiveRecord::HasManyThroughCantAssociateNewRecords.new(@owner, through) unless associate.respond_to?(:new_record?) && !associate.new_record?
-            klass.with_scope(:create => construct_association_attributes(through)) { klass.create! }
+            klass.with_scope(:create => construct_join_attributes(associate)) { klass.create! }
           end
         end
       end
@@ -99,7 +99,8 @@ module ActiveRecord
           @reflection.options[:uniq] ? records.to_set.to_a : records
         end
 
-        def construct_association_attributes(reflection)
+        # Construct attributes for associate pointing to owner.
+        def construct_owner_attributes(reflection)
           if as = reflection.options[:as]
             { "#{as}_id" => @owner.id,
               "#{as}_type" => @owner.class.base_class.name.to_s }
@@ -108,7 +109,13 @@ module ActiveRecord
           end
         end
 
-        def construct_quoted_association_attributes(reflection)
+        # Construct attributes for :through pointing to owner and associate.
+        def construct_join_attributes(associate)
+          construct_owner_attributes(@reflection.through_reflection).merge(@reflection.source_reflection.association_foreign_key => associate.id)
+        end
+
+        # Associate attributes pointing to owner, quoted.
+        def construct_quoted_owner_attributes(reflection)
           if as = reflection.options[:as]
             { "#{as}_id" => @owner.quoted_id,
               "#{as}_type" => reflection.klass.quote(
@@ -119,9 +126,10 @@ module ActiveRecord
           end
         end
 
+        # Build SQL conditions from attributes, qualified by table name.
         def construct_conditions
           table_name = @reflection.through_reflection.table_name
-          conditions = construct_quoted_association_attributes(@reflection.through_reflection).map do |attr, value|
+          conditions = construct_quoted_owner_attributes(@reflection.through_reflection).map do |attr, value|
             "#{table_name}.#{attr} = #{value}"
           end
           conditions << sql_conditions if sql_conditions
@@ -161,7 +169,7 @@ module ActiveRecord
         end
 
         def construct_scope
-          { :create => construct_association_attributes(@reflection),
+          { :create => construct_owner_attributes(@reflection),
             :find   => { :from        => construct_from,
                          :conditions  => construct_conditions,
                          :joins       => construct_joins,
diff --git a/activerecord/test/associations_join_model_test.rb b/activerecord/test/associations_join_model_test.rb
index 508628f37a..0f6a5e76c2 100644
--- a/activerecord/test/associations_join_model_test.rb
+++ b/activerecord/test/associations_join_model_test.rb
@@ -370,15 +370,16 @@ class AssociationsJoinModelTest < Test::Unit::TestCase
   end
 
   def test_create_associate_when_adding_to_has_many_through
-    count = Tagging.count
-    assert_nothing_raised { posts(:thinking).tags << tags(:general) }
-    assert_equal(count + 1, Tagging.count)
+    count = posts(:thinking).tags.count
+    push = Tag.create!(:name => 'pushme')
+    assert_nothing_raised { posts(:thinking).tags << push }
+    assert_equal(count + 1, posts(:thinking).tags(true).size)
 
     assert_nothing_raised { posts(:thinking).tags.create!(:name => 'foo') }
-    assert_equal(count + 2, Tagging.count)
+    assert_equal(count + 2, posts(:thinking).tags(true).size)
 
     assert_nothing_raised { posts(:thinking).tags.concat(Tag.create!(:name => 'abc'), Tag.create!(:name => 'def')) }
-    assert_equal(count + 4, Tagging.count)
+    assert_equal(count + 4, posts(:thinking).tags(true).size)
   end
 
   def test_has_many_through_sum_uses_calculations