DEV: Log site setting changes from the rails console (#31353)

When using the rails console to change site settings log them to the
staff actions logs so that there is a record.

lib/site_setting_extension.rb

@@ -4,6 +4,8 @@ module SiteSettingExtension
   include SiteSettings::DeprecatedSettings
   include HasSanitizableFields
 
+  SiteSettingChangeResult = Struct.new(:previous_value, :new_value)
+
   # support default_locale being set via global settings
   # this also adds support for testing the extension and global settings
   # for site locale
@@ -457,9 +459,13 @@ module SiteSettingExtension
     clear_uploads_cache(name)
     notify_clients!(name) if client_settings.include? name
     clear_cache!
-    if old_val != current[name]
-      DiscourseEvent.trigger(:site_setting_changed, name, old_val, current[name])
+
+    if defined?(Rails::Console)
+      details = "Updated via Rails console"
+      log(name, val, old_val, Discourse.system_user, details)
     end
+
+    DiscourseEvent.trigger(:site_setting_changed, name, old_val, current[name])
   end
 
   def notify_changed!
@@ -519,13 +525,9 @@ module SiteSettingExtension
     if has_setting?(name)
       prev_value = public_send(name)
       set(name, value)
-      value = prev_value = "[FILTERED]" if secret_settings.include?(name.to_sym)
-      StaffActionLogger.new(user).log_site_setting_change(
-        name,
-        prev_value,
-        value,
-        { details: detailed_message }.compact_blank,
-      )
+      # Logging via the rails console is already handled in add_override!
+      log(name, value, prev_value, user, detailed_message) unless defined?(Rails::Console)
+      SiteSettingChangeResult.new(prev_value, public_send(name))
     else
       raise Discourse::InvalidParameters.new(
               I18n.t("errors.site_settings.invalid_site_setting", name: name),
@@ -777,6 +779,17 @@ module SiteSettingExtension
     end
   end
 
+  def log(name, value, prev_value, user = Discourse.system_user, detailed_message = nil)
+    value = prev_value = "[FILTERED]" if secret_settings.include?(name.to_sym)
+    return if hidden_settings.include?(name.to_sym)
+    StaffActionLogger.new(user).log_site_setting_change(
+      name,
+      prev_value,
+      value,
+      { details: detailed_message }.compact_blank,
+    )
+  end
+
   def default_uploads
     @default_uploads ||= {}
 

spec/jobs/disable_bootstrap_mode_spec.rb

@@ -19,22 +19,25 @@ RSpec.describe Jobs::DisableBootstrapMode do
 
     it "turns off bootstrap mode if bootstrap_mode_min_users is set to 0" do
       SiteSetting.bootstrap_mode_min_users = 0
-      StaffActionLogger.any_instance.expects(:log_site_setting_change).times(4)
+      StaffActionLogger.any_instance.expects(:log_site_setting_change).times(3)
       Jobs::DisableBootstrapMode.new.execute(user_id: admin.id)
+      expect(SiteSetting.bootstrap_mode_enabled).to eq(false)
     end
 
     it "does not amend setting that is not in bootstrap state" do
       SiteSetting.bootstrap_mode_min_users = 0
       SiteSetting.default_trust_level = TrustLevel[3]
-      StaffActionLogger.any_instance.expects(:log_site_setting_change).times(3)
+      StaffActionLogger.any_instance.expects(:log_site_setting_change).times(2)
       Jobs::DisableBootstrapMode.new.execute(user_id: admin.id)
+      expect(SiteSetting.bootstrap_mode_enabled).to eq(false)
     end
 
     it "successfully turns off bootstrap mode" do
       SiteSetting.bootstrap_mode_min_users = 5
       6.times { Fabricate(:user) }
-      StaffActionLogger.any_instance.expects(:log_site_setting_change).times(4)
+      StaffActionLogger.any_instance.expects(:log_site_setting_change).times(3)
       Jobs::DisableBootstrapMode.new.execute(user_id: admin.id)
+      expect(SiteSetting.bootstrap_mode_enabled).to eq(false)
     end
   end
 end

spec/jobs/enable_bootstrap_mode_spec.rb

@@ -26,15 +26,16 @@ RSpec.describe Jobs::EnableBootstrapMode do
 
     it "does not amend setting that is not in default state" do
       SiteSetting.default_trust_level = TrustLevel[3]
-      StaffActionLogger.any_instance.expects(:log_site_setting_change).times(3)
+      StaffActionLogger.any_instance.expects(:log_site_setting_change).times(2)
       Jobs::EnableBootstrapMode.new.execute(user_id: admin.id)
+      expect(SiteSetting.bootstrap_mode_enabled).to eq(true)
     end
 
     it "successfully turns on bootstrap mode" do
-      StaffActionLogger.any_instance.expects(:log_site_setting_change).times(4)
       Jobs::EnableBootstrapMode.new.execute(user_id: admin.id)
       expect(admin.reload.moderator).to be_truthy
       expect(Jobs::SendSystemMessage.jobs.size).to eq(0)
+      expect(SiteSetting.bootstrap_mode_enabled).to eq(true)
     end
   end
 end

spec/lib/site_setting_extension_spec.rb

@@ -1056,4 +1056,91 @@ RSpec.describe SiteSettingExtension do
       end
     end
   end
+
+  describe "logging Site Settings via the Rails Console" do
+    around do |example|
+      # Ensure Rails::Console is defined for the duration of each example.
+      if !Rails.const_defined?(:Console)
+        Rails.const_set("Console", Module.new)
+        example.run
+        Rails.send(:remove_const, "Console")
+      else
+        example.run
+      end
+    end
+
+    before do
+      settings.setting(:log_test, "initial")
+      settings.refresh!
+    end
+
+    context "when using the direct setter" do
+      it "logs the change exactly once" do
+        logger_spy = instance_spy(StaffActionLogger)
+        allow(StaffActionLogger).to receive(:new).with(Discourse.system_user).and_return(logger_spy)
+
+        settings.log_test = "changed"
+        expect(settings.log_test).to eq("changed")
+        expect(logger_spy).to have_received(:log_site_setting_change).with(
+          :log_test,
+          "initial",
+          "changed",
+          { details: "Updated via Rails console" },
+        ).once
+      end
+    end
+
+    context "when using set_and_log" do
+      it "logs the change exactly once without double logging" do
+        logger_spy = instance_spy(StaffActionLogger)
+        allow(StaffActionLogger).to receive(:new).with(Discourse.system_user).and_return(logger_spy)
+
+        settings.set_and_log("log_test", "changed", Discourse.system_user)
+        expect(settings.log_test).to eq("changed")
+        expect(logger_spy).to have_received(:log_site_setting_change).with(
+          :log_test,
+          "initial",
+          "changed",
+          { details: "Updated via Rails console" },
+        ).once
+      end
+    end
+
+    context "for secret settings" do
+      before do
+        settings.setting(:secret_test, "old_secret", secret: true)
+        settings.refresh!
+      end
+
+      it "logs filtered values" do
+        logger_spy = instance_spy(StaffActionLogger)
+        allow(StaffActionLogger).to receive(:new).with(Discourse.system_user).and_return(logger_spy)
+
+        settings.secret_test = "new_secret"
+        expect(settings.secret_test).to eq("new_secret")
+        expect(logger_spy).to have_received(:log_site_setting_change).with(
+          :secret_test,
+          "[FILTERED]",
+          "[FILTERED]",
+          { details: "Updated via Rails console" },
+        ).once
+      end
+    end
+
+    context "for hidden settings" do
+      before do
+        settings.setting(:hidden_test, "old_hidden", hidden: true)
+        settings.refresh!
+      end
+
+      it "does not log the change" do
+        logger_spy = instance_spy(StaffActionLogger)
+        allow(StaffActionLogger).to receive(:new).with(Discourse.system_user).and_return(logger_spy)
+
+        settings.hidden_test = "changed"
+        expect(settings.hidden_test).to eq("changed")
+        expect(logger_spy).not_to have_received(:log_site_setting_change)
+      end
+    end
+  end
 end

spec/requests/categories_controller_spec.rb

@@ -555,7 +555,7 @@ RSpec.describe CategoriesController do
           expect(category.category_groups.map { |g| [g.group_id, g.permission_type] }.sort).to eq(
             [[Group[:everyone].id, readonly], [Group[:staff].id, create_post]],
           )
-          expect(UserHistory.count).to eq(6) # 1 + 5 (bootstrap mode)
+          expect(UserHistory.count).to eq(5)
         end
       end
     end
@@ -785,7 +785,7 @@ RSpec.describe CategoriesController do
                 },
               }
           expect(response.status).to eq(200)
-          expect(UserHistory.count).to eq(7) # 2 + 5 (bootstrap mode)
+          expect(UserHistory.count).to eq(6)
         end
 
         it "updates per-category settings correctly" do