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.

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