Merge pull request #4727 from railsaholic/dependent_restrict_should_add_error

has_many/has_one, :dependent => :restrict, deprecation added

2 files changed, 79 insertions, 14 deletions

diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index f1a341437f..8c7956406a 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -1140,16 +1140,41 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_nil companies(:leetsoft).reload.client_of
     assert_nil companies(:jadedpixel).reload.client_of
 
-
     assert_equal num_accounts, Account.count
   end
 
   def test_restrict
-    firm = RestrictedFirm.new(:name => 'restrict')
-    firm.save!
+    # ActiveRecord::Base.dependent_restrict_raises = true, by default
+
+    firm = RestrictedFirm.create!(:name => 'restrict')
     firm.companies.create(:name => 'child')
+
     assert !firm.companies.empty?
     assert_raise(ActiveRecord::DeleteRestrictionError) { firm.destroy }
+    assert RestrictedFirm.exists?(:name => 'restrict')
+    assert firm.companies.exists?(:name => 'child')
+  end
+
+  def test_restrict_when_dependent_restrict_raises_config_set_to_false
+    # ActiveRecord::Base.dependent_restrict_raises = true, by default
+
+    ActiveRecord::Base.dependent_restrict_raises = false
+    # add an error on the model instead of raising ActiveRecord::DeleteRestrictionError
+
+    firm = RestrictedFirm.create!(:name => 'restrict')
+    firm.companies.create(:name => 'child')
+
+    assert !firm.companies.empty?
+
+    firm.destroy
+
+    assert !firm.errors.empty?
+
+    assert_equal "Cannot delete record because dependent company exists", firm.errors[:base].first
+    assert RestrictedFirm.exists?(:name => 'restrict')
+    assert firm.companies.exists?(:name => 'child')
+  ensure
+    ActiveRecord::Base.dependent_restrict_raises = true
   end
 
   def test_included_in_collection
@@ -1401,29 +1426,29 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
       firm.clients.last
     end
   end
-  
+
   def test_custom_primary_key_on_new_record_should_fetch_with_query
     author = Author.new(:name => "David")
     assert !author.essays.loaded?
-    
-    assert_queries 1 do 
+
+    assert_queries 1 do
       assert_equal 1, author.essays.size
     end
-    
+
     assert_equal author.essays, Essay.find_all_by_writer_id("David")
-    
+
   end
-  
+
   def test_has_many_custom_primary_key
     david = authors(:david)
     assert_equal david.essays, Essay.find_all_by_writer_id("David")
   end
-  
+
   def test_blank_custom_primary_key_on_new_record_should_not_run_queries
     author = Author.new
     assert !author.essays.loaded?
-    
-    assert_queries 0 do 
+
+    assert_queries 0 do
       assert_equal 0, author.essays.size
     end
   end
@@ -1649,4 +1674,11 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal [bulb2], car.bulbs
     assert_equal [bulb2], car.reload.bulbs
   end
+
+  def test_building_has_many_association_with_restrict_dependency
+    klass = Class.new(ActiveRecord::Base)
+    
+    assert_deprecated     { klass.has_many :companies, :dependent => :restrict }  
+    assert_not_deprecated { klass.has_many :companies }
+  end
 end
diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb
index 26931e3e85..7c6736fb95 100644
--- a/activerecord/test/cases/associations/has_one_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_associations_test.rb
@@ -157,11 +157,37 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_dependence_with_restrict
-    firm = RestrictedFirm.new(:name => 'restrict')
-    firm.save!
+    # ActiveRecord::Base.dependent_restrict_raises = true, by default
+
+    firm = RestrictedFirm.create!(:name => 'restrict')
     firm.create_account(:credit_limit => 10)
+
     assert_not_nil firm.account
+
     assert_raise(ActiveRecord::DeleteRestrictionError) { firm.destroy }
+    assert RestrictedFirm.exists?(:name => 'restrict')
+    assert firm.account.present?
+  end
+
+  def test_dependence_with_restrict_with_dependent_restrict_raises_config_set_to_false
+    # ActiveRecord::Base.dependent_restrict_raises = true, by default
+
+    ActiveRecord::Base.dependent_restrict_raises = false
+    # adds an error on the model instead of raising ActiveRecord::DeleteRestrictionError
+
+    firm = RestrictedFirm.create!(:name => 'restrict')
+    firm.create_account(:credit_limit => 10)
+
+    assert_not_nil firm.account
+
+    firm.destroy
+
+    assert !firm.errors.empty?
+    assert_equal "Cannot delete record because dependent account exists", firm.errors[:base].first
+    assert RestrictedFirm.exists?(:name => 'restrict')
+    assert firm.account.present?
+  ensure
+    ActiveRecord::Base.dependent_restrict_raises = true
   end
 
   def test_successful_build_association
@@ -456,4 +482,11 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
 
     assert_equal car.id, bulb.attributes_after_initialize['car_id']
   end
+
+  def test_building_has_one_association_with_dependent_restrict
+    klass = Class.new(ActiveRecord::Base)
+    
+    assert_deprecated     { klass.has_one :account, :dependent => :restrict }
+    assert_not_deprecated { klass.has_one :account }
+  end
 end