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 = +"

#{CGI.escapeHTML(cur&.dig("name").to_s)}

" + 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 +

name

#{tag_group1.name}#{tag_group2.name}
+ HTML + expect(parsed["side_by_side"]).to include(name_diff.strip) + expect(parsed["side_by_side"]).to include("#{tag1.name}") + expect(parsed["side_by_side"]).to include("#{tag3.name}") + 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