Commit 1b6a671e authored by Rémy Coutable's avatar Rémy Coutable

Merge branch '28124-mrs-don-t-show-all-merge-errors' into 'master'

Show merge errors in merge request widget

Closes #28124 and gitlab-ee#1652

See merge request !9229
parents 93ae3063 7a9d3a3c
...@@ -110,7 +110,7 @@ require('./smart_interval'); ...@@ -110,7 +110,7 @@ require('./smart_interval');
urlSuffix = deleteSourceBranch ? '?deleted_source_branch=true' : ''; urlSuffix = deleteSourceBranch ? '?deleted_source_branch=true' : '';
return window.location.href = window.location.pathname + urlSuffix; return window.location.href = window.location.pathname + urlSuffix;
} else if (data.merge_error) { } else if (data.merge_error) {
return _this.$widgetBody.html("<h4>" + data.merge_error + "</h4>"); return $('.mr-widget-body').html("<h4>" + data.merge_error + "</h4>");
} else { } else {
callback = function() { callback = function() {
return merge_request_widget.mergeInProgress(deleteSourceBranch); return merge_request_widget.mergeInProgress(deleteSourceBranch);
......
...@@ -11,18 +11,20 @@ module MergeRequests ...@@ -11,18 +11,20 @@ module MergeRequests
def execute(merge_request) def execute(merge_request)
@merge_request = merge_request @merge_request = merge_request
return log_merge_error('Merge request is not mergeable', true) unless @merge_request.mergeable? unless @merge_request.mergeable?
return log_merge_error('Merge request is not mergeable', save_message_on_model: true)
end
@source = find_merge_source @source = find_merge_source
return log_merge_error('No source for merge', true) unless @source unless @source
log_merge_error('No source for merge', save_message_on_model: true)
end
merge_request.in_locked_state do merge_request.in_locked_state do
if commit if commit
after_merge after_merge
success success
else
log_merge_error('Can not merge changes', true)
end end
end end
end end
...@@ -43,11 +45,11 @@ module MergeRequests ...@@ -43,11 +45,11 @@ module MergeRequests
if commit_id if commit_id
merge_request.update(merge_commit_sha: commit_id) merge_request.update(merge_commit_sha: commit_id)
else else
merge_request.update(merge_error: 'Conflicts detected during merge') log_merge_error('Conflicts detected during merge', save_message_on_model: true)
false false
end end
rescue GitHooksService::PreReceiveError => e rescue GitHooksService::PreReceiveError => e
merge_request.update(merge_error: e.message) log_merge_error(e.message, save_message_on_model: true)
false false
rescue StandardError => e rescue StandardError => e
merge_request.update(merge_error: "Something went wrong during merge: #{e.message}") merge_request.update(merge_error: "Something went wrong during merge: #{e.message}")
...@@ -70,10 +72,10 @@ module MergeRequests ...@@ -70,10 +72,10 @@ module MergeRequests
@merge_request.force_remove_source_branch? ? @merge_request.author : current_user @merge_request.force_remove_source_branch? ? @merge_request.author : current_user
end end
def log_merge_error(message, http_error = false) def log_merge_error(message, save_message_on_model: false)
Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{message}") Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{message}")
error(message) if http_error @merge_request.update(merge_error: message) if save_message_on_model
end end
def merge_request_info def merge_request_info
......
---
title: Show merge errors in merge request widget
merge_request: 9229
author:
...@@ -52,4 +52,19 @@ describe 'Merge request', :feature, :js do ...@@ -52,4 +52,19 @@ describe 'Merge request', :feature, :js do
end end
end end
end end
context 'merge error' do
before do
allow_any_instance_of(Repository).to receive(:merge).and_return(false)
visit namespace_project_merge_request_path(project.namespace, project, merge_request)
click_button 'Accept Merge Request'
wait_for_ajax
end
it 'updates the MR widget' do
page.within('.mr-widget-body') do
expect(page).to have_content('Conflicts detected during merge')
end
end
end
end end
...@@ -149,35 +149,46 @@ describe MergeRequests::MergeService, services: true do ...@@ -149,35 +149,46 @@ describe MergeRequests::MergeService, services: true do
context "error handling" do context "error handling" do
let(:service) { MergeRequests::MergeService.new(project, user, commit_message: 'Awesome message') } let(:service) { MergeRequests::MergeService.new(project, user, commit_message: 'Awesome message') }
it 'saves error if there is an exception' do before do
allow(service).to receive(:repository).and_raise("error message") allow(Rails.logger).to receive(:error)
end
it 'logs and saves error if there is an exception' do
error_message = 'error message'
allow(service).to receive(:repository).and_raise("error message")
allow(service).to receive(:execute_hooks) allow(service).to receive(:execute_hooks)
service.execute(merge_request) service.execute(merge_request)
expect(merge_request.merge_error).to eq("Something went wrong during merge: error message") expect(merge_request.merge_error).to include(error_message)
expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message))
end end
it 'saves error if there is an PreReceiveError exception' do it 'logs and saves error if there is an PreReceiveError exception' do
allow(service).to receive(:repository).and_raise(GitHooksService::PreReceiveError, "error") error_message = 'error message'
allow(service).to receive(:repository).and_raise(GitHooksService::PreReceiveError, error_message)
allow(service).to receive(:execute_hooks) allow(service).to receive(:execute_hooks)
service.execute(merge_request) service.execute(merge_request)
expect(merge_request.merge_error).to eq("error") expect(merge_request.merge_error).to include(error_message)
expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message))
end end
it 'aborts if there is a merge conflict' do it 'logs and saves error if there is a merge conflict' do
error_message = 'Conflicts detected during merge'
allow_any_instance_of(Repository).to receive(:merge).and_return(false) allow_any_instance_of(Repository).to receive(:merge).and_return(false)
allow(service).to receive(:execute_hooks) allow(service).to receive(:execute_hooks)
service.execute(merge_request) service.execute(merge_request)
expect(merge_request.open?).to be_truthy expect(merge_request).to be_open
expect(merge_request.merge_commit_sha).to be_nil expect(merge_request.merge_commit_sha).to be_nil
expect(merge_request.merge_error).to eq("Conflicts detected during merge") expect(merge_request.merge_error).to include(error_message)
expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message))
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