Commit ea25fbb8 authored by Mark Chao's avatar Mark Chao Committed by Sean McGivern

Notify conflict only for opened/locked merge requests

parent 6938335c
...@@ -129,9 +129,7 @@ class MergeRequest < ActiveRecord::Base ...@@ -129,9 +129,7 @@ class MergeRequest < ActiveRecord::Base
after_transition unchecked: :cannot_be_merged do |merge_request, transition| after_transition unchecked: :cannot_be_merged do |merge_request, transition|
begin begin
# Merge request can become unmergeable due to many reasons. if merge_request.notify_conflict?
# 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) NotificationService.new.merge_request_unmergeable(merge_request)
TodoService.new.merge_request_became_unmergeable(merge_request) TodoService.new.merge_request_became_unmergeable(merge_request)
end end
...@@ -708,6 +706,10 @@ class MergeRequest < ActiveRecord::Base ...@@ -708,6 +706,10 @@ class MergeRequest < ActiveRecord::Base
should_remove_source_branch? || force_remove_source_branch? should_remove_source_branch? || force_remove_source_branch?
end end
def notify_conflict?
(opened? || locked?) && !project.repository.can_be_merged?(diff_head_sha, target_branch)
end
def related_notes def related_notes
# Fetch comments only from last 100 commits # Fetch comments only from last 100 commits
commits_for_notes_limit = 100 commits_for_notes_limit = 100
......
---
title: Notify conflict for only open merge request
merge_request: 20125
author:
type: fixed
...@@ -111,7 +111,7 @@ by yourself (except when an issue is due). You will only receive automatic ...@@ -111,7 +111,7 @@ 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 an open merge request becomes unmergeable due to conflict, its author will be notified about the cause.
If a user has also set the merge request to automatically merge once pipeline succeeds, If a user has also set the merge request to automatically merge once pipeline succeeds,
then that user will also be notified. then that user will also be notified.
......
...@@ -31,7 +31,7 @@ A Todo appears in your Todos dashboard when: ...@@ -31,7 +31,7 @@ A Todo appears in your Todos dashboard when:
- you are `@mentioned` in a comment on a commit, - you are `@mentioned` in a comment on a commit,
- a job in the CI pipeline running for your merge request failed, but this - a job in the CI pipeline running for your merge request failed, but this
job is not allowed to fail. job is not allowed to fail.
- a merge request becomes unmergeable, and you are either: - an open merge request becomes unmergeable due to conflict, and you are either:
- the author, or - the author, or
- have set it to automatically merge once pipeline succeeds. - have set it to automatically merge once pipeline succeeds.
......
...@@ -2134,8 +2134,7 @@ describe MergeRequest do ...@@ -2134,8 +2134,7 @@ describe MergeRequest do
describe 'transition to cannot_be_merged' do describe 'transition to cannot_be_merged' do
let(:notification_service) { double(:notification_service) } let(:notification_service) { double(:notification_service) }
let(:todo_service) { double(:todo_service) } let(:todo_service) { double(:todo_service) }
subject { create(:merge_request, state, merge_status: :unchecked) }
subject { create(:merge_request, merge_status: :unchecked) }
before do before do
allow(NotificationService).to receive(:new).and_return(notification_service) allow(NotificationService).to receive(:new).and_return(notification_service)
...@@ -2144,6 +2143,10 @@ describe MergeRequest do ...@@ -2144,6 +2143,10 @@ describe MergeRequest do
allow(subject.project.repository).to receive(:can_be_merged?).and_return(false) allow(subject.project.repository).to receive(:can_be_merged?).and_return(false)
end end
[:opened, :locked].each do |state|
context state do
let(:state) { state }
it 'notifies conflict, but does not notify again if rechecking still results in cannot_be_merged' do it 'notifies conflict, but does not notify again if rechecking still results in cannot_be_merged' do
expect(notification_service).to receive(:merge_request_unmergeable).with(subject).once expect(notification_service).to receive(:merge_request_unmergeable).with(subject).once
expect(todo_service).to receive(:merge_request_became_unmergeable).with(subject).once expect(todo_service).to receive(:merge_request_became_unmergeable).with(subject).once
...@@ -2173,6 +2176,21 @@ describe MergeRequest do ...@@ -2173,6 +2176,21 @@ describe MergeRequest do
subject.mark_as_unmergeable subject.mark_as_unmergeable
end end
end end
end
[:closed, :merged].each do |state|
let(:state) { state }
context state do
it 'does not notify' do
expect(notification_service).not_to receive(:merge_request_unmergeable)
expect(todo_service).not_to receive(:merge_request_became_unmergeable)
subject.mark_as_unmergeable
end
end
end
end
describe 'check_state?' do describe 'check_state?' do
it 'indicates whether MR is still checking for mergeability' do it 'indicates whether MR is still checking for mergeability' do
......
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