From 31e8309f0584d8260bb937b45a055c12b21d381e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Wed, 9 Sep 2015 14:34:44 +0200
Subject: [PATCH] FIX: ensure we never have a string when an enum is Fixnum -
 Take 2

---
 .../auto_track_duration_site_setting.rb       |  2 +-
 app/models/new_topic_duration_site_setting.rb |  2 +-
 app/models/trust_level_setting.rb             |  4 ++--
 lib/site_setting_extension.rb                 | 14 +++++-------
 .../components/site_setting_extension_spec.rb | 22 +++++++++----------
 5 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/app/models/auto_track_duration_site_setting.rb b/app/models/auto_track_duration_site_setting.rb
index 6b10f80a5fc..ce4c7b68789 100644
--- a/app/models/auto_track_duration_site_setting.rb
+++ b/app/models/auto_track_duration_site_setting.rb
@@ -3,7 +3,7 @@ require_dependency 'enum_site_setting'
 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
diff --git a/app/models/new_topic_duration_site_setting.rb b/app/models/new_topic_duration_site_setting.rb
index 92a00ea1c66..f04b9e67e5a 100644
--- a/app/models/new_topic_duration_site_setting.rb
+++ b/app/models/new_topic_duration_site_setting.rb
@@ -3,7 +3,7 @@ require_dependency 'enum_site_setting'
 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
diff --git a/app/models/trust_level_setting.rb b/app/models/trust_level_setting.rb
index e76f258280a..4b3e5d42e66 100644
--- a/app/models/trust_level_setting.rb
+++ b/app/models/trust_level_setting.rb
@@ -3,11 +3,11 @@ require_dependency 'enum_site_setting'
 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
diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb
index b6500cf5640..90be6860fc3 100644
--- a/lib/site_setting_extension.rb
+++ b/lib/site_setting_extension.rb
@@ -102,9 +102,7 @@ module SiteSettingExtension
 
       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 @@ module SiteSettingExtension
   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 @@ module SiteSettingExtension
     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 @@ module SiteSettingExtension
   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 @@ module SiteSettingExtension
     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
diff --git a/spec/components/site_setting_extension_spec.rb b/spec/components/site_setting_extension_spec.rb
index cf1a34229ea..78c0e7d5edb 100644
--- a/spec/components/site_setting_extension_spec.rb
+++ b/spec/components/site_setting_extension_spec.rb
@@ -7,12 +7,11 @@ describe SiteSettingExtension do
     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 @@ describe SiteSettingExtension do
     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 @@ describe SiteSettingExtension do
       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