Add deleted_by to Trashable tables

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

@@ -77,9 +77,7 @@ Discourse.FlaggedPost = Discourse.Post.extend({
     return Discourse.ajax('/admin/flags/agree/' + this.id, { type: 'POST', cache: false });
   },
 
-  postHidden: function() {
+  postHidden: Em.computed.alias('hidden'),
-    return (this.get('hidden'));
-  }.property(),
 
   extraClasses: function() {
     var classes = [];
@@ -92,9 +90,8 @@ Discourse.FlaggedPost = Discourse.Post.extend({
     return classes.join(' ');
   }.property(),
 
-  deleted: function() {
+  deleted: Em.computed.or('deleted_at', 'topic_deleted_at')
-    return (this.get('deleted_at') || this.get('topic_deleted_at'));
+
-  }.property()
 });
 
 Discourse.FlaggedPost.reopenClass({

app/controllers/invites_controller.rb

@@ -32,7 +32,7 @@ class InvitesController < ApplicationController
 
     invite = Invite.where(invited_by_id: current_user.id, email: params[:email]).first
     raise Discourse::InvalidParameters.new(:email) if invite.blank?
-    invite.trash!
+    invite.trash!(current_user)
 
     render nothing: true
   end

app/controllers/topics_controller.rb

@@ -169,7 +169,7 @@ class TopicsController < ApplicationController
   def destroy
     topic = Topic.where(id: params[:id]).first
     guardian.ensure_can_delete!(topic)
-    topic.trash!
+    topic.trash!(current_user)
     render nothing: true
   end
 

app/models/post.rb

@@ -54,9 +54,9 @@ class Post < ActiveRecord::Base
     @types ||= Enum.new(:regular, :moderator_action)
   end
 
-  def trash!
+  def trash!(trashed_by=nil)
     self.topic_links.each(&:destroy)
-    super
+    super(trashed_by)
   end
 
   def recover!

app/models/post_action.rb

@@ -63,7 +63,7 @@ class PostAction < ActiveRecord::Base
       moderator_id == -1 ? PostActionType.auto_action_flag_types.values : PostActionType.flag_types.values
     end
 
-    PostAction.where({ post_id: post.id, post_action_type_id: actions }).update_all({ deleted_at: Time.zone.now, deleted_by: moderator_id })
+    PostAction.where({ post_id: post.id, post_action_type_id: actions }).update_all({ deleted_at: Time.zone.now, deleted_by_id: moderator_id })
     f = actions.map{|t| ["#{PostActionType.types[t]}_count", 0]}
     Post.where(id: post.id).with_deleted.update_all(Hash[*f.flatten])
     update_flagged_posts_count
@@ -140,13 +140,13 @@ class PostAction < ActiveRecord::Base
                       user_id: user.id,
                       post_action_type_id:
                       post_action_type_id).first
-      action.trash!
+      action.trash!(user)
       action.run_callbacks(:save)
     end
   end
 
   def remove_act!(user)
-    trash!
+    trash!(user)
     run_callbacks(:save)
   end
 
@@ -390,7 +390,7 @@ end
 #  deleted_at          :datetime
 #  created_at          :datetime         not null
 #  updated_at          :datetime         not null
-#  deleted_by          :integer
+#  deleted_by_id       :integer
 #  message             :text
 #  related_post_id     :integer
 #  staff_took_action   :boolean          default(FALSE), not null

app/models/topic.rb

@@ -21,9 +21,9 @@ class Topic < ActiveRecord::Base
 
   versioned if: :new_version_required?
 
-  def trash!
+  def trash!(trashed_by=nil)
     update_category_topic_count_by(-1) if deleted_at.nil?
-    super
+    super(trashed_by)
     update_flagged_posts_count
   end
 

db/migrate/20130709184941_add_deleted_by_id_to_posts.rb

@@ -0,0 +1,8 @@
+class AddDeletedByIdToPosts < ActiveRecord::Migration
+  def change
+    add_column :posts, :deleted_by_id, :integer, null: true
+    add_column :topics, :deleted_by_id, :integer, null: true
+    add_column :invites, :deleted_by_id, :integer, null: true
+    rename_column :post_actions, :deleted_by, :deleted_by_id
+  end
+end

lib/post_destroyer.rb

@@ -37,12 +37,14 @@ class PostDestroyer
       # Feature users in the topic
       Jobs.enqueue(:feature_topic_users, topic_id: @post.topic_id, except_post_id: @post.id)
 
-      @post.post_actions.map(&:trash!)
+      @post.post_actions.each do |pa|
+        pa.trash!(@user)
+      end
 
       f = PostActionType.types.map{|k,v| ["#{k}_count", 0]}
       Post.with_deleted.where(id: @post.id).update_all(Hash[*f.flatten])
 
-      @post.trash!
+      @post.trash!(@user)
 
       Topic.reset_highest(@post.topic_id)
 
@@ -59,7 +61,7 @@ class PostDestroyer
       # Remove any notifications that point to this deleted post
       Notification.delete_all topic_id: @post.topic_id, post_number: @post.post_number
 
-      @post.topic.trash! if @post.post_number == 1
+      @post.topic.trash!(@user) if @post.post_number == 1
     end
   end
 

lib/trashable.rb

@@ -5,6 +5,8 @@ module Trashable
     default_scope where(with_deleted_scope_sql)
 
     # scope unscoped does not work
+
+    belongs_to :deleted_by, class_name: 'User'
   end
 
 
@@ -24,23 +26,31 @@ module Trashable
     end
   end
 
-  def trash!
+  def trash!(trashed_by=nil)
     # note, an argument could be made that the column should probably called trashed_at
     # however, deleted_at is the terminology used in the UI
     #
     # we could hijack use a delete! and delete - redirecting the originals elsewhere, but that is
     # confusing as well. So for now, we go with trash!
     #
-    update_column(:deleted_at, DateTime.now)
+    trash_update(DateTime.now, trashed_by.try(:id))
   end
 
   def recover!
-    # see: https://github.com/rails/rails/issues/8436
+    trash_update(nil, nil)
-    #
-    # Fixed in Rails 4
-    #
-    self.class.unscoped.where(id: self.id).update_all({deleted_at: nil})
-    raw_write_attribute :deleted_at, nil
   end
 
+
+  private
+
+    def trash_update(deleted_at, deleted_by_id)
+      # see: https://github.com/rails/rails/issues/8436
+      #
+      # Fixed in Rails 4
+      #
+      self.class.unscoped.where(id: self.id).update_all(deleted_at: deleted_at, deleted_by_id: deleted_by_id)
+      raw_write_attribute :deleted_at, deleted_at
+      raw_write_attribute :deleted_by_id, deleted_by_id
+    end
+
 end

spec/components/guardian_spec.rb

@@ -229,20 +229,22 @@ describe Guardian do
     end
 
     describe 'a Post' do
+
+      let(:another_admin) { Fabricate(:admin) }
       it 'correctly handles post visibility' do
         post = Fabricate(:post)
         topic = post.topic
 
         Guardian.new(user).can_see?(post).should be_true
 
-        post.trash!
+        post.trash!(another_admin)
         post.reload
         Guardian.new(user).can_see?(post).should be_false
         Guardian.new(admin).can_see?(post).should be_true
 
         post.recover!
         post.reload
-        topic.trash!
+        topic.trash!(another_admin)
         topic.reload
         Guardian.new(user).can_see?(post).should be_false
         Guardian.new(admin).can_see?(post).should be_true

spec/components/post_destroyer_spec.rb

@@ -23,6 +23,7 @@ describe PostDestroyer do
 
       it "doesn't delete the post" do
         post.deleted_at.should be_blank
+        post.deleted_by.should be_blank
         post.raw.should == I18n.t('js.post.deleted_by_author')
         post.version.should == 2
       end
@@ -35,6 +36,7 @@ describe PostDestroyer do
 
       it "deletes the post" do
         post.deleted_at.should be_present
+        post.deleted_by.should == moderator
       end
     end
 
@@ -45,6 +47,7 @@ describe PostDestroyer do
 
       it "deletes the post" do
         post.deleted_at.should be_present
+        post.deleted_by.should == admin
       end
     end
 

spec/controllers/invites_controller_spec.rb

@@ -29,7 +29,7 @@ describe InvitesController do
       end
 
       it "destroys the invite" do
-        Invite.any_instance.expects(:trash!)
+        Invite.any_instance.expects(:trash!).with(user)
         delete :destroy, email: invite.email
       end
 

spec/controllers/post_actions_controller_spec.rb

@@ -137,7 +137,7 @@ describe PostActionsController do
         end
 
         it "works with a deleted post" do
-          flagged_post.trash!
+          flagged_post.trash!(user)
           xhr :post, :clear_flags, id: flagged_post.id, post_action_type_id: PostActionType.types[:spam]
           response.should be_success
         end

spec/controllers/posts_controller_spec.rb

@@ -14,7 +14,8 @@ describe PostsController do
 
   describe 'show' do
 
-    let(:post) { Fabricate(:post, user: log_in) }
+    let(:user) { log_in }
+    let(:post) { Fabricate(:post, user: user) }
 
     it 'ensures the user can see the post' do
       Guardian.any_instance.expects(:can_see?).with(post).returns(false)
@@ -30,7 +31,7 @@ describe PostsController do
     context "deleted post" do
 
       before do
-        post.trash!
+        post.trash!(user)
       end
 
       it "can't find deleted posts as an anonymous user" do

spec/integration/spam_rules_spec.rb

@@ -58,7 +58,7 @@ describe PostAction do
         end
 
         context "a post is deleted" do
-          When { spam_post.trash!; spammer.reload }
+          When { spam_post.trash!(moderator); spammer.reload }
           Then { expect(spammer.reload).to be_blocked }
         end
 

spec/models/notification_spec.rb

@@ -215,7 +215,7 @@ describe Notification do
 
       Notification.create!(read: false, user_id: p2.user_id, topic_id: p2.topic_id, post_number: p2.post_number, data: '[]',
                            notification_type: Notification.types[:liked])
-      p2.trash!
+      p2.trash!(p.user)
 
       # we may want to make notification "trashable" but for now we nuke pm notifications from deleted topics/posts
       Notification.ensure_consistency!

spec/models/topic_spec.rb

@@ -1296,16 +1296,17 @@ describe Topic do
 
   describe 'trash!' do
     context "its category's topic count" do
+      let(:moderator) { Fabricate(:moderator) }
       let(:category) { Fabricate(:category) }
 
       it "subtracts 1 if topic is being deleted" do
         topic = Fabricate(:topic, category: category)
-        expect { topic.trash! }.to change { category.reload.topic_count }.by(-1)
+        expect { topic.trash!(moderator) }.to change { category.reload.topic_count }.by(-1)
       end
 
       it "doesn't subtract 1 if topic is already deleted" do
         topic = Fabricate(:topic, category: category, deleted_at: 1.day.ago)
-        expect { topic.trash! }.to_not change { category.reload.topic_count }
+        expect { topic.trash!(moderator) }.to_not change { category.reload.topic_count }
       end
     end
   end

spec/models/user_action_spec.rb

@@ -62,7 +62,7 @@ describe UserAction do
 
       other_stats.should == expecting
 
-      public_topic.trash!
+      public_topic.trash!(user)
       stats_for_user.should == []
       stream_count.should == 0