diff options
Diffstat (limited to 'app')
| -rw-r--r-- | app/mailers/emails/merge_requests.rb | 8 | ||||
| -rw-r--r-- | app/models/merge_request.rb | 29 | ||||
| -rw-r--r-- | app/models/project_services/drone_ci_service.rb | 2 | ||||
| -rw-r--r-- | app/presenters/merge_request_presenter.rb | 11 | ||||
| -rw-r--r-- | app/services/merge_requests/merge_when_pipeline_succeeds_service.rb | 6 | ||||
| -rw-r--r-- | app/services/notification_recipient_service.rb | 25 | ||||
| -rw-r--r-- | app/services/notification_service.rb | 18 | ||||
| -rw-r--r-- | app/services/todo_service.rb | 30 | ||||
| -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 | 11 |
10 files changed, 122 insertions, 24 deletions
diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb index b3f2aeb08ca..5ba3a4a322c 100644 --- a/app/mailers/emails/merge_requests.rb +++ b/app/mailers/emails/merge_requests.rb @@ -56,6 +56,14 @@ module Emails mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason)) end + 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 + 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) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 628c61d5d69..9c4384a6e42 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -104,24 +104,35 @@ class MergeRequest < ActiveRecord::Base state_machine :merge_status, initial: :unchecked do event :mark_as_unchecked do - transition [:can_be_merged, :cannot_be_merged] => :unchecked + transition [:can_be_merged, :unchecked] => :unchecked + transition [:cannot_be_merged, :cannot_be_merged_recheck] => :cannot_be_merged_recheck end event :mark_as_mergeable do - transition [:unchecked, :cannot_be_merged] => :can_be_merged + transition [:unchecked, :cannot_be_merged_recheck] => :can_be_merged end event :mark_as_unmergeable do - transition [:unchecked, :can_be_merged] => :cannot_be_merged + transition [:unchecked, :cannot_be_merged_recheck] => :cannot_be_merged end state :unchecked + state :cannot_be_merged_recheck state :can_be_merged state :cannot_be_merged around_transition do |merge_request, transition, block| Gitlab::Timeless.timeless(merge_request, &block) 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) + end + + def check_state?(merge_status) + [:unchecked, :cannot_be_merged_recheck].include?(merge_status.to_sym) + end end validates :source_project, presence: true, unless: [:allow_broken, :importing?, :closed_without_fork?] @@ -327,6 +338,16 @@ class MergeRequest < ActiveRecord::Base update_column(:merge_jid, jid) end + def merge_participants + participants = [author] + + if merge_when_pipeline_succeeds? && !participants.include?(merge_user) + participants << merge_user + end + + participants + end + def first_commit merge_request_diff ? merge_request_diff.first_commit : compare_commits.first end @@ -602,7 +623,7 @@ class MergeRequest < ActiveRecord::Base end def check_if_can_be_merged - return unless unchecked? && Gitlab::Database.read_write? + return unless self.class.state_machines[:merge_status].check_state?(merge_status) && Gitlab::Database.read_write? can_be_merged = !broken? && project.repository.can_be_merged?(diff_head_sha, target_branch) diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb index 71b10fc6bc1..a4bf427ac0b 100644 --- a/app/models/project_services/drone_ci_service.rb +++ b/app/models/project_services/drone_ci_service.rb @@ -115,6 +115,6 @@ class DroneCiService < CiService def merge_request_valid?(data) data[:object_attributes][:state] == 'opened' && - data[:object_attributes][:merge_status] == 'unchecked' + MergeRequest.state_machines[:merge_status].check_state?(data[:object_attributes][:merge_status]) end end diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb index 4b4132af2d0..ad839d9840a 100644 --- a/app/presenters/merge_request_presenter.rb +++ b/app/presenters/merge_request_presenter.rb @@ -20,6 +20,17 @@ 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/services/merge_requests/merge_when_pipeline_succeeds_service.rb b/app/services/merge_requests/merge_when_pipeline_succeeds_service.rb index 850deb0ac7a..9a4e6eb2e88 100644 --- a/app/services/merge_requests/merge_when_pipeline_succeeds_service.rb +++ b/app/services/merge_requests/merge_when_pipeline_succeeds_service.rb @@ -24,11 +24,7 @@ module MergeRequests pipeline_merge_requests(pipeline) do |merge_request| next unless merge_request.merge_when_pipeline_succeeds? - - unless merge_request.mergeable? - todo_service.merge_request_became_unmergeable(merge_request) - next - end + next unless merge_request.mergeable? merge_request.merge_async(merge_request.merge_user_id, merge_request.merge_params) end diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb index 5658699664d..4fa38665abc 100644 --- a/app/services/notification_recipient_service.rb +++ b/app/services/notification_recipient_service.rb @@ -18,6 +18,10 @@ module NotificationRecipientService Builder::NewNote.new(*a).notification_recipients end + def self.build_merge_request_unmergeable_recipients(*a) + Builder::MergeRequestUnmergeable.new(*a).notification_recipients + end + module Builder class Base def initialize(*) @@ -330,5 +334,26 @@ module NotificationRecipientService note.author 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 diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 55a1735e54b..636cfbf5b45 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -129,6 +129,7 @@ class NotificationService # When create a merge request we should send an email to: # + # * mr author # * mr assignee if their notification level is not Disabled # * project team members with notification level higher then Participating # * watchers of the mr's labels @@ -148,6 +149,15 @@ class NotificationService 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: # # * newly mentioned project team members with notification level higher than Participating @@ -484,6 +494,14 @@ class NotificationService 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 Notify end diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index ffd48e842c2..f91cd03bf5c 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -98,12 +98,12 @@ class TodoService # When a build fails on the HEAD of a merge request we should: # - # * create a todo for author of MR to fix it - # * create a todo for merge_user to keep an eye on it + # * create a todo for each merge participant # def merge_request_build_failed(merge_request) - create_build_failed_todo(merge_request, merge_request.author) - create_build_failed_todo(merge_request, merge_request.merge_user) if merge_request.merge_when_pipeline_succeeds? + merge_request.merge_participants.each do |user| + create_build_failed_todo(merge_request, user) + end end # When a new commit is pushed to a merge request we should: @@ -116,20 +116,22 @@ class TodoService # When a build is retried to a merge request we should: # - # * mark all pending todos related to the merge request for the author as done - # * mark all pending todos related to the merge request for the merge_user as done + # * mark all pending todos related to the merge request as done for each merge participant # def merge_request_build_retried(merge_request) - mark_pending_todos_as_done(merge_request, merge_request.author) - mark_pending_todos_as_done(merge_request, merge_request.merge_user) if merge_request.merge_when_pipeline_succeeds? + merge_request.merge_participants.each do |user| + mark_pending_todos_as_done(merge_request, user) + end end - # When a merge request could not be automatically merged due to its unmergeable state we should: + # When a merge request could not be merged due to its unmergeable state we should: # - # * create a todo for a merge_user + # * create a todo for each merge participant # def merge_request_became_unmergeable(merge_request) - create_unmergeable_todo(merge_request, merge_request.merge_user) if merge_request.merge_when_pipeline_succeeds? + merge_request.merge_participants.each do |user| + create_unmergeable_todo(merge_request, user) + end end # When create a note we should: @@ -275,9 +277,9 @@ class TodoService create_todos(todo_author, attributes) end - def create_unmergeable_todo(merge_request, merge_user) - attributes = attributes_for_todo(merge_request.project, merge_request, merge_user, Todo::UNMERGEABLE) - create_todos(merge_user, attributes) + def create_unmergeable_todo(merge_request, todo_author) + attributes = attributes_for_todo(merge_request.project, merge_request, todo_author, Todo::UNMERGEABLE) + create_todos(todo_author, attributes) end def attributes_for_target(target) diff --git a/app/views/notify/merge_request_unmergeable_email.html.haml b/app/views/notify/merge_request_unmergeable_email.html.haml new file mode 100644 index 00000000000..e84905a5fad --- /dev/null +++ b/app/views/notify/merge_request_unmergeable_email.html.haml @@ -0,0 +1,6 @@ +%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 #{pluralize(@reasons, 'reason')}: + + %ul + - @reasons.each do |reason| + %li= reason diff --git a/app/views/notify/merge_request_unmergeable_email.text.haml b/app/views/notify/merge_request_unmergeable_email.text.haml new file mode 100644 index 00000000000..4f0901c761a --- /dev/null +++ b/app/views/notify/merge_request_unmergeable_email.text.haml @@ -0,0 +1,11 @@ +Merge Request #{@merge_request.to_reference} can no longer be merged due to the following #{pluralize(@reasons, 'reason')}: + +- @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} |