From f2b022816495e479ef6c97a01d701a4e6d4cbe99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Tue, 19 Aug 2014 16:14:17 +0200 Subject: [PATCH] FIX: unhide post when a moderator undos the flag on which s/he took action --- app/controllers/post_actions_controller.rb | 1 - app/models/post_action.rb | 5 +- spec/models/post_action_spec.rb | 65 +++++++++++++--------- 3 files changed, 42 insertions(+), 29 deletions(-) diff --git a/app/controllers/post_actions_controller.rb b/app/controllers/post_actions_controller.rb index aa1d14b9e..68ab3f959 100644 --- a/app/controllers/post_actions_controller.rb +++ b/app/controllers/post_actions_controller.rb @@ -36,7 +36,6 @@ class PostActionsController < ApplicationController def destroy post_action = current_user.post_actions.find_by(post_id: params[:id].to_i, post_action_type_id: @post_action_type_id, deleted_at: nil) - raise Discourse::NotFound if post_action.blank? guardian.ensure_can_delete!(post_action) diff --git a/app/models/post_action.rb b/app/models/post_action.rb index 6e3b5b6c3..8e4e48452 100644 --- a/app/models/post_action.rb +++ b/app/models/post_action.rb @@ -249,9 +249,10 @@ class PostAction < ActiveRecord::Base def self.remove_act(user, post, post_action_type_id) finder = PostAction.where(post_id: post.id, user_id: user.id, post_action_type_id: post_action_type_id) - finder = finder.with_deleted if user.try(:staff?) + finder = finder.with_deleted.includes(:post) if user.try(:staff?) if action = finder.first action.remove_act!(user) + action.post.unhide! if action.staff_took_action end end @@ -407,7 +408,7 @@ class PostAction < ActiveRecord::Base end Post.where(id: post.id).update_all(["hidden = true, hidden_at = CURRENT_TIMESTAMP, hidden_reason_id = COALESCE(hidden_reason_id, ?)", reason]) - Topic.where(["id = :topic_id AND NOT EXISTS(SELECT 1 FROM POSTS WHERE topic_id = :topic_id AND NOT hidden)", topic_id: post.topic_id]).update_all(visible: false) + Topic.where("id = :topic_id AND NOT EXISTS(SELECT 1 FROM POSTS WHERE topic_id = :topic_id AND NOT hidden)", topic_id: post.topic_id).update_all(visible: false) # inform user if post.user diff --git a/spec/models/post_action_spec.rb b/spec/models/post_action_spec.rb index 6fadf3a43..ce95700f2 100644 --- a/spec/models/post_action_spec.rb +++ b/spec/models/post_action_spec.rb @@ -9,6 +9,7 @@ describe PostAction do let(:moderator) { Fabricate(:moderator) } let(:codinghorror) { Fabricate(:coding_horror) } + let(:eviltrout) { Fabricate(:evil_trout) } let(:admin) { Fabricate(:admin) } let(:post) { Fabricate(:post) } let(:second_post) { Fabricate(:post, topic_id: post.topic_id) } @@ -110,17 +111,19 @@ describe PostAction do it "should ignore validated flags" do post = create_post - admin = Fabricate(:admin) + PostAction.act(codinghorror, post, PostActionType.types[:off_topic]) post.hidden.should be_false post.hidden_at.should be_blank PostAction.defer_flags!(post, admin) PostAction.flagged_posts_count.should == 0 + post.reload post.hidden.should be_false post.hidden_at.should be_blank PostAction.hide_post!(post, PostActionType.types[:off_topic]) + post.reload post.hidden.should be_true post.hidden_at.should be_present @@ -133,6 +136,7 @@ describe PostAction do it "properly updates topic counters" do PostAction.act(moderator, post, PostActionType.types[:like]) PostAction.act(codinghorror, second_post, PostActionType.types[:like]) + post.topic.reload post.topic.like_count.should == 2 end @@ -237,11 +241,10 @@ describe PostAction do # A post with no flags has 0 for flag counts PostAction.flag_counts_for(post.id).should == [0, 0] - flag = PostAction.act(Fabricate(:evil_trout), post, PostActionType.types[:spam]) + flag = PostAction.act(eviltrout, post, PostActionType.types[:spam]) PostAction.flag_counts_for(post.id).should == [0, 1] # If staff takes action, it is ranked higher - admin = Fabricate(:admin) PostAction.act(admin, post, PostActionType.types[:spam], take_action: true) PostAction.flag_counts_for(post.id).should == [0, 8] @@ -251,45 +254,41 @@ describe PostAction do end end - it 'does not allow you to flag stuff with 2 reasons' do + it 'does not allow you to flag stuff with the same reason more than once' do post = Fabricate(:post) - u1 = Fabricate(:evil_trout) - PostAction.act(u1, post, PostActionType.types[:spam]) - lambda { PostAction.act(u1, post, PostActionType.types[:off_topic]) }.should raise_error(PostAction::AlreadyActed) + PostAction.act(eviltrout, post, PostActionType.types[:spam]) + lambda { PostAction.act(eviltrout, post, PostActionType.types[:off_topic]) }.should raise_error(PostAction::AlreadyActed) end it 'allows you to flag stuff with another reason' do post = Fabricate(:post) - u1 = Fabricate(:evil_trout) - PostAction.act(u1, post, PostActionType.types[:spam]) - PostAction.remove_act(u1, post, PostActionType.types[:spam]) - lambda { PostAction.act(u1, post, PostActionType.types[:off_topic]) }.should_not raise_error() + PostAction.act(eviltrout, post, PostActionType.types[:spam]) + PostAction.remove_act(eviltrout, post, PostActionType.types[:spam]) + lambda { PostAction.act(eviltrout, post, PostActionType.types[:off_topic]) }.should_not raise_error() end it 'should update counts when you clear flags' do post = Fabricate(:post) - u1 = Fabricate(:evil_trout) - PostAction.act(u1, post, PostActionType.types[:spam]) + PostAction.act(eviltrout, post, PostActionType.types[:spam]) post.reload post.spam_count.should == 1 PostAction.clear_flags!(post, Discourse.system_user) - post.reload + post.reload post.spam_count.should == 0 end it 'should follow the rules for automatic hiding workflow' do post = create_post - u1 = Fabricate(:evil_trout) - u2 = Fabricate(:walter_white) - admin = Fabricate(:admin) # we need an admin for the messages + walterwhite = Fabricate(:walter_white) SiteSetting.stubs(:flags_required_to_hide_post).returns(2) + Discourse.stubs(:site_contact_user).returns(admin) - PostAction.act(u1, post, PostActionType.types[:spam]) - PostAction.act(u2, post, PostActionType.types[:spam]) + PostAction.act(eviltrout, post, PostActionType.types[:spam]) + PostAction.act(walterwhite, post, PostActionType.types[:spam]) post.reload @@ -307,8 +306,8 @@ describe PostAction do post.hidden_at.should be_blank post.topic.visible.should be_true - PostAction.act(u1, post, PostActionType.types[:spam]) - PostAction.act(u2, post, PostActionType.types[:off_topic]) + PostAction.act(eviltrout, post, PostActionType.types[:spam]) + PostAction.act(walterwhite, post, PostActionType.types[:off_topic]) post.reload @@ -330,29 +329,43 @@ describe PostAction do it "can flag the topic instead of a post" do post1 = create_post post2 = create_post(topic: post1.topic) - post_action = PostAction.act(Fabricate(:user), post1, PostActionType.types[:spam], {flag_topic: true}) + post_action = PostAction.act(Fabricate(:user), post1, PostActionType.types[:spam], { flag_topic: true }) post_action.targets_topic.should == true end it "will flag the first post if you flag a topic but there is only one post in the topic" do post = create_post - post_action = PostAction.act(Fabricate(:user), post, PostActionType.types[:spam], {flag_topic: true}) + post_action = PostAction.act(Fabricate(:user), post, PostActionType.types[:spam], { flag_topic: true }) post_action.targets_topic.should == false post_action.post_id.should == post.id end + it "will unhide the post when a moderator undos the flag on which s/he took action" do + Discourse.stubs(:site_contact_user).returns(admin) + + post = create_post + PostAction.act(moderator, post, PostActionType.types[:spam], { take_action: true }) + + post.reload + post.hidden.should == true + + PostAction.remove_act(moderator, post, PostActionType.types[:spam]) + + post.reload + post.hidden.should == false + end + end it "prevents user to act twice at the same time" do post = Fabricate(:post) - user = Fabricate(:evil_trout) # flags are already being tested all_types_except_flags = PostActionType.types.except(PostActionType.flag_types) all_types_except_flags.values.each do |action| lambda do - PostAction.act(user, post, action) - PostAction.act(user, post, action) + PostAction.act(eviltrout, post, action) + PostAction.act(eviltrout, post, action) end.should raise_error(PostAction::AlreadyActed) end end