Merge pull request rails#23675 from kachick/activemodel-errors-indifferent

Adjust to indifferent access around some ActiveModel::Errors methods

Author: rafaelfranca

activemodel/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Allow indifferent access in `ActiveModel::Errors`.
+
+    `#include?`, `#has_key?`, `#key?`, `#delete` and `#full_messages_for`.
+
+    *Kenichi Kamiya*
+
 *   Removed deprecated `:tokenizer` in the length validator.
 
     *Rafael Mendonça França*

activemodel/lib/active_model/errors.rb

@@ -110,6 +110,7 @@ def clear
     #   person.errors.include?(:name) # => true
     #   person.errors.include?(:age)  # => false
     def include?(attribute)
+      attribute = attribute.to_sym
       messages.key?(attribute) && messages[attribute].present?
     end
     alias :has_key? :include?
@@ -121,8 +122,9 @@ def include?(attribute)
     #   person.errors.delete(:name) # => ["cannot be nil"]
     #   person.errors[:name]        # => []
     def delete(key)
-      details.delete(key)
-      messages.delete(key)
+      attribute = key.to_sym
+      details.delete(attribute)
+      messages.delete(attribute)
     end
 
     # When passed a symbol or a name of a method, returns an array of errors
@@ -342,6 +344,7 @@ def full_messages
     #   person.errors.full_messages_for(:name)
     #   # => ["Name is too short (minimum is 5 characters)", "Name can't be blank"]
     def full_messages_for(attribute)
+      attribute = attribute.to_sym
       messages[attribute].map { |message| full_message(attribute, message) }
     end
 

activemodel/test/cases/errors_test.rb

@@ -30,14 +30,15 @@ def self.lookup_ancestors
   def test_delete
     errors = ActiveModel::Errors.new(self)
     errors[:foo] << "omg"
-    errors.delete(:foo)
+    errors.delete("foo")
     assert_empty errors[:foo]
   end
 
   def test_include?
     errors = ActiveModel::Errors.new(self)
     errors[:foo] << "omg"
     assert_includes errors, :foo, "errors should include :foo"
+    assert_includes errors, "foo", "errors should include 'foo' as :foo"
   end
 
   def test_dup
@@ -52,6 +53,7 @@ def test_has_key?
     errors = ActiveModel::Errors.new(self)
     errors[:foo] << "omg"
     assert_equal true, errors.has_key?(:foo), "errors should have key :foo"
+    assert_equal true, errors.has_key?("foo"), "errors should have key 'foo' as :foo"
   end
 
   def test_has_no_key
@@ -63,6 +65,7 @@ def test_key?
     errors = ActiveModel::Errors.new(self)
     errors[:foo] << "omg"
     assert_equal true, errors.key?(:foo), "errors should have key :foo"
+    assert_equal true, errors.key?("foo"), "errors should have key 'foo' as :foo"
   end
 
   def test_no_key
@@ -150,10 +153,11 @@ def test_no_key
     assert_equal ["cannot be blank"], person.errors[:name]
   end
 
-  test "added? detects if a specific error was added to the object" do
+  test "added? detects indifferent if a specific error was added to the object" do
     person = Person.new
     person.errors.add(:name, "cannot be blank")
     assert person.errors.added?(:name, "cannot be blank")
+    assert person.errors.added?("name", "cannot be blank")
   end
 
   test "added? handles symbol message" do
@@ -241,7 +245,7 @@ def test_no_key
     assert_equal ["name cannot be blank", "name cannot be nil"], person.errors.full_messages
   end
 
-  test "full_messages_for contains all the error messages for the given attribute" do
+  test "full_messages_for contains all the error messages for the given attribute indifferent" do
     person = Person.new
     person.errors.add(:name, "cannot be blank")
     person.errors.add(:name, "cannot be nil")
@@ -253,6 +257,7 @@ def test_no_key
     person.errors.add(:name, "cannot be blank")
     person.errors.add(:email, "cannot be blank")
     assert_equal ["name cannot be blank"], person.errors.full_messages_for(:name)
+    assert_equal ["name cannot be blank"], person.errors.full_messages_for("name")
   end
 
   test "full_messages_for returns an empty list in case there are no errors for the given attribute" do