DEV: Remove site_setting_saved event (#15164)
We said we would drop it from 2.4, so this is long overdue Co-authored-by: Jarek Radosz <jradosz@gmail.com>
This commit is contained in:
parent
cfb6199a95
commit
2f04a9b9fb
|
@ -7,11 +7,6 @@ class SiteSetting < ActiveRecord::Base
|
||||||
validates_presence_of :name
|
validates_presence_of :name
|
||||||
validates_presence_of :data_type
|
validates_presence_of :data_type
|
||||||
|
|
||||||
after_save do |site_setting|
|
|
||||||
DiscourseEvent.trigger(:site_setting_saved, site_setting)
|
|
||||||
true
|
|
||||||
end
|
|
||||||
|
|
||||||
def self.load_settings(file, plugin: nil)
|
def self.load_settings(file, plugin: nil)
|
||||||
SiteSettings::YamlLoader.new(file).load do |category, name, default, opts|
|
SiteSettings::YamlLoader.new(file).load do |category, name, default, opts|
|
||||||
setting(name, default, opts.merge(category: category, plugin: plugin))
|
setting(name, default, opts.merge(category: category, plugin: plugin))
|
||||||
|
|
|
@ -17,7 +17,7 @@ class DiscourseEvent
|
||||||
|
|
||||||
def self.on(event_name, &block)
|
def self.on(event_name, &block)
|
||||||
if event_name == :site_setting_saved
|
if event_name == :site_setting_saved
|
||||||
Discourse.deprecate("The :site_setting_saved event is deprecated. Please use :site_setting_changed instead", since: "2.3.0beta8", drop_from: "2.4")
|
Discourse.deprecate("The :site_setting_saved event is deprecated. Please use :site_setting_changed instead", since: "2.3.0beta8", drop_from: "2.4", raise_error: true)
|
||||||
end
|
end
|
||||||
events[event_name] << block
|
events[event_name] << block
|
||||||
end
|
end
|
||||||
|
|
|
@ -45,7 +45,6 @@ class SiteSettings::LocalProcessProvider
|
||||||
settings[name] = setting
|
settings[name] = setting
|
||||||
end
|
end
|
||||||
setting.value = value.to_s
|
setting.value = value.to_s
|
||||||
DiscourseEvent.trigger(:site_setting_saved, setting)
|
|
||||||
setting
|
setting
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -176,9 +176,9 @@ describe SiteSettingExtension do
|
||||||
no_change_events = DiscourseEvent.track_events { settings.test_setting = 2 }
|
no_change_events = DiscourseEvent.track_events { settings.test_setting = 2 }
|
||||||
default_events = DiscourseEvent.track_events { settings.test_setting = 1 }
|
default_events = DiscourseEvent.track_events { settings.test_setting = 1 }
|
||||||
|
|
||||||
expect(override_events.map { |e| e[:event_name] }).to contain_exactly(:site_setting_changed, :site_setting_saved)
|
expect(override_events.map { |e| e[:event_name] }).to contain_exactly(:site_setting_changed)
|
||||||
expect(no_change_events.map { |e| e[:event_name] }).to contain_exactly(:site_setting_saved)
|
expect(no_change_events.map { |e| e[:event_name] }).to be_empty
|
||||||
expect(default_events.map { |e| e[:event_name] }).to contain_exactly(:site_setting_changed, :site_setting_saved)
|
expect(default_events.map { |e| e[:event_name] }).to contain_exactly(:site_setting_changed)
|
||||||
|
|
||||||
changed_event_1 = override_events.find { |e| e[:event_name] == :site_setting_changed }
|
changed_event_1 = override_events.find { |e| e[:event_name] == :site_setting_changed }
|
||||||
changed_event_2 = default_events.find { |e| e[:event_name] == :site_setting_changed }
|
changed_event_2 = default_events.find { |e| e[:event_name] == :site_setting_changed }
|
||||||
|
@ -186,53 +186,6 @@ describe SiteSettingExtension do
|
||||||
expect(changed_event_1[:params]).to eq([:test_setting, 1, 2])
|
expect(changed_event_1[:params]).to eq([:test_setting, 1, 2])
|
||||||
expect(changed_event_2[:params]).to eq([:test_setting, 2, 1])
|
expect(changed_event_2[:params]).to eq([:test_setting, 2, 1])
|
||||||
end
|
end
|
||||||
|
|
||||||
it "provides the correct values when using site_setting_changed" do
|
|
||||||
event_new_value = nil
|
|
||||||
event_old_value = nil
|
|
||||||
site_setting_value = nil
|
|
||||||
|
|
||||||
test_lambda = -> (name, old_val, new_val) do
|
|
||||||
event_old_value = old_val
|
|
||||||
event_new_value = new_val
|
|
||||||
site_setting_value = settings.test_setting
|
|
||||||
end
|
|
||||||
|
|
||||||
begin
|
|
||||||
DiscourseEvent.on(:site_setting_changed, &test_lambda)
|
|
||||||
settings.test_setting = 2
|
|
||||||
ensure
|
|
||||||
DiscourseEvent.off(:site_setting_changed, &test_lambda)
|
|
||||||
end
|
|
||||||
|
|
||||||
expect(event_old_value).to eq(1)
|
|
||||||
expect(event_new_value).to eq(2)
|
|
||||||
expect(site_setting_value).to eq(2)
|
|
||||||
end
|
|
||||||
|
|
||||||
it "can produce confusing results when using site_setting_saved" do
|
|
||||||
# site_setting_saved is deprecated. This test case illustrates why it can be confusing
|
|
||||||
|
|
||||||
active_record_value = nil
|
|
||||||
site_setting_value = nil
|
|
||||||
|
|
||||||
test_lambda = -> (setting) do
|
|
||||||
active_record_value = setting.value
|
|
||||||
site_setting_value = settings.test_setting
|
|
||||||
end
|
|
||||||
|
|
||||||
begin
|
|
||||||
DiscourseEvent.on(:site_setting_saved, &test_lambda)
|
|
||||||
settings.test_setting = 2
|
|
||||||
ensure
|
|
||||||
DiscourseEvent.off(:site_setting_saved, &test_lambda)
|
|
||||||
end
|
|
||||||
|
|
||||||
# Problem 1, the site_setting_changed event gives us the database value, not the ruby value
|
|
||||||
expect(active_record_value).to eq("2")
|
|
||||||
# Problem 2, calling SiteSetting.test_setting inside the event will still return the old value
|
|
||||||
expect(site_setting_value).to eq(1)
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "int setting" do
|
describe "int setting" do
|
||||||
|
|
Loading…
Reference in New Issue