make sure we don't notify watching users multiple times

plugins/chat/app/jobs/regular/chat_notify_watching.rb

@@ -19,7 +19,7 @@ module Jobs
 
       members =
         UserChatChannelMembership
-          .includes(user: :groups)
+          .includes(:user)
           .joins(user: :user_option)
           .where(user_option: { chat_enabled: true })
           .where(chat_channel_id: @chat_channel.id)
@@ -29,7 +29,6 @@ module Jobs
             @chat_message.id
           )
           .where.not(user_id: direct_mentioned_user_ids)
-          .where.not(groups: { id: mentioned_group_ids })
           .where(
             "desktop_notification_level = ? OR mobile_notification_level = ?",
             always_notification_level,
@@ -37,6 +36,13 @@ module Jobs
           )
           .merge(User.not_suspended)
 
+      if mentioned_group_ids.present?
+        members = members
+          .joins("LEFT OUTER JOIN group_users gu ON gu.user_id = users.id")
+          .group("user_chat_channel_memberships.id")
+          .having("COUNT(gu.group_id) = 0 OR bool_and(gu.group_id NOT IN (?))", mentioned_group_ids)
+      end
+
       if global_mentions.include?(Chat::ChatNotifier::ALL_KEYWORD)
         members = members.where(user_option: { ignore_channel_wide_mention: true })
       elsif global_mentions.include?(Chat::ChatNotifier::HERE_KEYWORD)

plugins/chat/lib/chat_notifier.rb

@@ -154,6 +154,8 @@ class Chat::ChatNotifier
     inaccessible_mentions[:group_mentions_disabled] = mentions_disabled
     inaccessible_mentions[:too_many_members] = too_many_members
 
+    return if mentionable.blank?
+
     mentioned_by_group(mentionable).find_in_batches(batch_size: MENTION_BATCH_SIZE) do |reached_by_group|
       grouped = group_users_to_notify(reached_by_group)
       ordered_group_names = group_name_mentions & mentionable.map { |mg| mg.name.downcase }
@@ -321,7 +323,7 @@ class Chat::ChatNotifier
   # Jobs to create notifications
 
   def notify_mentioned_users(mention_type, user_ids)
-    return if user_ids.blank?
+    return if user_ids.blank? || mention_type.blank?
 
     Jobs.enqueue(
       :chat_notify_mentioned,

plugins/chat/spec/jobs/regular/chat_notify_watching_spec.rb

@@ -92,7 +92,7 @@ RSpec.describe Jobs::ChatNotifyWatching do
     end
 
     it "skips the watching notifications if it was mentioned through a group" do
-      group.add(user1)
+      group.add(user2)
       PostAlerter.expects(:push_notification).never
 
       messages = listen_for_notifications(user2, mentioned_group_ids: [group.id])
@@ -100,6 +100,14 @@ RSpec.describe Jobs::ChatNotifyWatching do
       expect(messages.size).to be_zero
     end
 
+    it "doesn't skip watching notifications if the user is not a member of the mentioned group" do
+      PostAlerter.expects(:push_notification).once
+
+      messages = listen_for_notifications(user2, mentioned_group_ids: [group.id])
+
+      expect(messages).to be_present
+    end
+
     it "skips the watching notifications if it was mentioned via @all mention" do
       PostAlerter.expects(:push_notification).never
 
@@ -135,6 +143,20 @@ RSpec.describe Jobs::ChatNotifyWatching do
 
       assert_notification_alert_is_correct(messages.first.data, user1, user2, message)
     end
+
+    context "when among the user groups there is a mentioned one" do
+      it 'skips the watching notification' do
+        group.add(user2)
+        another_group = Fabricate(:group)
+        another_group.add(user2)
+
+        PostAlerter.expects(:push_notification).never
+
+        messages = listen_for_notifications(user2, mentioned_group_ids: [group.id])
+
+        expect(messages.size).to be_zero
+      end
+    end
   end
 
   context "for a category channel" do
@@ -182,7 +204,7 @@ RSpec.describe Jobs::ChatNotifyWatching do
         )
       end
 
-      it "sends a mobile notification" do
+      it "only sends a mobile notification" do
         expects_push_notification(user1, user2, message)
 
         messages = listen_for_notifications(user2)
@@ -193,7 +215,7 @@ RSpec.describe Jobs::ChatNotifyWatching do
       context "when the channel is muted via membership preferences" do
         before { membership2.update!(muted: true) }
 
-        it "does not send a desktop or mobile notification" do
+        it "does not send any notification" do
           PostAlerter.expects(:push_notification).never
           messages = listen_for_notifications(user2)
           expect(messages).to be_empty