From 902c5d11cf195b70c060bffeec3e134717130fce Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Wed, 28 Feb 2018 11:22:51 +0800
Subject: [PATCH] FIX: Don't allow other flag actions after `notify_moderator`
 has happened.

https://meta.discourse.org/t/receiving-sorry-an-error-has-occurred-during-flagging-step-of-discobot-tutorial/77233/5
---
 app/models/post_action.rb        |  4 ++--
 lib/guardian/post_guardian.rb    | 19 +++++++++----------
 spec/components/guardian_spec.rb | 10 ++++++----
 spec/models/post_action_spec.rb  | 26 ++++++++++++++++++++++++++
 4 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/app/models/post_action.rb b/app/models/post_action.rb
index 75b0b13c109..471cebe9aba 100644
--- a/app/models/post_action.rb
+++ b/app/models/post_action.rb
@@ -364,7 +364,7 @@ SQL
   end
 
   def is_flag?
-    !!PostActionType.flag_types[post_action_type_id]
+    !!PostActionType.notify_flag_types[post_action_type_id]
   end
 
   def is_private_message?
@@ -396,7 +396,7 @@ SQL
   end
 
   before_create do
-    post_action_type_ids = is_flag? ? PostActionType.flag_types_without_custom.values : post_action_type_id
+    post_action_type_ids = is_flag? ? PostActionType.notify_flag_types.values : post_action_type_id
     raise AlreadyActed if PostAction.where(user_id: user_id)
         .where(post_id: post_id)
         .where(post_action_type_id: post_action_type_ids)
diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb
index c0e110a1bdb..a364423fb96 100644
--- a/lib/guardian/post_guardian.rb
+++ b/lib/guardian/post_guardian.rb
@@ -15,19 +15,22 @@ module PostGuardian
     return false if (action_key == :notify_user && !is_staff? && opts[:is_warning].present? && opts[:is_warning] == 'true')
 
     taken = opts[:taken_actions].try(:keys).to_a
-    is_flag = PostActionType.flag_types_without_custom[action_key]
+    is_flag = PostActionType.notify_flag_types[action_key]
     already_taken_this_action = taken.any? && taken.include?(PostActionType.types[action_key])
-    already_did_flagging      = taken.any? && (taken & PostActionType.flag_types_without_custom.values).any?
+    already_did_flagging      = taken.any? && (taken & PostActionType.notify_flag_types.values).any?
 
     result = if authenticated? && post && !@user.anonymous?
-
       # post made by staff, but we don't allow staff flags
       return false if is_flag &&
         (!SiteSetting.allow_flagging_staff?) &&
         post.user.staff?
 
-      return false if [:notify_user, :notify_moderators].include?(action_key) &&
-        !SiteSetting.enable_personal_messages?
+      if [:notify_user, :notify_moderators].include?(action_key) &&
+         (!SiteSetting.enable_personal_messages? ||
+         !@user.has_trust_level?(SiteSetting.min_trust_to_send_messages))
+
+        return false
+      end
 
       # we allow flagging for trust level 1 and higher
       # always allowed for private messages
@@ -37,7 +40,7 @@ module PostGuardian
       not(is_flag || already_taken_this_action) &&
 
       # nothing except flagging on archived topics
-      not(post.topic.try(:archived?)) &&
+      not(post.topic&.archived?) &&
 
       # nothing except flagging on deleted posts
       not(post.trashed?) &&
@@ -45,10 +48,6 @@ module PostGuardian
       # don't like your own stuff
       not(action_key == :like && is_my_own?(post)) &&
 
-      # new users can't notify_user or notify_moderators because they are not allowed to send private messages
-      not((action_key == :notify_user || action_key == :notify_moderators) &&
-        !@user.has_trust_level?(SiteSetting.min_trust_to_send_messages)) &&
-
       # no voting more than once on single vote topics
       not(action_key == :vote && opts[:voted_in_topic] && post.topic.has_meta_data_boolean?(:single_vote))
     end
diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb
index c0775b839b8..e6e05828f61 100644
--- a/spec/components/guardian_spec.rb
+++ b/spec/components/guardian_spec.rb
@@ -44,7 +44,7 @@ describe Guardian do
     end
   end
 
-  describe 'post_can_act?' do
+  describe '#post_can_act?' do
     let(:post) { build(:post) }
     let(:user) { build(:user) }
 
@@ -100,9 +100,11 @@ describe Guardian do
     end
 
     it "returns false when you already flagged a post" do
-      expect(Guardian.new(user).post_can_act?(post, :off_topic, opts: {
-        taken_actions: { PostActionType.types[:spam] => 1 }
-      })).to be_falsey
+      PostActionType.notify_flag_types.each do |type, _id|
+        expect(Guardian.new(user).post_can_act?(post, :off_topic, opts: {
+          taken_actions: { PostActionType.types[type] => 1 }
+        })).to be_falsey
+      end
     end
 
     it "returns false for notify_user if private messages are disabled" do
diff --git a/spec/models/post_action_spec.rb b/spec/models/post_action_spec.rb
index f17d1f79865..02b205276f7 100644
--- a/spec/models/post_action_spec.rb
+++ b/spec/models/post_action_spec.rb
@@ -659,4 +659,30 @@ describe PostAction do
 
   end
 
+  describe '#is_flag?' do
+    describe 'when post action is a flag' do
+      it 'should return true' do
+        PostActionType.notify_flag_types.each do |_type, id|
+          post_action = PostAction.new(
+            user: codinghorror,
+            post_action_type_id: id
+          )
+
+          expect(post_action.is_flag?).to eq(true)
+        end
+      end
+    end
+
+    describe 'when post action is not a flag' do
+      it 'should return false' do
+        post_action = PostAction.new(
+          user: codinghorror,
+          post_action_type_id: 99
+        )
+
+        expect(post_action.is_flag?).to eq(false)
+      end
+    end
+  end
+
 end