From 9ed6aae3488e855f9fb789bcbba9d47f9c15f78a Mon Sep 17 00:00:00 2001
From: Julian Herrero <microweb10@gmail.com>
Date: Wed, 13 Mar 2019 11:54:20 +0100
Subject: [PATCH] Refactor Setting model

- Make easier to group settings by using prefixes
- Add method to rename setting keys
- Add method to remove setting keys
---
 app/models/setting.rb       | 26 +++++++----
 spec/models/setting_spec.rb | 87 ++++++++++++++++++++++++++++++++-----
 2 files changed, 93 insertions(+), 20 deletions(-)

diff --git a/app/models/setting.rb b/app/models/setting.rb
index 00b87a4db..764408d27 100644
--- a/app/models/setting.rb
+++ b/app/models/setting.rb
@@ -4,19 +4,16 @@ class Setting < ActiveRecord::Base
   default_scope { order(id: :asc) }
 
   def type
-    if feature_flag?
-      "feature"
+    prefix = key.split(".").first
+    if %w[feature process map html homepage].include? prefix
+      prefix
     else
-      "common"
+      "configuration"
     end
   end
 
-  def feature_flag?
-    key.start_with?("feature.")
-  end
-
   def enabled?
-    feature_flag? && value.present?
+    value.present?
   end
 
   class << self
@@ -30,5 +27,18 @@ class Setting < ActiveRecord::Base
       setting.save!
       value
     end
+
+    def rename_key(from:, to:)
+      if where(key: to).empty?
+        value = where(key: from).pluck(:value).first.presence
+        create!(key: to, value: value)
+      end
+      remove(from)
+    end
+
+    def remove(key)
+      setting = where(key: key).first
+      setting.destroy if setting.present?
+    end
   end
 end
diff --git a/spec/models/setting_spec.rb b/spec/models/setting_spec.rb
index 77f2d3daf..c0e060a3c 100644
--- a/spec/models/setting_spec.rb
+++ b/spec/models/setting_spec.rb
@@ -17,20 +17,40 @@ describe Setting do
     expect(described_class.where(key: "official_level_1_name", value: "Stormtrooper")).to exist
   end
 
-  describe "#feature_flag?" do
-    it "is true if key starts with 'feature.'" do
-      setting = described_class.create(key: "feature.whatever")
-      expect(setting.feature_flag?).to eq true
+  describe "#type" do
+    it "returns the key prefix for 'process' settings" do
+      process_setting = Setting.create(key: "process.whatever")
+      expect(process_setting.type).to eq "process"
     end
 
-    it "is false if key does not start with 'feature.'" do
-      setting = described_class.create(key: "whatever")
-      expect(setting.feature_flag?).to eq false
+    it "returns the key prefix for 'feature' settings" do
+      feature_setting = Setting.create(key: "feature.whatever")
+      expect(feature_setting.type).to eq "feature"
+    end
+
+    it "returns the key prefix for 'map' settings" do
+      map_setting = Setting.create(key: "map.whatever")
+      expect(map_setting.type).to eq "map"
+    end
+
+    it "returns the key prefix for 'html' settings" do
+      html_setting = Setting.create(key: "html.whatever")
+      expect(html_setting.type).to eq "html"
+    end
+
+    it "returns the key prefix for 'homepage' settings" do
+      homepage_setting = Setting.create(key: "homepage.whatever")
+      expect(homepage_setting.type).to eq "homepage"
+    end
+
+    it "returns 'configuration' for the rest of the settings" do
+      configuration_setting = Setting.create(key: "whatever")
+      expect(configuration_setting.type).to eq "configuration"
     end
   end
 
   describe "#enabled?" do
-    it "is true if feature_flag and value present" do
+    it "is true if value is present" do
       setting = described_class.create(key: "feature.whatever", value: 1)
       expect(setting.enabled?).to eq true
 
@@ -41,17 +61,60 @@ describe Setting do
       expect(setting.enabled?).to eq true
     end
 
-    it "is false if feature_flag and value blank" do
+    it "is false if value is blank" do
       setting = described_class.create(key: "feature.whatever")
       expect(setting.enabled?).to eq false
 
       setting.value = ""
       expect(setting.enabled?).to eq false
     end
+  end
 
-    it "is false if not feature_flag" do
-      setting = described_class.create(key: "whatever", value: "whatever")
-      expect(setting.enabled?).to eq false
+  describe ".rename_key" do
+    it "renames the setting keeping the original value and deletes the old setting" do
+      Setting["old_key"] = "old_value"
+
+      Setting.rename_key from: "old_key", to: "new_key"
+
+      expect(Setting.where(key: "new_key", value: "old_value")).to exist
+      expect(Setting.where(key: "old_key")).not_to exist
+    end
+
+    it "initialize the setting with null value if old key doesn't exist" do
+      expect(Setting.where(key: "old_key")).not_to exist
+
+      Setting.rename_key from: "old_key", to: "new_key"
+
+      expect(Setting.where(key: "new_key", value: nil)).to exist
+      expect(Setting.where(key: "old_key")).not_to exist
+    end
+
+    it "does not change value if new key already exists, but deletes setting with old key" do
+      Setting["new_key"] = "new_value"
+      Setting["old_key"] = "old_value"
+
+      Setting.rename_key from: "old_key", to: "new_key"
+
+      expect(Setting["new_key"]).to eq "new_value"
+      expect(Setting.where(key: "old_key")).not_to exist
+    end
+  end
+
+  describe ".remove" do
+    it "deletes the setting by given key" do
+      expect(Setting.where(key: "official_level_1_name")).to exist
+
+      Setting.remove("official_level_1_name")
+
+      expect(Setting.where(key: "official_level_1_name")).not_to exist
+    end
+
+    it "does nothing if key doesn't exists" do
+      all_settings = Setting.all
+
+      Setting.remove("not_existing_key")
+
+      expect(Setting.all).to eq all_settings
     end
   end
 end