Deprecate Errors#on_base/add_to_base/invalid?/each_full

7 files changed, 34 insertions, 19 deletions

diff --git a/activemodel/lib/active_model/deprecated_error_methods.rb b/activemodel/lib/active_model/deprecated_error_methods.rb
index 2f2e804ade..433de8931a 100644
--- a/activemodel/lib/active_model/deprecated_error_methods.rb
+++ b/activemodel/lib/active_model/deprecated_error_methods.rb
@@ -11,22 +11,22 @@ module ActiveModel
     end
 
     def on_base
-      # ActiveSupport::Deprecation.warn "Errors#on_base have been deprecated, use Errors#[:base] instead"
-      on(:base)
+      ActiveSupport::Deprecation.warn "Errors#on_base have been deprecated, use Errors#[:base] instead"
+      ActiveSupport::Deprecation.silence { on(:base) }
     end
 
     def add_to_base(msg)
-      # ActiveSupport::Deprecation.warn "Errors#add_to_base(msg) has been deprecated, use Errors#[:base] << msg instead"
+      ActiveSupport::Deprecation.warn "Errors#add_to_base(msg) has been deprecated, use Errors#[:base] << msg instead"
       self[:base] << msg
     end
   
     def invalid?(attribute)
-      # ActiveSupport::Deprecation.warn "Errors#invalid?(attribute) has been deprecated, use Errors#[attribute].any? instead"
+      ActiveSupport::Deprecation.warn "Errors#invalid?(attribute) has been deprecated, use Errors#[attribute].any? instead"
       self[attribute].any?
     end
 
     def each_full
-      # ActiveSupport::Deprecation.warn "Errors#each_full has been deprecated, use Errors#to_a.each instead"
+      ActiveSupport::Deprecation.warn "Errors#each_full has been deprecated, use Errors#to_a.each instead"
       to_a.each { |error| yield error }
     end
   end
diff --git a/activemodel/test/cases/validations/conditional_validation_test.rb b/activemodel/test/cases/validations/conditional_validation_test.rb
index 72b4800dce..4c716d5d48 100644
--- a/activemodel/test/cases/validations/conditional_validation_test.rb
+++ b/activemodel/test/cases/validations/conditional_validation_test.rb
@@ -124,15 +124,15 @@ class ConditionalValidationTest < ActiveModel::TestCase
     Topic.validates_presence_of(:author_name, :if => "title.to_s.match('important')")
 
     t = Topic.new
-    assert !t.valid?, "A topic without a title should not be valid"
-    assert !t.errors.invalid?("author_name"), "A topic without an 'important' title should not require an author"
+    assert t.invalid?, "A topic without a title should not be valid"
+    assert t.errors[:author_name].empty?, "A topic without an 'important' title should not require an author"
 
     t.title = "Just a title"
     assert t.valid?, "A topic with a basic title should be valid"
 
     t.title = "A very important title"
     assert !t.valid?, "A topic with an important title, but without an author, should not be valid"
-    assert t.errors.invalid?("author_name"), "A topic with an 'important' title should require an author"
+    assert t.errors[:author_name].any?, "A topic with an 'important' title should require an author"
 
     t.author_name = "Hubert J. Farnsworth"
     assert t.valid?, "A topic with an important title and author should be valid"
diff --git a/activemodel/test/cases/validations_test.rb b/activemodel/test/cases/validations_test.rb
index a5dd7af04d..8c89494247 100644
--- a/activemodel/test/cases/validations_test.rb
+++ b/activemodel/test/cases/validations_test.rb
@@ -27,7 +27,7 @@ class ValidationsTest < ActiveModel::TestCase
     r = Reply.new
     r.title = "There's no content!"
     assert !r.valid?
-    assert r.errors.invalid?("content"), "A reply without content should mark that attribute as invalid"
+    assert r.errors[:content].any?, "A reply without content should mark that attribute as invalid"
     assert_equal ["Empty"], r.errors["content"], "A reply without content should contain an error"
     assert_equal 1, r.errors.count
   end
@@ -36,10 +36,10 @@ class ValidationsTest < ActiveModel::TestCase
     r = Reply.new
     assert !r.valid?
 
-    assert r.errors.invalid?("title"), "A reply without title should mark that attribute as invalid"
+    assert r.errors[:title].any?, "A reply without title should mark that attribute as invalid"
     assert_equal ["Empty"], r.errors["title"], "A reply without title should contain an error"
 
-    assert r.errors.invalid?("content"), "A reply without content should mark that attribute as invalid"
+    assert r.errors[:content].any?, "A reply without content should mark that attribute as invalid"
     assert_equal ["Empty"], r.errors["content"], "A reply without content should contain an error"
 
     assert_equal 2, r.errors.count
