summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2016-07-29 11:14:53 -0700
committerStan Hu <stanhu@gmail.com>2016-07-29 11:21:21 -0700
commit60529e021667194c402d4f6e85a83e02a8bb9f75 (patch)
treeae816493b2f43ec236f274e0edb9ddc46f0366d3
parent6ad514d0663bad80e24586be0919a580cdaab8d2 (diff)
downloadgitlab-ce-reject-merge-conflicts.tar.gz
Properly abort a merge when merge conflicts occurreject-merge-conflicts
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
-rw-r--r--CHANGELOG1
-rw-r--r--app/services/merge_requests/merge_service.rb8
-rw-r--r--spec/services/merge_requests/merge_service_spec.rb11
3 files changed, 19 insertions, 1 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 31a7eae90b9..6f7cf029440 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -38,6 +38,7 @@ v 8.11.0 (unreleased)
v 8.10.3 (unreleased)
- Fix hooks missing on imported GitLab projects
+ - Properly abort a merge when merge conflicts occur
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