summaryrefslogtreecommitdiff
path: root/spec
diff options
context:
space:
mode:
authorMario de la Ossa <mariodelaossa@gmail.com>2017-12-28 11:25:02 -0600
committerMario de la Ossa <mariodelaossa@gmail.com>2018-01-16 19:17:55 -0600
commit23a20c20f826f090446587881df7008a137d2d34 (patch)
treef1f491f1ab62f40c093ba0f5bd645606c681fd8d /spec
parent3228ac06a019c9126b965ff32e354d10011a4f76 (diff)
downloadgitlab-ce-23a20c20f826f090446587881df7008a137d2d34.tar.gz
Initial work to add notification reason to emails
Adds `#build_notification_recipients` to `NotificationRecipientService` that returns the `NotificationRecipient` objects in order to be able to access the new attribute `reason`. This new attribute is used in the different notifier methods in order to add the reason as a header: `X-GitLab-NotificationReason`. Only the reason with the most priority gets sent.
Diffstat (limited to 'spec')
-rw-r--r--spec/mailers/notify_spec.rb49
-rw-r--r--spec/services/notification_service_spec.rb62
-rw-r--r--spec/support/email_helpers.rb4
-rw-r--r--spec/workers/new_issue_worker_spec.rb5
-rw-r--r--spec/workers/new_merge_request_worker_spec.rb6
5 files changed, 121 insertions, 5 deletions
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index cbc8c67da61..7a8e798e3c9 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -71,6 +71,18 @@ describe Notify do
is_expected.to have_html_escaped_body_text issue.description
end
+ it 'does not add a reason header' do
+ is_expected.not_to have_header('X-GitLab-NotificationReason', /.+/)
+ end
+
+ context 'when sent with a reason' do
+ subject { described_class.new_issue_email(issue.assignees.first.id, issue.id, NotificationReason::ASSIGNED) }
+
+ it 'includes the reason in a header' do
+ is_expected.to have_header('X-GitLab-NotificationReason', NotificationReason::ASSIGNED)
+ end
+ end
+
context 'when enabled email_author_in_body' do
before do
stub_application_setting(email_author_in_body: true)
@@ -108,6 +120,14 @@ describe Notify do
is_expected.to have_body_text(project_issue_path(project, issue))
end
end
+
+ context 'when sent with a reason' do
+ subject { described_class.reassigned_issue_email(recipient.id, issue.id, [previous_assignee.id], current_user.id, NotificationReason::ASSIGNED) }
+
+ it 'includes the reason in a header' do
+ is_expected.to have_header('X-GitLab-NotificationReason', NotificationReason::ASSIGNED)
+ end
+ end
end
describe 'that have been relabeled' do
@@ -226,6 +246,14 @@ describe Notify do
is_expected.to have_html_escaped_body_text merge_request.description
end
+ context 'when sent with a reason' do
+ subject { described_class.new_merge_request_email(merge_request.assignee_id, merge_request.id, NotificationReason::ASSIGNED) }
+
+ it 'includes the reason in a header' do
+ is_expected.to have_header('X-GitLab-NotificationReason', NotificationReason::ASSIGNED)
+ end
+ end
+
context 'when enabled email_author_in_body' do
before do
stub_application_setting(email_author_in_body: true)
@@ -263,6 +291,27 @@ describe Notify do
is_expected.to have_html_escaped_body_text(assignee.name)
end
end
+
+ context 'when sent with a reason' do
+ subject { described_class.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id, NotificationReason::ASSIGNED) }
+
+ it 'includes the reason in a header' do
+ is_expected.to have_header('X-GitLab-NotificationReason', NotificationReason::ASSIGNED)
+ end
+
+ it 'includes the reason in the footer' do
+ text = EmailsHelper.instance_method(:notification_reason_text).bind(self).call(NotificationReason::ASSIGNED)
+ is_expected.to have_body_text(text)
+
+ new_subject = described_class.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id, NotificationReason::MENTIONED)
+ text = EmailsHelper.instance_method(:notification_reason_text).bind(self).call(NotificationReason::MENTIONED)
+ expect(new_subject).to have_body_text(text)
+
+ new_subject = described_class.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id, nil)
+ text = EmailsHelper.instance_method(:notification_reason_text).bind(self).call(nil)
+ expect(new_subject).to have_body_text(text)
+ end
+ end
end
describe 'that have been relabeled' do
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 43e2643f709..5c59455e3e1 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe NotificationService, :mailer do
+ include EmailSpec::Matchers
+
let(:notification) { described_class.new }
let(:assignee) { create(:user) }
@@ -31,6 +33,14 @@ describe NotificationService, :mailer do
send_notifications(@u_disabled)
should_not_email_anyone
end
+
+ it 'sends the proper notification reason header' do
+ send_notifications(@u_watcher)
+ should_only_email(@u_watcher)
+ email = find_email_for(@u_watcher)
+
+ expect(email).to have_header('X-GitLab-NotificationReason', NotificationReason::MENTIONED)
+ end
end
# Next shared examples are intended to test notifications of "participants"
@@ -511,12 +521,35 @@ describe NotificationService, :mailer do
should_not_email(issue.assignees.first)
end
+ it 'properly prioritizes notification reason' do
+ # have assignee be both assigned and mentioned
+ issue.update_attribute(:description, "/cc #{assignee.to_reference} #{@u_mentioned.to_reference}")
+
+ notification.new_issue(issue, @u_disabled)
+
+ email = find_email_for(assignee)
+ expect(email).to have_header('X-GitLab-NotificationReason', 'assigned')
+
+ email = find_email_for(@u_mentioned)
+ expect(email).to have_header('X-GitLab-NotificationReason', 'mentioned')
+ end
+
+ it 'adds "assigned" reason for assignees if any' do
+ notification.new_issue(issue, @u_disabled)
+
+ email = find_email_for(assignee)
+
+ expect(email).to have_header('X-GitLab-NotificationReason', 'assigned')
+ end
+
it "emails any mentioned users with the mention level" do
issue.description = @u_mentioned.to_reference
notification.new_issue(issue, @u_disabled)
- should_email(@u_mentioned)
+ email = find_email_for(@u_mentioned)
+ expect(email).not_to be_nil
+ expect(email).to have_header('X-GitLab-NotificationReason', 'mentioned')
end
it "emails the author if they've opted into notifications about their activity" do
@@ -620,6 +653,14 @@ describe NotificationService, :mailer do
should_not_email(@u_lazy_participant)
end
+ it 'adds "assigned" reason for new assignee' do
+ notification.reassigned_issue(issue, @u_disabled, [assignee])
+
+ email = find_email_for(assignee)
+
+ expect(email).to have_header('X-GitLab-NotificationReason', NotificationReason::ASSIGNED)
+ end
+
it 'emails previous assignee even if he has the "on mention" notif level' do
issue.assignees = [@u_mentioned]
notification.reassigned_issue(issue, @u_disabled, [@u_watcher])
@@ -910,6 +951,14 @@ describe NotificationService, :mailer do
should_not_email(@u_lazy_participant)
end
+ it 'adds "assigned" reason for assignee, if any' do
+ notification.new_merge_request(merge_request, @u_disabled)
+
+ email = find_email_for(merge_request.assignee)
+
+ expect(email).to have_header('X-GitLab-NotificationReason', NotificationReason::ASSIGNED)
+ end
+
it "emails any mentioned users with the mention level" do
merge_request.description = @u_mentioned.to_reference
@@ -924,6 +973,9 @@ describe NotificationService, :mailer do
notification.new_merge_request(merge_request, merge_request.author)
should_email(merge_request.author)
+
+ email = find_email_for(merge_request.author)
+ expect(email).to have_header('X-GitLab-NotificationReason', NotificationReason::OWN_ACTIVITY)
end
it "doesn't email the author if they haven't opted into notifications about their activity" do
@@ -1009,6 +1061,14 @@ describe NotificationService, :mailer do
should_not_email(@u_lazy_participant)
end
+ it 'adds "assigned" reason for new assignee' do
+ notification.reassigned_merge_request(merge_request, merge_request.author)
+
+ email = find_email_for(merge_request.assignee)
+
+ expect(email).to have_header('X-GitLab-NotificationReason', NotificationReason::ASSIGNED)
+ end
+
it_behaves_like 'participating notifications' do
let(:participant) { create(:user, username: 'user-participant') }
let(:issuable) { merge_request }
diff --git a/spec/support/email_helpers.rb b/spec/support/email_helpers.rb
index b39052923dd..1fb8252459f 100644
--- a/spec/support/email_helpers.rb
+++ b/spec/support/email_helpers.rb
@@ -30,4 +30,8 @@ module EmailHelpers
def email_recipients(kind: :to)
ActionMailer::Base.deliveries.flat_map(&kind)
end
+
+ def find_email_for(user)
+ ActionMailer::Base.deliveries.find { |d| d.to.include?(user.notification_email) }
+ end
end
diff --git a/spec/workers/new_issue_worker_spec.rb b/spec/workers/new_issue_worker_spec.rb
index 4e15ccc534b..baa8ddb59e5 100644
--- a/spec/workers/new_issue_worker_spec.rb
+++ b/spec/workers/new_issue_worker_spec.rb
@@ -44,8 +44,9 @@ describe NewIssueWorker do
expect { worker.perform(issue.id, user.id) }.to change { Event.count }.from(0).to(1)
end
- it 'creates a notification for the assignee' do
- expect(Notify).to receive(:new_issue_email).with(mentioned.id, issue.id).and_return(double(deliver_later: true))
+ it 'creates a notification for the mentioned user' do
+ expect(Notify).to receive(:new_issue_email).with(mentioned.id, issue.id, NotificationReason::MENTIONED)
+ .and_return(double(deliver_later: true))
worker.perform(issue.id, user.id)
end
diff --git a/spec/workers/new_merge_request_worker_spec.rb b/spec/workers/new_merge_request_worker_spec.rb
index 9e0cbde45b1..c3f29a40d58 100644
--- a/spec/workers/new_merge_request_worker_spec.rb
+++ b/spec/workers/new_merge_request_worker_spec.rb
@@ -46,8 +46,10 @@ describe NewMergeRequestWorker do
expect { worker.perform(merge_request.id, user.id) }.to change { Event.count }.from(0).to(1)
end
- it 'creates a notification for the assignee' do
- expect(Notify).to receive(:new_merge_request_email).with(mentioned.id, merge_request.id).and_return(double(deliver_later: true))
+ it 'creates a notification for the mentioned user' do
+ expect(Notify).to receive(:new_merge_request_email)
+ .with(mentioned.id, merge_request.id, NotificationReason::MENTIONED)
+ .and_return(double(deliver_later: true))
worker.perform(merge_request.id, user.id)
end