diff --git a/app/assets/javascripts/merge_request_widget.js.es6 b/app/assets/javascripts/merge_request_widget.js.es6
index 69aed77c83d83abbc6a39d7d5e5602f886120800..4ab33420e592b2c4bb57a49e52bf94511af00973 100644
--- a/app/assets/javascripts/merge_request_widget.js.es6
+++ b/app/assets/javascripts/merge_request_widget.js.es6
@@ -110,7 +110,7 @@ require('./smart_interval');
urlSuffix = deleteSourceBranch ? '?deleted_source_branch=true' : '';
return window.location.href = window.location.pathname + urlSuffix;
} 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 {
callback = function() {
return merge_request_widget.mergeInProgress(deleteSourceBranch);
diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb
index 5ca6fec962dcfe7673e81541e9bcf5202ef2894c..177b714b734a21f802f2c06e02424ab5d6351a95 100644
--- a/app/services/merge_requests/merge_service.rb
+++ b/app/services/merge_requests/merge_service.rb
@@ -11,18 +11,20 @@ module MergeRequests
def execute(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
- 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
if commit
after_merge
success
- else
- log_merge_error('Can not merge changes', true)
end
end
end
@@ -43,11 +45,11 @@ module MergeRequests
if commit_id
merge_request.update(merge_commit_sha: commit_id)
else
- merge_request.update(merge_error: 'Conflicts detected during merge')
+ log_merge_error('Conflicts detected during merge', save_message_on_model: true)
false
end
rescue GitHooksService::PreReceiveError => e
- merge_request.update(merge_error: e.message)
+ log_merge_error(e.message, save_message_on_model: true)
false
rescue StandardError => e
merge_request.update(merge_error: "Something went wrong during merge: #{e.message}")
@@ -70,10 +72,10 @@ module MergeRequests
@merge_request.force_remove_source_branch? ? @merge_request.author : current_user
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}")
- error(message) if http_error
+ @merge_request.update(merge_error: message) if save_message_on_model
end
def merge_request_info
diff --git a/changelogs/unreleased/28124-mrs-don-t-show-all-merge-errors.yml b/changelogs/unreleased/28124-mrs-don-t-show-all-merge-errors.yml
new file mode 100644
index 0000000000000000000000000000000000000000..cd61c38e1bcb48fd0f046ef5fc68f0c59426c14e
--- /dev/null
+++ b/changelogs/unreleased/28124-mrs-don-t-show-all-merge-errors.yml
@@ -0,0 +1,4 @@
+---
+title: Show merge errors in merge request widget
+merge_request: 9229
+author:
diff --git a/spec/features/merge_requests/widget_spec.rb b/spec/features/merge_requests/widget_spec.rb
index 957e913bf95fa74548fc31092eaf31548bc3224f..4ad944366c8285196df8831bf9b9e7745efcc71a 100644
--- a/spec/features/merge_requests/widget_spec.rb
+++ b/spec/features/merge_requests/widget_spec.rb
@@ -52,4 +52,19 @@ describe 'Merge request', :feature, :js do
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
diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb
index 5a89acc96a4f941a180bdeaa0013f90171cbbd18..d96f819e66ac6c666b4ddca23f066fed3349b7aa 100644
--- a/spec/services/merge_requests/merge_service_spec.rb
+++ b/spec/services/merge_requests/merge_service_spec.rb
@@ -149,35 +149,46 @@ describe MergeRequests::MergeService, services: true do
context "error handling" do
let(:service) { MergeRequests::MergeService.new(project, user, commit_message: 'Awesome message') }
- it 'saves error if there is an exception' do
- allow(service).to receive(:repository).and_raise("error message")
+ before do
+ 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)
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
- it 'saves error if there is an PreReceiveError exception' do
- allow(service).to receive(:repository).and_raise(GitHooksService::PreReceiveError, "error")
+ it 'logs and saves error if there is an PreReceiveError exception' do
+ error_message = 'error message'
+ allow(service).to receive(:repository).and_raise(GitHooksService::PreReceiveError, error_message)
allow(service).to receive(:execute_hooks)
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
- 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(service).to receive(:execute_hooks)
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_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