From 01269aede398f63e5ef68ebe0f0dafbc472c686e Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Tue, 28 Mar 2017 12:05:14 +0300 Subject: [PATCH] Fix ActiveModel::Errors #keys, #values Before: person.errors.keys # => [] person.errors.values # => [] person.errors[:name] # => [] person.errors.keys # => [:name] person.errors.values # => [[]] After: person.errors.keys # => [] person.errors.values # => [] person.errors[:name] # => [] person.errors.keys # => [] person.errors.values # => [] Related to #23468 --- activemodel/CHANGELOG.md | 30 ++++++++++++++++++++++++++ activemodel/lib/active_model/errors.rb | 17 ++++++--------- activemodel/test/cases/errors_test.rb | 16 ++++++++++++++ 3 files changed, 52 insertions(+), 11 deletions(-) diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 6b9b3dd7ff..16b1b50cb3 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,3 +1,33 @@ +* Fix methods `#keys`, `#values` in `ActiveModel::Errors`. + + Change `#keys` to only return the keys that don't have empty messages. + + Change `#values` to only return the not empty values. + + Example: + + # Before + person = Person.new + person.errors.keys # => [] + person.errors.values # => [] + person.errors.messages # => {} + person.errors[:name] # => [] + person.errors.messages # => {:name => []} + person.errors.keys # => [:name] + person.errors.values # => [[]] + + # After + person = Person.new + person.errors.keys # => [] + person.errors.values # => [] + person.errors.messages # => {} + person.errors[:name] # => [] + person.errors.messages # => {:name => []} + person.errors.keys # => [] + person.errors.values # => [] + + *bogdanvlviv* + * Avoid converting integer as a string into float. *namusyaka* diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 9df4ca51fe..5d3472802b 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -132,15 +132,6 @@ module ActiveModel # # person.errors[:name] # => ["cannot be nil"] # person.errors['name'] # => ["cannot be nil"] - # - # Note that, if you try to get errors of an attribute which has - # no errors associated with it, this method will instantiate - # an empty error list for it and +keys+ will return an array - # of error keys which includes this attribute. - # - # person.errors.keys # => [] - # person.errors[:name] # => [] - # person.errors.keys # => [:name] def [](attribute) messages[attribute.to_sym] end @@ -181,7 +172,9 @@ module ActiveModel # person.errors.messages # => {:name=>["cannot be nil", "must be specified"]} # person.errors.values # => [["cannot be nil", "must be specified"]] def values - messages.values + messages.reject do |key, value| + value.empty? + end.values end # Returns all message keys. @@ -189,7 +182,9 @@ module ActiveModel # person.errors.messages # => {:name=>["cannot be nil", "must be specified"]} # person.errors.keys # => [:name] def keys - messages.keys + messages.reject do |key, value| + value.empty? + end.keys end # Returns +true+ if no errors are found, +false+ otherwise. diff --git a/activemodel/test/cases/errors_test.rb b/activemodel/test/cases/errors_test.rb index 0872084cf5..43aee5a814 100644 --- a/activemodel/test/cases/errors_test.rb +++ b/activemodel/test/cases/errors_test.rb @@ -99,6 +99,14 @@ class ErrorsTest < ActiveModel::TestCase assert_equal ["omg", "zomg"], errors.values end + test "values returns an empty array after try to get a message only" do + errors = ActiveModel::Errors.new(self) + errors.messages[:foo] + errors.messages[:baz] + + assert_equal [], errors.values + end + test "keys returns the error keys" do errors = ActiveModel::Errors.new(self) errors.messages[:foo] << "omg" @@ -107,6 +115,14 @@ class ErrorsTest < ActiveModel::TestCase assert_equal [:foo, :baz], errors.keys end + test "keys returns an empty array after try to get a message only" do + errors = ActiveModel::Errors.new(self) + errors.messages[:foo] + errors.messages[:baz] + + assert_equal [], errors.keys + end + test "detecting whether there are errors with empty?, blank?, include?" do person = Person.new person.errors[:foo]