diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2017-10-12 14:39:51 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2017-10-12 14:39:51 +0000 |
commit | ca06ff27874d96705b7878a17179ffb25d809aba (patch) | |
tree | b7722e47f160ffcb9b7f41e4a74017c930d460fb | |
parent | f383220674a40e2c542ba03e88c4296c6042d1f1 (diff) | |
parent | 486da72f2801cae0a170e4e4eafc4b928cd3f060 (diff) | |
download | gitlab-ce-ca06ff27874d96705b7878a17179ffb25d809aba.tar.gz |
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
-rw-r--r-- | app/services/notification_service.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/37691-subscription-fires-multiple-notifications.yml | 5 | ||||
-rw-r--r-- | spec/services/notification_service_spec.rb | 12 | ||||
-rw-r--r-- | spec/support/email_helpers.rb | 14 |
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 |