Merge branch 'mr_improve_errors_handling'

CHANGELOG

@@ -20,6 +20,7 @@ v 8.1.0 (unreleased)
   - Move CI project settings page to CE project settings area
   - Fix bug when removed file was not appearing in merge request diff
   - Note the original location of a moved project when notifying users of the move
+  - Improve error message when merging fails
 
 v 8.0.3
   - Fix URL shown in Slack notifications

app/assets/javascripts/merge_request_widget.js.coffee

@@ -15,11 +15,12 @@ class @MergeRequestWidget
       type: 'GET'
       url: $('.merge-request').data('url')
       success: (data) =>
-        switch data.state
-          when 'merged'
-            location.reload()
-          else
-            setTimeout(merge_request_widget.mergeInProgress, 2000)
+        if data.state == "merged"
+          location.reload()
+        else if data.merge_error
+          $('.mr-widget-body').html("<h4>" + data.merge_error + "</h4>")
+        else
+          setTimeout(merge_request_widget.mergeInProgress, 2000)
       dataType: 'json'
 
   getMergeStatus: ->

app/controllers/projects/merge_requests_controller.rb

@@ -150,6 +150,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     return access_denied! unless @merge_request.can_be_merged_by?(current_user)
 
     if @merge_request.mergeable?
+      @merge_request.update(merge_error: nil)
       MergeWorker.perform_async(@merge_request.id, current_user.id, params)
       @status = true
     else

app/services/merge_requests/merge_service.rb

@@ -38,6 +38,10 @@ module MergeRequests
       }
 
       repository.merge(current_user, merge_request.source_sha, merge_request.target_branch, options)
+    rescue Exception => e
+      merge_request.update(merge_error: "Something went wrong during merge")
+      Rails.logger.error(e.message)
+      return false
     end
 
     def after_merge

db/migrate/20150930001110_merge_request_error_field.rb

@@ -0,0 +1,5 @@
+class MergeRequestErrorField < ActiveRecord::Migration
+  def up
+    add_column :merge_requests, :merge_error, :string
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150930095736) do
+ActiveRecord::Schema.define(version: 20150930001110) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -453,6 +453,7 @@ ActiveRecord::Schema.define(version: 20150930095736) do
     t.integer  "position",          default: 0
     t.datetime "locked_at"
     t.integer  "updated_by_id"
+    t.string   "merge_error"
   end
 
   add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree

spec/services/merge_requests/merge_service_spec.rb

@@ -35,5 +35,19 @@ describe MergeRequests::MergeService do
         expect(note.note).to include 'Status changed to merged'
       end
     end
+
+    context "error handling" do
+      let(:service) { MergeRequests::MergeService.new(project, user, {}) }
+
+      it 'saves error if there is an exception' do
+        allow(service).to receive(:repository).and_raise("error")
+
+        allow(service).to receive(:execute_hooks)
+
+        service.execute(merge_request, 'Awesome message')
+
+        expect(merge_request.merge_error).to eq("Something went wrong during merge")
+      end
+    end
   end
 end