From 26b35a4096e4725098b447c57f7b663a15caa8b9 Mon Sep 17 00:00:00 2001
From: Radan Skoric <radan.skoric@gmail.com>
Date: Sun, 22 Mar 2015 23:10:28 +0100
Subject: Fix ActiveModel::Errors#delete return value to stay backward
 compatible

Rails 5.0 changes to ActiveModel::Errors include addition of `details`
that also accidentally changed the return value of `delete`. Since
there was no test for that behavior it went unnoticed. This commit
adds a test and fixes the regression.

Small improvements to comments have also been made. Since `get` is
getting deprecated it is better to use `[]` in other methods' code
examples. Also, in the module usage example, `def Person.method`
was replaced with a more commonly used `def self.method` code style.
---
 activemodel/lib/active_model/errors.rb | 14 +++++++-------
 activemodel/test/cases/errors_test.rb  |  6 ++++++
 2 files changed, 13 insertions(+), 7 deletions(-)

(limited to 'activemodel')

diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb
index cbca76d949..e35ed03e74 100644
--- a/activemodel/lib/active_model/errors.rb
+++ b/activemodel/lib/active_model/errors.rb
@@ -34,11 +34,11 @@ module ActiveModel
   #       send(attr)
   #     end
   #
-  #     def Person.human_attribute_name(attr, options = {})
+  #     def self.human_attribute_name(attr, options = {})
   #       attr
   #     end
   #
-  #     def Person.lookup_ancestors
+  #     def self.lookup_ancestors
   #       [self]
   #     end
   #   end
@@ -124,9 +124,9 @@ module ActiveModel
 
     # Set messages for +key+ to +value+.
     #
-    #   person.errors.get(:name) # => ["cannot be nil"]
+    #   person.errors[:name] # => ["cannot be nil"]
     #   person.errors.set(:name, ["can't be nil"])
-    #   person.errors.get(:name) # => ["can't be nil"]
+    #   person.errors[:name] # => ["can't be nil"]
     def set(key, value)
       ActiveSupport::Deprecation.warn(<<-MESSAGE.squish)
         ActiveModel::Errors#set is deprecated and will be removed in Rails 5.1.
@@ -139,12 +139,12 @@ module ActiveModel
 
     # Delete messages for +key+. Returns the deleted messages.
     #
-    #   person.errors.get(:name)    # => ["cannot be nil"]
+    #   person.errors[:name]    # => ["cannot be nil"]
     #   person.errors.delete(:name) # => ["cannot be nil"]
-    #   person.errors.get(:name)    # => []
+    #   person.errors[:name]    # => []
     def delete(key)
-      messages.delete(key)
       details.delete(key)
+      messages.delete(key)
     end
 
     # When passed a symbol or a name of a method, returns an array of errors
diff --git a/activemodel/test/cases/errors_test.rb b/activemodel/test/cases/errors_test.rb
index da142ea2c0..27631005e5 100644
--- a/activemodel/test/cases/errors_test.rb
+++ b/activemodel/test/cases/errors_test.rb
@@ -377,6 +377,12 @@ class ErrorsTest < ActiveModel::TestCase
     assert_empty errors.details[:name]
   end
 
+  test "delete returns the deleted messages" do
+    errors = ActiveModel::Errors.new(Person.new)
+    errors.add(:name, :invalid)
+    assert_equal errors.delete(:name), ["is invalid"]
+  end
+
   test "clear removes details" do
     person = Person.new
     person.errors.add(:name, :invalid)
-- 
cgit v1.2.3