Fix notification targeting: don't notify unrelated reviewers

engine/app/services/coplan/notifications/create.rb

@@ -53,7 +53,7 @@ def plan_author_ids
         ids = Set[plan.created_by_user_id]
         ids.merge(
           plan.plan_collaborators
-            .where(role: %w[author reviewer])
+            .where(role: "author")
             .pluck(:user_id)
         )
         ids

spec/services/notifications/create_spec.rb

@@ -28,15 +28,24 @@
       expect(result).to be_nil
     end
 
-    it "notifies plan collaborators with author/reviewer role" do
-      create(:plan_collaborator, plan: plan, user: reviewer, role: "reviewer")
+    it "notifies plan collaborators with author role" do
+      co_author = create(:coplan_user)
+      create(:plan_collaborator, plan: plan, user: co_author, role: "author")
 
       expect {
         described_class.call(comment_thread: thread, actor_id: commenter.id, reason: "new_comment")
       }.to change(CoPlan::Notification, :count).by(2)
 
       notified_user_ids = CoPlan::Notification.pluck(:user_id)
-      expect(notified_user_ids).to contain_exactly(plan_author.id, reviewer.id)
+      expect(notified_user_ids).to contain_exactly(plan_author.id, co_author.id)
+    end
+
+    it "does not notify reviewer collaborators" do
+      create(:plan_collaborator, plan: plan, user: reviewer, role: "reviewer")
+
+      described_class.call(comment_thread: thread, actor_id: commenter.id, reason: "new_comment")
+      notified_user_ids = CoPlan::Notification.pluck(:user_id)
+      expect(notified_user_ids).not_to include(reviewer.id)
     end
 
     it "does not notify viewer collaborators" do
@@ -59,6 +68,15 @@
       expect(notified_user_ids).to contain_exactly(plan_author.id, commenter.id)
     end
 
+    it "does not notify reviewer collaborators who are not thread participants" do
+      other_reviewer = create(:coplan_user)
+      create(:plan_collaborator, plan: plan, user: other_reviewer, role: "reviewer")
+
+      described_class.call(comment_thread: thread, actor_id: create(:coplan_user).id, reason: "reply")
+      notified_user_ids = CoPlan::Notification.pluck(:user_id)
+      expect(notified_user_ids).not_to include(other_reviewer.id)
+    end
+
     it "notifies prior human commenters in the thread" do
       prior_commenter = create(:coplan_user)
       create(:comment, comment_thread: thread, author_type: "human", author_id: prior_commenter.id)