@@ -73,10 +73,10 @@ class ValidationsTest < ActiveModel::TestCase
     r = Reply.new
     r.content = "Mismatch"
     r.save
-    r.errors.add_to_base "Reply is not dignifying"
+    r.errors[:base] << "Reply is not dignifying"
 
     errors = []
-    r.errors.each_full { |error| errors << error }
+    r.errors.to_a.each { |error| errors << error }
 
     assert_equal ["Reply is not dignifying"], r.errors[:base]
 
@@ -134,7 +134,7 @@ class ValidationsTest < ActiveModel::TestCase
 
     t = Topic.new
     assert t.invalid?
-    assert t.errors.invalid?(:title)
+    assert t.errors[:title].any?
 
     t.title = 'Things are going to change'
     assert !t.invalid?
@@ -163,4 +163,16 @@ class ValidationsTest < ActiveModel::TestCase
       end
     end
   end
+
+  def test_deprecated_errors_on_base_and_each
+    t = Topic.new
+    assert t.valid?
+
+    assert_deprecated { t.errors.add_to_base "invalid topic" }
+    assert_deprecated { assert_equal "invalid topic", t.errors.on_base }
+    assert_deprecated { assert t.errors.invalid?(:base) }
+
+    all_errors = t.errors.to_a
+    assert_deprecated { assert_equal all_errors, t.errors.each_full{|err| err} }
+  end
 end
\ No newline at end of file
diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb
index 69691aad02..a220580a8d 100644
--- a/activerecord/test/cases/validations_test.rb
+++ b/activerecord/test/cases/validations_test.rb
@@ -27,7 +27,7 @@ class ValidationsTest < ActiveRecord::TestCase
     r = Reply.new
     r.title = "Wrong Create"
     assert !r.valid?
-    assert r.errors.invalid?("title"), "A reply with a bad title should mark that attribute as invalid"
+    assert r.errors[:title].any?, "A reply with a bad title should mark that attribute as invalid"
     assert_equal ["is Wrong Create"], r.errors[:title], "A reply with a bad content should contain an error"
   end
 
@@ -40,7 +40,7 @@ class ValidationsTest < ActiveRecord::TestCase
     r.title = "Wrong Update"
     assert !r.save, "Second save should fail"
 
-    assert r.errors.invalid?("title"), "A reply with a bad title should mark that attribute as invalid"
+    assert r.errors[:title].any?, "A reply with a bad title should mark that attribute as invalid"
     assert_equal ["is Wrong Update"], r.errors[:title], "A reply with a bad content should contain an error"
   end
 
diff --git a/activeresource/lib/active_resource/validations.rb b/activeresource/lib/active_resource/validations.rb
index 7fe3162c02..9d3d45b010 100644
--- a/activeresource/lib/active_resource/validations.rb
+++ b/activeresource/lib/active_resource/validations.rb
@@ -17,7 +17,7 @@ module ActiveResource
           end
         end
         
-        add_to_base message if attr_message.nil?
+        self[:base] << message if attr_message.nil?
       end
     end
   end
diff --git a/activeresource/test/abstract_unit.rb b/activeresource/test/abstract_unit.rb
index 0f11ea482a..ce9371d050 100644
--- a/activeresource/test/abstract_unit.rb
+++ b/activeresource/test/abstract_unit.rb
@@ -14,6 +14,9 @@ require 'setter_trap'
 
 ActiveResource::Base.logger = Logger.new("#{File.dirname(__FILE__)}/debug.log")
 
+# Show backtraces for deprecated behavior for quicker cleanup.
+ActiveSupport::Deprecation.debug = true
+
 def uses_gem(gem_name, test_name, version = '> 0')
   gem gem_name.to_s, version
   require gem_name.to_s
diff --git a/activeresource/test/base_errors_test.rb b/activeresource/test/base_errors_test.rb
index 8c177c8006..28813821df 100644
--- a/activeresource/test/base_errors_test.rb
+++ b/activeresource/test/base_errors_test.rb
@@ -20,7 +20,7 @@ class BaseErrorsTest < Test::Unit::TestCase
   end
 
   def test_should_parse_errors_to_individual_attributes
-    assert @person.errors.invalid?(:name)    
+    assert @person.errors[:name].any?
     assert_equal ["can't be blank"], @person.errors[:age]
     assert_equal ["can't be blank", "must start with a letter"], @person.errors[:name]
     assert_equal ["Person quota full for today."], @person.errors[:base]
@@ -34,7 +34,7 @@ class BaseErrorsTest < Test::Unit::TestCase
 
   def test_should_iterate_over_full_errors
     errors = []
-    @person.errors.each_full { |message| errors << message }
+    @person.errors.to_a.each { |message| errors << message }
     assert errors.include?("Name can't be blank")
   end