[dm-validations] Removed allow_blank option from acceptance validator

* Acceptance criteria can sometimes be true/false, and if false
  is provided and the allow_blank is true (which was the default)
  then the validator would incorrectly return true.
* Blank values like "" will now (correctly) return false in the
  acceptance validator.
* Only use :allow_nil to with acceptance validator

dm-validations/lib/dm-validations/validators/acceptance_validator.rb

@@ -10,7 +10,7 @@ class AcceptanceValidator < GenericValidator
       def initialize(field_name, options = {})
         super
 
-        set_optional_by_default
+        @options[:allow_nil] = true unless @options.key?(:allow_nil)
 
         @options[:accept] ||= [ '1', 1, 'true', true, 't' ]
         @options[:accept] = Array(@options[:accept])
@@ -29,7 +29,7 @@ def call(target)
 
       def valid?(target)
         value = target.validation_property_value(field_name)
-        return true if optional?(value)
+        return true if allow_nil?(value)
         @options[:accept].include?(value)
       end
     end # class AcceptanceValidator
@@ -39,12 +39,10 @@ module ValidatesIsAccepted
       ##
       # Validates that the attributes's value is in the set of accepted values.
       #
-      # @option :allow_nil<Boolean>   true if nil is allowed, false if not
-      #                               allowed. Default is true.
-      # @option :allow_blank<Boolean> true if blank is allowed, false if not
-      #                               allowed. Default is true.
-      # @option :accept<Array>        a list of accepted values.
-      #                               Default are ["1", 1, "true", true, "t"]).
+      # @option :allow_nil<Boolean> true if nil is allowed, false if not
+      #                             allowed. Default is true.
+      # @option :accept<Array>      a list of accepted values.
+      #                             Default are ["1", 1, "true", true, "t"]).
       #
       # @example [Usage]
       #   require 'dm-validations'

dm-validations/lib/dm-validations/validators/generic_validator.rb

@@ -95,11 +95,27 @@ def set_optional_by_default(default = true)
       # @param <Object> value to test
       # @return <Boolean> true if blank/nil is allowed, and the value is blank/nil
       def optional?(value)
-        return true if @options[:allow_nil]   && value.nil?
-        return true if @options[:allow_blank] && value.blank?
+        return allow_nil?(value)   if value.nil?
+        return allow_blank?(value) if value.blank?
         false
       end
 
+      # Test if the value is nil and is allowed
+      #
+      # @param <Object> value to test
+      # @return <Boolean> true if nil is allowed and value is nil
+      def allow_nil?(value)
+        @options[:allow_nil] if value.nil?
+      end
+
+      # Test if the value is blank and is allowed
+      #
+      # @param <Object> value to test
+      # @return <Boolean> true if blank is allowed and value is blank
+      def allow_blank?(value)
+        @options[:allow_blank] if value.blank?
+      end
+
       # Returns true if validators are equal
       #
       # Note that this intentionally do

dm-validations/spec/fixtures/beta_tester_account.rb

@@ -24,7 +24,7 @@ class BetaTesterAccount
         # Validations
         #
 
-        validates_is_accepted :user_agreement, :allow_nil => false, :allow_blank => false
+        validates_is_accepted :user_agreement, :allow_nil => false
         validates_is_accepted :newsletter_signup
         validates_is_accepted :privacy_agreement, :accept => %w(agreed accept), :message => "You must accept this agreement in order to proceed"
       end # BetaTesterAccount

dm-validations/spec/integration/acceptance_validator/acceptance_validator_spec.rb

@@ -4,7 +4,7 @@
 describe DataMapper::Validate::Fixtures::BetaTesterAccount do
   before :each do
     @model = DataMapper::Validate::Fixtures::BetaTesterAccount.new(:user_agreement    => true,
-                                                                   :newsletter_signup => "",
+                                                                   :newsletter_signup => nil,
                                                                    :privacy_agreement => "accept")
     @model.should be_valid
   end
@@ -24,12 +24,11 @@
       @model.newsletter_signup = ""
     end
 
-    it "is perfectly valid" do
-      @model.should be_valid
+    it "is NOT valid" do
+      @model.should_not be_valid
     end
   end
 
-
   describe "with a blank user agreement field" do
     before :each do
       @model.user_agreement = ""
@@ -40,7 +39,6 @@
     end
   end
 
-
   describe "with a nil user agreement field" do
     before :each do
       @model.user_agreement = nil
@@ -51,7 +49,6 @@
     end
   end
 
-
   describe "with user agreement field having value of 1 (as integer)" do
     before :each do
       @model.user_agreement = 1
@@ -62,7 +59,6 @@
     end
   end
 
-
   describe "with user agreement field having value of 1 (as a string)" do
     before :each do
       @model.user_agreement = "1"
@@ -73,7 +69,6 @@
     end
   end
 
-
   describe "with user agreement field having value of 'true' (as a string)" do
     before :each do
       @model.user_agreement = 'true'
@@ -84,7 +79,6 @@
     end
   end
 
-
   describe "with user agreement field having value of true (TrueClass instance)" do
     before :each do
       @model.user_agreement = true
@@ -95,7 +89,6 @@
     end
   end
 
-
   describe "with user agreement field having value of 't' (The Lisp Way)" do
     before :each do
       @model.user_agreement = 't'
@@ -106,7 +99,6 @@
     end
   end
 
-
   describe "with user agreement field having value of 'f'" do
     before :each do
       @model.user_agreement = 'f'
@@ -117,6 +109,15 @@
     end
   end
 
+  describe "with user agreement field having value of false" do
+    before :each do
+      @model.user_agreement = false
+    end
+
+    it "is NOT valid" do
+      @model.should_not be_valid
+    end
+  end
 
   describe "with privacy agreement field having value of 1" do
     before :each do
@@ -138,7 +139,6 @@
     end
   end
 
-
   describe "with privacy agreement field having value of '1'" do
     before :each do
       @model.privacy_agreement = '1'
@@ -149,7 +149,6 @@
     end
   end
 
-
   describe "with privacy agreement field having value of 't'" do
     before :each do
       @model.privacy_agreement = 't'
@@ -160,7 +159,6 @@
     end
   end
 
-
   describe "with privacy agreement field having value of 'accept'" do
     before :each do
       @model.privacy_agreement = 'accept'
@@ -171,7 +169,6 @@
     end
   end
 
-
   describe "with privacy agreement field having value of 'agreed'" do
     before :each do
       @model.privacy_agreement = 'agreed'
@@ -182,7 +179,6 @@
     end
   end
 
-
   describe "with privacy agreement field having value of 'ah, greed'" do
     before :each do
       @model.privacy_agreement = 'ah, greed'