Commit bb937e7a authored by Robert Speicher's avatar Robert Speicher Committed by Rémy Coutable

Merge branch 'reject-merge-conflicts' into 'master'

Properly abort a merge when merge conflicts occur

If somehow a user attempted to accept a merge request that had conflicts (e.g. the "Accept Merge Request" button or the MR itself was not updated), `MergeService` did not properly detect that a conflict
occurred. It would assume that the MR went through without any issues and close the MR as though everything was fine. This could cause data loss if the source branch were removed.

Closes #20425

See merge request !5569
Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent c004ed5f
...@@ -5,6 +5,7 @@ v 8.10.3 (unreleased) ...@@ -5,6 +5,7 @@ v 8.10.3 (unreleased)
- Fix timing problems running imports on production. !5523 - Fix timing problems running imports on production. !5523
- Add a log message when a project is scheduled for destruction for debugging. !5540 - Add a log message when a project is scheduled for destruction for debugging. !5540
- Fix hooks missing on imported GitLab projects. !5549 - Fix hooks missing on imported GitLab projects. !5549
- Properly abort a merge when merge conflicts occur. !5569
v 8.10.2 v 8.10.2
- User can now search branches by name. !5144 - User can now search branches by name. !5144
......
...@@ -35,7 +35,13 @@ module MergeRequests ...@@ -35,7 +35,13 @@ module MergeRequests
} }
commit_id = repository.merge(current_user, merge_request, options) commit_id = repository.merge(current_user, merge_request, options)
if commit_id
merge_request.update(merge_commit_sha: commit_id) merge_request.update(merge_commit_sha: commit_id)
else
merge_request.update(merge_error: 'Conflicts detected during merge')
false
end
rescue GitHooksService::PreReceiveError => e rescue GitHooksService::PreReceiveError => e
merge_request.update(merge_error: e.message) merge_request.update(merge_error: e.message)
false false
......
...@@ -75,6 +75,17 @@ describe MergeRequests::MergeService, services: true do ...@@ -75,6 +75,17 @@ describe MergeRequests::MergeService, services: true do
expect(merge_request.merge_error).to eq("error") expect(merge_request.merge_error).to eq("error")
end end
it 'aborts if there is a merge conflict' do
allow_any_instance_of(Repository).to receive(:merge).and_return(false)
allow(service).to receive(:execute_hooks)
service.execute(merge_request)
expect(merge_request.open?).to be_truthy
expect(merge_request.merge_commit_sha).to be_nil
expect(merge_request.merge_error).to eq("Conflicts detected during merge")
end
end end
end end
end end
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