coerce blank strings to nil values for boolean and integer fields

[#860 state:resolved]
rails · Aug 22, 2008 · e48e77e · e48e77e
1 parent 707ee0e
commit e48e77e
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 12 deletions.
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
@@ -2607,11 +2607,14 @@ def ensure_proper_type
       end
 
       def convert_number_column_value(value)
-        case value
-          when FalseClass; 0
-          when TrueClass;  1
-          when '';         nil
-          else value
+        if value == false
+          0
+        elsif value == true
+          1
+        elsif value.is_a?(String) && value.blank?
+          nil
+        else
+          value
         end
       end
 

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -138,7 +138,11 @@ def string_to_dummy_time(string)
 
         # convert something to a boolean
         def value_to_boolean(value)
-          TRUE_VALUES.include?(value)
+          if value.is_a?(String) && value.blank?
+            nil
+          else
+            TRUE_VALUES.include?(value)
+          end
         end
 
         # convert something to a BigDecimal

diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
@@ -138,7 +138,7 @@ def test_read_attributes_before_type_cast
   if current_adapter?(:MysqlAdapter)
     def test_read_attributes_before_type_cast_on_boolean
       bool = Booleantest.create({ "value" => false })
-      assert_equal 0, bool.attributes_before_type_cast["value"]
+      assert_equal "0", bool.reload.attributes_before_type_cast["value"]
     end
   end
 
@@ -1114,23 +1114,31 @@ def test_attributes_on_dummy_time
   end
 
   def test_boolean
+    b_nil = Booleantest.create({ "value" => nil })
+    nil_id = b_nil.id
     b_false = Booleantest.create({ "value" => false })
     false_id = b_false.id
     b_true = Booleantest.create({ "value" => true })
     true_id = b_true.id
 
+    b_nil = Booleantest.find(nil_id)
+    assert_nil b_nil.value
     b_false = Booleantest.find(false_id)
     assert !b_false.value?
     b_true = Booleantest.find(true_id)
     assert b_true.value?
   end
 
   def test_boolean_cast_from_string
+    b_blank = Booleantest.create({ "value" => "" })
+    blank_id = b_blank.id
     b_false = Booleantest.create({ "value" => "0" })
     false_id = b_false.id
     b_true = Booleantest.create({ "value" => "1" })
     true_id = b_true.id
 
+    b_blank = Booleantest.find(blank_id)
+    assert_nil b_blank.value
     b_false = Booleantest.find(false_id)
     assert !b_false.value?
     b_true = Booleantest.find(true_id)

diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb
@@ -1420,8 +1420,8 @@ def test_default_validates_numericality_of
   def test_validates_numericality_of_with_nil_allowed
     Topic.validates_numericality_of :approved, :allow_nil => true
 
-    invalid!(BLANK + JUNK)
-    valid!(NIL + FLOATS + INTEGERS + BIGDECIMAL + INFINITY)
+    invalid!(JUNK)
+    valid!(NIL + BLANK + FLOATS + INTEGERS + BIGDECIMAL + INFINITY)
   end
 
   def test_validates_numericality_of_with_integer_only
@@ -1434,8 +1434,8 @@ def test_validates_numericality_of_with_integer_only
   def test_validates_numericality_of_with_integer_only_and_nil_allowed
     Topic.validates_numericality_of :approved, :only_integer => true, :allow_nil => true
 
-    invalid!(BLANK + JUNK + FLOATS + BIGDECIMAL + INFINITY)
-    valid!(NIL + INTEGERS)
+    invalid!(JUNK + FLOATS + BIGDECIMAL + INFINITY)
+    valid!(NIL + BLANK + INTEGERS)
   end
 
   def test_validates_numericality_with_greater_than

diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
@@ -60,7 +60,7 @@ def create_table(*args, &block)
   end
 
   create_table :booleantests, :force => true do |t|
-    t.integer :value
+    t.boolean :value
   end
 
   create_table :categories, :force => true do |t|