FIX: ensure we never have a string when an enum is Fixnum

config/locales/server.en.yml

@@ -1199,8 +1199,8 @@ en:
     default_email_mailing_list_mode: "Send an email for every new post by default."
     default_email_always: "Send an email notification even when the user is active by default."
 
-    default_other_new_topic_duration_minutes: "Global default number of minutes a topic is considered new, users can override (-1 for always, -2 for last visit)"
+    default_other_new_topic_duration_minutes: "Global default number of minutes a topic is considered new."
-    default_other_auto_track_topics_after_msecs: "Global default milliseconds before a topic is automatically tracked, users can override (0 for always, -1 for never)"
+    default_other_auto_track_topics_after_msecs: "Global default milliseconds before a topic is automatically tracked."
     default_other_external_links_in_new_tab: "Open external links in a new tab by default."
     default_other_enable_quoting: "Enable quote reply for highlighted text by default."
     default_other_dynamic_favicon: "Show new/updated topic count on browser icon by default."

lib/site_setting_extension.rb

@@ -217,26 +217,20 @@ module SiteSettingExtension
       ensure_listen_for_changes
       old = current
 
-      new_hash =  Hash[*(provider.all.map{ |s|
+      new_hash =  Hash[*(provider.all.map { |s|
-        [s.name.intern, convert(s.value,s.data_type)]
+        [s.name.intern, convert(s.value, s.data_type, s.name)]
       }.to_a.flatten)]
 
       # add defaults, cause they are cached
       new_hash = defaults.merge(new_hash)
 
       # add shadowed
-      shadowed_settings.each do |ss|
+      shadowed_settings.each { |ss| new_hash[ss] = GlobalSetting.send(ss) }
-        new_hash[ss] = GlobalSetting.send(ss)
-      end
 
       changes, deletions = diff_hash(new_hash, old)
 
-      changes.each do |name, val|
+      changes.each   { |name, val| current[name] = val }
-        current[name] = val
+      deletions.each { |name, val| current[name] = defaults[name] }
-      end
-      deletions.each do |name, val|
-        current[name] = defaults[name]
-      end
 
       clear_cache!
     end
@@ -321,7 +315,7 @@ module SiteSettingExtension
     end
 
     provider.save(name, val, type)
-    current[name] = convert(val, type)
+    current[name] = convert(val, type, name)
     notify_clients!(name) if client_settings.include? name
     clear_cache!
   end
@@ -421,7 +415,7 @@ module SiteSettingExtension
     end
   end
 
-  def convert(value, type)
+  def convert(value, type, name)
     case type
     when types[:float]
       value.to_f
@@ -431,9 +425,10 @@ module SiteSettingExtension
       value == true || value == "t" || value == "true"
     when types[:null]
       nil
+    when types[:enum]
+      defaults[name.to_sym] === Fixnum ? value.to_i : value
     else
       return value if types[type]
-
       # Otherwise it's a type error
       raise ArgumentError.new :type
     end

lib/site_settings/db_provider.rb

@@ -23,19 +23,15 @@ class SiteSettings::DbProvider
 
     # note, not leaking out AR records, cause I want all editing to happen
     # via this API
-    SqlBuilder.new("select name, data_type, value from #{@model.table_name} where name = :name")
+    SqlBuilder.new("SELECT name, data_type, value FROM #{@model.table_name} WHERE name = :name")
       .map_exec(OpenStruct, name: name)
       .first
   end
 
   def save(name, value, data_type)
-
     return unless table_exists?
 
-    model = @model.find_by({
+    model = @model.find_by(name: name)
-      name: name
-    })
-
     model ||= @model.new
 
     model.name = name

lib/site_settings/local_process_provider.rb

@@ -24,7 +24,7 @@ class SiteSettings::LocalProcessProvider
   end
 
   def save(name, value, data_type)
-    settings[name] = Setting.new(name,value, data_type)
+    settings[name] = Setting.new(name, value, data_type)
   end
 
   def destroy(name)

spec/components/site_setting_extension_spec.rb

@@ -232,23 +232,16 @@ describe SiteSettingExtension do
       def self.values
         [1,2,3]
       end
-      def self.translate_names?
-        true
-      end
-    end
-
-    let :test_enum_class do
-      TestEnumClass
     end
 
     before do
-      settings.setting(:test_enum, 1, enum: TestEnumClass)
+      settings.setting(:test_int_enum, 1, enum: TestIntEnumClass)
       settings.refresh!
     end
 
     it 'should coerce correctly' do
-      settings.test_enum = "2"
+      settings.test_int_enum = "2"
-      expect(settings.test_enum).to eq(2)
+      expect(settings.test_int_enum).to eq(2)
     end
 
   end