Only force recheck when merge-ref is outdated
When recheck flag is true, we make sure the merge-ref is indeed outdated. If it is, we update it along the merge status.
This commit is contained in:
parent
3af348b6cf
commit
1f0b50c418
2 changed files with 61 additions and 3 deletions
|
@ -15,7 +15,7 @@ module MergeRequests
|
||||||
# the merge-ref is refreshed.
|
# the merge-ref is refreshed.
|
||||||
#
|
#
|
||||||
# recheck - When given, it'll enforce a merge-ref refresh if the current merge_status is
|
# recheck - When given, it'll enforce a merge-ref refresh if the current merge_status is
|
||||||
# can_be_merged or cannot_be_merged.
|
# can_be_merged or cannot_be_merged and merge-ref is outdated.
|
||||||
# Given MergeRequests::RefreshService is called async, it might happen that the target
|
# Given MergeRequests::RefreshService is called async, it might happen that the target
|
||||||
# branch gets updated, but the MergeRequest#merge_status lags behind. So in scenarios
|
# branch gets updated, but the MergeRequest#merge_status lags behind. So in scenarios
|
||||||
# where we need the current state of the merge ref in repository, the `recheck`
|
# where we need the current state of the merge ref in repository, the `recheck`
|
||||||
|
@ -79,7 +79,22 @@ module MergeRequests
|
||||||
end
|
end
|
||||||
|
|
||||||
def recheck!
|
def recheck!
|
||||||
merge_request.mark_as_unchecked unless merge_request.recheck_merge_status?
|
if !merge_request.recheck_merge_status? && outdated_merge_ref?
|
||||||
|
merge_request.mark_as_unchecked
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Checks if the existing merge-ref is synced with the target branch.
|
||||||
|
#
|
||||||
|
# Returns true if the merge-ref does not exists or is out of sync.
|
||||||
|
def outdated_merge_ref?
|
||||||
|
return false unless merge_ref_auto_sync_enabled?
|
||||||
|
|
||||||
|
return true unless ref_head = merge_request.merge_ref_head
|
||||||
|
return true unless target_sha = merge_request.target_branch_sha
|
||||||
|
return true unless source_sha = merge_request.source_branch_sha
|
||||||
|
|
||||||
|
ref_head.parent_ids != [target_sha, source_sha]
|
||||||
end
|
end
|
||||||
|
|
||||||
def can_git_merge?
|
def can_git_merge?
|
||||||
|
|
|
@ -91,7 +91,7 @@ describe MergeRequests::MergeabilityCheckService do
|
||||||
expect(result.payload).to be_empty
|
expect(result.payload).to be_empty
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'updates the merge_status' do
|
it 'ignores merge-ref and updates merge status' do
|
||||||
expect { subject }.to change(merge_request, :merge_status).from('unchecked').to('can_be_merged')
|
expect { subject }.to change(merge_request, :merge_status).from('unchecked').to('can_be_merged')
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -206,6 +206,28 @@ describe MergeRequests::MergeabilityCheckService do
|
||||||
context 'recheck enforced' do
|
context 'recheck enforced' do
|
||||||
subject { described_class.new(merge_request).execute(recheck: true) }
|
subject { described_class.new(merge_request).execute(recheck: true) }
|
||||||
|
|
||||||
|
context 'when MR is mergeable and merge-ref auto-sync is disabled' do
|
||||||
|
before do
|
||||||
|
stub_feature_flags(merge_ref_auto_sync: false)
|
||||||
|
merge_request.mark_as_mergeable!
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'returns ServiceResponse.error' do
|
||||||
|
result = subject
|
||||||
|
|
||||||
|
expect(result).to be_a(ServiceResponse)
|
||||||
|
expect(result.error?).to be(true)
|
||||||
|
expect(result.message).to eq('Merge ref is outdated due to disabled feature')
|
||||||
|
expect(result.payload).to be_empty
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'merge status is not changed' do
|
||||||
|
subject
|
||||||
|
|
||||||
|
expect(merge_request.merge_status).to eq('can_be_merged')
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
context 'when MR is mergeable but merge-ref does not exists' do
|
context 'when MR is mergeable but merge-ref does not exists' do
|
||||||
before do
|
before do
|
||||||
merge_request.mark_as_mergeable!
|
merge_request.mark_as_mergeable!
|
||||||
|
@ -213,6 +235,27 @@ describe MergeRequests::MergeabilityCheckService do
|
||||||
|
|
||||||
it_behaves_like 'mergeable merge request'
|
it_behaves_like 'mergeable merge request'
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'when MR is mergeable but merge-ref is already updated' do
|
||||||
|
before do
|
||||||
|
MergeRequests::MergeToRefService.new(project, merge_request.author).execute(merge_request)
|
||||||
|
merge_request.mark_as_mergeable!
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'returns ServiceResponse.success' do
|
||||||
|
result = subject
|
||||||
|
|
||||||
|
expect(result).to be_a(ServiceResponse)
|
||||||
|
expect(result).to be_success
|
||||||
|
expect(result.payload[:merge_ref_head]).to be_present
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'does not recreate the merge-ref' do
|
||||||
|
expect(MergeRequests::MergeToRefService).not_to receive(:new)
|
||||||
|
|
||||||
|
subject
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in a new issue