Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fix preference definition types not being used to typecast values

  • Loading branch information...
commit eaa4f475b8d57d61b0463a0246663e04a5c859c6 1 parent 32dff7c
@obrie obrie authored
View
1  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
View
11 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)
View
5 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.
View
15 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!
View
16 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
Please sign in to comment.
Something went wrong with that request. Please try again.