kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Merge branch '41532-email-reason' into 'master'

Browse source 
Show why a notification email was sent

Closes #41532 and #1366

See merge request gitlab-org/gitlab-ce!16160
This commit is contained in:
Sean McGivern 2018-01-19 11:01:46 +00:00
commit e4952f1703
17 changed files with 288 additions and 94 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
app/helpers/emails_helper.rb
View file

@ -80,4 +80,20 @@ module EmailsHelper
'text-align:center'
].join(';')
end
# "You are receiving this email because #{reason}"
def notification_reason_text(reason)
string = case reason
when NotificationReason::OWN_ACTIVITY
'of your activity'
when NotificationReason::ASSIGNED
'you have been assigned an item'
when NotificationReason::MENTIONED
'you have been mentioned'
else
'of your account'
end
"#{string} on #{Gitlab.config.gitlab.host}"
end
end

33
app/mailers/emails/issues.rb
View file

@ -1,54 +1,54 @@
module Emails
module Issues
def new_issue_email(recipient_id, issue_id)
def new_issue_email(recipient_id, issue_id, reason = nil)
setup_issue_mail(issue_id, recipient_id)
mail_new_thread(@issue, issue_thread_options(@issue.author_id, recipient_id))
mail_new_thread(@issue, issue_thread_options(@issue.author_id, recipient_id, reason))
end
def new_mention_in_issue_email(recipient_id, issue_id, updated_by_user_id)
def new_mention_in_issue_email(recipient_id, issue_id, updated_by_user_id, reason = nil)
setup_issue_mail(issue_id, recipient_id)
mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id))
mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id, reason))
end
def reassigned_issue_email(recipient_id, issue_id, previous_assignee_ids, updated_by_user_id)
def reassigned_issue_email(recipient_id, issue_id, previous_assignee_ids, updated_by_user_id, reason = nil)
setup_issue_mail(issue_id, recipient_id)
@previous_assignees = []
@previous_assignees = User.where(id: previous_assignee_ids) if previous_assignee_ids.any?
mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id))
mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id, reason))
end
def closed_issue_email(recipient_id, issue_id, updated_by_user_id)
def closed_issue_email(recipient_id, issue_id, updated_by_user_id, reason = nil)
setup_issue_mail(issue_id, recipient_id)
@updated_by = User.find(updated_by_user_id)
mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id))
mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id, reason))
end
def relabeled_issue_email(recipient_id, issue_id, label_names, updated_by_user_id)
def relabeled_issue_email(recipient_id, issue_id, label_names, updated_by_user_id, reason = nil)
setup_issue_mail(issue_id, recipient_id)
@label_names = label_names
@labels_url = project_labels_url(@project)
mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id))
mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id, reason))
end
def issue_status_changed_email(recipient_id, issue_id, status, updated_by_user_id)
def issue_status_changed_email(recipient_id, issue_id, status, updated_by_user_id, reason = nil)
setup_issue_mail(issue_id, recipient_id)
@issue_status = status
@updated_by = User.find(updated_by_user_id)
mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id))
mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id, reason))
end
def issue_moved_email(recipient, issue, new_issue, updated_by_user)
def issue_moved_email(recipient, issue, new_issue, updated_by_user, reason = nil)
setup_issue_mail(issue.id, recipient.id)
@new_issue = new_issue
@new_project = new_issue.project
mail_answer_thread(issue, issue_thread_options(updated_by_user.id, recipient.id))
mail_answer_thread(issue, issue_thread_options(updated_by_user.id, recipient.id, reason))
end
private
@ -61,11 +61,12 @@ module Emails
@sent_notification = SentNotification.record(@issue, recipient_id, reply_key)
end
def issue_thread_options(sender_id, recipient_id)
def issue_thread_options(sender_id, recipient_id, reason)
{
from: sender(sender_id),
to: recipient(recipient_id),
subject: subject("#{@issue.title} (##{@issue.iid})")
subject: subject("#{@issue.title} (##{@issue.iid})"),
'X-GitLab-NotificationReason' => reason
}
end
end

37
app/mailers/emails/merge_requests.rb
View file

