From 23074c81a5e0b1e08e2e6555053678e8d656f484 Mon Sep 17 00:00:00 2001 From: Manoj Date: Tue, 31 Jan 2012 13:46:07 +0530 Subject: [PATCH] suggested fixes for :dependent => :restrict deprecation. --- .../lib/active_record/associations.rb | 6 ++++-- .../associations/builder/association.rb | 2 +- activerecord/lib/active_record/locale/en.yml | 2 +- .../has_many_associations_test.rb | 21 +++++-------------- .../associations/has_one_associations_test.rb | 21 +++++-------------- 5 files changed, 16 insertions(+), 36 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index b44ab26a73..958821add6 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1097,7 +1097,8 @@ module ActiveRecord # alongside this object by calling their +destroy+ method. If set to :delete_all all associated # objects are deleted *without* calling their +destroy+ method. If set to :nullify all associated # objects' foreign keys are set to +NULL+ *without* calling their +save+ callbacks. If set to - # :restrict this object cannot be deleted if it has any associated objects. + # :restrict an error will be added to the object, preventing its deletion, if any associated + # objects are present. # # If using with the :through option, the association on the join model must be # a +belongs_to+, and the records which get deleted are the join records, rather than @@ -1250,7 +1251,8 @@ module ActiveRecord # If set to :destroy, the associated object is destroyed when this object is. If set to # :delete, the associated object is deleted *without* calling its destroy method. # If set to :nullify, the associated object's foreign key is set to +NULL+. - # If set to :restrict, this object cannot be deleted if it has any associated object. + # If set to :restrict, an error will be added to the object, preventing its deletion, if an + # associated object is present. # [:foreign_key] # Specify the foreign key used for the association. By default this is guessed to be the name # of this class in lower-case and "_id" suffixed. So a Person class that makes a +has_one+ association diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index 45d3f37089..ba2fcf5ed7 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -74,7 +74,7 @@ module ActiveRecord::Associations::Builder if dependent_restrict_raises? raise ActiveRecord::DeleteRestrictionError.new(name) else - errors.add(:base, :restrict_dependent_destroy, :model => name) + errors.add(:base, :restrict_dependent_destroy, :model => name.to_s.singularize) return false end end diff --git a/activerecord/lib/active_record/locale/en.yml b/activerecord/lib/active_record/locale/en.yml index 88edabfd41..8892f7ef2f 100644 --- a/activerecord/lib/active_record/locale/en.yml +++ b/activerecord/lib/active_record/locale/en.yml @@ -10,7 +10,7 @@ en: messages: taken: "has already been taken" record_invalid: "Validation failed: %{errors}" - restrict_dependent_destroy: "Cannot delete record because dependent %{model} exist" + restrict_dependent_destroy: "Cannot delete record because dependent %{model} exists" # Append your own errors here or at the model/attributes scope. # You can define own errors for models or model attributes. diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index a710900054..8c7956406a 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1170,7 +1170,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert !firm.errors.empty? - assert_equal "Cannot delete record because dependent companies exist", firm.errors[:base].first + 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 @@ -1676,20 +1676,9 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_building_has_many_association_with_restrict_dependency - assert_deprecated do - class_eval <<-EOF - class RestrictedFirm < ActiveRecord::Base - has_many :companies, :dependent => :restrict - end - EOF - end - - assert_not_deprecated do - class_eval <<-EOF - class Firm < ActiveRecord::Base - has_many :companies - end - EOF - end + 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 a17f33c8df..7c6736fb95 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -183,7 +183,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase firm.destroy assert !firm.errors.empty? - assert_equal "Cannot delete record because dependent account exist", firm.errors[:base].first + assert_equal "Cannot delete record because dependent account exists", firm.errors[:base].first assert RestrictedFirm.exists?(:name => 'restrict') assert firm.account.present? ensure @@ -484,20 +484,9 @@ class HasOneAssociationsTest < ActiveRecord::TestCase end def test_building_has_one_association_with_dependent_restrict - assert_deprecated do - class_eval <<-EOF - class RestrictedFirm < ActiveRecord::Base - has_one :account, :dependent => :restrict - end - EOF - end - - assert_not_deprecated do - class_eval <<-EOF - class Firm < ActiveRecord::Base - has_one :account - end - EOF - end + klass = Class.new(ActiveRecord::Base) + + assert_deprecated { klass.has_one :account, :dependent => :restrict } + assert_not_deprecated { klass.has_one :account } end end