Allow :if, :unless, :on, :allow_nil and :allow_blank as shared options in validates.

4 files changed, 61 insertions, 12 deletions

diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb
index 573c914e63..276472ea46 100644
--- a/activemodel/lib/active_model/validations.rb
+++ b/activemodel/lib/active_model/validations.rb
@@ -121,9 +121,7 @@ module ActiveModel
     #     end
     #   end
     #
-    def read_attribute_for_validation(key)
-      send(key)
-    end
+    alias :read_attribute_for_validation :send
   end
 end
 
diff --git a/activemodel/lib/active_model/validations/validates.rb b/activemodel/lib/active_model/validations/validates.rb
index e8935d3794..61caf32c06 100644
--- a/activemodel/lib/active_model/validations/validates.rb
+++ b/activemodel/lib/active_model/validations/validates.rb
@@ -31,7 +31,7 @@ module ActiveModel
       #     attr_accessor :name, :email
       # 
       #     validates :name, :presence => true, :uniqueness => true, :length => { :maximum => 100 }
-      #     validates :email, :presence => true, :email => true
+      #     validates :email, :presence => true, :format => { :with => /@/ }
       #   end
       # 
       # Validator classes my also exist within the class being validated
@@ -44,21 +44,33 @@ module ActiveModel
       #       end
       #     end
       #   end
-      # 
+      #
       #   class Film
       #     include ActiveModel::Validations
       #     include MyValidators
       # 
       #     validates :name, :title => true
       #   end 
-      # 
+      #
+      # The options :if, :unless, :on, :allow_blank and :allow_nil can be given to one specific
+      # validator:
+      #
+      #   validates :password, :presence => { :if => :password_required? }, :confirmation => true
+      #
+      # Or to all at the same time:
+      #
+      #   validates :password, :presence => true, :confirmation => true, :if => :password_required?
+      #
       def validates(*attributes)
-        validations = attributes.extract_options!
+        defaults = attributes.extract_options!
+        validations = defaults.slice!(:if, :unless, :on, :allow_blank, :allow_nil)
 
         raise ArgumentError, "You need to supply at least one attribute" if attributes.empty?
         raise ArgumentError, "Attribute names must be symbols" if attributes.any?{ |attribute| !attribute.is_a?(Symbol) }
         raise ArgumentError, "You need to supply at least one validation" if validations.empty?
-        
+
+        defaults.merge!(:attributes => attributes)
+
         validations.each do |key, options|
           begin
             validator = const_get("#{key.to_s.camelize}Validator")
@@ -66,7 +78,7 @@ module ActiveModel
             raise ArgumentError, "Unknown validator: '#{key}'"
           end
 
-          validates_with(validator, (options == true ? {} : options).merge(:attributes => attributes))
+          validates_with(validator, defaults.merge(options == true ? {} : options))
         end
       end
     end
diff --git a/activemodel/test/cases/validations/validates_test.rb b/activemodel/test/cases/validations/validates_test.rb
index c0bf03efc7..181ff38b64 100644
--- a/activemodel/test/cases/validations/validates_test.rb
+++ b/activemodel/test/cases/validations/validates_test.rb
@@ -4,14 +4,21 @@ require 'models/person'
 require 'models/person_with_validator'
 require 'validators/email_validator'
 
-class ValidatesTest < ActiveRecord::TestCase  
+class ValidatesTest < ActiveModel::TestCase
+  setup :reset_callbacks
+  teardown :reset_callbacks
+
+  def reset_callbacks
+    Person.reset_callbacks(:validate)
+  end
+
   def test_validates_with_built_in_validation
     Person.validates :title, :numericality => true
     person = Person.new
     person.valid?
     assert person.errors[:title].include?('is not a number')
   end
-  
+
   def test_validates_with_built_in_validation_and_options
     Person.validates :title, :numericality => { :message => 'my custom message' }
     person = Person.new
@@ -25,7 +32,35 @@ class ValidatesTest < ActiveRecord::TestCase
     person.valid?
     assert person.errors[:karma].include?('is not an email')
   end
-  
+
+  def test_validates_with_if_as_local_conditions
+    Person.validates :karma, :presence => true, :email => { :unless => :condition_is_true }
+    person = Person.new
+    person.valid?
+    assert !person.errors[:karma].include?('is not an email')
+    assert person.errors[:karma].include?('can\'t be blank')
+  end
+
+  def test_validates_with_if_as_shared_conditions
+    Person.validates :karma, :presence => true, :email => true, :if => :condition_is_true
+    person = Person.new
+    person.valid?
+    assert person.errors[:karma].include?('is not an email')
+    assert person.errors[:karma].include?('can\'t be blank')
+  end
+
+  def test_validates_with_unless_shared_conditions
+    Person.validates :karma, :presence => true, :email => true, :unless => :condition_is_true
+    person = Person.new
+    assert person.valid?
+  end
+
+  def test_validates_with_allow_nil_shared_conditions
+    Person.validates :karma, :length => { :minimum => 20 }, :email => true, :allow_nil => true
+    person = Person.new
+    assert person.valid?
+  end
+
   def test_validates_with_validator_class_and_options
     Person.validates :karma, :email => { :message => 'my custom message' }
     person = Person.new
diff --git a/activemodel/test/models/person.rb b/activemodel/test/models/person.rb
index c83d768379..53ac68055f 100644
--- a/activemodel/test/models/person.rb
+++ b/activemodel/test/models/person.rb
@@ -3,6 +3,10 @@ class Person
   extend  ActiveModel::Translation
 
   attr_accessor :title, :karma, :salary
+
+  def condition_is_true
+    true
+  end
 end
 
 class Child < Person