Commit d57b6519 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'ee-ccr/51520_change_milestone_email' into 'master'

Port of ccr/51520_change_milestone_email to EE

See merge request gitlab-org/gitlab-ee!7953
parents 43daa7c8 2ac20bc7
......@@ -45,6 +45,20 @@ module Emails
mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id, reason))
end
def removed_milestone_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, reason))
end
def changed_milestone_issue_email(recipient_id, issue_id, milestone, updated_by_user_id, reason = nil)
setup_issue_mail(issue_id, recipient_id)
@milestone = milestone
@milestone_url = milestone_url(@milestone)
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, reason = nil)
setup_issue_mail(issue_id, recipient_id)
......
......@@ -42,6 +42,20 @@ module Emails
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason))
end
def removed_milestone_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, reason))
end
def changed_milestone_merge_request_email(recipient_id, merge_request_id, milestone, updated_by_user_id, reason = nil)
setup_merge_request_mail(merge_request_id, recipient_id)
@milestone = milestone
@milestone_url = milestone_url(@milestone)
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, reason = nil)
setup_merge_request_mail(merge_request_id, recipient_id)
......
......@@ -70,6 +70,14 @@ class NotifyPreview < ActionMailer::Preview
Notify.issue_status_changed_email(user.id, issue.id, 'closed', user.id).message
end
def removed_milestone_issue_email
Notify.removed_milestone_issue_email(user.id, issue.id, user.id)
end
def changed_milestone_issue_email
Notify.changed_milestone_issue_email(user.id, issue.id, milestone, user.id)
end
def closed_merge_request_email
Notify.closed_merge_request_email(user.id, issue.id, user.id).message
end
......@@ -82,6 +90,14 @@ class NotifyPreview < ActionMailer::Preview
Notify.merged_merge_request_email(user.id, merge_request.id, user.id).message
end
def removed_milestone_merge_request_email
Notify.removed_milestone_merge_request_email(user.id, merge_request.id, user.id)
end
def changed_milestone_merge_request_email
Notify.changed_milestone_merge_request_email(user.id, merge_request.id, milestone, user.id)
end
def member_access_denied_email
Notify.member_access_denied_email('project', project.id, user.id).message
end
......@@ -145,6 +161,10 @@ class NotifyPreview < ActionMailer::Preview
@merge_request ||= project.merge_requests.first
end
def milestone
@milestone ||= issue.milestone
end
def pipeline
@pipeline = Ci::Pipeline.last
end
......
......@@ -5,6 +5,14 @@ class IssuableBaseService < BaseService
private
attr_accessor :params, :skip_milestone_email
def initialize(project, user = nil, params = {})
super
@skip_milestone_email = @params.delete(:skip_milestone_email)
end
def filter_params(issuable)
ability_name = :"admin_#{issuable.to_ability_name}"
......
......@@ -49,6 +49,8 @@ module Issues
notification_service.async.relabeled_issue(issue, added_labels, current_user)
end
handle_milestone_change(issue)
added_mentions = issue.mentioned_users - old_mentioned_users
if added_mentions.present?
......@@ -92,6 +94,18 @@ module Issues
private
def handle_milestone_change(issue)
return if skip_milestone_email
return unless issue.previous_changes.include?('milestone_id')
if issue.milestone.nil?
notification_service.async.removed_milestone_issue(issue, current_user)
else
notification_service.async.changed_milestone_issue(issue, issue.milestone, current_user)
end
end
# rubocop: disable CodeReuse/ActiveRecord
def get_issue_if_allowed(id, board_group_id = nil)
return unless id
......
......@@ -60,6 +60,8 @@ module MergeRequests
merge_request.mark_as_unchecked
end
handle_milestone_change(merge_request)
added_labels = merge_request.labels - old_labels
if added_labels.present?
notification_service.async.relabeled_merge_request(
......@@ -107,6 +109,18 @@ module MergeRequests
private
def handle_milestone_change(merge_request)
return if skip_milestone_email
return unless merge_request.previous_changes.include?('milestone_id')
if merge_request.milestone.nil?
notification_service.async.removed_milestone_merge_request(merge_request, current_user)
else
notification_service.async.changed_milestone_merge_request(merge_request, merge_request.milestone, current_user)
end
end
def create_branch_change_note(issuable, branch_type, old_branch, new_branch)
SystemNoteService.change_branch(
issuable, issuable.project, current_user, branch_type,
......
......@@ -4,7 +4,7 @@ module Milestones
class DestroyService < Milestones::BaseService
def execute(milestone)
Milestone.transaction do
update_params = { milestone: nil }
update_params = { milestone: nil, skip_milestone_email: true }
milestone.issues.each do |issue|
Issues::UpdateService.new(parent, current_user, update_params).execute(issue)
......
......@@ -131,6 +131,14 @@ class NotificationService
relabeled_resource_email(issue, added_labels, current_user, :relabeled_issue_email)
end
def removed_milestone_issue(issue, current_user)
removed_milestone_resource_email(issue, current_user, :removed_milestone_issue_email)
end
def changed_milestone_issue(issue, new_milestone, current_user)
changed_milestone_resource_email(issue, new_milestone, current_user, :changed_milestone_issue_email)
end
# When create a merge request we should send an email to:
#
# * mr author
......@@ -140,9 +148,6 @@ class NotificationService
# * users with custom level checked with "new merge request"
#
# In EE, approvers of the merge request are also included
#
# In EE, approvers of the merge request are also included
#
def new_merge_request(merge_request, current_user)
new_resource_email(merge_request, :new_merge_request_email)
end
......@@ -212,6 +217,14 @@ class NotificationService
relabeled_resource_email(merge_request, added_labels, current_user, :relabeled_merge_request_email)
end
def removed_milestone_merge_request(merge_request, current_user)
removed_milestone_resource_email(merge_request, current_user, :removed_milestone_merge_request_email)
end
def changed_milestone_merge_request(merge_request, new_milestone, current_user)
changed_milestone_resource_email(merge_request, new_milestone, current_user, :changed_milestone_merge_request_email)
end
def close_mr(merge_request, current_user)
close_resource_email(merge_request, current_user, :closed_merge_request_email)
end
......@@ -504,6 +517,30 @@ class NotificationService
end
end
def removed_milestone_resource_email(target, current_user, method)
recipients = NotificationRecipientService.build_recipients(
target,
current_user,
action: 'removed_milestone'
)
recipients.each do |recipient|
mailer.send(method, recipient.user.id, target.id, current_user.id).deliver_later
end
end
def changed_milestone_resource_email(target, milestone, current_user, method)
recipients = NotificationRecipientService.build_recipients(
target,
current_user,
action: 'changed_milestone'
)
recipients.each do |recipient|
mailer.send(method, recipient.user.id, target.id, milestone, current_user.id).deliver_later
end
end
def reopen_resource_email(target, current_user, method, status)
recipients = NotificationRecipientService.build_recipients(target, current_user, action: "reopen")
......
%p
Milestone changed to
%strong= link_to(@milestone.name, @milestone_url)
Milestone changed to <%= @milestone.name %> ( <%= @milestone_url %> )
%p
Milestone changed to
%strong= link_to(@milestone.name, @milestone_url)
Milestone changed to <%= @milestone.name %> ( <%= @milestone_url %> )
---
title: Add email for milestone change
merge_request: 22279
author:
type: added
......@@ -92,12 +92,16 @@ In most of the below cases, the notification will be sent to:
| Reassign issue | The above, plus the old assignee |
| Reopen issue | |
| Due issue | Participants and Custom notification level with this event selected |
| Change milestone issue | Subscribers, participants mentioned, and Custom notification level with this event selected |
| Remove milestone issue | Subscribers, participants mentioned, and Custom notification level with this event selected |
| New merge request | |
| Push to merge request | Participants and Custom notification level with this event selected |
| Reassign merge request | The above, plus the old assignee |
| Close merge request | |
| Reopen merge request | |
| Merge merge request | |
| Change milestone merge request | Subscribers, participants mentioned, and Custom notification level with this event selected |
| Remove milestone merge request | Subscribers, participants mentioned, and Custom notification level with this event selected |
| New comment | The above, plus anyone mentioned by `@username` in the comment, with notification level "Mention" or higher |
| Failed pipeline | The author of the pipeline |
| Successful pipeline | The author of the pipeline, if they have the custom notification setting for successful pipelines set |
......
......@@ -343,7 +343,42 @@ describe Issues::UpdateService, :mailer do
end
end
context 'when the milestone change' do
context 'when the milestone is removed' do
let!(:non_subscriber) { create(:user) }
let!(:subscriber) do
create(:user) do |u|
issue.toggle_subscription(u, project)
project.add_developer(u)
end
end
it_behaves_like 'system notes for milestones'
it 'sends notifications for subscribers of changed milestone' do
issue.milestone = create(:milestone)
issue.save
perform_enqueued_jobs do
update_issue(milestone_id: "")
end
should_email(subscriber)
should_not_email(non_subscriber)
end
end
context 'when the milestone is changed' do
let!(:non_subscriber) { create(:user) }
let!(:subscriber) do
create(:user) do |u|
issue.toggle_subscription(u, project)
project.add_developer(u)
end
end
it 'marks todos as done' do
update_issue(milestone: create(:milestone))
......@@ -351,6 +386,15 @@ describe Issues::UpdateService, :mailer do
end
it_behaves_like 'system notes for milestones'
it 'sends notifications for subscribers of changed milestone' do
perform_enqueued_jobs do
update_issue(milestone: create(:milestone))
end
should_email(subscriber)
should_not_email(non_subscriber)
end
end
context 'when the labels change' do
......@@ -374,7 +418,7 @@ describe Issues::UpdateService, :mailer do
let!(:non_subscriber) { create(:user) }
let!(:subscriber) do
create(:user).tap do |u|
create(:user) do |u|
label.toggle_subscription(u, project)
project.add_developer(u)
end
......
......@@ -344,7 +344,42 @@ describe MergeRequests::UpdateService, :mailer do
end
end
context 'when the milestone change' do
context 'when the milestone is removed' do
let!(:non_subscriber) { create(:user) }
let!(:subscriber) do
create(:user) do |u|
merge_request.toggle_subscription(u, project)
project.add_developer(u)
end
end
it_behaves_like 'system notes for milestones'
it 'sends notifications for subscribers of changed milestone' do
merge_request.milestone = create(:milestone)
merge_request.save
perform_enqueued_jobs do
update_merge_request(milestone_id: "")
end
should_email(subscriber)
should_not_email(non_subscriber)
end
end
context 'when the milestone is changed' do
let!(:non_subscriber) { create(:user) }
let!(:subscriber) do
create(:user) do |u|
merge_request.toggle_subscription(u, project)
project.add_developer(u)
end
end
it 'marks pending todos as done' do
update_merge_request({ milestone: create(:milestone) })
......@@ -352,6 +387,15 @@ describe MergeRequests::UpdateService, :mailer do
end
it_behaves_like 'system notes for milestones'
it 'sends notifications for subscribers of changed milestone' do
perform_enqueued_jobs do
update_merge_request(milestone: create(:milestone))
end
should_email(subscriber)
should_not_email(non_subscriber)
end
end
context 'when the labels change' do
......
......@@ -13,6 +13,54 @@ describe NotificationService, :mailer do
end
end
shared_examples 'altered milestone notification on issue' do
it 'sends the email to the correct people' do
should_email(subscriber_to_new_milestone)
issue.assignees.each do |a|
should_email(a)
end
should_email(@u_watcher)
should_email(@u_guest_watcher)
should_email(@u_participant_mentioned)
should_email(@subscriber)
should_email(@subscribed_participant)
should_email(@watcher_and_subscriber)
should_not_email(@u_guest_custom)
should_not_email(@u_committer)
should_not_email(@unsubscriber)
should_not_email(@u_participating)
should_not_email(@u_lazy_participant)
should_not_email(issue.author)
should_not_email(@u_disabled)
should_not_email(@u_custom_global)
should_not_email(@u_mentioned)
end
end
shared_examples 'altered milestone notification on merge request' do
it 'sends the email to the correct people' do
should_email(subscriber_to_new_milestone)
merge_request.assignees.each do |a|
should_email(a)
end
should_email(@u_watcher)
should_email(@u_guest_watcher)
should_email(@u_participant_mentioned)
should_email(@subscriber)
should_email(@subscribed_participant)
should_email(@watcher_and_subscriber)
should_not_email(@u_guest_custom)
should_not_email(@u_committer)
should_not_email(@unsubscriber)
should_not_email(@u_participating)
should_not_email(@u_lazy_participant)
should_not_email(merge_request.author)
should_not_email(@u_disabled)
should_not_email(@u_custom_global)
should_not_email(@u_mentioned)
end
end
shared_examples 'notifications for new mentions' do
it 'sends no emails when no new mentions are present' do
send_notifications
......@@ -952,6 +1000,96 @@ describe NotificationService, :mailer do
end
end
describe '#removed_milestone_issue' do
it_behaves_like 'altered milestone notification on issue' do
let(:milestone) { create(:milestone, project: project, issues: [issue]) }
let!(:subscriber_to_new_milestone) { create(:user) { |u| issue.toggle_subscription(u, project) } }
before do
notification.removed_milestone_issue(issue, issue.author)
end
end
context 'confidential issues' do
let(:author) { create(:user) }
let(:assignee) { create(:user) }
let(:non_member) { create(:user) }
let(:member) { create(:user) }
let(:guest) { create(:user) }
let(:admin) { create(:admin) }
let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignees: [assignee]) }
let(:milestone) { create(:milestone, project: project, issues: [confidential_issue]) }
it "emails subscribers of the issue's milestone that can read the issue" do
project.add_developer(member)
project.add_guest(guest)
confidential_issue.subscribe(non_member, project)
confidential_issue.subscribe(author, project)
confidential_issue.subscribe(assignee, project)
confidential_issue.subscribe(member, project)
confidential_issue.subscribe(guest, project)
confidential_issue.subscribe(admin, project)
reset_delivered_emails!
notification.removed_milestone_issue(confidential_issue, @u_disabled)
should_not_email(non_member)
should_not_email(guest)
should_email(author)
should_email(assignee)
should_email(member)
should_email(admin)
end
end
end
describe '#changed_milestone_issue' do
it_behaves_like 'altered milestone notification on issue' do
let(:new_milestone) { create(:milestone, project: project, issues: [issue]) }
let!(:subscriber_to_new_milestone) { create(:user) { |u| issue.toggle_subscription(u, project) } }
before do
notification.changed_milestone_issue(issue, new_milestone, issue.author)
end
end
context 'confidential issues' do
let(:author) { create(:user) }
let(:assignee) { create(:user) }
let(:non_member) { create(:user) }
let(:member) { create(:user) }
let(:guest) { create(:user) }
let(:admin) { create(:admin) }
let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignees: [assignee]) }
let(:new_milestone) { create(:milestone, project: project, issues: [confidential_issue]) }
it "emails subscribers of the issue's milestone that can read the issue" do
project.add_developer(member)
project.add_guest(guest)
confidential_issue.subscribe(non_member, project)
confidential_issue.subscribe(author, project)
confidential_issue.subscribe(assignee, project)
confidential_issue.subscribe(member, project)
confidential_issue.subscribe(guest, project)
confidential_issue.subscribe(admin, project)
reset_delivered_emails!
notification.changed_milestone_issue(confidential_issue, new_milestone, @u_disabled)
should_not_email(non_member)
should_not_email(guest)
should_email(author)
should_email(assignee)
should_email(member)
should_email(admin)
end
end
end
describe '#close_issue' do
before do
update_custom_notification(:close_issue, @u_guest_custom, resource: project)
......@@ -1346,6 +1484,28 @@ describe NotificationService, :mailer do
end
end
describe '#removed_milestone_merge_request' do
it_behaves_like 'altered milestone notification on merge request' do
let(:milestone) { create(:milestone, project: project, merge_requests: [merge_request]) }
let!(:subscriber_to_new_milestone) { create(:user) { |u| merge_request.toggle_subscription(u, project) } }
before do
notification.removed_milestone_merge_request(merge_request, merge_request.author)
end
end
end
describe '#changed_milestone_merge_request' do
it_behaves_like 'altered milestone notification on merge request' do
let(:new_milestone) { create(:milestone, project: project, merge_requests: [merge_request]) }
let!(:subscriber_to_new_milestone) { create(:user) { |u| merge_request.toggle_subscription(u, project) } }
before do
notification.changed_milestone_merge_request(merge_request, new_milestone, merge_request.author)
end
end
end
describe '#merge_request_unmergeable' do
it "sends email to merge request author" do
notification.merge_request_unmergeable(merge_request)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment