FEATURE: don't notify about changed tags for a private message (#10408)

* FEATURE: don't notify about changed tags for a private message

Only staff members observing specific tag should receive a notification

* FIX: remove other category which is not used

* FIX: improved specs to ensure that revise was succesful

app/jobs/regular/notify_tag_change.rb

@@ -7,7 +7,7 @@ module Jobs
 
       if post&.topic&.visible?
         post_alerter = PostAlerter.new
-        post_alerter.notify_post_users(post, User.where(id: args[:notified_user_ids]), include_category_watchers: false)
+        post_alerter.notify_post_users(post, User.where(id: args[:notified_user_ids]), include_topic_watchers: !post.topic.private_message?, include_category_watchers: false)
         post_alerter.notify_first_post_watchers(post, post_alerter.tag_watchers(post.topic))
       end
     end

app/services/post_alerter.rb

@@ -613,21 +613,28 @@ class PostAlerter
     end
   end
 
-  def notify_post_users(post, notified, include_category_watchers: true, include_tag_watchers: true, new_record: false)
+  def notify_post_users(post, notified, include_topic_watchers: true, include_category_watchers: true, include_tag_watchers: true, new_record: false)
     return unless post.topic
 
     warn_if_not_sidekiq
 
     condition = +<<~SQL
       id IN (
+        SELECT id FROM users WHERE false
+        /*topic*/
+        /*category*/
+        /*tags*/
+      )
+    SQL
+    if include_topic_watchers
+      condition.sub! "/*topic*/", <<~SQL
+        UNION
         SELECT user_id
           FROM topic_users
          WHERE notification_level = :watching
            AND topic_id = :topic_id
-         /*category*/
+      SQL
-         /*tags*/
+    end
-      )
-    SQL
 
     if include_category_watchers
       condition.sub! "/*category*/", <<~SQL
@@ -639,7 +646,7 @@ class PostAlerter
                              AND tu.topic_id = :topic_id
          WHERE cu.notification_level = :watching
            AND cu.category_id = :category_id
-           AND tu.user_id IS NULL
+           AND (tu.user_id IS NULL OR tu.notification_level = :watching)
       SQL
     end
 
@@ -655,7 +662,7 @@ class PostAlerter
                              AND tu.topic_id = :topic_id
          WHERE tag_users.notification_level = :watching
            AND tag_users.tag_id IN (:tag_ids)
-           AND tu.user_id IS NULL
+           AND (tu.user_id IS NULL OR tu.notification_level = :watching)
       SQL
     end
 
@@ -666,6 +673,10 @@ class PostAlerter
       tag_ids: tag_ids
     )
 
+    if post.topic.private_message?
+      notify = notify.where(staged: false).staff
+    end
+
     exclude_user_ids = notified.map(&:id)
     notify = notify.where("id NOT IN (?)", exclude_user_ids) if exclude_user_ids.present?
 

spec/services/post_alerter_spec.rb

@@ -1098,6 +1098,38 @@ describe PostAlerter do
         expect(user.notifications.where(notification_type: Notification.types[:watching_first_post]).count).to eq(0)
       end
     end
+
+    context "private message" do
+      fab!(:post) { Fabricate(:private_message_post) }
+      fab!(:other_tag) { Fabricate(:tag) }
+      fab!(:other_tag2) { Fabricate(:tag) }
+      fab!(:other_tag3) { Fabricate(:tag) }
+      fab!(:user) { Fabricate(:user) }
+      fab!(:staged) { Fabricate(:staged) }
+      fab!(:admin) { Fabricate(:admin) }
+
+      before do
+        SiteSetting.tagging_enabled = true
+        SiteSetting.allow_staff_to_tag_pms = true
+        Jobs.run_immediately!
+        TopicUser.change(user.id, post.topic.id, notification_level: TopicUser.notification_levels[:watching])
+        TopicUser.change(staged.id, post.topic.id, notification_level: TopicUser.notification_levels[:watching])
+        TopicUser.change(admin.id, post.topic.id, notification_level: TopicUser.notification_levels[:watching])
+        TagUser.change(staged.id, other_tag.id, TagUser.notification_levels[:watching])
+        TagUser.change(admin.id, other_tag3.id, TagUser.notification_levels[:watching])
+        post.topic.allowed_users << user
+        post.topic.allowed_users << staged
+      end
+
+      it "only notifes staff watching added tag" do
+        expect(PostRevisor.new(post).revise!(Fabricate(:admin), tags: [other_tag.name])).to be true
+        expect(Notification.where(user_id: staged.id).count).to eq(0)
+        expect(PostRevisor.new(post).revise!(Fabricate(:admin), tags: [other_tag2.name])).to be true
+        expect(Notification.where(user_id: admin.id).count).to eq(0)
+        expect(PostRevisor.new(post).revise!(Fabricate(:admin), tags: [other_tag3.name])).to be true
+        expect(Notification.where(user_id: admin.id).count).to eq(1)
+      end
+    end
   end
 
   describe '#extract_linked_users' do