From 0ed784b4fc037446bb415e47504e5ca6f10bdac7 Mon Sep 17 00:00:00 2001 From: jbrw Date: Tue, 21 Jul 2020 14:29:02 -0400 Subject: [PATCH] FEATURE: Create logs for Group Moderator changes (#10271) --- .../admin/components/staff-actions.js | 12 +++++ .../admin/models/staff-action-log.js | 6 ++- app/controllers/posts_controller.rb | 5 ++ app/models/topic.rb | 6 +++ app/models/user_history.rb | 14 +++++- app/services/staff_action_logger.rb | 29 +++++++++++ config/locales/client.en.yml | 6 +++ spec/models/topic_spec.rb | 8 +++ spec/requests/posts_controller_spec.rb | 2 + spec/services/staff_action_logger_spec.rb | 49 +++++++++++++++++++ 10 files changed, 135 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/admin/components/staff-actions.js b/app/assets/javascripts/admin/components/staff-actions.js index 1c295799dd1..fe650878781 100644 --- a/app/assets/javascripts/admin/components/staff-actions.js +++ b/app/assets/javascripts/admin/components/staff-actions.js @@ -23,5 +23,17 @@ export default Component.extend({ return false; } ); + + $(this.element).on( + "click.discourse-staff-logs", + "[data-link-topic-id]", + e => { + let topicId = $(e.target).attr("data-link-topic-id"); + + DiscourseURL.routeTo(`/t/${topicId}`); + + return false; + } + ); } }); diff --git a/app/assets/javascripts/admin/models/staff-action-log.js b/app/assets/javascripts/admin/models/staff-action-log.js index a077d054c0d..5dc942da957 100644 --- a/app/assets/javascripts/admin/models/staff-action-log.js +++ b/app/assets/javascripts/admin/models/staff-action-log.js @@ -47,10 +47,14 @@ const StaffActionLog = RestModel.extend({ ? `${postId}` : null; + const topicLink = topicId + ? `${topicId}` + : null; + let lines = [ format("email", email), format("admin.logs.ip_address", ipAddress), - format("admin.logs.topic_id", topicId), + format("admin.logs.topic_id", topicLink, false), format("admin.logs.post_id", postLink, false), format("admin.logs.category_id", categoryId) ]; diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 09cde3a54d9..ade423ce7c4 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -477,6 +477,8 @@ class PostsController < ApplicationController post = find_post_from_params raise Discourse::NotFound unless guardian.can_edit_staff_notes?(post.topic) + previous_notice = post.custom_fields[Post::NOTICE_ARGS] + if params[:notice].present? post.custom_fields[Post::NOTICE_TYPE] = Post.notices[:custom] post.custom_fields[Post::NOTICE_ARGS] = PrettyText.cook(params[:notice], features: { onebox: false }) @@ -485,6 +487,9 @@ class PostsController < ApplicationController post.delete_post_notices end + details = { new_raw_value: params[:notice], old_value: previous_notice } + StaffActionLogger.new(current_user).log_post_staff_note(post, details) + render body: nil end diff --git a/app/models/topic.rb b/app/models/topic.rb index cbf29d19189..de3034e3249 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -621,6 +621,12 @@ class Topic < ActiveRecord::Base TopicStatusUpdater.new(self, user).update!(status, enabled, opts) DiscourseEvent.trigger(:topic_status_updated, self, status, enabled) + if status == 'closed' + StaffActionLogger.new(user).log_topic_closed(self, closed: enabled) + elsif status == 'archived' + StaffActionLogger.new(user).log_topic_archived(self, archived: enabled) + end + if enabled && private_message? && status.to_s["closed"] group_ids = user.groups.pluck(:id) if group_ids.present? diff --git a/app/models/user_history.rb b/app/models/user_history.rb index 26e7b8ab739..d9b7848bdd0 100644 --- a/app/models/user_history.rb +++ b/app/models/user_history.rb @@ -109,6 +109,12 @@ class UserHistory < ActiveRecord::Base add_email: 88, update_email: 89, destroy_email: 90, + topic_closed: 91, + topic_opened: 92, + topic_archived: 93, + topic_unarchived: 94, + post_staff_note_create: 95, + post_staff_note_destroy: 96 ) end @@ -193,7 +199,13 @@ class UserHistory < ActiveRecord::Base :page_unpublished, :add_email, :update_email, - :destroy_email + :destroy_email, + :topic_closed, + :topic_opened, + :topic_archived, + :topic_unarchived, + :post_staff_note_create, + :post_staff_note_destroy ] end diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index 0181a558745..aa5e52d34c8 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -148,6 +148,35 @@ class StaffActionLogger )) end + def log_topic_closed(topic, opts = {}) + raise Discourse::InvalidParameters.new(:topic) unless topic && topic.is_a?(Topic) + UserHistory.create!(params(opts).merge( + action: UserHistory.actions[opts[:closed] ? :topic_closed : :topic_opened], + topic_id: topic.id + )) + end + + def log_topic_archived(topic, opts = {}) + raise Discourse::InvalidParameters.new(:topic) unless topic && topic.is_a?(Topic) + UserHistory.create!(params(opts).merge( + action: UserHistory.actions[opts[:archived] ? :topic_archived : :topic_unarchived], + topic_id: topic.id + )) + end + + def log_post_staff_note(post, opts = {}) + raise Discourse::InvalidParameters.new(:post) unless post && post.is_a?(Post) + + args = params(opts).merge( + action: UserHistory.actions[opts[:new_raw_value].present? ? :post_staff_note_create : :post_staff_note_destroy], + post_id: post.id + ) + args[:new_value] = opts[:new_raw_value] if opts[:new_raw_value].present? + args[:previous_value] = opts[:old_value] if opts[:old_value].present? + + UserHistory.create!(params(opts).merge(args)) + end + def log_site_setting_change(setting_name, previous_value, new_value, opts = {}) raise Discourse::InvalidParameters.new(:setting_name) unless setting_name.present? && SiteSetting.respond_to?(setting_name) UserHistory.create!(params(opts).merge( diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 9ddc636610b..1e3ddae9de9 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -4236,6 +4236,12 @@ en: add_email: "add email" update_email: "update email" destroy_email: "destroy email" + topic_closed: "topic closed" + topic_opened: "topic opened" + topic_archived: "topic archived" + topic_unarchived: "topic unarchived" + post_staff_note_create: "add staff note" + post_staff_note_destroy: "destroy staff note" 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/models/topic_spec.rb b/spec/models/topic_spec.rb index dbe0c7460f1..b8be9f34684 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -1082,6 +1082,10 @@ describe Topic do end context 'archived' do + it 'should create a staff action log entry' do + expect { topic.update_status('archived', true, @user) }.to change { UserHistory.where(action: UserHistory.actions[:topic_archived]).count }.by(1) + end + context 'disable' do before do @archived_topic = Fabricate(:topic, archived: true, bumped_at: 1.hour.ago) @@ -1155,6 +1159,10 @@ describe Topic do expect { topic.update_status(status, true, @user) }.to change(topic.group_archived_messages, :count).by(1) end + + it 'should create a staff action log entry' do + expect { topic.update_status(status, true, @user) }.to change { UserHistory.where(action: UserHistory.actions[:topic_closed]).count }.by(1) + end end context 'autoclosed' do diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index 65d11dd5736..ef5bf4e46f4 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/spec/requests/posts_controller_spec.rb @@ -1805,6 +1805,7 @@ describe PostsController do expect(public_post.custom_fields[Post::NOTICE_TYPE]).to eq(Post.notices[:custom]) expect(public_post.custom_fields[Post::NOTICE_ARGS]).to include('

Hello world!

') expect(public_post.custom_fields[Post::NOTICE_ARGS]).not_to include('onebox') + expect(UserHistory.where(action: UserHistory.actions[:post_staff_note_create]).count).to eq(1) put "/posts/#{public_post.id}/notice.json", params: { notice: nil } @@ -1812,6 +1813,7 @@ describe PostsController do public_post.reload expect(public_post.custom_fields[Post::NOTICE_TYPE]).to eq(nil) expect(public_post.custom_fields[Post::NOTICE_ARGS]).to eq(nil) + expect(UserHistory.where(action: UserHistory.actions[:post_staff_note_destroy]).count).to eq(1) end describe 'group moderators' do diff --git a/spec/services/staff_action_logger_spec.rb b/spec/services/staff_action_logger_spec.rb index 68ccc7c1069..1d6bb3bea76 100644 --- a/spec/services/staff_action_logger_spec.rb +++ b/spec/services/staff_action_logger_spec.rb @@ -546,4 +546,53 @@ describe StaffActionLogger do end end + + describe 'log_topic_closed' do + fab!(:topic) { Fabricate(:topic) } + + it "raises an error when argument is missing" do + expect { logger.log_topic_closed(nil) }.to raise_error(Discourse::InvalidParameters) + end + + it "creates a new UserHistory record" do + expect { logger.log_topic_closed(topic, closed: true) }.to change { UserHistory.where(action: UserHistory.actions[:topic_closed]).count }.by(1) + expect { logger.log_topic_closed(topic, closed: false) }.to change { UserHistory.where(action: UserHistory.actions[:topic_opened]).count }.by(1) + end + end + + describe 'log_topic_archived' do + fab!(:topic) { Fabricate(:topic) } + + it "raises an error when argument is missing" do + expect { logger.log_topic_archived(nil) }.to raise_error(Discourse::InvalidParameters) + end + + it "creates a new UserHistory record" do + expect { logger.log_topic_archived(topic, archived: true) }.to change { UserHistory.where(action: UserHistory.actions[:topic_archived]).count }.by(1) + expect { logger.log_topic_archived(topic, archived: false) }.to change { UserHistory.where(action: UserHistory.actions[:topic_unarchived]).count }.by(1) + end + end + + describe 'log_post_staff_note' do + fab!(:post) { Fabricate(:post) } + + it "raises an error when argument is missing" do + expect { logger.log_topic_archived(nil) }.to raise_error(Discourse::InvalidParameters) + end + + it "creates a new UserHistory record" do + expect { logger.log_post_staff_note(post, { new_raw_value: 'my note', old_value: nil }) }.to change { UserHistory.count }.by(1) + user_history = UserHistory.last + expect(user_history.action).to eq(UserHistory.actions[:post_staff_note_create]) + expect(user_history.new_value).to eq('my note') + expect(user_history.previous_value).to eq(nil) + + expect { logger.log_post_staff_note(post, { new_raw_value: '', old_value: 'my note' }) }.to change { UserHistory.count }.by(1) + user_history = UserHistory.last + expect(user_history.action).to eq(UserHistory.actions[:post_staff_note_destroy]) + expect(user_history.new_value).to eq(nil) + expect(user_history.previous_value).to eq('my note') + end + end + end