diff --git a/app/observers/merge_request_observer.rb b/app/observers/merge_request_observer.rb
index 15214016b0d15f6d48c27ced5bd4da7ebcd5e98e..10f30155740ce48cad6b288b8523a2e0b9496257 100644
--- a/app/observers/merge_request_observer.rb
+++ b/app/observers/merge_request_observer.rb
@@ -7,6 +7,12 @@ class MergeRequestObserver < BaseObserver
def after_close(merge_request, transition)
Note.create_status_change_note(merge_request, current_user, merge_request.state)
+
+ notification.close_mr(merge_request, current_user)
+ end
+
+ def after_merge(merge_request, transition)
+ notification.merge_mr(merge_request, current_user)
end
def after_reopen(merge_request, transition)
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 47d535fec771d956d6ffa0f1e7e0fbbdc7b7383f..82ecd25eb1cf2155fa012e16d83265391538598a 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -17,7 +17,21 @@ class NotificationService
end
end
- # TODO: When we close an issue we should send next emails:
+ # When create an issue we should send next emails:
+ #
+ # * issue assignee if his notification level is not Disabled
+ # * project team members with notification level higher then Participating
+ #
+ def new_issue(issue, current_user)
+ recipients = reject_muted_users([issue.assignee])
+ recipients = recipients.concat(project_watchers(issue.project)).uniq
+
+ recipients.each do |recipient|
+ Notify.delay.new_issue_email(recipient.id, issue.id)
+ end
+ end
+
+ # When we close an issue we should send next emails:
#
# * issue author if his notification level is not Disabled
# * issue assignee if his notification level is not Disabled
@@ -26,6 +40,9 @@ class NotificationService
def close_issue(issue, current_user)
recipients = reject_muted_users([issue.author, issue.assignee])
+ # Add watchers to email list
+ recipients = recipients.concat(project_watchers(issue.project)).uniq
+
# Dont send email to me when I close an issue
recipients.delete(current_user)
@@ -43,30 +60,17 @@ class NotificationService
reassign_email(issue, current_user, 'reassigned_issue_email')
end
- # When create an issue we should send next emails:
- #
- # * issue assignee if his notification level is not Disabled
- #
- def new_issue(issue, current_user)
-
- if issue.assignee && issue.assignee != current_user
- # skip if assignee notification disabled
- return true if issue.assignee.notification.disabled?
-
- Notify.delay.new_issue_email(issue.id)
- end
- end
# When create a merge request we should send next emails:
#
# * mr assignee if his notification level is not Disabled
#
def new_merge_request(merge_request, current_user)
- if merge_request.assignee && merge_request.assignee != current_user
- # skip if assignee notification disabled
- return true if merge_request.assignee.notification.disabled?
+ recipients = reject_muted_users([merge_request.assignee])
+ recipients = recipients.concat(project_watchers(merge_request.project)).uniq
- Notify.delay.new_merge_request_email(merge_request.id)
+ recipients.each do |recipient|
+ Notify.delay.new_merge_request_email(recipient.id, merge_request.id)
end
end
@@ -79,6 +83,36 @@ class NotificationService
reassign_email(merge_request, current_user, 'reassigned_merge_request_email')
end
+ # When we close a merge request we should send next emails:
+ #
+ # * merge_request author if his notification level is not Disabled
+ # * merge_request assignee if his notification level is not Disabled
+ # * project team members with notification level higher then Participating
+ #
+ def close_mr(merge_request)
+ recipients = reject_muted_users([merge_request.author, merge_request.assignee])
+ recipients = recipients.concat(project_watchers(merge_request.project)).uniq
+
+ recipients.each do |recipient|
+ Notify.delay.closed_merge_request_email(recipient.id, merge_request.id)
+ end
+ end
+
+ # When we merge a merge request we should send next emails:
+ #
+ # * merge_request author if his notification level is not Disabled
+ # * merge_request assignee if his notification level is not Disabled
+ # * project team members with notification level higher then Participating
+ #
+ def merge_mr(merge_request)
+ recipients = reject_muted_users([merge_request.author, merge_request.assignee])
+ recipients = recipients.concat(project_watchers(merge_request.project)).uniq
+
+ recipients.each do |recipient|
+ Notify.delay.merged_merge_request_email(recipient.id, merge_request.id)
+ end
+ end
+
# Notify new user with email after creation
def new_user(user)
# Dont email omniauth created users
@@ -119,6 +153,11 @@ class NotificationService
protected
+ # Get project users with WATCH notification level
+ def project_watchers(project)
+ project.users.where(notification_level: Notification::N_WATCH)
+ end
+
# Remove users with disabled notifications from array
# Also remove duplications and nil recipients
def reject_muted_users(users)
@@ -130,6 +169,9 @@ class NotificationService
def reassign_email(target, current_user, method)
recipients = User.where(id: [target.assignee_id, target.assignee_id_was])
+ # Add watchers to email list
+ recipients = recipients.concat(project_watchers(target.project))
+
# reject users with disabled notifications
recipients = reject_muted_users(recipients)
diff --git a/app/views/notify/closed_merge_request_email.html.haml b/app/views/notify/closed_merge_request_email.html.haml
new file mode 100644
index 0000000000000000000000000000000000000000..c0b08fccda21edb25806147b80370f00d0a8edb2
--- /dev/null
+++ b/app/views/notify/closed_merge_request_email.html.haml
@@ -0,0 +1,9 @@
+%p
+ = "Merge Request #{@merge_request.id} was closed"
+%p
+ = link_to_gfm truncate(@merge_request.title, length: 40), project_merge_request_url(@merge_request.project, @merge_request)
+%p
+ Branches: #{@merge_request.source_branch} → #{@merge_request.target_branch}
+%p
+ Assignee: #{@merge_request.author_name} → #{@merge_request.assignee_name}
+
diff --git a/app/views/notify/merged_merge_request_email.html.haml b/app/views/notify/merged_merge_request_email.html.haml
new file mode 100644
index 0000000000000000000000000000000000000000..2b8cc030b234dedfbae73b119c0ebf07046864f7
--- /dev/null
+++ b/app/views/notify/merged_merge_request_email.html.haml
@@ -0,0 +1,9 @@
+%p
+ = "Merge Request #{@merge_request.id} was merged"
+%p
+ = link_to_gfm truncate(@merge_request.title, length: 40), project_merge_request_url(@merge_request.project, @merge_request)
+%p
+ Branches: #{@merge_request.source_branch} → #{@merge_request.target_branch}
+%p
+ Assignee: #{@merge_request.author_name} → #{@merge_request.assignee_name}
+
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index e9e4770d9a8f7d24276e99c89f3a1aa2bab5025b..6fe18bb798a49b0f40888c468455229530e4f2b2 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -23,6 +23,10 @@ describe NotificationService do
describe 'Issues' do
let(:issue) { create :issue, assignee: create(:user) }
+ before do
+ build_team(issue.project)
+ end
+
describe :new_issue do
it 'should sent email to issue assignee' do
Notify.should_receive(:new_issue_email).with(issue.id)
@@ -31,16 +35,41 @@ describe NotificationService do
end
describe :reassigned_issue do
- it 'should sent email to issue old assignee and new issue assignee' do
- Notify.should_receive(:reassigned_issue_email)
- notification.reassigned_issue(issue, issue.author)
+ it 'should email new assignee' do
+ should_email(issue.assignee_id)
+ should_email(@u_watcher.id)
+ should_not_email(@u_participating.id)
+ should_not_email(@u_disabled.id)
+
+ notification.reassigned_issue(issue, @u_disabled)
+ end
+
+ def should_email(user_id)
+ Notify.should_receive(:reassigned_issue_email).with(user_id, issue.id, issue.assignee_id)
+ end
+
+ def should_not_email(user_id)
+ Notify.should_not_receive(:reassigned_issue_email).with(user_id, issue.id, issue.assignee_id)
end
end
describe :close_issue do
it 'should sent email to issue assignee and issue author' do
- Notify.should_receive(:issue_status_changed_email)
- notification.close_issue(issue, issue.author)
+ should_email(issue.assignee_id)
+ should_email(issue.author_id)
+ should_email(@u_watcher.id)
+ should_not_email(@u_participating.id)
+ should_not_email(@u_disabled.id)
+
+ notification.close_issue(issue, @u_disabled)
+ end
+
+ def should_email(user_id)
+ Notify.should_receive(:issue_status_changed_email).with(user_id, issue.id, issue.assignee_id)
+ end
+
+ def should_not_email(user_id)
+ Notify.should_not_receive(:issue_status_changed_email).with(user_id, issue.id, issue.assignee_id)
end
end
end
@@ -48,16 +77,90 @@ describe NotificationService do
describe 'Merge Requests' do
let(:merge_request) { create :merge_request, assignee: create(:user) }
+ before do
+ build_team(merge_request.project)
+ end
+
describe :new_merge_request do
- it 'should send email to merge_request assignee' do
+ it do
+ should_email(merge_request.assignee_id)
+ should_email(@u_watcher.id)
+ should_not_email(@u_participating.id)
+ should_not_email(@u_disabled.id)
+ notification.new_merge_request(merge_request, @u_disabled)
+ end
+
+ def should_email(user_id)
Notify.should_receive(:new_merge_request_email).with(merge_request.id)
- notification.new_merge_request(merge_request, merge_request.author)
end
- it 'should not send email to merge_request assignee if he is current_user' do
+ def should_not_email(user_id)
Notify.should_not_receive(:new_merge_request_email).with(merge_request.id)
- notification.new_merge_request(merge_request, merge_request.assignee)
end
end
+
+ describe :reassigned_merge_request do
+ it do
+ should_email(merge_request.assignee_id)
+ should_email(@u_watcher.id)
+ should_not_email(@u_participating.id)
+ should_not_email(@u_disabled.id)
+ notification.reassigned_merge_request(merge_request, merge_request.author)
+ end
+
+ def should_email(user_id)
+ Notify.should_receive(:reassigned_merge_request_email).with(user_id, merge_request.id, merge_request.assignee_id)
+ end
+
+ def should_not_email(user_id)
+ Notify.should_not_receive(:reassigned_merge_request_email).with(user_id, merge_request.id, merge_request.assignee_id)
+ end
+ end
+
+ describe :closed_merge_request do
+ it do
+ should_email(merge_request.assignee_id)
+ should_email(@u_watcher.id)
+ should_not_email(@u_participating.id)
+ should_not_email(@u_disabled.id)
+ notification.close_mr(merge_request)
+ end
+
+ def should_email(user_id)
+ Notify.should_receive(:closed_merge_request_email).with(user_id, merge_request.id)
+ end
+
+ def should_not_email(user_id)
+ Notify.should_not_receive(:closed_merge_request_email).with(user_id, merge_request.id)
+ end
+ end
+
+ describe :merged_merge_request do
+ it do
+ should_email(merge_request.assignee_id)
+ should_email(@u_watcher.id)
+ should_not_email(@u_participating.id)
+ should_not_email(@u_disabled.id)
+ notification.merge_mr(merge_request)
+ end
+
+ def should_email(user_id)
+ Notify.should_receive(:merged_merge_request_email).with(user_id, merge_request.id)
+ end
+
+ def should_not_email(user_id)
+ Notify.should_not_receive(:merged_merge_request_email).with(user_id, merge_request.id)
+ end
+ end
+ end
+
+ def build_team(project)
+ @u_watcher = create(:user, notification_level: Notification::N_WATCH)
+ @u_participating = create(:user, notification_level: Notification::N_PARTICIPATING)
+ @u_disabled = create(:user, notification_level: Notification::N_DISABLED)
+
+ project.team << [@u_watcher, :master]
+ project.team << [@u_participating, :master]
+ project.team << [@u_disabled, :master]
end
end