From 10203bd57a13b243b03491f107efbe9566e89ed6 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 9 Jan 2024 15:01:53 +0100 Subject: [PATCH] Clean up `Setting` model and remove dead code (#28661) --- app/lib/settings/scoped_settings.rb | 79 ----------------------------- app/models/setting.rb | 12 ++--- spec/models/setting_spec.rb | 30 +++++------ 3 files changed, 15 insertions(+), 106 deletions(-) delete mode 100644 app/lib/settings/scoped_settings.rb diff --git a/app/lib/settings/scoped_settings.rb b/app/lib/settings/scoped_settings.rb deleted file mode 100644 index 3ad57cc1e..000000000 --- a/app/lib/settings/scoped_settings.rb +++ /dev/null @@ -1,79 +0,0 @@ -# frozen_string_literal: true - -module Settings - class ScopedSettings - DEFAULTING_TO_UNSCOPED = %w( - theme - noindex - ).freeze - - def initialize(object) - @object = object - end - - def method_missing(method, *args) - method_name = method.to_s - # set a value for a variable - if method_name[-1] == '=' - var_name = method_name.sub('=', '') - value = args.first - self[var_name] = value - else - # retrieve a value - self[method_name] - end - end - - def respond_to_missing?(*) - true - end - - def all_as_records - vars = thing_scoped - records = vars.index_by(&:var) - - Setting.default_settings.each do |key, default_value| - next if records.key?(key) || default_value.is_a?(Hash) - - records[key] = Setting.new(var: key, value: default_value) - end - - records - end - - def []=(key, value) - key = key.to_s - record = thing_scoped.find_or_initialize_by(var: key) - record.update!(value: value) - - Rails.cache.write(Setting.cache_key(key, @object), value) - end - - def [](key) - Rails.cache.fetch(Setting.cache_key(key, @object)) do - db_val = thing_scoped.find_by(var: key.to_s) - if db_val - default_value = ScopedSettings.default_settings[key] - return default_value.with_indifferent_access.merge!(db_val.value) if default_value.is_a?(Hash) - - db_val.value - else - ScopedSettings.default_settings[key] - end - end - end - - class << self - def default_settings - defaulting = DEFAULTING_TO_UNSCOPED.index_with { |k| Setting[k] } - Setting.default_settings.merge!(defaulting) - end - end - - protected - - def thing_scoped - Setting.unscoped.where(thing_type: @object.class.base_class.to_s, thing_id: @object.id) - end - end -end diff --git a/app/models/setting.rb b/app/models/setting.rb index ecd9d2d73..6af7a98c6 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -67,10 +67,6 @@ class Setting < ApplicationRecord end # rubocop:enable Style/MissingRespondToMissing - def object(var_name) - find_by(var: var_name.to_s) - end - def cache_prefix_by_startup @cache_prefix_by_startup ||= Digest::MD5.hexdigest(default_settings.to_s) end @@ -81,16 +77,14 @@ class Setting < ApplicationRecord def [](key) Rails.cache.fetch(cache_key(key)) do - db_val = object(key) + db_val = find_by(var: key) db_val ? db_val.value : default_settings[key] end end # set a setting value by [] notation def []=(var_name, value) - var_name = var_name.to_s - - record = object(var_name) || new(var: var_name) + record = find_or_initialize_by(var: var_name.to_s) record.value = value record.save! end @@ -100,7 +94,7 @@ class Setting < ApplicationRecord content = Rails.root.join('config', 'settings.yml').read hash = content.empty? ? {} : YAML.safe_load(ERB.new(content).result, aliases: true).to_hash - @default_settings = hash[Rails.env] || {} + @default_settings = (hash[Rails.env] || {}).freeze end end diff --git a/spec/models/setting_spec.rb b/spec/models/setting_spec.rb index ca9c65511..a1e24e835 100644 --- a/spec/models/setting_spec.rb +++ b/spec/models/setting_spec.rb @@ -23,38 +23,32 @@ RSpec.describe Setting do context 'when Rails.cache does not exists' do before do - allow(described_class).to receive(:object).with(key).and_return(object) allow(described_class).to receive(:default_settings).and_return(default_settings) - Fabricate(:setting, var: key, value: nil) + Fabricate(:setting, var: key, value: 42) if save_setting Rails.cache.delete(cache_key) end - let(:object) { nil } let(:default_value) { 'default_value' } let(:default_settings) { { key => default_value } } + let(:save_setting) { true } - it 'calls Setting.object' do - allow(described_class).to receive(:object).with(key) + context 'when the setting has been saved to database' do + it 'returns the value from database' do + callback = double + allow(callback).to receive(:call) - described_class[key] + ActiveSupport::Notifications.subscribed callback, 'sql.active_record' do + expect(described_class[key]).to eq 42 + end - expect(described_class).to have_received(:object).with(key) - end - - context 'when Setting.object returns truthy' do - let(:object) { db_val } - let(:db_val) { instance_double(described_class, value: 'db_val') } - let(:default_value) { 'default_value' } - - it 'returns db_val.value' do - expect(described_class[key]).to be db_val.value + expect(callback).to have_received(:call) end end - context 'when Setting.object returns falsey' do - let(:object) { nil } + context 'when the setting has not been saved to database' do + let(:save_setting) { false } it 'returns default_settings[key]' do expect(described_class[key]).to be default_settings[key]