diff options
author | Mark Chao <mchao@gitlab.com> | 2018-06-07 23:56:59 +0900 |
---|---|---|
committer | Mark Chao <mchao@gitlab.com> | 2018-06-20 23:27:17 +0800 |
commit | 5b994b8199a3679b6f5ef0d00edce22ea9662664 (patch) | |
tree | 6574c99eb3b963d0518e079f041552bb10367756 /app | |
parent | 222284a6bf9348b30b5ce2b1b04370163e2120e5 (diff) | |
download | gitlab-ce-5b994b8199a3679b6f5ef0d00edce22ea9662664.tar.gz |
Notify only when unmergeable due to conflict
There is still the edge case when 'no commits' changes to 'conflict'
would not trigger notification, which we ignore for now.
Calling can_be_merged? can cause exception (e.g. non-UTF8)
Ignore those by rescueing.
Remove unmergeable_reason as now only conflict is notified
Update spec
Diffstat (limited to 'app')
-rw-r--r-- | app/mailers/emails/merge_requests.rb | 2 | ||||
-rw-r--r-- | app/models/merge_request.rb | 13 | ||||
-rw-r--r-- | app/presenters/merge_request_presenter.rb | 11 | ||||
-rw-r--r-- | app/views/notify/merge_request_unmergeable_email.html.haml | 6 | ||||
-rw-r--r-- | app/views/notify/merge_request_unmergeable_email.text.haml | 5 |
5 files changed, 13 insertions, 24 deletions
diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb index 5ba3a4a322c..70509e9066d 100644 --- a/app/mailers/emails/merge_requests.rb +++ b/app/mailers/emails/merge_requests.rb @@ -59,8 +59,6 @@ module Emails def merge_request_unmergeable_email(recipient_id, merge_request_id, reason = nil) setup_merge_request_mail(merge_request_id, recipient_id) - @reasons = MergeRequestPresenter.new(@merge_request, current_user: current_user).unmergeable_reasons - mail_answer_thread(@merge_request, merge_request_thread_options(@merge_request.author_id, recipient_id, reason)) end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 324065c1162..4b78ba1029f 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -128,8 +128,17 @@ class MergeRequest < ActiveRecord::Base end after_transition unchecked: :cannot_be_merged do |merge_request, transition| - NotificationService.new.merge_request_unmergeable(merge_request) - TodoService.new.merge_request_became_unmergeable(merge_request) + begin + # Merge request can become unmergeable due to many reasons. + # We only notify if it is due to conflict. + unless merge_request.project.repository.can_be_merged?(merge_request.diff_head_sha, merge_request.target_branch) + NotificationService.new.merge_request_unmergeable(merge_request) + TodoService.new.merge_request_became_unmergeable(merge_request) + end + rescue Gitlab::Git::CommandError + # Checking mergeability can trigger exception, e.g. non-utf8 + # We ignore this type of errors. + end end def check_state?(merge_status) diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb index 8d466c33510..eb54ab2cda6 100644 --- a/app/presenters/merge_request_presenter.rb +++ b/app/presenters/merge_request_presenter.rb @@ -20,17 +20,6 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated 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 if can_cancel_merge_when_pipeline_succeeds?(current_user) cancel_merge_when_pipeline_succeeds_project_merge_request_path(project, merge_request) diff --git a/app/views/notify/merge_request_unmergeable_email.html.haml b/app/views/notify/merge_request_unmergeable_email.html.haml index 578fa1fbce7..7ec0c1ef390 100644 --- a/app/views/notify/merge_request_unmergeable_email.html.haml +++ b/app/views/notify/merge_request_unmergeable_email.html.haml @@ -1,6 +1,2 @@ %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'.pluralize(@reasons.count)}: - - %ul - - @reasons.each do |reason| - %li= reason + 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 conflict. diff --git a/app/views/notify/merge_request_unmergeable_email.text.haml b/app/views/notify/merge_request_unmergeable_email.text.haml index e4f9f1bf5e7..dcdd6db69d6 100644 --- a/app/views/notify/merge_request_unmergeable_email.text.haml +++ b/app/views/notify/merge_request_unmergeable_email.text.haml @@ -1,7 +1,4 @@ -Merge Request #{@merge_request.to_reference} can no longer be merged due to the following #{'reason'.pluralize(@reasons.count)}: - -- @reasons.each do |reason| - * #{reason} +Merge Request #{@merge_request.to_reference} can no longer be merged due to conflict. Merge Request url: #{project_merge_request_url(@merge_request.target_project, @merge_request)} |