@ -1,57 +1,57 @@
module Emails
module MergeRequests
def new_merge_request_email(recipient_id, merge_request_id)
def new_merge_request_email(recipient_id, merge_request_id, reason = nil)
setup_merge_request_mail(merge_request_id, recipient_id)
mail_new_thread(@merge_request, merge_request_thread_options(@merge_request.author_id, recipient_id))
mail_new_thread(@merge_request, merge_request_thread_options(@merge_request.author_id, recipient_id, reason))
end
def new_mention_in_merge_request_email(recipient_id, merge_request_id, updated_by_user_id)
def new_mention_in_merge_request_email(recipient_id, merge_request_id, updated_by_user_id, reason = nil)
setup_merge_request_mail(merge_request_id, recipient_id)
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id))
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason))
end
def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id, updated_by_user_id)
def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id, updated_by_user_id, reason = nil)
setup_merge_request_mail(merge_request_id, recipient_id)
@previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id))
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason))
end
def relabeled_merge_request_email(recipient_id, merge_request_id, label_names, updated_by_user_id)
def relabeled_merge_request_email(recipient_id, merge_request_id, label_names, updated_by_user_id, reason = nil)
setup_merge_request_mail(merge_request_id, recipient_id)
@label_names = label_names
@labels_url = project_labels_url(@project)
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id))
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason))
end
def closed_merge_request_email(recipient_id, merge_request_id, updated_by_user_id)
def closed_merge_request_email(recipient_id, merge_request_id, updated_by_user_id, reason = nil)
setup_merge_request_mail(merge_request_id, recipient_id)
@updated_by = User.find(updated_by_user_id)
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id))
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason))
end
def merged_merge_request_email(recipient_id, merge_request_id, updated_by_user_id)
def merged_merge_request_email(recipient_id, merge_request_id, updated_by_user_id, reason = nil)
setup_merge_request_mail(merge_request_id, recipient_id)
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id))
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason))
end
def merge_request_status_email(recipient_id, merge_request_id, status, updated_by_user_id)
def merge_request_status_email(recipient_id, merge_request_id, status, updated_by_user_id, reason = nil)
setup_merge_request_mail(merge_request_id, recipient_id)
@mr_status = status
@updated_by = User.find(updated_by_user_id)
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id))
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason))
end
def resolved_all_discussions_email(recipient_id, merge_request_id, resolved_by_user_id)
def resolved_all_discussions_email(recipient_id, merge_request_id, resolved_by_user_id, reason = nil)
setup_merge_request_mail(merge_request_id, recipient_id)
@resolved_by = User.find(resolved_by_user_id)
mail_answer_thread(@merge_request, merge_request_thread_options(resolved_by_user_id, recipient_id))
mail_answer_thread(@merge_request, merge_request_thread_options(resolved_by_user_id, recipient_id, reason))
end
private
@ -64,11 +64,12 @@ module Emails
@sent_notification = SentNotification.record(@merge_request, recipient_id, reply_key)
end
def merge_request_thread_options(sender_id, recipient_id)
def merge_request_thread_options(sender_id, recipient_id, reason = nil)
{
from: sender(sender_id),
to: recipient(recipient_id),
subject: subject("#{@merge_request.title} (#{@merge_request.to_reference})")
subject: subject("#{@merge_request.title} (#{@merge_request.to_reference})"),
'X-GitLab-NotificationReason' => reason
}
end
end

2
app/mailers/notify.rb
View file

@ -112,6 +112,8 @@ class Notify < BaseMailer
headers["X-GitLab-#{model.class.name}-ID"] = model.id
headers['X-GitLab-Reply-Key'] = reply_key
@reason = headers['X-GitLab-NotificationReason']
if Gitlab::IncomingEmail.enabled? && @sent_notification
address = Mail::Address.new(Gitlab::IncomingEmail.reply_address(reply_key))
address.display_name = @project.name_with_namespace

19
app/models/notification_reason.rb Normal file
View file

