diff --git a/plugins/chat/app/jobs/regular/chat_notify_watching.rb b/plugins/chat/app/jobs/regular/chat_notify_watching.rb index 36dfd355e2..0504e9545f 100644 --- a/plugins/chat/app/jobs/regular/chat_notify_watching.rb +++ b/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) diff --git a/plugins/chat/lib/chat_notifier.rb b/plugins/chat/lib/chat_notifier.rb index 12025935ba..7d647f7d81 100644 --- a/plugins/chat/lib/chat_notifier.rb +++ b/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, diff --git a/plugins/chat/spec/jobs/regular/chat_notify_watching_spec.rb b/plugins/chat/spec/jobs/regular/chat_notify_watching_spec.rb index 175f7a640f..f94dfeafec 100644 --- a/plugins/chat/spec/jobs/regular/chat_notify_watching_spec.rb +++ b/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