Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

No longer allow both group and non-group preferences to be looked up …

…at once (except for named scopes)
  • Loading branch information...
commit 32dff7cbdbcbae09cc4ff77eb4766388b48b7c7e 1 parent 9bfe0ee
@obrie obrie authored
View
1  CHANGELOG.rdoc
@@ -1,5 +1,6 @@
== master
+* 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
* Raise exception if unknown preference is accessed
View
121 lib/preferences.rb
@@ -248,8 +248,8 @@ def self.included(base) #:nodoc:
end
# Finds all preferences, including defaults, for the current record. If
- # any custom group preferences have been stored, then this will include
- # all default preferences within that particular group.
+ # looking up custom group preferences, then this will include all default
+ # preferences within that particular group as well.
#
# == Examples
#
@@ -261,59 +261,25 @@ def self.included(base) #:nodoc:
#
# A user with stored values for a particular group:
#
- # user.preferred_color = 'red', 'cars'
- # user.preferences
- # => {"language"=>"English", "color"=>nil, "cars"=>{"language=>"English", "color"=>"red"}}
- #
- # Getting preference values *just* for the owning record (i.e. excluding groups):
- #
- # user.preferences(nil)
- # => {"language"=>"English", "color"=>nil}
- #
- # Getting preference values for a particular group:
- #
- # user.preferences('cars')
- # => {"language"=>"English", "color"=>"red"}
- def preferences(*args)
- group = args.first
+ # user.preferred_color = 'red', :cars
+ # user.preferences(:cars)
+ # => {"language=>"English", "color"=>"red"}
+ def preferences(group = nil)
group = group.is_a?(Symbol) ? group.to_s : group
- unless @all_preferences_loaded
- if args.empty?
- # Looking up all available preferences
- loaded_preferences = stored_preferences
- @all_preferences_loaded = true
-
- preferences_group(nil).reverse_merge!(self.class.default_preferences.dup)
- elsif !preferences_group_loaded?(group)
- # Looking up group preferences
- group_id, group_type = Preference.split_group(group)
- loaded_preferences = stored_preferences.find(:all, :conditions => {:group_id => group_id, :group_type => group_type})
-
- preferences_group(group).reverse_merge!(self.class.default_preferences.dup)
+ unless preferences_group_loaded?(group)
+ preferences = preferences_group(group)
+
+ group_id, group_type = Preference.split_group(group)
+ find_preferences(:group_id => group_id, :group_type => group_type).each do |preference|
+ preferences[preference.name] ||= preference.value
end
- # Find all stored preferences and hashify group -> name -> value
- loaded_preferences.inject(@preferences) do |preferences, preference|
- preferences[preference.group] ||= self.class.default_preferences.dup
- preferences[preference.group][preference.name] = preference.value
- preferences
- end if loaded_preferences
+ # Add defaults
+ preferences.reverse_merge!(self.class.default_preferences.dup)
end
- # Generate a deep copy
- if args.empty?
- @preferences.inject({}) do |preferences, (group, group_preferences)|
- if group.nil?
- preferences.merge!(group_preferences)
- else
- preferences[group] = group_preferences.dup
- end
- preferences
- end
- else
- @preferences[group].dup
- end
+ preferences_group(group).dup
end
# Queries whether or not a value is present for the given preference.
@@ -370,13 +336,9 @@ def preferred(name, group = nil)
# grab from the pending values
value = preferences_group(group)[name]
else
- # Split the group being filtered
- group_id, group_type = Preference.split_group(group)
-
# Grab the first preference; if it doesn't exist, use the default value
- unless preferences_group_loaded?(group)
- preference = stored_preferences.find(:first, :conditions => {:name => name, :group_id => group_id, :group_type => group_type})
- end
+ group_id, group_type = Preference.split_group(group)
+ preference = find_preferences(:name => name, :group_id => group_id, :group_type => group_type).first unless preferences_group_loaded?(group)
value = preference ? preference.value : preference_definitions[name].default_value
preferences_group(group)[name] = value
@@ -419,9 +381,8 @@ def write_preference(name, value, group = nil)
def reload(*args)
result = super
- @all_preferences_loaded = false
@preferences.clear if @preferences
- preferences_changed.clear
+ @preferences_changed.clear if @preferences_changed
result
end
@@ -442,7 +403,7 @@ def preferences_group(group)
# Determines whether the given group of preferences has already been
# loaded from the database
def preferences_group_loaded?(group)
- @all_preferences_loaded || preference_definitions.length == preferences_group(group).length
+ preference_definitions.length == preferences_group(group).length
end
# Generates a clone of the current value stored for the preference with
@@ -456,13 +417,9 @@ def clone_preference_value(name, group)
# Keeps track of all preferences that have been changed so that they can
# be properly updated in the database. Maps group -> preference -> value.
- def preferences_changed(*args)
+ def preferences_changed(group = nil)
@preferences_changed ||= {}
- if args.empty?
- @preferences_changed
- else
- @preferences_changed[args.first] ||= {}
- end
+ @preferences_changed[group] ||= {}
end
# Determines whether the old value is different from the new value for the
@@ -484,20 +441,34 @@ def preference_value_changed?(name, old, value)
# Updates any preferences that have been changed/added since the record
# was last saved
def update_preferences
- preferences_changed.each do |group, preferences|
- group_id, group_type = Preference.split_group(group)
-
- preferences.keys.each do |name|
- attributes = {:name => name, :group_id => group_id, :group_type => group_type}
+ if @preferences_changed
+ @preferences_changed.each do |group, preferences|
+ group_id, group_type = Preference.split_group(group)
- # Find an existing preference or build a new one
- preference = stored_preferences.find(:first, :conditions => attributes) || stored_preferences.build(attributes)
- preference.value = preferred(name, group)
- preference.save!
+ preferences.keys.each do |name|
+ # Find an existing preference or build a new one
+ attributes = {:name => name, :group_id => group_id, :group_type => group_type}
+ preference = find_preferences(attributes).first || stored_preferences.build(attributes)
+ preference.value = preferred(name, group)
+ preference.save!
+ end
end
+
+ @preferences_changed.clear
+ end
+ end
+
+ # Finds all stored preferences with the given attributes. This will do a
+ # smart lookup by looking at the in-memory collection if it was eager-
+ # loaded.
+ def find_preferences(attributes)
+ if stored_preferences.loaded?
+ stored_preferences.select do |preference|
+ attributes.all? {|attribute, value| preference[attribute] == value}
+ end
+ else
+ stored_preferences.find(:all, :conditions => attributes)
end
-
- preferences_changed.clear
end
end
end
View
36 test/functional/preferences_test.rb
@@ -531,6 +531,12 @@ def test_should_not_remove_preference_if_set_to_nil
preference = @user.stored_preferences.first
assert_nil preference.value
end
+
+ def test_should_not_query_for_old_value_if_preferences_loaded
+ @user.preferences
+
+ assert_queries(0) { @user.write_preference(:notifications, false) }
+ end
end
class PreferencesGroupWriterTest < ModelPreferenceTest
@@ -655,6 +661,12 @@ def test_should_merge_stored_preferences_with_unsaved_changes
assert_equal e = {'notifications' => false, 'language' => 'Latin'}, @user.preferences
end
+ def test_should_use_unsaved_changes_over_stored_preferences
+ create_preference(:owner => @user, :name => 'notifications', :value => false)
+ @user.write_preference(:notifications, true)
+ assert_equal e = {'notifications' => true, 'language' => 'English'}, @user.preferences
+ end
+
def test_should_cache_results
assert_queries(1) { @user.preferences }
assert_queries(0) { @user.preferences }
@@ -718,15 +730,6 @@ def test_should_not_query_if_all_preferences_individually_loaded
assert_queries(0) { @user.preferences(:chat) }
end
- def test_should_not_query_if_all_preferences_previously_loaded
- create_preference(:owner => @user, :group_type => 'chat', :name => 'notifications', :value => false)
- @user.preferences
-
- assert_queries(0) do
- assert_equal e = {'notifications' => false, 'language' => 'English'}, @user.preferences(:chat)
- end
- end
-
def test_should_not_generate_same_object_twice
assert_not_same @user.preferences(:chat), @user.preferences(:chat)
end
@@ -803,19 +806,8 @@ def setup
create_preference(:owner => @user, :group_type => 'chat', :name => 'notifications', :value => false)
end
- def test_include_group_preferences_if_known
- assert_equal e = {
- 'notifications' => true, 'language' => 'English',
- 'chat' => {'notifications' => false, 'language' => 'English'}
- }, @user.preferences
- end
-
- def test_should_not_generate_same_object_twice
- assert_not_same @user.preferences, @user.preferences
- end
-
- def test_should_not_generate_same_group_object_twice
- assert_not_same @user.preferences['chat'], @user.preferences['chat']
+ def test_not_include_group_preferences_by_default
+ assert_equal e = {'notifications' => true, 'language' => 'English'}, @user.preferences
end
end
Please sign in to comment.
Something went wrong with that request. Please try again.