@ -0,0 +1,19 @@
# Holds reasons for a notification to have been sent as well as a priority list to select which reason to use
# above the rest
class NotificationReason
OWN_ACTIVITY = 'own_activity'.freeze
ASSIGNED = 'assigned'.freeze
MENTIONED = 'mentioned'.freeze
# Priority list for selecting which reason to return in the notification
REASON_PRIORITY = [
OWN_ACTIVITY,
ASSIGNED,
MENTIONED
].freeze
# returns the priority of a reason as an integer
def self.priority(reason)
REASON_PRIORITY.index(reason) || REASON_PRIORITY.length + 1
end
end

36
app/models/notification_recipient.rb
View file

@ -1,27 +1,19 @@
class NotificationRecipient
attr_reader :user, :type
def initialize(
user, type,
custom_action: nil,
target: nil,
acting_user: nil,
project: nil,
group: nil,
skip_read_ability: false
)
attr_reader :user, :type, :reason
def initialize(user, type, **opts)
unless NotificationSetting.levels.key?(type) || type == :subscription
raise ArgumentError, "invalid type: #{type.inspect}"
end
@custom_action = custom_action
@acting_user = acting_user
@target = target
@project = project || default_project
@group = group || @project&.group
@custom_action = opts[:custom_action]
@acting_user = opts[:acting_user]
@target = opts[:target]
@project = opts[:project] || default_project
@group = opts[:group] || @project&.group
@user = user
@type = type
@skip_read_ability = skip_read_ability
@reason = opts[:reason]
@skip_read_ability = opts[:skip_read_ability]
end
def notification_setting
@ -77,9 +69,15 @@ class NotificationRecipient
def own_activity?
return false unless @acting_user
return false if @acting_user.notified_of_own_activity?
user == @acting_user
if user == @acting_user
# if activity was generated by the same user, change reason to :own_activity
@reason = NotificationReason::OWN_ACTIVITY
# If the user wants to be notified, we must return `false`
!@acting_user.notified_of_own_activity?
else
false
end
end
def has_access?

48
app/services/notification_recipient_service.rb
View file

@ -11,11 +11,11 @@ module NotificationRecipientService
end
def self.build_recipients(*a)
Builder::Default.new(*a).recipient_users
Builder::Default.new(*a).notification_recipients
end
def self.build_new_note_recipients(*a)
Builder::NewNote.new(*a).recipient_users
Builder::NewNote.new(*a).notification_recipients
end
module Builder
@ -49,25 +49,24 @@ module NotificationRecipientService
@recipients ||= []
end
def <<(pair)
users, type = pair
def add_recipients(users, type, reason)
if users.is_a?(ActiveRecord::Relation)
users = users.includes(:notification_settings)
end
users = Array(users)
users.compact!
recipients.concat(users.map { |u| make_recipient(u, type) })
recipients.concat(users.map { |u| make_recipient(u, type, reason) })
end
def user_scope
User.includes(:notification_settings)
end
def make_recipient(user, type)
def make_recipient(user, type, reason)
NotificationRecipient.new(
user, type,
reason: reason,
project: project,
custom_action: custom_action,
target: target,
@ -75,14 +74,13 @@ module NotificationRecipientService
)
end
def recipient_users
@recipient_users ||=
def notification_recipients
@notification_recipients ||=
begin
build!
filter!
users = recipients.map(&:user)
users.uniq!
users.freeze
recipients = self.recipients.sort_by { |r| NotificationReason.priority(r.reason) }.uniq(&:user)
recipients.freeze
end
end
@ -95,13 +93,13 @@ module NotificationRecipientService
def add_participants(user)
return unless target.respond_to?(:participants)
self << [target.participants(user), :participating]
add_recipients(target.participants(user), :participating, nil)
end
def add_mentions(user, target:)
return unless target.respond_to?(:mentioned_users)
self << [target.mentioned_users(user), :mention]
add_recipients(target.mentioned_users(user), :mention, NotificationReason::MENTIONED)
end
# Get project/group users with CUSTOM notification level
@ -119,11 +117,11 @@ module NotificationRecipientService
global_users_ids = user_ids_with_project_level_global.concat(user_ids_with_group_level_global)
user_ids += user_ids_with_global_level_custom(global_users_ids, custom_action)
self << [user_scope.where(id: user_ids), :watch]
add_recipients(user_scope.where(id: user_ids), :watch, nil)
end
def add_project_watchers
self << [project_watchers, :watch]
add_recipients(project_watchers, :watch, nil)
end
# Get project users with WATCH notification level
@ -144,7 +142,7 @@ module NotificationRecipientService
def add_subscribed_users
return unless target.respond_to? :subscribers
self << [target.subscribers(project), :subscription]
add_recipients(target.subscribers(project), :subscription, nil)
end
def user_ids_notifiable_on(resource, notification_level = nil)
@ -195,7 +193,7 @@ module NotificationRecipientService
return unless target.respond_to? :labels
(labels || target.labels).each do |label|
self << [label.subscribers(project), :subscription]
add_recipients(label.subscribers(project), :subscription, nil)
end
end
end
@ -222,12 +220,12 @@ module NotificationRecipientService
# Re-assign is considered as a mention of the new assignee
case custom_action
when :reassign_merge_request
self << [previous_assignee, :mention]
self << [target.assignee, :mention]
add_recipients(previous_assignee, :mention, nil)
add_recipients(target.assignee, :mention, NotificationReason::ASSIGNED)
when :reassign_issue
previous_assignees = Array(previous_assignee)
self << [previous_assignees, :mention]
self << [target.assignees, :mention]
add_recipients(previous_assignees, :mention, nil)
add_recipients(target.assignees, :mention, NotificationReason::ASSIGNED)
end
add_subscribed_users
@ -238,6 +236,12 @@ module NotificationRecipientService
# receive them, too.
add_mentions(current_user, target: target)
# Add the assigned users, if any
assignees = custom_action == :new_issue ? target.assignees : target.assignee
# We use the `:participating` notification level in order to match existing legacy behavior as captured
# in existing specs (notification_service_spec.rb ~ line 507)
add_recipients(assignees, :participating, NotificationReason::ASSIGNED) if assignees
add_labels_subscribers
end
end

