Added that has_and_belongs_to_many associations with additional attributes also can be created between unsaved objects and only committed to the database when Base#save is called on the associator #524 [Eric Anderson]

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

4 files changed, 54 insertions, 18 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 2595960978..00419e9855 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Added that has_and_belongs_to_many associations with additional attributes also can be created between unsaved objects and only committed to the database when Base#save is called on the associator #524 [Eric Anderson]
+
 * Fixed that records fetched with piggy-back attributes or through rich has_and_belongs_to_many associations couldn't be saved due to the extra attributes not part of the table #522 [Eric Anderson]
 
 * Added mass-assignment protection for the inheritance column -- regardless of a custom column is used or not
diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
index 83b87547ee..77d2fb9cde 100644
--- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -81,8 +81,8 @@ module ActiveRecord
 
       def push_with_attributes(record, join_attributes = {})
         raise_on_type_mismatch(record)
-        insert_record_with_join_attributes(record, join_attributes)
-        join_attributes.each { |key, value| record.send(:write_attribute, key, value) }
+        join_attributes.each { |key, value| record[key.to_s] = value }
+        insert_record(record) unless @owner.new_record?
         @target << record
         self
       end
@@ -105,22 +105,33 @@ module ActiveRecord
 
         def insert_record(record)
           return false unless record.save
+
           if @options[:insert_sql]
             @owner.connection.execute(interpolate_sql(@options[:insert_sql], record))
           else
-            sql = "INSERT INTO #{@join_table} (#{@association_class_primary_key_name}, #{@association_foreign_key}) " + 
-                  "VALUES (#{@owner.quoted_id},#{record.quoted_id})"
+            columns = @owner.connection.columns(@join_table, "#{@join_table} Columns")
+
+            attributes = columns.inject({}) do |attributes, column|
+              case column.name
+                when @association_class_primary_key_name
+                  attributes[column.name] = @owner.quoted_id
+                when @association_foreign_key
+                  attributes[column.name] = record.quoted_id
+                else
+                  value = record[column.name]
+                  attributes[column.name] = value unless value.nil?
+              end
+              attributes
+            end
+
+            sql =
+              "INSERT INTO #{@join_table} (#{@owner.send(:quoted_column_names, attributes).join(', ')}) " +
+              "VALUES (#{attributes.values.collect { |value| @owner.send(:quote, value) }.join(', ')})"
+
             @owner.connection.execute(sql)
           end
-          true
-        end
-        
-        def insert_record_with_join_attributes(record, join_attributes)
-          attributes = { @association_class_primary_key_name => @owner.id, @association_foreign_key => record.id }.update(join_attributes)
-          sql = 
-            "INSERT INTO #{@join_table} (#{@owner.send(:quoted_column_names, attributes).join(', ')}) " +
-            "VALUES (#{attributes.values.collect { |value| @owner.send(:quote, value) }.join(', ')})"
-          @owner.connection.execute(sql)
+          
+          return true
         end
         
         def delete_records(records)
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index ede60e3f6d..55fe29c8f5 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1115,11 +1115,13 @@ module ActiveRecord #:nodoc:
       # an SQL statement. 
       def attributes_with_quotes(include_primary_key = true)
         columns_hash = self.class.columns_hash
+
         attrs_quoted = @attributes.inject({}) do |attrs_quoted, pair| 
           attrs_quoted[pair.first] = quote(pair.last, columns_hash[pair.first]) unless !include_primary_key && pair.first == self.class.primary_key
           attrs_quoted
         end
-        attrs_quoted.delete_if { | key, value | !self.class.columns_hash.keys.include?(key) }
+
+        attrs_quoted.delete_if { |key, value| !self.class.columns_hash.keys.include?(key) }
       end
       
       # Quote strings appropriately for SQL statements.
@@ -1178,7 +1180,7 @@ module ActiveRecord #:nodoc:
 
           unless value.empty?
             attributes[attribute_name] << 
-              [find_parameter_position(multiparameter_name), type_cast_attribute_value(multiparameter_name, value)]
+              [ find_parameter_position(multiparameter_name), type_cast_attribute_value(multiparameter_name, value) ]
           end
         end
 
@@ -1203,10 +1205,10 @@ module ActiveRecord #:nodoc:
       end
 
       def quote_columns(column_quoter, hash)
-        hash.inject({}) {|list, pair|
+        hash.inject({}) do |list, pair|
           list[column_quoter.quote_column_name(pair.first)] = pair.last
           list
-        }
+        end
       end
 
       def quoted_comma_pair_list(column_quoter, hash)
@@ -1231,4 +1233,4 @@ module ActiveRecord #:nodoc:
         string[0..3] == "--- "
       end
   end
-end
+end
\ No newline at end of file
diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb
index 420fae069b..7ede042879 100755
--- a/activerecord/test/associations_test.rb
+++ b/activerecord/test/associations_test.rb
@@ -724,6 +724,27 @@ class HasAndBelongsToManyAssociationsTest < Test::Unit::TestCase
     assert_equal 2, aridridel.projects(true).size
   end
 
+  def test_habtm_adding_before_save_with_join_attributes
+    no_of_devels = Developer.count
+    no_of_projects = Project.count
+    now = Date.today
+    ken = Developer.new("name" => "Ken")
+    ken.projects.push_with_attributes( Project.find(1), :joined_on => now )
+    p = Project.new("name" => "Foomatic")
+    ken.projects.push_with_attributes( p, :joined_on => now )
+    assert ken.new_record?
+    assert p.new_record?
+    assert ken.save
+    assert !ken.new_record?
+    assert_equal no_of_devels+1, Developer.count
+    assert_equal no_of_projects+1, Project.count
+    assert_equal 2, ken.projects.size
+    assert_equal 2, ken.projects(true).size
+
+    kenReloaded = Developer.find_by_name 'Ken'
+    kenReloaded.projects.each { |prj| assert_equal(now.to_s, prj.joined_on.to_s) }
+  end
+
   def test_build
     devel = Developer.find(1)
     proj = devel.projects.build("name" => "Projekt")