diff options
author | Robert Speicher <robert@gitlab.com> | 2016-07-30 18:05:54 +0000 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-08-01 15:30:58 +0200 |
commit | bb937e7ab7cfcc3d816ccf4d34b67b342882bece (patch) | |
tree | c80597b20e16a4a18a31ec5194229a2da57ad848 | |
parent | c004ed5f73e7680c70f520f2f920987348ee0c9a (diff) | |
download | gitlab-ce-bb937e7ab7cfcc3d816ccf4d34b67b342882bece.tar.gz |
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: Rémy Coutable <remy@rymai.me>
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/services/merge_requests/merge_service.rb | 8 | ||||
-rw-r--r-- | spec/services/merge_requests/merge_service_spec.rb | 11 |
3 files changed, 19 insertions, 1 deletions
diff --git a/CHANGELOG b/CHANGELOG index ccd454ddf3c..3f4a504630b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,7 @@ v 8.10.3 (unreleased) - Fix timing problems running imports on production. !5523 - Add a log message when a project is scheduled for destruction for debugging. !5540 - Fix hooks missing on imported GitLab projects. !5549 + - Properly abort a merge when merge conflicts occur. !5569 v 8.10.2 - User can now search branches by name. !5144 diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 0dac0614141..b037780c431 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -35,7 +35,13 @@ module MergeRequests } commit_id = repository.merge(current_user, merge_request, options) - merge_request.update(merge_commit_sha: commit_id) + + if 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 merge_request.update(merge_error: e.message) false diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index f5bf3c1e367..8ffebcac698 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -75,6 +75,17 @@ describe MergeRequests::MergeService, services: true do expect(merge_request.merge_error).to eq("error") 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 |