26
app/services/notification_service.rb
View file

@ -85,10 +85,11 @@ class NotificationService
recipients.each do |recipient|
mailer.send(
:reassigned_issue_email,
recipient.id,
recipient.user.id,
issue.id,
previous_assignee_ids,
current_user.id
current_user.id,
recipient.reason
).deliver_later
end
end
@ -176,7 +177,7 @@ class NotificationService
action: "resolve_all_discussions")
recipients.each do |recipient|
mailer.resolved_all_discussions_email(recipient.id, merge_request.id, current_user.id).deliver_later
mailer.resolved_all_discussions_email(recipient.user.id, merge_request.id, current_user.id, recipient.reason).deliver_later
end
end
@ -199,7 +200,7 @@ class NotificationService
recipients = NotificationRecipientService.build_new_note_recipients(note)
recipients.each do |recipient|
mailer.send(notify_method, recipient.id, note.id).deliver_later
mailer.send(notify_method, recipient.user.id, note.id).deliver_later
end
end
@ -299,7 +300,7 @@ class NotificationService
recipients = NotificationRecipientService.build_recipients(issue, current_user, action: 'moved')
recipients.map do |recipient|
email = mailer.issue_moved_email(recipient, issue, new_issue, current_user)
email = mailer.issue_moved_email(recipient.user, issue, new_issue, current_user, recipient.reason)
email.deliver_later
email
end
@ -339,16 +340,16 @@ class NotificationService
recipients = NotificationRecipientService.build_recipients(target, target.author, action: "new")
recipients.each do |recipient|
mailer.send(method, recipient.id, target.id).deliver_later
mailer.send(method, recipient.user.id, target.id, recipient.reason).deliver_later
end
end
def new_mentions_in_resource_email(target, new_mentioned_users, current_user, method)
recipients = NotificationRecipientService.build_recipients(target, current_user, action: "new")
recipients = recipients & new_mentioned_users
recipients = recipients.select {|r| new_mentioned_users.include?(r.user) }
recipients.each do |recipient|
mailer.send(method, recipient.id, target.id, current_user.id).deliver_later
mailer.send(method, recipient.user.id, target.id, current_user.id, recipient.reason).deliver_later
end
end
@ -363,7 +364,7 @@ class NotificationService
)
recipients.each do |recipient|
mailer.send(method, recipient.id, target.id, current_user.id).deliver_later
mailer.send(method, recipient.user.id, target.id, current_user.id, recipient.reason).deliver_later
end
end
@ -381,10 +382,11 @@ class NotificationService
recipients.each do |recipient|
mailer.send(
method,
recipient.id,
recipient.user.id,
target.id,
previous_assignee_id,
current_user.id
current_user.id,
recipient.reason
).deliver_later
end
end
@ -408,7 +410,7 @@ class NotificationService
recipients = NotificationRecipientService.build_recipients(target, current_user, action: "reopen")
recipients.each do |recipient|
mailer.send(method, recipient.id, target.id, status, current_user.id).deliver_later
mailer.send(method, recipient.user.id, target.id, status, current_user.id, recipient.reason).deliver_later
end
end

