From aab2987438ad2587f568c63978d0e8acdbd96e8e Mon Sep 17 00:00:00 2001
From: Linca <41134017+Lhcfl@users.noreply.github.com>
Date: Mon, 9 Sep 2024 10:50:48 +0800
Subject: [PATCH] FEATURE: Log tag group changes in staff action log (#28787)

* FEATURE: Log tag group changes in staff action log

This commit records every change (add, change, delete) to a tag group in
the staff action log.

It uses a modal that was originally called ThemeChangeModal to display
changes, allowing staffs to see the specific changes clearly. The modal
is renamed to StaffActionLogChangeModal in this PR.

ref: https://meta.discourse.org/t/-/325011/14

Co-authored-by: Keegan George <kgeorge13@gmail.com>
---
 ...change.hbs => staff-action-log-change.hbs} |  0
 ...e-change.js => staff-action-log-change.js} |  2 +-
 .../admin-logs-staff-action-logs.js           |  6 +-
 .../admin/addon/models/staff-action-log.js    |  8 +-
 .../admin/staff_action_logs_controller.rb     | 46 +++++++---
 app/controllers/tag_groups_controller.rb      | 11 +++
 app/models/user_history.rb                    | 10 +-
 app/services/staff_action_logger.rb           | 27 ++++++
 config/locales/client.en.yml                  |  3 +
 .../staff_action_logs_controller_spec.rb      | 36 ++++++++
 spec/requests/tag_groups_controller_spec.rb   | 92 +++++++++++++++++++
 11 files changed, 222 insertions(+), 19 deletions(-)
 rename app/assets/javascripts/admin/addon/components/modal/{theme-change.hbs => staff-action-log-change.hbs} (100%)
 rename app/assets/javascripts/admin/addon/components/modal/{theme-change.js => staff-action-log-change.js} (86%)

diff --git a/app/assets/javascripts/admin/addon/components/modal/theme-change.hbs b/app/assets/javascripts/admin/addon/components/modal/staff-action-log-change.hbs
similarity index 100%
rename from app/assets/javascripts/admin/addon/components/modal/theme-change.hbs
rename to app/assets/javascripts/admin/addon/components/modal/staff-action-log-change.hbs
diff --git a/app/assets/javascripts/admin/addon/components/modal/theme-change.js b/app/assets/javascripts/admin/addon/components/modal/staff-action-log-change.js
similarity index 86%
rename from app/assets/javascripts/admin/addon/components/modal/theme-change.js
rename to app/assets/javascripts/admin/addon/components/modal/staff-action-log-change.js
index ff54a362ff2..eddda604925 100644
--- a/app/assets/javascripts/admin/addon/components/modal/theme-change.js
+++ b/app/assets/javascripts/admin/addon/components/modal/staff-action-log-change.js
@@ -3,7 +3,7 @@ import { tracked } from "@glimmer/tracking";
 import { action } from "@ember/object";
 import { ajax } from "discourse/lib/ajax";
 
