Merge branch 'dedupe-notifications-watcher-and-subscriber' into 'master'

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

Closes #4708

See merge request !2290

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)

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

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)