2
app/views/layouts/notify.html.haml
View file

@ -20,7 +20,7 @@
#{link_to "View it on GitLab", @target_url}.
%br
-# Don't link the host in the line below, one link in the email is easier to quickly click than two.
You're receiving this email because of your account on #{Gitlab.config.gitlab.host}.
You're receiving this email because #{notification_reason_text(@reason)}.
If you'd like to receive fewer emails, you can
- if @labels_url
adjust your #{link_to 'label subscriptions', @labels_url}.

2
app/views/layouts/notify.text.erb
View file

@ -9,4 +9,4 @@
<% end -%>
<% end -%>
You're receiving this email because of your account on <%= Gitlab.config.gitlab.host %>.
<%= "You're receiving this email because #{notification_reason_text(@reason)}." %>

5
changelogs/unreleased/41532-email-reason.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Initial work to add notification reason to emails
merge_request: 16160
author: Mario de la Ossa
type: added

30
doc/workflow/notifications.md
View file

@ -104,3 +104,33 @@ You won't receive notifications for Issues, Merge Requests or Milestones
created by yourself. You will only receive automatic notifications when
somebody else comments or adds changes to the ones that you've created or
mentions you.
### Email Headers
Notification emails include headers that provide extra content about the notification received:
| Header | Description |
|-----------------------------|-------------------------------------------------------------------------|
| X-GitLab-Project | The name of the project the notification belongs to |
| X-GitLab-Project-Id | The ID of the project |
| X-GitLab-Project-Path | The path of the project |
| X-GitLab-(Resource)-ID | The ID of the resource the notification is for, where resource is `Issue`, `MergeRequest`, `Commit`, etc|
| X-GitLab-Discussion-ID | Only in comment emails, the ID of the discussion the comment is from |
| X-GitLab-Pipeline-Id | Only in pipeline emails, the ID of the pipeline the notification is for |
| X-GitLab-Reply-Key | A unique token to support reply by email |
| X-GitLab-NotificationReason | The reason for being notified. "mentioned", "assigned", etc |
#### X-GitLab-NotificationReason
This header holds the reason for the notification to have been sent out,
where reason can be `mentioned`, `assigned`, `own_activity`, etc.
Only one reason is sent out according to its priority:
- `own_activity`
- `assigned`
- `mentioned`
The reason in this header will also be shown in the footer of the notification email. For example an email with the
reason `assigned` will have this sentence in the footer:
`"You are receiving this email because you have been assigned an item on {configured GitLab hostname}"`
**Note: Only reasons listed above have been implemented so far**
Further implementation is [being discussed here](https://gitlab.com/gitlab-org/gitlab-ce/issues/42062)

49
spec/mailers/notify_spec.rb
View file

@ -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

62
spec/services/notification_service_spec.rb
View file

@ -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 }

4
spec/support/email_helpers.rb
View file

@ -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

5
spec/workers/new_issue_worker_spec.rb
View file

@ -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

6
spec/workers/new_merge_request_worker_spec.rb
View file

@ -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