diff options
author | Edouard CHIN <edouard.chin@shopify.com> | 2019-07-10 17:52:43 +0200 |
---|---|---|
committer | Edouard CHIN <edouard.chin@shopify.com> | 2019-07-10 23:33:32 +0200 |
commit | 06756290d5000034e6815f57d5003111ff0b2865 (patch) | |
tree | 21d9459ff213c4416edc39c5430599d6478b6300 /activemodel | |
parent | 84ff4f6ea2a118b47160e3fe7ed29bff52c2a7a2 (diff) | |
download | rails-06756290d5000034e6815f57d5003111ff0b2865.tar.gz rails-06756290d5000034e6815f57d5003111ff0b2865.tar.bz2 rails-06756290d5000034e6815f57d5003111ff0b2865.zip |
Fix `AM::Errors.added?` trying to generate a message:
- When a ActiveRecord record get saved and validated as part of a
collection association, the errors attribute are changed to reflect
the children names. You end up with an error attribute that will
look like this:
`author.errors # {:'books.title' => [:blank]}`
https://github.com/rails/rails/blob/2fe20cb55c76e6e50ec3a4ec5b03bbb65adba290/activerecord/lib/active_record/autosave_association.rb#L331-L340
We then can't check if the `books.title` errors was added using
`ActiveModel::Errors#added?` because it tries to generate a message
to make the match and end up calling the "books.title" method
on the Author.
```
author.errors.added?(:'books.title', :blank) => NoMethodError: undefined method `books.title'
```
This patch modify the behaviour of `strict_match?` to not generate
a message to make the comparison but instead make a strict
comparison with the `options` from the error.
Diffstat (limited to 'activemodel')
-rw-r--r-- | activemodel/lib/active_model/error.rb | 4 | ||||
-rw-r--r-- | activemodel/test/cases/errors_test.rb | 22 |
2 files changed, 24 insertions, 2 deletions
diff --git a/activemodel/lib/active_model/error.rb b/activemodel/lib/active_model/error.rb index f7267fc7bf..b45d432bb2 100644 --- a/activemodel/lib/active_model/error.rb +++ b/activemodel/lib/active_model/error.rb @@ -58,9 +58,9 @@ module ActiveModel end def strict_match?(attribute, type, **options) - return false unless match?(attribute, type, **options) + return false unless match?(attribute, type) - full_message == Error.new(@base, attribute, type, **options).full_message + options == @options.except(*CALLBACKS_OPTIONS + MESSAGE_OPTIONS) end def ==(other) diff --git a/activemodel/test/cases/errors_test.rb b/activemodel/test/cases/errors_test.rb index baaf404f2e..e7ec9225b0 100644 --- a/activemodel/test/cases/errors_test.rb +++ b/activemodel/test/cases/errors_test.rb @@ -274,6 +274,28 @@ class ErrorsTest < ActiveModel::TestCase assert_equal [msg], person.errors[:name] end + test "added? when attribute was added through a collection" do + person = Person.new + person.errors.add(:"family_members.name", :too_long, count: 25) + assert person.errors.added?(:"family_members.name", :too_long, count: 25) + assert_not person.errors.added?(:"family_members.name", :too_long) + assert_not person.errors.added?(:"family_members.name", :too_long, name: "hello") + end + + test "added? ignores callback option" do + person = Person.new + + person.errors.add(:name, :too_long, if: -> { true }) + assert person.errors.added?(:name, :too_long) + end + + test "added? ignores message option" do + person = Person.new + + person.errors.add(:name, :too_long, message: proc { "foo" }) + assert person.errors.added?(:name, :too_long) + end + test "added? detects indifferent if a specific error was added to the object" do person = Person.new person.errors.add(:name, "cannot be blank") |