Stop users from calling .create on a has_many / habtm association when the owner is a new_record?

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

activerecord/lib/active_record/associations/association_collection.rb

@@ -85,6 +85,7 @@ module ActiveRecord
       end
       
       def create(attrs = {})
+        ensure_owner_is_not_new
         record = @reflection.klass.send(:with_scope, :create => construct_scope[:create]) { @reflection.klass.create(attrs) }                
         @target ||= [] unless loaded?
         @target << record 
@@ -92,6 +93,7 @@ module ActiveRecord
       end
 
       def create!(attrs = {})
+        ensure_owner_is_not_new
         record = @reflection.klass.send(:with_scope, :create => construct_scope[:create]) { @reflection.klass.create!(attrs) }                
         @target ||= [] unless loaded?
         @target << record 
@@ -206,7 +208,14 @@ module ActiveRecord
         def callbacks_for(callback_name)
           full_callback_name = "#{callback_name}_for_#{@reflection.name}"
           @owner.class.read_inheritable_attribute(full_callback_name.to_sym) || []
-        end        
+        end   
+        
+        def ensure_owner_is_not_new
+          if @owner.new_record?
+            raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved"
+          end
+        end
+               
     end
   end
 end

activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb

@@ -15,6 +15,7 @@ module ActiveRecord
 
       def create(attributes = {})
         # Can't use Base.create because the foreign key may be a protected attribute.
+        ensure_owner_is_not_new
         if attributes.is_a?(Array)
           attributes.collect { |attr| create(attr) }
         else
@@ -23,6 +24,18 @@ module ActiveRecord
           record
         end
       end
+      
+      def create!(attributes = {})
+        # Can't use Base.create! because the foreign key may be a protected attribute.
+        ensure_owner_is_not_new
+        if attributes.is_a?(Array)
+          attributes.collect { |attr| create(attr) }
+        else
+          record = build(attributes)
+          insert_record(record, true) unless @owner.new_record?
+          record
+        end        
+      end
 
       def find_first
         load_target.first
@@ -75,9 +88,13 @@ module ActiveRecord
           load_target.size
         end
 
-        def insert_record(record)
+        def insert_record(record, force=true)
           if record.new_record?
-            return false unless record.save
+            if force
+              record.save!
+            else
+              return false unless record.save
+            end
           end
 
           if @reflection.options[:insert_sql]

activerecord/test/associations_test.rb

@@ -576,6 +576,26 @@ class HasManyAssociationsTest < Test::Unit::TestCase
     assert_equal 3, first_firm.plain_clients.length
     assert_equal 3, first_firm.plain_clients.size
   end
+  
+  def test_create_with_bang_on_has_many_when_parent_is_new_raises
+    assert_raises(ActiveRecord::RecordNotSaved) do 
+      firm = Firm.new
+      firm.plain_clients.create! :name=>"Whoever"
+    end
+  end
+
+  def test_regular_create_on_has_many_when_parent_is_new_raises
+    assert_raises(ActiveRecord::RecordNotSaved) do 
+      firm = Firm.new
+      firm.plain_clients.create :name=>"Whoever"
+    end
+  end
+  
+  def test_create_with_bang_on_habtm_when_parent_is_new_raises
+    assert_raises(ActiveRecord::RecordNotSaved) do 
+      Developer.new("name" => "Aredridel").projects.create!    
+    end
+  end
 
   def test_adding_a_mismatch_class
     assert_raises(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).clients_of_firm << nil }
@@ -1540,8 +1560,8 @@ class HasAndBelongsToManyAssociationsTest < Test::Unit::TestCase
 
   def test_create_by_new_record
     devel = Developer.new(:name => "Marcel", :salary => 75000)
-    proj1 = devel.projects.create(:name => "Make bed")
-    proj2 = devel.projects.create(:name => "Lie in it")
+    proj1 = devel.projects.build(:name => "Make bed")
+    proj2 = devel.projects.build(:name => "Lie in it")
     assert_equal devel.projects.last, proj2
     assert proj2.new_record?
     devel.save

activerecord/test/validations_test.rb

@@ -675,7 +675,7 @@ class ValidationsTest < Test::Unit::TestCase
     t = Topic.new('title' => 'noreplies', 'content' => 'whatever')
     assert !t.save
     assert t.errors.on(:replies)
-    t.replies.create('title' => 'areply', 'content' => 'whateveragain')
+    reply = t.replies.build('title' => 'areply', 'content' => 'whateveragain')
     assert t.valid?
   end
 
@@ -868,7 +868,7 @@ class ValidationsTest < Test::Unit::TestCase
       t = Topic.new('title' => 'あいうえお', 'content' => 'かきくけこ')
       assert !t.save
       assert t.errors.on(:replies)
-      t.replies.create('title' => 'あいうえお', 'content' => 'かきくけこ')
+      t.replies.build('title' => 'あいうえお', 'content' => 'かきくけこ')
       assert t.valid?
     end
   end