Merge pull request rails#20668 from repinel/rename-restrict-dependent-destroy-i18n-key

Deprecate the keys for association `restrict_dependent_destroy` errors in favor of new key names

Author: rafaelfranca

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.

activerecord/lib/active_record/associations/has_many_association.rb

@@ -16,7 +16,14 @@ def handle_dependency
         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
 

activerecord/lib/active_record/associations/has_one_association.rb

@@ -12,7 +12,14 @@ def handle_dependency
         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
 

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.

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -1426,6 +1426,26 @@ def test_restrict_with_exception
     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')

activerecord/test/cases/associations/has_one_associations_test.rb

@@ -178,6 +178,25 @@ def test_restrict_with_exception
     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)