Skip to content

Commit

Permalink
Deprecate and rename the keys for association restrict_dependent_destroy
Browse files Browse the repository at this point in the history
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]
  • Loading branch information
repinel committed Jun 23, 2015
1 parent 5e75f51 commit 9911b08
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 4 deletions.
16 changes: 16 additions & 0 deletions 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*

* Prevent error when using `force_reload: true` on an unassigned polymorphic
belongs_to association.

Expand Down
Expand Up @@ -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

Expand Down
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/locale/en.yml
Expand Up @@ -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.
Expand Down
20 changes: 20 additions & 0 deletions activerecord/test/cases/associations/has_many_associations_test.rb
Expand Up @@ -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')
Expand Down
19 changes: 19 additions & 0 deletions activerecord/test/cases/associations/has_one_associations_test.rb
Expand Up @@ -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)
Expand Down

0 comments on commit 9911b08

Please sign in to comment.