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

app/models/auto_track_duration_site_setting.rb

@@ -3,7 +3,7 @@
 class AutoTrackDurationSiteSetting < EnumSiteSetting
 
   def self.valid_value?(val)
-    values.any? { |v| v[:value].to_s == val.to_s }
+    values.any? { |v| v[:value] == val.to_i }
   end
 
   def self.values

app/models/new_topic_duration_site_setting.rb

@@ -3,7 +3,7 @@
 class NewTopicDurationSiteSetting < EnumSiteSetting
 
   def self.valid_value?(val)
-    values.any? { |v| v[:value].to_s == val.to_s }
+    values.any? { |v| v[:value] == val.to_i }
   end
 
   def self.values

app/models/trust_level_setting.rb

@@ -3,11 +3,11 @@
 class TrustLevelSetting < EnumSiteSetting
 
   def self.valid_value?(val)
-    valid_values.any? { |v| v.to_s == val.to_s }
+    valid_values.any? { |v| v == val.to_i }
   end
 
   def self.values
-    @values ||= valid_values.map {|x| {name: x.to_s, value: x} }
+    @values ||= valid_values.map { |x| { name: x.to_s, value: x } }
   end
 
   def self.valid_values

lib/site_setting_extension.rb

@@ -102,9 +102,7 @@ def setting(name_arg, default = nil, opts = {})
 
       if new_choices = opts[:choices]
 
-        if String === new_choices
-          new_choices = eval(new_choices)
-        end
+        new_choices = eval(new_choices) if new_choices.is_a?(String)
 
         choices.has_key?(name) ?
           choices[name].concat(new_choices) :
@@ -280,7 +278,7 @@ def remove_override!(name)
   end
 
   def add_override!(name, val)
-    type = get_data_type(name, defaults[name])
+    type = get_data_type(name, defaults[name.to_sym])
 
     if type == types[:bool] && val != true && val != false
       val = (val == "t" || val == "true") ? 't' : 'f'
@@ -295,7 +293,7 @@ def add_override!(name, val)
     end
 
     if type == types[:enum]
-      val = val.to_i if Fixnum === defaults[name.to_sym]
+      val = val.to_i if defaults[name.to_sym].is_a?(Fixnum)
       if enum_class(name)
         raise Discourse::InvalidParameters.new(:value) unless enum_class(name).valid_value?(val)
       else
@@ -395,8 +393,8 @@ def diff_hash(new_hash, old)
   def get_data_type(name, val)
     return types[:null] if val.nil?
 
-    # Some types are just for validations like email. Only consider
-    # it valid if includes in `types`
+    # Some types are just for validations like email.
+    # Only consider it valid if includes in `types`
     if static_type = static_types[name.to_sym]
       return types[static_type] if types.keys.include?(static_type)
     end
@@ -426,7 +424,7 @@ def convert(value, type, name)
     when types[:null]
       nil
     when types[:enum]
-      defaults[name.to_sym] === Fixnum ? value.to_i : value
+      defaults[name.to_sym].is_a?(Fixnum) ? value.to_i : value
     else
       return value if types[type]
       # Otherwise it's a type error

spec/components/site_setting_extension_spec.rb

@@ -7,12 +7,11 @@
     SiteSettings::LocalProcessProvider.new
   end
 
-  def new_settings(provider)
-    c = Class.new do
+  def new_settings(provider=nil)
+    Class.new do
       extend SiteSettingExtension
-      self.provider = provider
+      self.provider = provider if provider
     end
-    c
   end
 
   let :settings do
@@ -23,6 +22,10 @@ def new_settings(provider)
     new_settings(provider)
   end
 
+  let :settings_db do
+    new_settings
+  end
+
   describe "refresh!" do
 
     it "will reset to default if provider vanishes" do
@@ -234,14 +237,11 @@ def self.values
       end
     end
 
-    before do
-      settings.setting(:test_int_enum, 1, enum: TestIntEnumClass)
-      settings.refresh!
-    end
-
     it 'should coerce correctly' do
-      settings.test_int_enum = "2"
-      expect(settings.test_int_enum).to eq(2)
+      settings_db.setting(:test_int_enum, 1, enum: TestIntEnumClass)
+      settings_db.test_int_enum = "2"
+      settings_db.refresh!
+      expect(settings_db.test_int_enum).to eq(2)
     end
 
   end