-export default class AdminThemeChangeComponent extends Component {
+export default class AdminStaffActionLogComponent extends Component {
   @tracked diff;
 
   constructor() {
diff --git a/app/assets/javascripts/admin/addon/controllers/admin-logs-staff-action-logs.js b/app/assets/javascripts/admin/addon/controllers/admin-logs-staff-action-logs.js
index 2f5a972a82e..1d4401ca74d 100644
--- a/app/assets/javascripts/admin/addon/controllers/admin-logs-staff-action-logs.js
+++ b/app/assets/javascripts/admin/addon/controllers/admin-logs-staff-action-logs.js
@@ -6,8 +6,8 @@ import { exportEntity } from "discourse/lib/export-csv";
 import { outputExportResult } from "discourse/lib/export-result";
 import discourseComputed from "discourse-common/utils/decorators";
 import I18n from "discourse-i18n";
+import AdminStaffActionLogComponent from "../components/modal/staff-action-log-change";
 import StaffActionLogDetailsModal from "../components/modal/staff-action-log-details";
-import ThemeChangeModal from "../components/modal/theme-change";
 
 export default class AdminLogsStaffActionLogsController extends Controller {
   @service modal;
@@ -164,6 +164,8 @@ export default class AdminLogsStaffActionLogsController extends Controller {
   @action
   showCustomDetailsModal(model, event) {
     event?.preventDefault();
-    this.modal.show(ThemeChangeModal, { model: { staffActionLog: model } });
+    this.modal.show(AdminStaffActionLogComponent, {
+      model: { staffActionLog: model },
+    });
   }
 }
diff --git a/app/assets/javascripts/admin/addon/models/staff-action-log.js b/app/assets/javascripts/admin/addon/models/staff-action-log.js
index f208451a07a..66474f6d121 100644
--- a/app/assets/javascripts/admin/addon/models/staff-action-log.js
+++ b/app/assets/javascripts/admin/addon/models/staff-action-log.js
@@ -104,6 +104,12 @@ export default class StaffActionLog extends RestModel {
 
   @discourseComputed("action_name")
   useCustomModalForDetails(actionName) {
-    return ["change_theme", "delete_theme"].includes(actionName);
+    return [
+      "change_theme",
+      "delete_theme",
+      "tag_group_create",
+      "tag_group_destroy",
+      "tag_group_change",
+    ].includes(actionName);
   }
 }
diff --git a/app/controllers/admin/staff_action_logs_controller.rb b/app/controllers/admin/staff_action_logs_controller.rb
index f419f47c5bf..87ec7432267 100644
--- a/app/controllers/admin/staff_action_logs_controller.rb
+++ b/app/controllers/admin/staff_action_logs_controller.rb
@@ -34,26 +34,18 @@ class Admin::StaffActionLogsController < Admin::StaffController
     prev = JSON.parse(prev) if prev
     cur = JSON.parse(cur) if cur
 
-    diff_fields = {}
-
     output = +"<h2>#{CGI.escapeHTML(cur&.dig("name").to_s)}</h2><p></p>"
 
+    diff_fields = {}
     diff_fields["name"] = { prev: prev&.dig("name").to_s, cur: cur&.dig("name").to_s }
 
-    %w[default user_selectable].each do |f|
-      diff_fields[f] = { prev: (!!prev&.dig(f)).to_s, cur: (!!cur&.dig(f)).to_s }
+    case @history.action
+    when *%i[change_theme delete_theme].map { |i| UserHistory.actions[i] }
+      diff_theme(diff_fields, prev, cur)
+    when *%i[tag_group_create tag_group_destroy tag_group_change].map { |i| UserHistory.actions[i] }
+      diff_tag_group(diff_fields, prev, cur)
     end
 
-    diff_fields["color scheme"] = {
-      prev: prev&.dig("color_scheme", "name").to_s,
-      cur: cur&.dig("color_scheme", "name").to_s,
-    }
-
-    diff_fields["included themes"] = { prev: child_themes(prev), cur: child_themes(cur) }
-
-    load_diff(diff_fields, :cur, cur)
-    load_diff(diff_fields, :prev, prev)
-
     diff_fields.delete_if { |k, v| v[:cur] == v[:prev] }
 
     diff_fields.each do |k, v|
@@ -89,4 +81,30 @@ class Admin::StaffActionLogsController < Admin::StaffController
       { id: name, action_id: UserHistory.actions[name] || UserHistory.actions[:custom_staff] }
     end
   end
+
+  def diff_theme(diff_fields, prev, cur)
+    %w[default user_selectable].each do |f|
+      diff_fields[f] = { prev: (!!prev&.dig(f)).to_s, cur: (!!cur&.dig(f)).to_s }
+    end
+
+    diff_fields["color scheme"] = {
+      prev: prev&.dig("color_scheme", "name").to_s,
+      cur: cur&.dig("color_scheme", "name").to_s,
+    }
+
+    diff_fields["included themes"] = { prev: child_themes(prev), cur: child_themes(cur) }
+
+    load_diff(diff_fields, :cur, cur)
+    load_diff(diff_fields, :prev, prev)
+  end
+
+  def diff_tag_group(diff_fields, prev, cur)
+    %w[parent_tag_name one_per_topic permissions].each do |f|
+      diff_fields[f] = { prev: prev&.dig(f).to_s, cur: cur&.dig(f).to_s }
+    end
+    diff_fields["tag_names"] = {
+      prev: prev&.dig("tag_names")&.join("\n"),
+      cur: cur&.dig("tag_names")&.join("\n"),
+    }
+  end
 end
diff --git a/app/controllers/tag_groups_controller.rb b/app/controllers/tag_groups_controller.rb
index 74aa7528af0..6741838ee63 100644
--- a/app/controllers/tag_groups_controller.rb
+++ b/app/controllers/tag_groups_controller.rb
@@ -51,6 +51,10 @@ class TagGroupsController < ApplicationController
     guardian.ensure_can_admin_tag_groups!
     @tag_group = TagGroup.new(tag_groups_params)
     if @tag_group.save
+      StaffActionLogger.new(current_user).log_tag_group_create(
+        @tag_group.name,
+        TagGroupSerializer.new(@tag_group).to_json(root: false),
+      )
       render_serialized(@tag_group, TagGroupSerializer)
     else
       render_json_error(@tag_group)
@@ -59,13 +63,20 @@ class TagGroupsController < ApplicationController
 
   def update
     guardian.ensure_can_admin_tag_groups!
+    old_data = TagGroupSerializer.new(@tag_group).to_json(root: false)
     json_result(@tag_group, serializer: TagGroupSerializer) do |tag_group|
       @tag_group.update(tag_groups_params)
+      new_data = TagGroupSerializer.new(@tag_group).to_json(root: false)
+      StaffActionLogger.new(current_user).log_tag_group_change(@tag_group.name, old_data, new_data)
     end
   end
 
   def destroy
     guardian.ensure_can_admin_tag_groups!
+    StaffActionLogger.new(current_user).log_tag_group_destroy(
+      @tag_group.name,
+      TagGroupSerializer.new(@tag_group).to_json(root: false),
+    )
     @tag_group.destroy
     render json: success_json
   end
diff --git a/app/models/user_history.rb b/app/models/user_history.rb
index b3c41856e5e..14b83cc3454 100644
--- a/app/models/user_history.rb
+++ b/app/models/user_history.rb
@@ -152,6 +152,9 @@ class UserHistory < ActiveRecord::Base
         custom_emoji_destroy: 113,
         delete_post_permanently: 114,
         delete_topic_permanently: 115,
+        tag_group_create: 116,
+        tag_group_destroy: 117,
+        tag_group_change: 118,
       )
   end
 
@@ -266,6 +269,9 @@ class UserHistory < ActiveRecord::Base
       custom_emoji_destroy
       delete_post_permanently
       delete_topic_permanently
+      tag_group_create
+      tag_group_destroy
+      tag_group_change
     ]
   end
 
@@ -332,7 +338,9 @@ class UserHistory < ActiveRecord::Base
   end
 
   def new_value_is_json?
-    [UserHistory.actions[:change_theme], UserHistory.actions[:delete_theme]].include?(action)
+    %i[change_theme delete_theme tag_group_create tag_group_destroy tag_group_change]
+      .map { |i| UserHistory.actions[i] }
+      .include?(action)
   end
 
   def previous_value_is_json?
diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb
index c39befbe89f..ee16aa51c9d 100644
--- a/app/services/staff_action_logger.rb
+++ b/app/services/staff_action_logger.rb
@@ -1052,6 +1052,33 @@ class StaffActionLogger
     )
   end
 
+  def log_tag_group_create(name, new_value, opts = {})
+    UserHistory.create!(
+      params(opts).merge(json_params(nil, new_value)).merge(
+        action: UserHistory.actions[:tag_group_create],
+        subject: name,
+      ),
+    )
+  end
+
+  def log_tag_group_destroy(name, old_value, opts = {})
+    UserHistory.create!(
+      params(opts).merge(json_params(old_value, nil)).merge(
+        action: UserHistory.actions[:tag_group_destroy],
+        subject: name,
+      ),
+    )
+  end
+
+  def log_tag_group_change(name, old_data, new_data)
+    UserHistory.create!(
+      params.merge(json_params(old_data, new_data)).merge(
+        action: UserHistory.actions[:tag_group_change],
+        subject: name,
+      ),
+    )
+  end
+
   private
 
   def json_params(previous_value, new_value)
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 9ad93f9f287..8da67ce6fb4 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -6339,6 +6339,9 @@ en:
             custom_emoji_destroy: "delete custom emoji"
             delete_post_permanently: "permanently delete post"
             delete_topic_permanently: "permanently delete topic"
+            tag_group_create: "create tag group"
+            tag_group_destroy: "delete tag group"
+            tag_group_change: "change tag group"
         screened_emails:
           title: "Screened Emails"
           description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed."
diff --git a/spec/requests/admin/staff_action_logs_controller_spec.rb b/spec/requests/admin/staff_action_logs_controller_spec.rb
index 182aabf4f44..4dde1bca9a7 100644
--- a/spec/requests/admin/staff_action_logs_controller_spec.rb
+++ b/spec/requests/admin/staff_action_logs_controller_spec.rb
@@ -129,10 +129,45 @@ RSpec.describe Admin::StaffActionLogsController do
       end
     end
 
+    shared_examples "tag_group diffs accessible" do
+      it "generates diffs for tag_group changes" do
+        tag1 = Fabricate(:tag)
+        tag2 = Fabricate(:tag)
+        tag3 = Fabricate(:tag)
+        tag_group1 = Fabricate(:tag_group, tags: [tag1, tag2])
+
+        old_json = TagGroupSerializer.new(tag_group1, root: false).to_json
+
+        tag_group2 = Fabricate(:tag_group, tags: [tag2, tag3])
+
+        new_json = TagGroupSerializer.new(tag_group2, root: false).to_json
+
+        record =
+          StaffActionLogger.new(Discourse.system_user).log_tag_group_change(
+            tag_group2.name,
+            old_json,
+            new_json,
+          )
+
+        get "/admin/logs/staff_action_logs/#{record.id}/diff.json"
+        expect(response.status).to eq(200)
+
+        parsed = response.parsed_body
+
+        name_diff = <<-HTML
+          <h3>name</h3><p></p><table class="markdown"><tr><td class="diff-del"><del>#{tag_group1.name}</del></td><td class="diff-ins"><ins>#{tag_group2.name}</ins></td></tr></table>
+        HTML
+        expect(parsed["side_by_side"]).to include(name_diff.strip)
+        expect(parsed["side_by_side"]).to include("<del>#{tag1.name}</del>")
+        expect(parsed["side_by_side"]).to include("<ins>#{tag3.name}</ins>")
+      end
+    end
+
     context "when logged in as an admin" do
       before { sign_in(admin) }
 
       include_examples "theme diffs accessible"
+      include_examples "tag_group diffs accessible"
 
       it "is not erroring when current value is empty" do
         theme = Fabricate(:theme)
@@ -146,6 +181,7 @@ RSpec.describe Admin::StaffActionLogsController do
       before { sign_in(moderator) }
 
       include_examples "theme diffs accessible"
+      include_examples "tag_group diffs accessible"
     end
 
     context "when logged in as a non-staff user" do
diff --git a/spec/requests/tag_groups_controller_spec.rb b/spec/requests/tag_groups_controller_spec.rb
index e53ef2f243b..2051771ddd0 100644
--- a/spec/requests/tag_groups_controller_spec.rb
+++ b/spec/requests/tag_groups_controller_spec.rb
@@ -147,4 +147,96 @@ RSpec.describe TagGroupsController do
       end
     end
   end
+
+  describe "#create" do
+    fab!(:admin)
+
+    fab!(:tag1) { Fabricate(:tag) }
+    fab!(:tag2) { Fabricate(:tag) }
+
+    before { sign_in(admin) }
+
+    it "should create a tag group and log the creation" do
+      post "/tag_groups.json",
+           params: {
+             tag_group: {
+               name: "test_tag_group_log",
+               tag_names: [tag1.name, tag2.name],
+             },
+           }
+
+      expect(response.status).to eq(200)
+
+      expect(TagGroup.last.id).to eq(response.parsed_body["tag_group"]["id"])
+
+      expect(UserHistory.last).to have_attributes(
+        acting_user_id: admin.id,
+        action: UserHistory.actions[:tag_group_create],
+        subject: "test_tag_group_log",
+        new_value: response.parsed_body["tag_group"].to_json,
+      )
+    end
+  end
+
+  describe "#delete" do
+    fab!(:admin)
+
+    fab!(:tag1) { Fabricate(:tag) }
+    fab!(:tag2) { Fabricate(:tag) }
+    fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag2]) }
+
+    before { sign_in(admin) }
+
+    it "should delete the tag group and log the deletion" do
+      previous_value = TagGroupSerializer.new(tag_group).to_json(root: false)
+
+      delete "/tag_groups/#{tag_group.id}.json"
+
+      expect(response.status).to eq(200)
+
+      expect(TagGroup.find_by(id: tag_group.id)).to eq(nil)
+
+      expect(UserHistory.last).to have_attributes(
+        acting_user_id: admin.id,
+        action: UserHistory.actions[:tag_group_destroy],
+        subject: tag_group.name,
+        previous_value:,
+      )
+    end
+  end
+
+  describe "#update" do
+    fab!(:admin)
+
+    fab!(:tag1) { Fabricate(:tag) }
+    fab!(:tag2) { Fabricate(:tag) }
+    fab!(:tag3) { Fabricate(:tag) }
+    fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag2]) }
+
+    before { sign_in(admin) }
+
+    it "should update the tag group and log the modification" do
+      previous_value = TagGroupSerializer.new(tag_group).to_json(root: false)
+
+      put "/tag_groups/#{tag_group.id}.json",
+          params: {
+            tag_group: {
+              tag_group: {
+                name: "test_tag_group_new_name",
+                tag_names: [tag2.name, tag3.name],
+              },
+            },
+          }
+
+      expect(response.status).to eq(200)
+
+      expect(UserHistory.last).to have_attributes(
+        acting_user_id: admin.id,
+        action: UserHistory.actions[:tag_group_change],
+        subject: tag_group.name,
+        previous_value:,
+        new_value: response.parsed_body["tag_group"].to_json,
+      )
+    end
+  end
 end