diff --git a/engine/app/services/coplan/notifications/create.rb b/engine/app/services/coplan/notifications/create.rb index 8f81d54..ed9502d 100644 --- a/engine/app/services/coplan/notifications/create.rb +++ b/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 diff --git a/spec/services/notifications/create_spec.rb b/spec/services/notifications/create_spec.rb index 9febac7..dbeca23 100644 --- a/spec/services/notifications/create_spec.rb +++ b/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)