FIX: blue notification instead of green for replies/mentions in PMs

app/services/post_alerter.rb

@@ -47,12 +47,12 @@ class PostAlerter
       end
 
       expand_group_mentions(mentioned_groups, post) do |group, users|
-        notify_users(users - notified, :group_mentioned, post, mentioned_opts.merge({group: group}))
+        notify_non_pm_users(users - notified, :group_mentioned, post, mentioned_opts.merge({group: group}))
         notified += users
       end
 
       if mentioned_users
-        notify_users(mentioned_users - notified, :mentioned, post, mentioned_opts)
+        notify_non_pm_users(mentioned_users - notified, :mentioned, post, mentioned_opts)
         notified += mentioned_users
       end
     end
@@ -61,41 +61,42 @@ class PostAlerter
     reply_to_user = post.reply_notification_target
 
     if new_record && reply_to_user && !notified.include?(reply_to_user) && post.post_type == Post.types[:regular]
-      notify_users(reply_to_user, :replied, post)
+      notify_non_pm_users(reply_to_user, :replied, post)
       notified += [reply_to_user]
     end
 
     # quotes
     quoted_users = extract_quoted_users(post)
-    notify_users(quoted_users - notified, :quoted, post)
+    notify_non_pm_users(quoted_users - notified, :quoted, post)
     notified += quoted_users
 
     # linked
     linked_users = extract_linked_users(post)
-    notify_users(linked_users - notified, :linked, post)
+    notify_non_pm_users(linked_users - notified, :linked, post)
     notified += linked_users
 
     # private messages
     if new_record
       if post.topic.private_message?
-        # users that aren't part of any mentionned groups
+        # users that aren't part of any mentioned groups
         directly_targeted_users(post).each do |user|
-          if !notified.include?(user)
+          notification_level = TopicUser.get(post.topic, user).try(:notification_level)
+          if notified.include?(user) || notification_level == TopicUser.notification_levels[:watching]
             create_notification(user, Notification.types[:private_message], post)
-            notified += [user]
           end
         end
         # users that are part of all mentionned groups
         indirectly_targeted_users(post).each do |user|
-          if !notified.include?(user)
-            # only create a notification when watching the group
-            notification_level = TopicUser.get(post.topic, user).try(:notification_level)
-            if notification_level == TopicUser.notification_levels[:watching]
+          # only create a notification when watching the group
+          notification_level = TopicUser.get(post.topic, user).try(:notification_level)
+
+          if notification_level == TopicUser.notification_levels[:watching]
+            create_notification(user, Notification.types[:private_message], post)
+          elsif notification_level == TopicUser.notification_levels[:tracking]
+            if notified.include?(user)
               create_notification(user, Notification.types[:private_message], post)
-              notified += [user]
-            elsif notification_level == TopicUser.notification_levels[:tracking]
+            else
               notify_group_summary(user, post)
-              notified += [user]
             end
           end
         end
@@ -400,13 +401,11 @@ class PostAlerter
   end
 
   # Notify a bunch of users
-  def notify_users(users, type, post, opts=nil)
-    users = [users] unless users.is_a?(Array)
+  def notify_non_pm_users(users, type, post, opts=nil)
 
-    if post.topic.try(:private_message?)
-      whitelist = all_allowed_users(post)
-      users.reject! { |u| !whitelist.include?(u) }
-    end
+    return if post.topic.try(:private_message?)
+
+    users = [users] unless users.is_a?(Array)
 
     users.each do |u|
       create_notification(u, Notification.types[type], post, opts)

spec/services/post_alerter_spec.rb

@@ -10,6 +10,43 @@ describe PostAlerter do
     PostAlerter.post_created(post)
   end
 
+  context "private message" do
+    it "notifies for pms correctly" do
+      pm = Fabricate(:topic, archetype: 'private_message', category_id: nil)
+      op = Fabricate(:post, user_id: pm.user_id)
+      pm.allowed_users << pm.user
+      PostAlerter.post_created(op)
+      reply = Fabricate(:post, user_id: pm.user_id, topic_id: pm.id, reply_to_post_number: 1)
+      PostAlerter.post_created(reply)
+
+      reply2 = Fabricate(:post, topic_id: pm.id, reply_to_post_number: 1)
+      PostAlerter.post_created(reply2)
+
+      # we get a green notification for a reply
+      expect(Notification.where(user_id: pm.user_id).pluck(:notification_type).first).to eq(Notification.types[:private_message])
+
+      TopicUser.change(pm.user_id, pm.id, notification_level: TopicUser.notification_levels[:tracking])
+
+      Notification.destroy_all
+
+      reply3 = Fabricate(:post, topic_id: pm.id)
+      PostAlerter.post_created(reply3)
+
+      # no notification cause we are tracking
+      expect(Notification.where(user_id: pm.user_id).count).to eq(0)
+
+      Notification.destroy_all
+
+      reply4 = Fabricate(:post, topic_id: pm.id, reply_to_post_number: 1)
+      PostAlerter.post_created(reply4)
+
+      # yes notification cause we were replied to
+      expect(Notification.where(user_id: pm.user_id).count).to eq(1)
+
+
+    end
+  end
+
   context "unread" do
     it "does not return whispers as unread posts" do
       op = Fabricate(:post)