Commit dc174e96 authored by lulalala's avatar lulalala Committed by Mark Chao

Notify with email when merge request became unmergeable

Display MR unmergeable reasons
parent 179a1ee7
...@@ -56,6 +56,14 @@ module Emails ...@@ -56,6 +56,14 @@ module Emails
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason)) mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason))
end end
def merge_request_unmergeable_email(recipient_id, merge_request_id, reason = nil)
setup_merge_request_mail(merge_request_id, recipient_id)
@merge_request_presenter = MergeRequestPresenter.new(@merge_request, current_user: current_user)
mail_answer_thread(@merge_request, merge_request_thread_options(@merge_request.author_id, recipient_id, reason))
end
def resolved_all_discussions_email(recipient_id, merge_request_id, resolved_by_user_id, reason = nil) 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) setup_merge_request_mail(merge_request_id, recipient_id)
......
...@@ -125,6 +125,10 @@ class MergeRequest < ActiveRecord::Base ...@@ -125,6 +125,10 @@ class MergeRequest < ActiveRecord::Base
Gitlab::Timeless.timeless(merge_request, &block) Gitlab::Timeless.timeless(merge_request, &block)
end end
after_transition unchecked: :cannot_be_merged do |merge_request, transition|
NotificationService.new.merge_request_unmergeable(merge_request)
end
def check_state?(merge_status) def check_state?(merge_status)
[:unchecked, :cannot_be_merged_recheck].include?(merge_status.to_sym) [:unchecked, :cannot_be_merged_recheck].include?(merge_status.to_sym)
end end
......
...@@ -20,6 +20,17 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated ...@@ -20,6 +20,17 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
end end
end end
def unmergeable_reasons
strong_memoize(:unmergeable_reasons) do
reasons = []
reasons << "no commits" if merge_request.has_no_commits?
reasons << "source branch is missing" unless merge_request.source_branch_exists?
reasons << "target branch is missing" unless merge_request.target_branch_exists?
reasons << "has merge conflicts" unless merge_request.project.repository.can_be_merged?(merge_request.diff_head_sha, merge_request.target_branch)
reasons
end
end
def cancel_merge_when_pipeline_succeeds_path def cancel_merge_when_pipeline_succeeds_path
if can_cancel_merge_when_pipeline_succeeds?(current_user) if can_cancel_merge_when_pipeline_succeeds?(current_user)
cancel_merge_when_pipeline_succeeds_project_merge_request_path(project, merge_request) cancel_merge_when_pipeline_succeeds_project_merge_request_path(project, merge_request)
......
...@@ -18,6 +18,10 @@ module NotificationRecipientService ...@@ -18,6 +18,10 @@ module NotificationRecipientService
Builder::NewNote.new(*a).notification_recipients Builder::NewNote.new(*a).notification_recipients
end end
def self.build_merge_request_unmergeable_recipients(*a)
Builder::MergeRequestUnmergeable.new(*a).notification_recipients
end
module Builder module Builder
class Base class Base
def initialize(*) def initialize(*)
...@@ -330,5 +334,26 @@ module NotificationRecipientService ...@@ -330,5 +334,26 @@ module NotificationRecipientService
note.author note.author
end end
end end
class MergeRequestUnmergeable < Base
attr_reader :target
def initialize(merge_request)
@target = merge_request
end
def build!
target.merge_participants.each do |user|
add_recipients(user, :participating, nil)
end
end
def custom_action
:unmergeable_merge_request
end
def acting_user
nil
end
end
end end
end end
...@@ -149,6 +149,15 @@ class NotificationService ...@@ -149,6 +149,15 @@ class NotificationService
end end
end end
# When a merge request is found to be unmergeable, we should send an email to:
#
# * mr author
# * mr merge user if set
#
def merge_request_unmergeable(merge_request)
merge_request_unmergeable_email(merge_request)
end
# When merge request text is updated, we should send an email to: # When merge request text is updated, we should send an email to:
# #
# * newly mentioned project team members with notification level higher than Participating # * newly mentioned project team members with notification level higher than Participating
...@@ -485,6 +494,14 @@ class NotificationService ...@@ -485,6 +494,14 @@ class NotificationService
end end
end end
def merge_request_unmergeable_email(merge_request)
recipients = NotificationRecipientService.build_merge_request_unmergeable_recipients(merge_request)
recipients.each do |recipient|
mailer.merge_request_unmergeable_email(recipient.user.id, merge_request.id).deliver_later
end
end
def mailer def mailer
Notify Notify
end end
......
%p
Merge Request #{link_to @merge_request.to_reference, project_merge_request_url(@merge_request.target_project, @merge_request)} can no longer be merged due to the following reason(s):
%ul
- @merge_request_presenter.unmergeable_reasons.each do |reason|
%li= reason
Merge Request #{@merge_request.to_reference} can no longer be merged due to the following reason(s):
- @merge_request_presenter.unmergeable_reasons.each do |reason|
* #{reason}
Merge Request url: #{project_merge_request_url(@merge_request.target_project, @merge_request)}
= merge_path_description(@merge_request, 'to')
Author: #{@merge_request.author_name}
Assignee: #{@merge_request.assignee_name}
---
title: When MR becomes unmergeable, notify and create todo for author and merge user
merge_request: 18042
author:
type: added
...@@ -106,6 +106,10 @@ by yourself (except when an issue is due). You will only receive automatic ...@@ -106,6 +106,10 @@ by yourself (except when an issue is due). You will only receive automatic
notifications when somebody else comments or adds changes to the ones that notifications when somebody else comments or adds changes to the ones that
you've created or mentions you. you've created or mentions you.
If a merge request becomes unmergeable, its author will be notified about the cause.
If a user has also set the merge request to automatically merge once pipeline succeeds,
then that user will also be notified.
### Email Headers ### Email Headers
Notification emails include headers that provide extra content about the notification received: Notification emails include headers that provide extra content about the notification received:
......
...@@ -390,6 +390,45 @@ describe Notify do ...@@ -390,6 +390,45 @@ describe Notify do
end end
end end
describe 'that are unmergeable' do
set(:merge_request) do
create(:merge_request, :conflict,
source_project: project,
target_project: project,
author: current_user,
assignee: assignee,
description: 'Awesome description')
end
subject { described_class.merge_request_unmergeable_email(recipient.id, merge_request.id) }
it_behaves_like 'a multiple recipients email'
it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
let(:model) { merge_request }
end
it_behaves_like 'it should show Gmail Actions View Merge request link'
it_behaves_like 'an unsubscribeable thread'
it 'is sent as the merge request author' do
sender = subject.header[:from].addrs[0]
expect(sender.display_name).to eq(merge_request.author.name)
expect(sender.address).to eq(gitlab_sender)
end
it 'has the correct subject and body' do
reasons = %w[foo bar]
allow_any_instance_of(MergeRequestPresenter).to receive(:unmergeable_reasons).and_return(reasons)
aggregate_failures do
is_expected.to have_referable_subject(merge_request, reply: true)
is_expected.to have_body_text(project_merge_request_path(project, merge_request))
reasons.each do |reason|
is_expected.to have_body_text(reason)
end
end
end
end
shared_examples 'a push to an existing merge request' do shared_examples 'a push to an existing merge request' do
let(:push_user) { create(:user) } let(:push_user) { create(:user) }
......
...@@ -70,6 +70,41 @@ describe MergeRequestPresenter do ...@@ -70,6 +70,41 @@ describe MergeRequestPresenter do
end end
end end
describe "#unmergeable_reasons" do
let(:presenter) { described_class.new(resource, current_user: user) }
before do
# Mergeable base state
allow(resource).to receive(:has_no_commits?).and_return(false)
allow(resource).to receive(:source_branch_exists?).and_return(true)
allow(resource).to receive(:target_branch_exists?).and_return(true)
allow(resource.project.repository).to receive(:can_be_merged?).and_return(true)
end
it "handles mergeable request" do
expect(presenter.unmergeable_reasons).to eq([])
end
it "handles no commit" do
allow(resource).to receive(:has_no_commits?).and_return(true)
expect(presenter.unmergeable_reasons).to eq(["no commits"])
end
it "handles branches missing" do
allow(resource).to receive(:source_branch_exists?).and_return(false)
allow(resource).to receive(:target_branch_exists?).and_return(false)
expect(presenter.unmergeable_reasons).to eq(["source branch is missing", "target branch is missing"])
end
it "handles merge conflict" do
allow(resource.project.repository).to receive(:can_be_merged?).and_return(false)
expect(presenter.unmergeable_reasons).to eq(["has merge conflicts"])
end
end
describe '#conflict_resolution_path' do describe '#conflict_resolution_path' do
let(:project) { create :project } let(:project) { create :project }
let(:user) { create :user } let(:user) { create :user }
......
...@@ -1224,6 +1224,32 @@ describe NotificationService, :mailer do ...@@ -1224,6 +1224,32 @@ describe NotificationService, :mailer do
end end
end end
describe '#merge_request_unmergeable' do
it "sends email to merge request author" do
notification.merge_request_unmergeable(merge_request)
should_email(merge_request.author)
expect(email_recipients.size).to eq(1)
end
describe 'when merge_when_pipeline_succeeds is true' do
before do
merge_request.update_attributes(
merge_when_pipeline_succeeds: true,
merge_user: create(:user)
)
end
it "sends email to merge request author and merge_user" do
notification.merge_request_unmergeable(merge_request)
should_email(merge_request.author)
should_email(merge_request.merge_user)
expect(email_recipients.size).to eq(2)
end
end
end
describe '#closed_merge_request' do describe '#closed_merge_request' do
before do before do
update_custom_notification(:close_merge_request, @u_guest_custom, resource: project) update_custom_notification(:close_merge_request, @u_guest_custom, resource: project)
......
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