From 2f04a9b9fb850df90bc021d7b5ee73cf61ab6c61 Mon Sep 17 00:00:00 2001 From: Daniel Waterworth Date: Thu, 2 Dec 2021 09:33:03 -0600 Subject: [PATCH] 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 --- app/models/site_setting.rb | 5 -- lib/discourse_event.rb | 2 +- lib/site_settings/local_process_provider.rb | 1 - .../components/site_setting_extension_spec.rb | 53 ++----------------- 4 files changed, 4 insertions(+), 57 deletions(-) diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 8f0b2516281..33799aba840 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -7,11 +7,6 @@ class SiteSetting < ActiveRecord::Base validates_presence_of :name 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) SiteSettings::YamlLoader.new(file).load do |category, name, default, opts| setting(name, default, opts.merge(category: category, plugin: plugin)) diff --git a/lib/discourse_event.rb b/lib/discourse_event.rb index b2e03b8b1d1..c2db7d10025 100644 --- a/lib/discourse_event.rb +++ b/lib/discourse_event.rb @@ -17,7 +17,7 @@ class DiscourseEvent def self.on(event_name, &block) 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 events[event_name] << block end diff --git a/lib/site_settings/local_process_provider.rb b/lib/site_settings/local_process_provider.rb index 4294a3f02e2..37e5faad56b 100644 --- a/lib/site_settings/local_process_provider.rb +++ b/lib/site_settings/local_process_provider.rb @@ -45,7 +45,6 @@ class SiteSettings::LocalProcessProvider settings[name] = setting end setting.value = value.to_s - DiscourseEvent.trigger(:site_setting_saved, setting) setting end diff --git a/spec/components/site_setting_extension_spec.rb b/spec/components/site_setting_extension_spec.rb index 5be49b0ff03..956a390fae9 100644 --- a/spec/components/site_setting_extension_spec.rb +++ b/spec/components/site_setting_extension_spec.rb @@ -176,9 +176,9 @@ describe SiteSettingExtension do no_change_events = DiscourseEvent.track_events { settings.test_setting = 2 } 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(no_change_events.map { |e| e[:event_name] }).to contain_exactly(:site_setting_saved) - expect(default_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 be_empty + 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_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_2[:params]).to eq([:test_setting, 2, 1]) 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 describe "int setting" do