Don't notify users twice if they are both project watchers and subscribers

Closes #4708

3 files changed, 14 insertions, 0 deletions

diff --git a/CHANGELOG b/CHANGELOG
index e4b35b281bb..e8eb5d568f5 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.4.0 (unreleased)
   - Expire view caches when application settings change (e.g. Gravatar disabled) (Stan Hu)
+  - Don't notify users twice if they are both project watchers and subscribers (Stan Hu)
   - Implement new UI for group page
   - Implement search inside emoji picker
   - Add API support for looking up a user by username (Stan Hu)
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index bdf7b3ad2bb..e4edc55bf69 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -413,6 +413,7 @@ class NotificationService
     recipients = reject_unsubscribed_users(recipients, target)
 
     recipients.delete(current_user)
+    recipients = recipients.uniq
 
     recipients
   end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index c103752198d..588ecc51382 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -61,6 +61,7 @@ describe NotificationService, services: true do
           should_email(note.noteable.assignee)
           should_email(@u_mentioned)
           should_email(@subscriber)
+          should_email(@watcher_and_subscriber)
           should_email(@subscribed_participant)
           should_not_email(note.author)
           should_not_email(@u_participating)
@@ -245,6 +246,7 @@ describe NotificationService, services: true do
         should_email(@u_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
+        should_email(@watcher_and_subscriber)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -260,6 +262,7 @@ describe NotificationService, services: true do
         should_email(@u_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
+        should_email(@watcher_and_subscriber)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
       end
@@ -282,6 +285,7 @@ describe NotificationService, services: true do
 
         should_email(merge_request.assignee)
         should_email(@u_watcher)
+        should_email(@watcher_and_subscriber)
         should_email(@u_participant_mentioned)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -296,6 +300,7 @@ describe NotificationService, services: true do
         should_email(@u_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
+        should_email(@watcher_and_subscriber)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -310,6 +315,7 @@ describe NotificationService, services: true do
         should_email(@u_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
+        should_email(@watcher_and_subscriber)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -324,6 +330,7 @@ describe NotificationService, services: true do
         should_email(@u_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
+        should_email(@watcher_and_subscriber)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -338,6 +345,7 @@ describe NotificationService, services: true do
         should_email(@u_watcher)
         should_email(@u_participant_mentioned)
         should_email(@subscriber)
+        should_email(@watcher_and_subscriber)
         should_not_email(@unsubscriber)
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
@@ -387,14 +395,18 @@ describe NotificationService, services: true do
     @subscriber = create :user
     @unsubscriber = create :user
     @subscribed_participant = create(:user, username: 'subscribed_participant', notification_level: Notification::N_PARTICIPATING)
+    @watcher_and_subscriber = create(:user, notification_level: Notification::N_WATCH)
 
     project.team << [@subscribed_participant, :master]
     project.team << [@subscriber, :master]
     project.team << [@unsubscriber, :master]
+    project.team << [@watcher_and_subscriber, :master]
 
     issuable.subscriptions.create(user: @subscriber, subscribed: true)
     issuable.subscriptions.create(user: @subscribed_participant, subscribed: true)
     issuable.subscriptions.create(user: @unsubscriber, subscribed: false)
+    # Make the watcher a subscriber to detect dupes
+    issuable.subscriptions.create(user: @watcher_and_subscriber, subscribed: true)
   end
 
   def sent_to_user?(user)