FEATURE: log topic/post deletions from staff members

app/assets/javascripts/admin/models/staff_action_log.js

@@ -17,6 +17,8 @@ Discourse.StaffActionLog = Discourse.Model.extend({
     var formatted = "";
     formatted += this.format('email', 'email');
     formatted += this.format('admin.logs.ip_address', 'ip_address');
+    formatted += this.format('admin.logs.topic_id', 'topic_id');
+    formatted += this.format('admin.logs.post_id', 'post_id');
     if (!this.get('useCustomModalForDetails')) {
       formatted += this.format('admin.logs.staff_actions.new_value', 'new_value');
       formatted += this.format('admin.logs.staff_actions.previous_value', 'previous_value');
@@ -25,7 +27,7 @@ Discourse.StaffActionLog = Discourse.Model.extend({
       if (this.get('details')) formatted += Handlebars.Utils.escapeExpression(this.get('details')) + '<br/>';
     }
     return formatted;
-  }.property('ip_address', 'email'),
+  }.property('ip_address', 'email', 'topic_id', 'post_id'),
 
   format: function(label, propertyName) {
     if (this.get(propertyName)) {

app/assets/javascripts/discourse/models/_post.js

@@ -280,7 +280,10 @@ Discourse.Post = Discourse.Model.extend({
   **/
   destroy: function(deletedBy) {
     this.setDeletedState(deletedBy);
-    return Discourse.ajax("/posts/" + (this.get('id')), { type: 'DELETE' });
+    return Discourse.ajax("/posts/" + this.get('id'), {
+      data: { context: window.location.pathname },
+      type: 'DELETE'
+    });
   },
 
   /**

app/assets/javascripts/discourse/models/topic.js

@@ -215,7 +215,10 @@ Discourse.Topic = Discourse.Model.extend({
       'details.can_delete': false,
       'details.can_recover': true
     });
-    return Discourse.ajax("/t/" + this.get('id'), { type: 'DELETE' });
+    return Discourse.ajax("/t/" + this.get('id'), {
+      data: { context: window.location.pathname },
+      type: 'DELETE'
+    });
   },
 
   // Recover this topic if deleted

app/controllers/posts_controller.rb

@@ -167,7 +167,7 @@ class PostsController < ApplicationController
 
     guardian.ensure_can_delete!(post)
 
-    destroyer = PostDestroyer.new(current_user, post)
+    destroyer = PostDestroyer.new(current_user, post, { context: params[:context] })
     destroyer.destroy
 
     render nothing: true

app/controllers/topics_controller.rb

@@ -211,7 +211,7 @@ class TopicsController < ApplicationController
     guardian.ensure_can_delete!(topic)
 
     first_post = topic.ordered_posts.first
-    PostDestroyer.new(current_user, first_post).destroy
+    PostDestroyer.new(current_user, first_post, { context: params[:context] }).destroy
 
     render nothing: true
   end

app/models/user_history.rb

@@ -5,6 +5,9 @@ class UserHistory < ActiveRecord::Base
   belongs_to :acting_user, class_name: 'User'
   belongs_to :target_user, class_name: 'User'
 
+  belongs_to :post
+  belongs_to :topic
+
   validates_presence_of :action
 
   scope :only_staff_actions, ->{ where("action IN (?)", UserHistory.staff_action_ids) }
@@ -12,22 +15,24 @@ class UserHistory < ActiveRecord::Base
   before_save :set_admin_only
 
   def self.actions
-    @actions ||= Enum.new( :delete_user,
-                           :change_trust_level,
-                           :change_site_setting,
-                           :change_site_customization,
-                           :delete_site_customization,
-                           :checked_for_custom_avatar,
-                           :notified_about_avatar,
-                           :notified_about_sequential_replies,
-                           :notified_about_dominating_topic,
-                           :suspend_user,
-                           :unsuspend_user,
-                           :facebook_no_email,
-                           :grant_badge,
-                           :revoke_badge,
-                           :auto_trust_level_change,
-                           :check_email)
+    @actions ||= Enum.new(:delete_user,
+                          :change_trust_level,
+                          :change_site_setting,
+                          :change_site_customization,
+                          :delete_site_customization,
+                          :checked_for_custom_avatar,
+                          :notified_about_avatar,
+                          :notified_about_sequential_replies,
+                          :notified_about_dominating_topic,
+                          :suspend_user,
+                          :unsuspend_user,
+                          :facebook_no_email,
+                          :grant_badge,
+                          :revoke_badge,
+                          :auto_trust_level_change,
+                          :check_email,
+                          :delete_post,
+                          :delete_topic)
   end
 
   # Staff actions is a subset of all actions, used to audit actions taken by staff users.
@@ -41,7 +46,9 @@ class UserHistory < ActiveRecord::Base
                         :unsuspend_user,
                         :grant_badge,
                         :revoke_badge,
-                        :check_email]
+                        :check_email,
+                        :delete_post,
+                        :delete_topic]
   end
 
   def self.staff_action_ids

app/serializers/user_history_serializer.rb

@@ -7,7 +7,9 @@ class UserHistorySerializer < ApplicationSerializer
              :created_at,
              :subject,
              :previous_value,
-             :new_value
+             :new_value,
+             :topic_id,
+             :post_id
 
   has_one :acting_user, serializer: BasicUserSerializer, embed: :objects
   has_one :target_user, serializer: BasicUserSerializer, embed: :objects
@@ -31,4 +33,4 @@ class UserHistorySerializer < ApplicationSerializer
       nil
     end
   end
-end
+end

app/services/staff_action_logger.rb

@@ -2,32 +2,74 @@
 class StaffActionLogger
   def initialize(admin)
     @admin = admin
-    raise Discourse::InvalidParameters.new('admin is nil') unless @admin and @admin.is_a?(User)
+    raise Discourse::InvalidParameters.new('admin is nil') unless @admin && @admin.is_a?(User)
   end
 
   def log_user_deletion(deleted_user, opts={})
-    raise Discourse::InvalidParameters.new('user is nil') unless deleted_user and deleted_user.is_a?(User)
+    raise Discourse::InvalidParameters.new('user is nil') unless deleted_user && deleted_user.is_a?(User)
     UserHistory.create( params(opts).merge({
       action: UserHistory.actions[:delete_user],
       email: deleted_user.email,
       ip_address: deleted_user.ip_address.to_s,
-      details: [:id, :username, :name, :created_at, :trust_level, :last_seen_at, :last_emailed_at].map { |x| "#{x}: #{deleted_user.send(x)}" }.join(', ')
+      details: [:id, :username, :name, :created_at, :trust_level, :last_seen_at, :last_emailed_at].map { |x| "#{x}: #{deleted_user.send(x)}" }.join("\n")
+    }))
+  end
+
+  def log_post_deletion(deleted_post, opts={})
+    raise Discourse::InvalidParameters.new("post is nil") unless deleted_post && deleted_post.is_a?(Post)
+
+    topic = deleted_post.topic || Topic.with_deleted.find(deleted_post.topic_id)
+
+    details = [
+      "id: #{deleted_post.id}",
+      "created_at: #{deleted_post.created_at}",
+      "user: #{deleted_post.user.username} (#{deleted_post.user.name})",
+      "topic: #{topic.title}",
+      "post_number: #{deleted_post.post_number}",
+      "raw: #{deleted_post.raw}"
+    ]
+
+    UserHistory.create(params(opts).merge({
+      action: UserHistory.actions[:delete_post],
+      post_id: deleted_post.id,
+      details: details.join("\n")
+    }))
+  end
+
+  def log_topic_deletion(deleted_topic, opts={})
+    raise Discourse::InvalidParameters.new("topic is nil") unless deleted_topic && deleted_topic.is_a?(Topic)
+
+    details = [
+      "id: #{deleted_topic.id}",
+      "created_at: #{deleted_topic.created_at}",
+      "user: #{deleted_topic.user.username} (#{deleted_topic.user.name})",
+      "title: #{deleted_topic.title}"
+    ]
+
+    if first_post = deleted_topic.ordered_posts.first
+      details << "raw: #{first_post.raw}"
+    end
+
+    UserHistory.create(params(opts).merge({
+      action: UserHistory.actions[:delete_topic],
+      topic_id: deleted_topic.id,
+      details: details.join("\n")
     }))
   end
 
   def log_trust_level_change(user, old_trust_level, new_trust_level, opts={})
-    raise Discourse::InvalidParameters.new('user is nil') unless user and user.is_a?(User)
+    raise Discourse::InvalidParameters.new('user is nil') unless user && user.is_a?(User)
     raise Discourse::InvalidParameters.new('old trust level is invalid') unless TrustLevel.valid? old_trust_level
     raise Discourse::InvalidParameters.new('new trust level is invalid') unless TrustLevel.valid? new_trust_level
     UserHistory.create!( params(opts).merge({
       action: UserHistory.actions[:change_trust_level],
       target_user_id: user.id,
-      details: "old trust level: #{old_trust_level}, new trust level: #{new_trust_level}"
+      details: "old trust level: #{old_trust_level}\nnew trust level: #{new_trust_level}"
     }))
   end
 
   def log_site_setting_change(setting_name, previous_value, new_value, opts={})
-    raise Discourse::InvalidParameters.new('setting_name is invalid') unless setting_name.present? and SiteSetting.respond_to?(setting_name)
+    raise Discourse::InvalidParameters.new('setting_name is invalid') unless setting_name.present? && SiteSetting.respond_to?(setting_name)
     UserHistory.create( params(opts).merge({
       action: UserHistory.actions[:change_site_setting],
       subject: setting_name,

config/locales/client.en.yml

@@ -1768,6 +1768,8 @@ en:
         last_match_at: "Last Matched"
         match_count: "Matches"
         ip_address: "IP"
+        topic_id: "Topic ID"
+        post_id: "Post ID"
         delete: 'Delete'
         edit: 'Edit'
         save: 'Save'
@@ -1802,6 +1804,8 @@ en:
             grant_badge: "grant badge"
             revoke_badge: "revoke badge"
             check_email: "check email"
+            delete_topic: "delete topic"
+            delete_post: "delete post"
         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."

db/migrate/20141001101041_add_post_id_to_user_histories.rb

@@ -0,0 +1,5 @@
+class AddPostIdToUserHistories < ActiveRecord::Migration
+  def change
+    add_column :user_histories, :post_id, :integer
+  end
+end

lib/post_destroyer.rb

@@ -33,10 +33,11 @@ class PostDestroyer
     end
   end
 
-  def initialize(user, post)
+  def initialize(user, post, opts={})
     @user = user
     @post = post
     @topic = post.topic if post
+    @opts = opts
   end
 
   def destroy
@@ -79,7 +80,12 @@ class PostDestroyer
       @post.update_flagged_posts_count
       remove_associated_replies
       remove_associated_notifications
-      @post.topic.trash!(@user) if @post.topic && @post.post_number == 1
+      if @post.topic && @post.post_number == 1
+        StaffActionLogger.new(@user).log_topic_deletion(@post.topic, @opts.slice(:context))
+        @post.topic.trash!(@user)
+      else
+        StaffActionLogger.new(@user).log_post_deletion(@post, @opts.slice(:context))
+      end
       update_associated_category_latest_topic
       update_user_counts
     end

spec/components/post_destroyer_spec.rb

@@ -170,6 +170,12 @@ describe PostDestroyer do
           author.reload
         }.to change { author.post_count }.by(-1)
       end
+
+      it "creates a new user history entry" do
+        expect {
+          PostDestroyer.new(moderator, post).destroy
+        }.to change { UserHistory.count}.by(1)
+      end
     end
 
     context "as an admin" do
@@ -258,6 +264,12 @@ describe PostDestroyer do
         post.deleted_at.should be_present
         post.deleted_by.should == admin
       end
+
+      it "creates a new user history entry" do
+        expect {
+          PostDestroyer.new(admin, post).destroy
+        }.to change { UserHistory.count}.by(1)
+      end
     end
   end
 

spec/controllers/posts_controller_spec.rb

@@ -143,7 +143,7 @@ describe PostsController do
 
       it "uses a PostDestroyer" do
         destroyer = mock
-        PostDestroyer.expects(:new).with(user, post).returns(destroyer)
+        PostDestroyer.expects(:new).returns(destroyer)
         destroyer.expects(:destroy)
         xhr :delete, :destroy, id: post.id
       end

spec/services/staff_action_logger_spec.rb

@@ -33,6 +33,42 @@ describe StaffActionLogger do
     end
   end
 
+  describe 'log_post_deletion' do
+    let(:deleted_post) { Fabricate(:post) }
+
+    subject(:log_post_deletion) { described_class.new(admin).log_post_deletion(deleted_post) }
+
+    it 'raises an error when post is nil' do
+      expect { logger.log_post_deletion(nil) }.to raise_error(Discourse::InvalidParameters)
+    end
+
+    it 'raises an error when post is not a Post' do
+      expect { logger.log_post_deletion(1) }.to raise_error(Discourse::InvalidParameters)
+    end
+
+    it 'creates a new UserHistory record' do
+      expect { log_post_deletion }.to change { UserHistory.count }.by(1)
+    end
+  end
+
+  describe 'log_topic_deletion' do
+    let(:deleted_topic) { Fabricate(:topic) }
+
+    subject(:log_topic_deletion) { described_class.new(admin).log_topic_deletion(deleted_topic) }
+
+    it 'raises an error when topic is nil' do
+      expect { logger.log_topic_deletion(nil) }.to raise_error(Discourse::InvalidParameters)
+    end
+
+    it 'raises an error when topic is not a Topic' do
+      expect { logger.log_topic_deletion(1) }.to raise_error(Discourse::InvalidParameters)
+    end
+
+    it 'creates a new UserHistory record' do
+      expect { log_topic_deletion }.to change { UserHistory.count }.by(1)
+    end
+  end
+
   describe 'log_trust_level_change' do
     let(:user) { Fabricate(:user) }
     let(:old_trust_level) { TrustLevel[0] }