diff --git a/app/models/theme.rb b/app/models/theme.rb index 6fd63b03940..607f354b955 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -647,14 +647,15 @@ class Theme < ActiveRecord::Base def settings field = settings_field - return [] unless field && field.error.nil? - settings = [] + settings = {} - ThemeSettingsParser - .new(field) - .load do |name, default, type, opts| - settings << ThemeSettingsManager.create(name, default, type, self, opts) - end + if field && field.error.nil? + ThemeSettingsParser + .new(field) + .load do |name, default, type, opts| + settings[name] = ThemeSettingsManager.create(name, default, type, self, opts) + end + end settings end @@ -668,7 +669,7 @@ class Theme < ActiveRecord::Base def cached_default_settings Theme.get_set_cache "default_settings_for_theme_#{self.id}" do settings_hash = {} - self.settings.each { |setting| settings_hash[setting.name] = setting.default } + self.settings.each { |name, setting| settings_hash[name] = setting.default } theme_uploads = build_theme_uploads_hash settings_hash["theme_uploads"] = theme_uploads if theme_uploads.present? @@ -682,7 +683,7 @@ class Theme < ActiveRecord::Base def build_settings_hash hash = {} - self.settings.each { |setting| hash[setting.name] = setting.value } + self.settings.each { |name, setting| hash[name] = setting.value } theme_uploads = build_theme_uploads_hash hash["theme_uploads"] = theme_uploads if theme_uploads.present? @@ -724,13 +725,13 @@ class Theme < ActiveRecord::Base # theme.get_setting(:some_string) => "hello" # def get_setting(setting_name) - target_setting = settings.find { |setting| setting.name == setting_name.to_sym } + target_setting = settings[setting_name.to_sym] raise Discourse::NotFound unless target_setting target_setting.value end def update_setting(setting_name, new_value) - target_setting = settings.find { |setting| setting.name == setting_name } + target_setting = settings[setting_name.to_sym] raise Discourse::NotFound unless target_setting target_setting.value = new_value diff --git a/app/serializers/theme_serializer.rb b/app/serializers/theme_serializer.rb index b1cbd1cb743..796724d1ada 100644 --- a/app/serializers/theme_serializer.rb +++ b/app/serializers/theme_serializer.rb @@ -56,7 +56,7 @@ class ThemeSerializer < BasicThemeSerializer end def settings - object.settings.map { |setting| ThemeSettingsSerializer.new(setting, root: false) } + object.settings.map { |_name, setting| ThemeSettingsSerializer.new(setting, root: false) } rescue ThemeSettingsParser::InvalidYaml => e @errors << e.message nil diff --git a/spec/lib/theme_settings_manager_spec.rb b/spec/lib/theme_settings_manager_spec.rb index ac507ad146e..52b2fc7b0b2 100644 --- a/spec/lib/theme_settings_manager_spec.rb +++ b/spec/lib/theme_settings_manager_spec.rb @@ -4,6 +4,7 @@ require "theme_settings_manager" RSpec.describe ThemeSettingsManager do let!(:theme) { Fabricate(:theme) } + let(:theme_settings) do yaml = File.read("#{Rails.root}/spec/fixtures/theme_settings/valid_settings.yaml") theme.set_field(target: :settings, name: "yaml", value: yaml) @@ -11,20 +12,16 @@ RSpec.describe ThemeSettingsManager do theme.settings end - def find_by_name(name) - theme_settings.find { |setting| setting.name == name } - end - describe "Enum" do it "only accepts values from its choices" do - enum_setting = find_by_name(:enum_setting) + enum_setting = theme_settings[:enum_setting] expect { enum_setting.value = "trust level 2" }.to raise_error(Discourse::InvalidParameters) expect { enum_setting.value = "trust level 0" }.not_to raise_error - enum_setting = find_by_name(:enum_setting_02) + enum_setting = theme_settings[:enum_setting_02] expect { enum_setting.value = "10" }.not_to raise_error - enum_setting = find_by_name(:enum_setting_03) + enum_setting = theme_settings[:enum_setting_03] expect { enum_setting.value = "10" }.not_to raise_error expect { enum_setting.value = 1 }.not_to raise_error expect { enum_setting.value = 15 }.to raise_error(Discourse::InvalidParameters) @@ -33,7 +30,7 @@ RSpec.describe ThemeSettingsManager do describe "Bool" do it "is either true or false" do - bool_setting = find_by_name(:boolean_setting) + bool_setting = theme_settings[:boolean_setting] expect(bool_setting.value).to eq(true) # default bool_setting.value = "true" @@ -52,7 +49,7 @@ RSpec.describe ThemeSettingsManager do describe "Integer" do it "is always an integer" do - int_setting = find_by_name(:integer_setting) + int_setting = theme_settings[:integer_setting] int_setting.value = 1.6 theme.reload expect(int_setting.value).to eq(1) @@ -71,7 +68,7 @@ RSpec.describe ThemeSettingsManager do end it "can have min or max value" do - int_setting = find_by_name(:integer_setting_02) + int_setting = theme_settings[:integer_setting_02] expect { int_setting.value = 0 }.to raise_error(Discourse::InvalidParameters) expect { int_setting.value = 61 }.to raise_error(Discourse::InvalidParameters) @@ -87,7 +84,7 @@ RSpec.describe ThemeSettingsManager do describe "Float" do it "is always a float" do - float_setting = find_by_name(:float_setting) + float_setting = theme_settings[:float_setting] float_setting.value = 1.615 theme.reload expect(float_setting.value).to eq(1.615) @@ -102,7 +99,7 @@ RSpec.describe ThemeSettingsManager do end it "can have min or max value" do - float_setting = find_by_name(:float_setting) + float_setting = theme_settings[:float_setting] expect { float_setting.value = 1.4 }.to raise_error(Discourse::InvalidParameters) expect { float_setting.value = 10.01 }.to raise_error(Discourse::InvalidParameters) expect { float_setting.value = "text" }.to raise_error(Discourse::InvalidParameters) @@ -115,7 +112,7 @@ RSpec.describe ThemeSettingsManager do describe "String" do it "can have min or max length" do - string_setting = find_by_name(:string_setting_02) + string_setting = theme_settings[:string_setting_02] expect { string_setting.value = "a" }.to raise_error(Discourse::InvalidParameters) string_setting.value = "ab" @@ -130,20 +127,20 @@ RSpec.describe ThemeSettingsManager do end it "can be a textarea" do - expect(find_by_name(:string_setting_02).textarea).to eq(false) - expect(find_by_name(:string_setting_03).textarea).to eq(true) + expect(theme_settings[:string_setting_02].textarea).to eq(false) + expect(theme_settings[:string_setting_03].textarea).to eq(true) end it "supports json schema" do - expect(find_by_name(:string_setting_03).json_schema).to eq(false) - expect(find_by_name(:invalid_json_schema_setting).json_schema).to eq(false) - expect(find_by_name(:valid_json_schema_setting).json_schema).to be_truthy + expect(theme_settings[:string_setting_03].json_schema).to eq(false) + expect(theme_settings[:invalid_json_schema_setting].json_schema).to eq(false) + expect(theme_settings[:valid_json_schema_setting].json_schema).to be_truthy end end describe "List" do it "can have a list type" do - list_setting = find_by_name(:compact_list_setting) + list_setting = theme_settings[:compact_list_setting] expect(list_setting.list_type).to eq("compact") end end @@ -152,7 +149,7 @@ RSpec.describe ThemeSettingsManager do let!(:upload) { Fabricate(:upload) } it "saves the upload id" do - upload_setting = find_by_name(:upload_setting) + upload_setting = theme_settings[:upload_setting] upload_setting.value = upload.url theme.reload @@ -164,7 +161,7 @@ RSpec.describe ThemeSettingsManager do describe "#value" do context "when it's changed to a custom upload" do it "returns CDN URL" do - upload_setting = find_by_name(:upload_setting) + upload_setting = theme_settings[:upload_setting] upload_setting.value = upload.url theme.reload @@ -181,7 +178,7 @@ RSpec.describe ThemeSettingsManager do upload_id: upload.id, ) theme.save! - upload_setting = find_by_name(:upload_setting) + upload_setting = theme_settings[:upload_setting] expect(upload_setting.value).to eq(Discourse.store.cdn_url(upload.url)) end end diff --git a/spec/models/remote_theme_spec.rb b/spec/models/remote_theme_spec.rb index b3fcdccd1f8..a4d70b1ca87 100644 --- a/spec/models/remote_theme_spec.rb +++ b/spec/models/remote_theme_spec.rb @@ -198,7 +198,7 @@ RSpec.describe RemoteTheme do expect(mapped.length).to eq(12) expect(theme.settings.length).to eq(1) - expect(theme.settings.first.value).to eq(true) + expect(theme.settings[:boolean_setting].value).to eq(true) expect(remote.remote_updated_at).to eq_time(time) @@ -247,7 +247,7 @@ RSpec.describe RemoteTheme do expect(mapped["1-scss"]).to eq(scss_data) expect(theme.settings.length).to eq(1) - expect(theme.settings.first.value).to eq(32) + expect(theme.settings[:integer_setting].value).to eq(32) expect(remote.remote_updated_at).to eq_time(time) expect(remote.about_url).to eq("https://newsite.com/about") diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb index 43e225079bd..014878be669 100644 --- a/spec/models/theme_spec.rb +++ b/spec/models/theme_spec.rb @@ -328,7 +328,7 @@ HTML expect(scss).to include("background-color:red") expect(scss).to include("font-size:25px") - setting = theme.settings.find { |s| s.name == :font_size } + setting = theme.settings[:font_size] setting.value = "30px" theme.save! @@ -418,7 +418,7 @@ HTML expect(theme_field.javascript_cache.content).to include("alert(settings.name)") expect(theme_field.javascript_cache.content).to include("let a = () => {}") - setting = theme.settings.find { |s| s.name == :name } + setting = theme.settings[:name] setting.value = "bill" theme.save!