From 65ac80b014638e5d9634b314725517ecba0606b9 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Mon, 12 Mar 2018 13:49:52 -0400 Subject: [PATCH] FEATURE: Log Staff edits in Staff Action Logs Why? Some edits by staff are not tracked. For example, during the grace period, or via the flags/silence dialog. If a staff member is editing someone else's post, it now goes into the Staff Action Logs so it can be audited by other staff members. --- app/controllers/admin/users_controller.rb | 3 +- app/models/user_history.rb | 6 ++- app/services/staff_action_logger.rb | 9 +++++ config/locales/client.en.yml | 1 + lib/post_revisor.rb | 8 ++++ spec/components/post_revisor_spec.rb | 49 ++++++++++++++++++++++- 6 files changed, 72 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 298161a88d1..d8775cb1772 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -504,7 +504,8 @@ class Admin::UsersController < Admin::AdminController revisor.revise!( current_user, { raw: params[:post_edit] }, - skip_validations: true, skip_revision: true + skip_validations: true, + skip_revision: true ) end end diff --git a/app/models/user_history.rb b/app/models/user_history.rb index 5e809af46e8..608ed772503 100644 --- a/app/models/user_history.rb +++ b/app/models/user_history.rb @@ -67,7 +67,8 @@ class UserHistory < ActiveRecord::Base post_locked: 49, post_unlocked: 50, check_personal_message: 51, - disabled_second_factor: 52) + disabled_second_factor: 52, + post_edit: 53) end # Staff actions is a subset of all actions, used to audit actions taken by staff users. @@ -112,7 +113,8 @@ class UserHistory < ActiveRecord::Base :post_locked, :post_unlocked, :check_personal_message, - :disabled_second_factor] + :disabled_second_factor, + :post_edit] end def self.staff_action_ids diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index 6058c1b5842..57ee6c20be9 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -103,6 +103,15 @@ class StaffActionLogger ) end + def log_post_edit(post, opts = {}) + raise Discourse::InvalidParameters.new(:post) unless post && post.is_a?(Post) + UserHistory.create!(params(opts).merge( + action: UserHistory.actions[:post_edit], + post_id: post.id, + details: "#{post.raw}\n\n---\n\n#{opts[:new_raw]}" + )) + 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(action: UserHistory.actions[:change_site_setting], diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 0df11e24ce6..96294e99a61 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3302,6 +3302,7 @@ en: reviewed_post: "reviewed post" custom_staff: "plugin custom action" post_locked: "post locked" + post_edit: "post edit" post_unlocked: "post unlocked" check_personal_message: "check personal message" disabled_second_factor: "disable Two Factor Authentication" diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb index bed0f684fe6..48dd64071c8 100644 --- a/lib/post_revisor.rb +++ b/lib/post_revisor.rb @@ -177,6 +177,14 @@ class PostRevisor PostLocker.new(@post, @editor).lock end + # We log staff edits to posts + if @editor.staff? && @editor.id != @post.user.id && @fields.has_key?('raw') + StaffActionLogger.new(@editor).log_post_edit( + @post, + new_raw: @fields['raw'] + ) + end + # WARNING: do not pull this into the transaction # it can fire events in sidekiq before the post is done saving # leading to corrupt state diff --git a/spec/components/post_revisor_spec.rb b/spec/components/post_revisor_spec.rb index 4a001a9ddf0..0beddbe6c87 100644 --- a/spec/components/post_revisor_spec.rb +++ b/spec/components/post_revisor_spec.rb @@ -437,21 +437,68 @@ describe PostRevisor do end end + context "logging staff edits" do + let(:moderator) { Fabricate(:moderator) } + + it "doesn't log when a regular user revises a post" do + subject.revise!( + post.user, + raw: "lets totally update the body" + ) + log = UserHistory.where( + acting_user_id: post.user.id, + action: UserHistory.actions[:post_edit] + ) + expect(log).to be_blank + end + + it "logs an edit when a staff member revises a post" do + subject.revise!( + moderator, + raw: "lets totally update the body" + ) + log = UserHistory.where( + acting_user_id: moderator.id, + action: UserHistory.actions[:post_edit] + ) + expect(log).to be_present + end + + it "doesn't log an edit when a staff member edits their own post" do + revisor = PostRevisor.new( + Fabricate(:post, user: moderator) + ) + revisor.revise!( + moderator, + raw: "my own edit to my own thing" + ) + + log = UserHistory.where( + acting_user_id: moderator.id, + action: UserHistory.actions[:post_edit] + ) + expect(log).to be_blank + end + end + context "staff_edit_locks_post" do context "disabled" do + let(:moderator) { Fabricate(:moderator) } + before do SiteSetting.staff_edit_locks_post = false end it "does not lock the post when revised" do result = subject.revise!( - Fabricate(:moderator), + moderator, raw: "lets totally update the body" ) expect(result).to eq(true) post.reload expect(post).not_to be_locked + end end