Deprecate and rename the keys for association restrict_dependent_destroy

Previously `has_one` and `has_many` associations were using the
`one` and `many` keys respectively. Both of these keys have special
meaning in I18n (they are considered to be pluralizations) so by
renaming them to `has_one` and `has_many` we make the messages more
explicit and most importantly they don't clash with linguistical
systems that need to validate translation keys (and their
pluralizations).

The `:'restrict_dependent_destroy.one'` key should be replaced with
`:'restrict_dependent_destroy.has_one'`, and
`:'restrict_dependent_destroy.many'` with
`:'restrict_dependent_destroy.has_many'`.

[Roque Pinel & Christopher Dell]

6 files changed, 73 insertions, 4 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index d9aa703d19..22f805fea4 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,19 @@
+*   Deprecate the keys for association `restrict_dependent_destroy` errors in favor
+    of new key names.
+
+    Previously `has_one` and `has_many` associations were using the
+    `one` and `many` keys respectively. Both of these keys have special
+    meaning in I18n (they are considered to be pluralizations) so by
+    renaming them to `has_one` and `has_many` we make the messages more explicit
+    and most importantly they don't clash with linguistical systems that need to
+    validate translation keys (and their pluralizations).
+
+    The `:'restrict_dependent_destroy.one'` key should be replaced with
+    `:'restrict_dependent_destroy.has_one'`, and `:'restrict_dependent_destroy.many'`
+    with `:'restrict_dependent_destroy.has_many'`.
+
+    *Roque Pinel*, *Christopher Dell*
+
 *   Fix state being carried over from previous transaction.
 
     Considering the following example where `name` is a required attribute.
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index ca27c9fdde..9f6c832c1b 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -16,7 +16,14 @@ module ActiveRecord
         when :restrict_with_error
           unless empty?
             record = klass.human_attribute_name(reflection.name).downcase
-            owner.errors.add(:base, :"restrict_dependent_destroy.many", record: record)
+            message = owner.errors.generate_message(:base, :'restrict_dependent_destroy.many', record: record, raise: true) rescue nil
+            if message
+              ActiveSupport::Deprecation.warn(<<-MESSAGE.squish)
+                The error key `:'restrict_dependent_destroy.many'` has been deprecated and will be removed in Rails 5.1.
+                Please use `:'restrict_dependent_destroy.has_many'` instead.
+              MESSAGE
+            end
+            owner.errors.add(:base, message || :'restrict_dependent_destroy.has_many', record: record)
             throw(:abort)
           end
 
diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb
index 41a75b820e..5a92bc5e8a 100644
--- a/activerecord/lib/active_record/associations/has_one_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_association.rb
@@ -12,7 +12,14 @@ module ActiveRecord
         when :restrict_with_error
           if load_target
             record = klass.human_attribute_name(reflection.name).downcase
-            owner.errors.add(:base, :"restrict_dependent_destroy.one", record: record)
+            message = owner.errors.generate_message(:base, :'restrict_dependent_destroy.one', record: record, raise: true) rescue nil
+            if message
+              ActiveSupport::Deprecation.warn(<<-MESSAGE.squish)
+                The error key `:'restrict_dependent_destroy.one'` has been deprecated and will be removed in Rails 5.1.
+                Please use `:'restrict_dependent_destroy.has_one'` instead.
+              MESSAGE
+            end
+            owner.errors.add(:base, message || :'restrict_dependent_destroy.has_one', record: record)
             throw(:abort)
           end
 
diff --git a/activerecord/lib/active_record/locale/en.yml b/activerecord/lib/active_record/locale/en.yml
index 8a3c27e6da..0b35027b2b 100644
--- a/activerecord/lib/active_record/locale/en.yml
+++ b/activerecord/lib/active_record/locale/en.yml
@@ -16,8 +16,8 @@ en:
       messages:
         record_invalid: "Validation failed: %{errors}"
         restrict_dependent_destroy:
-          one: "Cannot delete record because a dependent %{record} exists"
-          many: "Cannot delete record because dependent %{record} exist"
+          has_one: "Cannot delete record because a dependent %{record} exists"
+          has_many: "Cannot delete record because dependent %{record} exist"
         # 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 f4bacbea2e..c2ef59d0f7 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -1426,6 +1426,26 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert firm.companies.exists?(:name => 'child')
   end
 
+  def test_restrict_with_error_is_deprecated_using_key_many
+    I18n.backend = I18n::Backend::Simple.new
+    I18n.backend.store_translations :en, activerecord: { errors: { messages: { restrict_dependent_destroy: { many: 'message for deprecated key' } } } }
+
+    firm = RestrictedWithErrorFirm.create!(name: 'restrict')
+    firm.companies.create(name: 'child')
+
+    assert !firm.companies.empty?
+
+    assert_deprecated { firm.destroy }
+
+    assert !firm.errors.empty?
+
+    assert_equal 'message for deprecated key', firm.errors[:base].first
+    assert RestrictedWithErrorFirm.exists?(name: 'restrict')
+    assert firm.companies.exists?(name: 'child')
+  ensure
+    I18n.backend.reload!
+  end
+
   def test_restrict_with_error
     firm = RestrictedWithErrorFirm.create!(:name => 'restrict')
     firm.companies.create(:name => 'child')
diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb
index b7cb5a064f..5a8afaf4d2 100644
--- a/activerecord/test/cases/associations/has_one_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_associations_test.rb
@@ -178,6 +178,25 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
     assert firm.account.present?
   end
 
+  def test_restrict_with_error_is_deprecated_using_key_one
+    I18n.backend = I18n::Backend::Simple.new
+    I18n.backend.store_translations :en, activerecord: { errors: { messages: { restrict_dependent_destroy: { one: 'message for deprecated key' } } } }
+
+    firm = RestrictedWithErrorFirm.create!(name: 'restrict')
+    firm.create_account(credit_limit: 10)
+
+    assert_not_nil firm.account
+
+    assert_deprecated { firm.destroy }
+
+    assert !firm.errors.empty?
+    assert_equal 'message for deprecated key', firm.errors[:base].first
+    assert RestrictedWithErrorFirm.exists?(name: 'restrict')
+    assert firm.account.present?
+  ensure
+    I18n.backend.reload!
+  end
+
   def test_restrict_with_error
     firm = RestrictedWithErrorFirm.create!(:name => 'restrict')
     firm.create_account(:credit_limit => 10)