Skip to content
Snippets Groups Projects
Commit 85e0b994 authored by twonegatives's avatar twonegatives
Browse files

Notify the user who set auto-merge when merge conflict occurs

parent f14228f0
No related branches found
No related tags found
No related merge requests found
Loading
@@ -14,6 +14,7 @@ module TodosHelper
Loading
@@ -14,6 +14,7 @@ module TodosHelper
when Todo::BUILD_FAILED then 'The build failed for' when Todo::BUILD_FAILED then 'The build failed for'
when Todo::MARKED then 'added a todo for' when Todo::MARKED then 'added a todo for'
when Todo::APPROVAL_REQUIRED then 'set you as an approver for' when Todo::APPROVAL_REQUIRED then 'set you as an approver for'
when Todo::UNMERGEABLE then 'Could not merge'
end end
end end
   
Loading
Loading
Loading
@@ -91,6 +91,10 @@ class MergeRequest < ActiveRecord::Base
Loading
@@ -91,6 +91,10 @@ class MergeRequest < ActiveRecord::Base
around_transition do |merge_request, transition, block| around_transition do |merge_request, transition, block|
Gitlab::Timeless.timeless(merge_request, &block) Gitlab::Timeless.timeless(merge_request, &block)
end end
after_transition unchecked: :cannot_be_merged do |merge_request, transition|
TodoService.new.merge_request_became_unmergeable(merge_request)
end
end end
   
validates :source_project, presence: true, unless: [:allow_broken, :importing?, :closed_without_fork?] validates :source_project, presence: true, unless: [:allow_broken, :importing?, :closed_without_fork?]
Loading
Loading
Loading
@@ -6,13 +6,15 @@ class Todo < ActiveRecord::Base
Loading
@@ -6,13 +6,15 @@ class Todo < ActiveRecord::Base
BUILD_FAILED = 3 BUILD_FAILED = 3
MARKED = 4 MARKED = 4
APPROVAL_REQUIRED = 5 # This is an EE-only feature APPROVAL_REQUIRED = 5 # This is an EE-only feature
UNMERGEABLE = 6
   
ACTION_NAMES = { ACTION_NAMES = {
ASSIGNED => :assigned, ASSIGNED => :assigned,
MENTIONED => :mentioned, MENTIONED => :mentioned,
BUILD_FAILED => :build_failed, BUILD_FAILED => :build_failed,
MARKED => :marked, MARKED => :marked,
APPROVAL_REQUIRED => :approval_required APPROVAL_REQUIRED => :approval_required,
UNMERGEABLE => :unmergeable
} }
   
belongs_to :author, class_name: "User" belongs_to :author, class_name: "User"
Loading
@@ -66,6 +68,10 @@ class Todo < ActiveRecord::Base
Loading
@@ -66,6 +68,10 @@ class Todo < ActiveRecord::Base
end end
end end
   
def unmergeable?
action == UNMERGEABLE
end
def build_failed? def build_failed?
action == BUILD_FAILED action == BUILD_FAILED
end end
Loading
Loading
Loading
@@ -123,7 +123,15 @@ class TodoService
Loading
@@ -123,7 +123,15 @@ class TodoService
mark_pending_todos_as_done(merge_request, merge_request.author) 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_build_succeeds? mark_pending_todos_as_done(merge_request, merge_request.merge_user) if merge_request.merge_when_build_succeeds?
end end
# When a merge request could not be automatically merged due to its unmergeable state we should:
#
# * create a todo for a merge_user
#
def merge_request_became_unmergeable(merge_request)
create_unmergeable_todo(merge_request, merge_request.merge_user) if merge_request.merge_when_build_succeeds?
end
# When create a note we should: # When create a note we should:
# #
# * mark all pending todos related to the noteable for the note author as done # * mark all pending todos related to the noteable for the note author as done
Loading
@@ -245,6 +253,11 @@ class TodoService
Loading
@@ -245,6 +253,11 @@ class TodoService
create_todos(todo_author, attributes) create_todos(todo_author, attributes)
end 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)
end
def attributes_for_target(target) def attributes_for_target(target)
attributes = { attributes = {
project_id: target.project.id, project_id: target.project.id,
Loading
Loading
Loading
@@ -3,7 +3,7 @@
Loading
@@ -3,7 +3,7 @@
   
.todo-item.todo-block .todo-item.todo-block
.todo-title.title .todo-title.title
- unless todo.build_failed? - unless todo.build_failed? || todo.unmergeable?
= todo_target_state_pill(todo) = todo_target_state_pill(todo)
   
%span.author-name %span.author-name
Loading
Loading
--- ---
title: Create a TODO for user who set auto-merge when a build fails title: Create a TODO for user who set auto-merge when a build fails, merge conflict occurs
merge_request: merge_request: 8056
author: author: twonegatives
Loading
@@ -27,6 +27,10 @@ FactoryGirl.define do
Loading
@@ -27,6 +27,10 @@ FactoryGirl.define do
action { Todo::APPROVAL_REQUIRED } action { Todo::APPROVAL_REQUIRED }
end end
   
trait :unmergeable do
action { Todo::UNMERGEABLE }
end
trait :done do trait :done do
state :done state :done
end end
Loading
Loading
Loading
@@ -740,10 +740,12 @@ describe MergeRequest, models: true do
Loading
@@ -740,10 +740,12 @@ describe MergeRequest, models: true do
subject { create(:merge_request, source_project: project, merge_status: :unchecked) } subject { create(:merge_request, source_project: project, merge_status: :unchecked) }
   
context 'when it is not broken and has no conflicts' do context 'when it is not broken and has no conflicts' do
it 'is marked as mergeable' do before do
allow(subject).to receive(:broken?) { false } allow(subject).to receive(:broken?) { false }
allow(project.repository).to receive(:can_be_merged?).and_return(true) allow(project.repository).to receive(:can_be_merged?).and_return(true)
end
   
it 'is marked as mergeable' do
expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('can_be_merged') expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('can_be_merged')
end end
end end
Loading
@@ -754,6 +756,12 @@ describe MergeRequest, models: true do
Loading
@@ -754,6 +756,12 @@ describe MergeRequest, models: true do
it 'becomes unmergeable' do it 'becomes unmergeable' do
expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('cannot_be_merged') expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('cannot_be_merged')
end end
it 'creates Todo on unmergeability' do
expect_any_instance_of(TodoService).to receive(:merge_request_became_unmergeable).with(subject)
subject.check_if_can_be_merged
end
end end
   
context 'when it has conflicts' do context 'when it has conflicts' do
Loading
Loading
Loading
@@ -489,6 +489,15 @@ describe TodoService, services: true do
Loading
@@ -489,6 +489,15 @@ describe TodoService, services: true do
end end
end end
   
describe '#merge_request_became_unmergeable' do
it 'creates a pending todo for a merge_user' do
mr_unassigned.update(merge_when_build_succeeds: true, merge_user: admin)
service.merge_request_became_unmergeable(mr_unassigned)
should_create_todo(user: admin, author: admin, target: mr_unassigned, action: Todo::UNMERGEABLE)
end
end
describe '#mark_todo' do describe '#mark_todo' do
it 'creates a todo from a merge request' do it 'creates a todo from a merge request' do
service.mark_todo(mr_unassigned, author) service.mark_todo(mr_unassigned, author)
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment