DEV: Allow for setting a message with SiteSetting.set_and_log (#27447)

When we turn on settings automatically for customers,
we sometimes use `.set_and_log` which will make a staff
action log for the site setting change. This is fine, but
there is no context for customers.

This change allows setting a message with `.set_and_log`, which
will be stored in the `details` column of the staff action log
created, which will show up on `/admin/logs/staff_action_logs`

---------

Co-authored-by: Kelv <kelv@discourse.org>

app/assets/javascripts/admin/addon/templates/logs/staff-action-logs.hbs

@@ -68,6 +68,7 @@
       @value={{this.filterActionId}}
       @onChange={{action "filterActionIdChanged"}}
       @options={{hash none="admin.logs.staff_actions.all"}}
+      @id="staff-action-logs-action-filter"
     />
   {{/if}}
 
@@ -95,7 +96,7 @@
         </thead>
         <tbody>
           {{#each this.model.content as |item|}}
-            <tr class="admin-list-item">
+            <tr class="admin-list-item" data-user-history-id={{item.id}}>
               <td class="staff-users">
                 <div class="staff-user">
                   {{#if item.acting_user}}

lib/site_setting_extension.rb

@@ -442,12 +442,17 @@ module SiteSettingExtension
     end
   end
 
-  def set_and_log(name, value, user = Discourse.system_user)
+  def set_and_log(name, value, user = Discourse.system_user, detailed_message = nil)
     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)
+      StaffActionLogger.new(user).log_site_setting_change(
+        name,
+        prev_value,
+        value,
+        { details: detailed_message }.compact_blank,
+      )
     else
       raise Discourse::InvalidParameters.new(
               I18n.t("errors.site_settings.invalid_site_setting", name: name),

spec/fabricators/user_history_fabricator.rb

@@ -1,3 +1,16 @@
 # frozen_string_literal: true
 
-Fabricator(:user_history) {}
+Fabricator(:user_history) { acting_user { Fabricate(:user) } }
+
+Fabricator(:site_setting_change_history, from: :user_history) do
+  action { UserHistory.actions[:change_site_setting] }
+  previous_value { "old value" }
+  new_value { "new value" }
+  subject { "some_site_setting" }
+end
+
+Fabricator(:topic_closed_change_history, from: :user_history) do
+  action { UserHistory.actions[:topic_closed] }
+  subject { "some_site_setting" }
+  topic_id { Fabricate(:topic).id }
+end

spec/lib/site_setting_extension_spec.rb

@@ -569,6 +569,16 @@ RSpec.describe SiteSettingExtension do
       expect(UserHistory.last.previous_value).to eq("Discourse v1")
       expect(UserHistory.last.new_value).to eq("Discourse v2")
     end
+
+    context "when a detailed message is provided" do
+      let(:message) { "We really need to do this, see https://meta.discourse.org/t/123" }
+
+      it "adds the detailed message to the user history record" do
+        expect {
+          settings.set_and_log("title", "Discourse v2", Discourse.system_user, message)
+        }.to change { UserHistory.last.try(:details) }.to(message)
+      end
+    end
   end
 
   describe "filter domain name" do

spec/system/admin_staff_action_logs_spec.rb

@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+describe "Admin staff action logs", type: :system do
+  fab!(:current_user) { Fabricate(:admin) }
+  fab!(:history_1) do
+    Fabricate(
+      :site_setting_change_history,
+      subject: "enforce_second_factor",
+      previous_value: "no",
+      new_value: "all",
+    )
+  end
+  fab!(:history_2) { Fabricate(:topic_closed_change_history) }
+  let(:staff_action_logs_page) { PageObjects::Pages::AdminStaffActionLogs.new }
+
+  before { sign_in(current_user) }
+
+  it "shows details for an action" do
+    visit "/admin/logs/staff_action_logs"
+
+    expect(staff_action_logs_page.log_row(history_1)).to have_content(
+      I18n.t("admin_js.admin.logs.staff_actions.actions.change_site_setting"),
+    ).and have_content("enforce_second_factor").and have_content(
+                  I18n.t("admin_js.admin.logs.staff_actions.new_value", "all"),
+                ).and have_content(I18n.t("admin_js.admin.logs.staff_actions.previous_value", "no"))
+
+    expect(staff_action_logs_page.log_row(history_2)).to have_content(
+      I18n.t("admin_js.admin.logs.staff_actions.actions.topic_closed"),
+    ).and have_css("[data-link-topic-id='#{history_2.topic_id}']")
+  end
+
+  it "can filter by type of action" do
+    visit "/admin/logs/staff_action_logs"
+
+    expect(staff_action_logs_page).to have_log_row(history_1)
+    expect(staff_action_logs_page).to have_log_row(history_2)
+
+    staff_action_logs_page.filter_by_action(:change_site_setting)
+
+    expect(page).to have_css(
+      ".staff-action-logs-filters .filter",
+      text: I18n.t("admin_js.admin.logs.staff_actions.actions.change_site_setting"),
+    )
+
+    expect(staff_action_logs_page).to have_log_row(history_1)
+    expect(staff_action_logs_page).to have_no_log_row(history_2)
+  end
+
+  it "can show details for an action" do
+    history_1.update!(
+      details:
+        "Discourse is automatically enabling this for all our hosted customers, please see https://meta.discourse.org/t/123 for more information.",
+    )
+    visit "/admin/logs/staff_action_logs"
+
+    find("#{staff_action_logs_page.log_row_selector(history_1)} .col.value.details a").click
+    expect(PageObjects::Modals::Base.new).to have_content(history_1.details)
+  end
+end

spec/system/page_objects/pages/admin_staff_action_logs.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+module PageObjects
+  module Pages
+    class AdminStaffActionLogs < PageObjects::Pages::Base
+      def visit
+        page.visit "admin/logs/staff_action_logs"
+        self
+      end
+
+      def log_row_selector(user_history)
+        ".staff-logs tr[data-user-history-id='#{user_history.id}']"
+      end
+
+      def log_row(user_history)
+        find(log_row_selector(user_history))
+      end
+
+      def has_log_row?(user_history)
+        has_css?(log_row_selector(user_history))
+      end
+
+      def has_no_log_row?(user_history)
+        has_no_css?(log_row_selector(user_history))
+      end
+
+      def filter_by_action(action)
+        filter = PageObjects::Components::SelectKit.new("#staff-action-logs-action-filter")
+        filter.search(I18n.t("admin_js.admin.logs.staff_actions.actions.change_site_setting"))
+        filter.select_row_by_value(UserHistory.actions.key(UserHistory.actions[action.to_sym]).to_s)
+      end
+    end
+  end
+end