FIX: when inviting groups to message respect tracking state

FIX: when inviting groups to message respect tracking state

Previously we would unconditionally issue an “invited_to_pm” notification to all non muting users.

New behavior

  • Watching and Watching first post get notified
  • Tracking get a new “summary” message
  • The rest get nothing

This is consistent with topic creation and way clearer

diff --git a/app/models/topic.rb b/app/models/topic.rb
index cb12508..a37ea05 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -822,8 +822,17 @@ class Topic < ActiveRecord::Base
       group_id = group.id
 
       group.users.where(
-        "group_users.notification_level > ? AND user_id != ?",
-        NotificationLevels.all[:muted], user.id
+        "group_users.notification_level = :level",
+        level: NotificationLevels.all[:tracking],
+        id: user.id
+      ).find_each do |u|
+        PostAlerter.new.notify_group_summary(u, last_post)
+      end
+
+      group.users.where(
+        "group_users.notification_level in (:levels) AND user_id != :id",
+        levels: [NotificationLevels.all[:watching], NotificationLevels.all[:watching_first_post]],
+        id: user.id
       ).find_each do |u|
 
         u.notifications.create!(
diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb
index 7b35b38..e9da4a7 100644
--- a/spec/models/topic_spec.rb
+++ b/spec/models/topic_spec.rb
@@ -786,29 +786,65 @@ describe Topic do
             expect(topic.allowed_groups.include?(admins)).to eq(false)
           end
 
+          def set_state!(group, user, state)
+            group.group_users.find_by(user_id: user.id).update!(
+              notification_level: NotificationLevels.all[state]
+            )
+          end
+
           it 'creates a notification for each user in the group' do
-            user = Fabricate(:user)
-            user_2 = Fabricate(:user)
+
+            # trigger notification
+            user_watching_first = Fabricate(:user)
+            user_watching = Fabricate(:user)
+
+            # trigger rollup
+            user_tracking = Fabricate(:user)
+
+            # trigger nothing
+            user_normal = Fabricate(:user)
+            user_muted = Fabricate(:user)
+
             Fabricate(:post, topic: topic)
 
-            group.add(user)
-            group.add(user_2)
-            group.add(topic.user)
+            group.add(topic.user) # no notification even though watching
+            group.add(user_watching_first)
+            group.add(user_watching)
+            group.add(user_normal)
+            group.add(user_muted)
+            group.add(user_tracking)
 
-            group.group_users.find_by(user: user_2).update!(
-              notification_level: NotificationLevels.all[:muted]
-            )
+            set_state!(group, topic.user, :watching)
+            set_state!(group, user_watching, :watching)
+            set_state!(group, user_watching_first, :watching_first_post)
+            set_state!(group, user_tracking, :tracking)
+            set_state!(group, user_normal, :regular)
+            set_state!(group, user_muted, :muted)
+
+            Notification.delete_all
+            topic.invite_group(topic.user, group)
 
-            expect { topic.invite_group(topic.user, group) }
-              .to change { Notification.count }.by(1)
+            expect(Notification.count).to eq(3)
 
-            notification = Notification.last
+            [user_watching, user_watching_first].each do |u|
+              notifications = Notification.where(user_id: u.id).to_a
+              expect(notifications.length).to eq(1)
 
-            expect(notification.user).to eq(user)
-            expect(notification.topic).to eq(topic)
+              notification = notifications.first
+
+              expect(notification.topic).to eq(topic)
+              expect(notification.notification_type)
+                .to eq(Notification.types[:invited_to_private_message])
+
+            end
+
+            notifications = Notification.where(user_id: user_tracking.id).to_a
+            expect(notifications.length).to eq(1)
+            notification = notifications.first
 
             expect(notification.notification_type)
-              .to eq(Notification.types[:invited_to_private_message])
+              .to eq(Notification.types[:group_message_summary])
+
           end
         end
       end

GitHub sha: a590061a

1 Like