Show merge errors in merge request widget

There were two problems here:

1. On the JS side, the reference to $widgetBody didn't refer to the
   right DOM element any more. This might be because it was replaced by
   the `getMergeStatus` method. Even if it wasn't, ensuring we have the
   right element means that the content gets updated.

2. On the Ruby side, the `log_merge_error` method didn't update the
   `merge_error` column of the merge request. Change that to update if
   requested, and update in the most common cases by default.

   Additionally, this would sometimes return an error hash, but it
   doesn't look like this was ever used (the return value of
   `MergeService#execute` appears to be unused everywhere).

app/services/merge_requests/merge_service.rb

@@ -16,41 +16,45 @@ module MergeRequests
 
       @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
 
       if @merge_request.target_project.above_size_limit?
         message = Gitlab::RepositorySizeError.new(@merge_request.target_project).merge_error
-        @merge_request.update(merge_error: message)
-        return error(message)
+
+        return log_merge_error(message, save_message_on_model: true)
       end
 
       @source = find_merge_source
 
-      return log_merge_error('No source for merge', true) unless @source
+      unless @source
+        return 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
 
     def hooks_validation_pass?(merge_request)
+      @merge_request = merge_request
+
       return true if project.merge_requests_ff_only_enabled
 
       push_rule = merge_request.project.push_rule
       return true unless push_rule
 
       unless push_rule.commit_message_allowed?(params[:commit_message])
-        merge_request.update(merge_error: "Commit message does not follow the pattern '#{push_rule.commit_message_regex}'")
+        log_merge_error("Commit message does not follow the pattern '#{push_rule.commit_message_regex}'", save_message_on_model: true)
         return false
       end
 
       unless push_rule.author_email_allowed?(current_user.email)
-        merge_request.update(merge_error: "Commit author's email '#{current_user.email}' does not follow the pattern '#{push_rule.author_email_regex}'")
+        log_merge_error("Commit author's email '#{current_user.email}' does not follow the pattern '#{push_rule.author_email_regex}'", save_message_on_model: true)
         return false
       end
 
@@ -73,11 +77,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}")
@@ -100,10 +104,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

changelogs/unreleased/28124-mrs-don-t-show-all-merge-errors.yml

+---
+title: Show merge errors in merge request widget
+merge_request: 9229
+author:

spec/features/merge_requests/widget_spec.rb

@@ -53,20 +53,17 @@ describe 'Merge request', :feature, :js do
     end
   end
 
-  context 'view merge request' do
-    let!(:environment) { create(:environment, project: project) }
-    let!(:deployment) { create(:deployment, environment: environment, ref: 'feature', sha: merge_request.diff_head_sha) }
-
+  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)
-    end
-
-    it 'shows environments link' do
+      click_button 'Accept Merge Request'
       wait_for_ajax
+    end
 
-      page.within('.mr-widget-heading') do
-        expect(page).to have_content("Deployed to #{environment.name}")
-        expect(find('.js-environment-link')[:href]).to include(environment.formatted_external_url)
+    it 'updates the MR widget' do
+      page.within('.mr-widget-body') do
+        expect(page).to have_content('Conflicts detected during merge')
       end
     end
   end

spec/services/merge_requests/merge_service_spec.rb

@@ -165,35 +165,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