Fix preference definition types not being used to typecast values

CHANGELOG.rdoc

@@ -1,5 +1,6 @@
 == master
 
+* Fix preference definition types not being used to typecast values
 * No longer allow both group and non-group preferences to be looked up at once (except for named scopes)
 * Add support for using Symbols to reference groups
 * Fix #reload not causing unsaved preferences to get reset

lib/preferences.rb

@@ -344,6 +344,8 @@ def preferred(name, group = nil)
         preferences_group(group)[name] = value
       end
 
+      definition = preference_definitions[name]
+      value = definition.type_cast(value) unless value.nil?
       value
     end
     alias_method :prefers, :preferred
@@ -372,6 +374,7 @@ def write_preference(name, value, group = nil)
         preferences_changed(group)[name] = old if preference_value_changed?(name, old, value)
       end
 
+      value = convert_number_column_value(value) if preference_definitions[name].number?
       preferences_group(group)[name] = value
 
       value
@@ -427,9 +430,11 @@ def preferences_changed(group = nil)
       # equality.
       def preference_value_changed?(name, old, value)
         definition = preference_definitions[name]
-        if definition.type == :integer && old.nil?
-          # NULL gets stored in database for blank (i.e. '') values. Hence we
-          # don't record it as a change if the value changes from nil to ''.
+        if definition.type == :integer && (old.nil? || old == 0)
+          # For nullable numeric columns, NULL gets stored in database for blank (i.e. '') values.
+          # Hence we don't record it as a change if the value changes from nil to ''.
+          # If an old value of 0 is set to '' we want this to get changed to nil as otherwise it'll
+          # be typecast back to 0 (''.to_i => 0)
           value = nil if value.blank?
         else
           value = definition.type_cast(value)

lib/preferences/preference_definition.rb

@@ -25,6 +25,11 @@ def default_value
       @column.default
     end
 
+    # Determines whether column backing this preference stores numberic values
+    def number?
+      @column.number?
+    end
+
     # Typecasts the value based on the type of preference that was defined.
     # This uses ActiveRecord's typecast functionality so the same rules for
     # typecasting a model's columns apply here.

test/functional/preferences_test.rb

@@ -218,6 +218,11 @@ def test_should_use_stored_value_if_stored
     assert_equal false, @user.preferred(:notifications)
   end
 
+  def test_should_type_cast_based_on_preference_definition
+    @user.write_preference(:notifications, 'false')
+    assert_equal false, @user.preferred(:notifications)
+  end
+
   def test_should_cache_stored_values
     create_preference(:owner => @user, :name => 'notifications', :value => false)
     assert_queries(1) { @user.preferred(:notifications) }
@@ -476,6 +481,16 @@ def test_should_not_create_stored_integer_preference_if_typecast_not_changed
     assert_equal 0, @user.stored_preferences.count
   end
 
+  def test_should_create_stored_integer_preference_if_typecast_changed
+    User.preference :age, :integer, :default => 0
+
+    @user.write_preference(:age, '')
+    @user.save!
+
+    assert_nil @user.preferred(:age)
+    assert_equal 1, @user.stored_preferences.count
+  end
+
   def test_should_create_stored_preference_if_value_changed
     @user.write_preference(:notifications, false)
     @user.save!

test/unit/preference_definition_test.rb

@@ -61,6 +61,10 @@ def test_use_custom_type
     assert_equal :any, @definition.type
   end
 
+  def test_should_not_be_number
+    assert !@definition.number?
+  end
+
   def test_should_not_type_cast
     assert_equal nil, @definition.type_cast(nil)
     assert_equal 0, @definition.type_cast(0)
@@ -97,6 +101,10 @@ def setup
     @definition = Preferences::PreferenceDefinition.new(:notifications)
   end
 
+  def test_should_not_be_number
+    assert !@definition.number?
+  end
+
   def test_should_not_type_cast_if_value_is_nil
     assert_equal nil, @definition.type_cast(nil)
   end
@@ -143,6 +151,10 @@ def setup
     @definition = Preferences::PreferenceDefinition.new(:notifications, :integer)
   end
 
+  def test_should_be_number
+    assert @definition.number?
+  end
+
   def test_should_type_cast_true_to_integer
     assert_equal 1, @definition.type_cast(true)
   end
@@ -173,6 +185,10 @@ def setup
     @definition = Preferences::PreferenceDefinition.new(:notifications, :string)
   end
 
+  def test_should_not_be_number
+    assert !@definition.number?
+  end
+
   def test_should_type_cast_integers_to_strings
     assert_equal '1', @definition.type_cast('1')
   end