Make ActiveModel::Errors#add_on_blank and #add_on_empty accept an options hash and make various Validators pass their (filtered) options.

This makes it possible to pass additional options through Validators to message generation. E.g. plugin authors want to add validates_presence_of :foo, :format => "some format".

Also, cleanup the :default vs :message options confusion in ActiveModel validation message generation.

Also, deprecate ActiveModel::Errors#add_on_blank(attributes, custom_message) in favor of ActiveModel::Errors#add_on_blank(attributes, options).

Also, refactoring of ActiveModel and ActiveRecord Validation tests. Test are a lot more DRY now. Better test coverage as well now.

The first four points were reapplied from an older patch of Sven Fuchs which didn't apply cleanly anymore and was not complete yet.

Signed-off-by: José Valim <jose.valim@gmail.com>

2 files changed, 34 insertions, 28 deletions

diff --git a/activerecord/test/cases/validations/i18n_generate_message_validation_test.rb b/activerecord/test/cases/validations/i18n_generate_message_validation_test.rb
index 8ee2a5868c..454e42ed37 100644
--- a/activerecord/test/cases/validations/i18n_generate_message_validation_test.rb
+++ b/activerecord/test/cases/validations/i18n_generate_message_validation_test.rb
@@ -9,22 +9,22 @@ class I18nGenerateMessageValidationTest < ActiveRecord::TestCase
     I18n.backend = I18n::Backend::Simple.new
   end
 
-  # validates_associated: generate_message(attr_name, :invalid, :default => configuration[:message], :value => value)
+  # validates_associated: generate_message(attr_name, :invalid, :message => custom_message, :value => value)
   def test_generate_message_invalid_with_default_message
-    assert_equal 'is invalid', @topic.errors.generate_message(:title, :invalid, :default => nil, :value => 'title')
+    assert_equal 'is invalid', @topic.errors.generate_message(:title, :invalid, :value => 'title')
   end
 
   def test_generate_message_invalid_with_custom_message
-    assert_equal 'custom message title', @topic.errors.generate_message(:title, :invalid, :default => 'custom message %{value}', :value => 'title')
+    assert_equal 'custom message title', @topic.errors.generate_message(:title, :invalid, :message => 'custom message %{value}', :value => 'title')
   end
 
-  # validates_uniqueness_of: generate_message(attr_name, :taken, :default => configuration[:message])
+  # validates_uniqueness_of: generate_message(attr_name, :taken, :message => custom_message)
   def test_generate_message_taken_with_default_message
-    assert_equal "has already been taken", @topic.errors.generate_message(:title, :taken, :default => nil, :value => 'title')
+    assert_equal "has already been taken", @topic.errors.generate_message(:title, :taken, :value => 'title')
   end
 
   def test_generate_message_taken_with_custom_message
-    assert_equal 'custom message title', @topic.errors.generate_message(:title, :taken, :default => 'custom message %{value}', :value => 'title')
+    assert_equal 'custom message title', @topic.errors.generate_message(:title, :taken, :message => 'custom message %{value}', :value => 'title')
   end
 
   # ActiveRecord#RecordInvalid exception
diff --git a/activerecord/test/cases/validations/i18n_validation_test.rb b/activerecord/test/cases/validations/i18n_validation_test.rb
index 38fa2b821d..15b97c02c8 100644
--- a/activerecord/test/cases/validations/i18n_validation_test.rb
+++ b/activerecord/test/cases/validations/i18n_validation_test.rb
@@ -31,34 +31,40 @@ class I18nValidationTest < ActiveRecord::TestCase
     end
   end
 
-  # validates_uniqueness_of w/ mocha
+  # A set of common cases for ActiveModel::Validations message generation that
+  # are used to generate tests to keep things DRY
+  #
+  COMMON_CASES = [
+  # [ case,                                validation_options,            generate_message_options]
+    [ "given no options",                  {},                            {}],
+    [ "given custom message",              {:message => "custom"},        {:message => "custom"}],
+    [ "given if condition",                {:if     => lambda { true }},  {}],
+    [ "given unless condition",            {:unless => lambda { false }}, {}],
+    [ "given option that is not reserved", {:format => "jpg"},            {:format => "jpg" }]
+    # TODO Add :on case, but below doesn't work, because then the validation isn't run for some reason
+    #      even when using .save instead .valid?
+    # [ "given on condition",     {:on => :save},                {}]
+  ]
 
-  def test_validates_uniqueness_of_generates_message
-    Topic.validates_uniqueness_of :title
-    @topic.title = unique_topic.title
-    @topic.errors.expects(:generate_message).with(:title, :taken, {:default => nil, :value => 'unique!'})
-    @topic.valid?
-  end
+  # validates_uniqueness_of w/ mocha
 
-  def test_validates_uniqueness_of_generates_message_with_custom_default_message
-    Topic.validates_uniqueness_of :title, :message => 'custom'
-    @topic.title = unique_topic.title
-    @topic.errors.expects(:generate_message).with(:title, :taken, {:default => 'custom', :value => 'unique!'})
-    @topic.valid?
+  COMMON_CASES.each do |name, validation_options, generate_message_options|
+    test "validates_uniqueness_of on generated message #{name}" do
+      Topic.validates_uniqueness_of :title, validation_options
+      @topic.title = unique_topic.title
+      @topic.errors.expects(:generate_message).with(:title, :taken, generate_message_options.merge(:value => 'unique!'))
+      @topic.valid?
+    end
   end
 
   # validates_associated w/ mocha
 
-  def test_validates_associated_generates_message
-    Topic.validates_associated :replies
-    replied_topic.errors.expects(:generate_message).with(:replies, :invalid, {:value => replied_topic.replies, :default => nil})
-    replied_topic.valid?
-  end
-
-  def test_validates_associated_generates_message_with_custom_default_message
-    Topic.validates_associated :replies
-    replied_topic.errors.expects(:generate_message).with(:replies, :invalid, {:value => replied_topic.replies, :default => nil})
-    replied_topic.valid?
+  COMMON_CASES.each do |name, validation_options, generate_message_options|
+    test "validates_associated on generated message #{name}" do
+      Topic.validates_associated :replies, validation_options
+      replied_topic.errors.expects(:generate_message).with(:replies, :invalid, generate_message_options.merge(:value => replied_topic.replies))
+      replied_topic.save
+    end
   end
 
   # validates_associated w/o mocha