summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authormicael.bergeron <micaelbergeron@gmail.com>2017-10-10 14:45:43 -0400
committermicael.bergeron <micaelbergeron@gmail.com>2017-10-10 14:45:43 -0400
commite5ed2e4f4e82dcfdec4847f7d96094393a9f0839 (patch)
tree2f88d09c0d038f94ffbeb761762f160c707fa3ad
parentf9df0e13e3224e90dcddded6d8ae4f1eabc3b6db (diff)
downloadgitlab-ce-e5ed2e4f4e82dcfdec4847f7d96094393a9f0839.tar.gz
fix multiple notifications from being sent for multiple labels
This also refactor the email_helper support spec to watch for multiple emails being sent.
-rw-r--r--app/services/notification_service.rb2
-rw-r--r--spec/services/notification_service_spec.rb12
-rw-r--r--spec/support/email_helpers.rb14
3 files changed, 20 insertions, 8 deletions
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index e2a80db06a6..b1695f348ee 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -397,7 +397,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/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index f4b36eb7eeb..457acc86e50 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -743,6 +743,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..a9976972268 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)
end
def should_not_email_anyone