From c1a9e32b48e64c2d6adf7d2b1c9543d8a79f4f93 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Fri, 18 Sep 2015 12:48:43 -0400 Subject: [PATCH] FIX: When recovering a post, it should recreate user actions --- app/models/user_action_observer.rb | 4 +-- lib/post_destroyer.rb | 6 ++++ spec/components/post_destroyer_spec.rb | 44 ++++++++++++++++++-------- 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/app/models/user_action_observer.rb b/app/models/user_action_observer.rb index 34e6fcabedc..a98ab14320a 100644 --- a/app/models/user_action_observer.rb +++ b/app/models/user_action_observer.rb @@ -8,7 +8,7 @@ class UserActionObserver < ActiveRecord::Observer when (model.is_a?(Topic)) log_topic(model) when (model.is_a?(Post)) - log_post(model) + UserActionObserver.log_post(model) end end @@ -43,7 +43,7 @@ class UserActionObserver < ActiveRecord::Observer end end - def log_post(model) + def self.log_post(model) # first post gets nada return if model.is_first_post? diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb index 818f3766356..394b6ccc43f 100644 --- a/lib/post_destroyer.rb +++ b/lib/post_destroyer.rb @@ -58,6 +58,7 @@ class PostDestroyer topic = Topic.with_deleted.find @post.topic_id topic.recover! if @post.is_first_post? topic.update_statistics + recover_user_actions DiscourseEvent.trigger(:post_recovered, @post, @opts, @user) end @@ -166,6 +167,11 @@ class PostDestroyer end end + def recover_user_actions + # TODO: Use a trash concept for `user_actions` to avoid churn and simplify this? + UserActionObserver.log_post(@post) + end + def remove_associated_replies post_ids = PostReply.where(reply_id: @post.id).pluck(:post_id) diff --git a/spec/components/post_destroyer_spec.rb b/spec/components/post_destroyer_spec.rb index 53e8015cb46..a5a91c4c753 100644 --- a/spec/components/post_destroyer_spec.rb +++ b/spec/components/post_destroyer_spec.rb @@ -142,6 +142,28 @@ describe PostDestroyer do end end + describe "recovery and user actions" do + it "recreates user actions" do + reply = create_post(topic: post.topic) + author = reply.user + + post_action = author.user_actions.where(action_type: UserAction::REPLY, target_post_id: reply.id).first + expect(post_action).to be_present + + PostDestroyer.new(moderator, reply).destroy + + # User Action is removed + post_action = author.user_actions.where(action_type: UserAction::REPLY, target_post_id: reply.id).first + expect(post_action).to be_blank + + PostDestroyer.new(moderator, reply).recover + + # On recovery, the user action is recreated + post_action = author.user_actions.where(action_type: UserAction::REPLY, target_post_id: reply.id).first + expect(post_action).to be_present + end + end + describe 'basic destroying' do it "as the creator of the post, doesn't delete the post" do @@ -170,23 +192,19 @@ describe PostDestroyer do context "as a moderator" do it "deletes the post" do + author = post.user + + post_count = author.post_count + history_count = UserHistory.count + PostDestroyer.new(moderator, post).destroy + expect(post.deleted_at).to be_present expect(post.deleted_by).to eq(moderator) - end - it "updates the user's post_count" do - author = post.user - expect { - PostDestroyer.new(moderator, post).destroy - 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) + author.reload + expect(author.post_count).to eq(post_count - 1) + expect(UserHistory.count).to eq(history_count + 1) end end