Merge branch '37691-subscription-fires-multiple-notifications' into 'master'

fix multiple notifications from being sent for multiple labels

Closes #37691

See merge request gitlab-org/gitlab-ce!14798

4 files changed, 25 insertions, 8 deletions

diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 8d5da459882..be3b4b2ba07 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -390,7 +390,7 @@ class NotificationService
   end
 
   def relabeled_resource_email(target, labels, current_user, method)
-    recipients = labels.flat_map { |l| l.subscribers(target.project) }
+    recipients = labels.flat_map { |l| l.subscribers(target.project) }.uniq
     recipients = notifiable_users(
       recipients, :subscription,
       target: target,
diff --git a/changelogs/unreleased/37691-subscription-fires-multiple-notifications.yml b/changelogs/unreleased/37691-subscription-fires-multiple-notifications.yml
new file mode 100644
index 00000000000..c3c38b35fa7
--- /dev/null
+++ b/changelogs/unreleased/37691-subscription-fires-multiple-notifications.yml
@@ -0,0 +1,5 @@
+---
+title: Fixed duplicate notifications when added multiple labels on an issue
+merge_request: 14798
+author:
+type: fixed
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index b64ca5be8fc..b13e12e7c94 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -731,6 +731,18 @@ describe NotificationService, :mailer do
         should_not_email(@u_participating)
       end
 
+      it "doesn't send multiple email when a user is subscribed to multiple given labels" do
+        subscriber_to_both = create(:user) do |user|
+          [label_1, label_2].each { |label| label.toggle_subscription(user, project) }
+        end
+
+        notification.relabeled_issue(issue, [label_1, label_2], @u_disabled)
+
+        should_email(subscriber_to_label_1)
+        should_email(subscriber_to_label_2)
+        should_email(subscriber_to_both)
+      end
+
       context 'confidential issues' do
         let(:author) { create(:user) }
         let(:assignee) { create(:user) }
diff --git a/spec/support/email_helpers.rb b/spec/support/email_helpers.rb
index 3e979f2f470..b39052923dd 100644
--- a/spec/support/email_helpers.rb
+++ b/spec/support/email_helpers.rb
@@ -1,6 +1,6 @@
 module EmailHelpers
-  def sent_to_user?(user, recipients = email_recipients)
-    recipients.include?(user.notification_email)
+  def sent_to_user(user, recipients: email_recipients)
+    recipients.count { |to| to == user.notification_email }
   end
 
   def reset_delivered_emails!
@@ -10,17 +10,17 @@ module EmailHelpers
   def should_only_email(*users, kind: :to)
     recipients = email_recipients(kind: kind)
 
-    users.each { |user| should_email(user, recipients) }
+    users.each { |user| should_email(user, recipients: recipients) }
 
     expect(recipients.count).to eq(users.count)
   end
 
-  def should_email(user, recipients = email_recipients)
-    expect(sent_to_user?(user, recipients)).to be_truthy
+  def should_email(user, times: 1, recipients: email_recipients)
+    expect(sent_to_user(user, recipients: recipients)).to eq(times)
   end
 
-  def should_not_email(user, recipients = email_recipients)
-    expect(sent_to_user?(user, recipients)).to be_falsey
+  def should_not_email(user, recipients: email_recipients)
+    should_email(user, times: 0, recipients: recipients)
   end
 
   def should_not_email_anyone