Merge request, better error handling

CHANGELOG

@@ -16,6 +16,7 @@ v 8.1.0 (unreleased)
   - Move CI runners page to project settings area
   - Move CI variables page to project settings area
   - Move CI triggers page to project settings area
+  - Improve error message when merging fails
 
 v 8.0.3
   - Fix URL shown in Slack notifications

app/assets/javascripts/merge_request_widget.js.coffee

@@ -18,6 +18,8 @@ class @MergeRequestWidget
         switch data.state
           when 'merged'
             location.reload()
+          when 'opened'
+            $('.mr-widget-body').html("<h4>Something went wrong during merging</h4>")
           else
             setTimeout(merge_request_widget.mergeInProgress, 2000)
       dataType: 'json'

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.merging
       MergeWorker.perform_async(@merge_request.id, current_user.id, params)
       @status = true
     else

app/models/merge_request.rb

@@ -65,13 +65,21 @@ class MergeRequest < ActiveRecord::Base
     end
 
     event :lock_mr do
-      transition [:reopened, :opened] => :locked
+      transition [:reopened, :opened, :in_progress] => :locked
     end
 
     event :unlock_mr do
       transition locked: :reopened
     end
 
+    event :merging do
+      transition [:opened, :reopened] => :in_progress
+    end
+
+    event :merging_fail do
+      transition [:in_progress, :locked] => :opened
+    end
+
     after_transition any => :locked do |merge_request, transition|
       merge_request.locked_at = Time.now
       merge_request.save
@@ -86,6 +94,7 @@ class MergeRequest < ActiveRecord::Base
     state :reopened
     state :closed
     state :merged
+    state :in_progress
     state :locked
   end
 
@@ -231,7 +240,7 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def mergeable?
-    open? && !work_in_progress? && can_be_merged?
+    (open? || in_progress?) && !work_in_progress? && can_be_merged?
   end
 
   def gitlab_merge_status

app/services/merge_requests/merge_service.rb

@@ -21,6 +21,7 @@ module MergeRequests
           after_merge
           success
         else
+          @merge_request.merging_fail
           error('Can not merge changes')
         end
       end

app/views/projects/merge_requests/widget/_open.html.haml

@@ -9,6 +9,8 @@
       = render 'projects/merge_requests/widget/open/missing_branch'
     - elsif @merge_request.unchecked?
       = render 'projects/merge_requests/widget/open/check'
+    - elsif @merge_request.in_progress?
+      = render 'projects/merge_requests/widget/open/in_progress'
     - elsif @merge_request.cannot_be_merged?
       = render 'projects/merge_requests/widget/open/conflicts'
     - elsif @merge_request.work_in_progress?
@@ -18,6 +20,7 @@
     - elsif @merge_request.can_be_merged?
       = render 'projects/merge_requests/widget/open/accept'
 
+
   - if @closes_issues.present?
     .mr-widget-footer
       %span

app/views/projects/merge_requests/widget/_show.html.haml

-- if @merge_request.open?
+- if @merge_request.open? || @merge_request.in_progress?
   = render 'projects/merge_requests/widget/open'
 - elsif @merge_request.merged?
   = render 'projects/merge_requests/widget/merged'

app/views/projects/merge_requests/widget/open/_in_progress.html.haml

+.accept-merge-holder.clearfix.js-toggle-container
+  %i.fa.fa-spinner.fa-spin
+  Merge in progress
+
+:coffeescript
+  $ -> 
+    merge_request_widget.mergeInProgress()

spec/services/merge_requests/merge_service_spec.rb

@@ -12,9 +12,9 @@ describe MergeRequests::MergeService do
   end
 
   describe :execute do
-    context 'valid params' do
-      let(:service) { MergeRequests::MergeService.new(project, user, {}) }
+    let(:service) { MergeRequests::MergeService.new(project, user, {}) }
 
+    context 'valid params' do
       before do
         allow(service).to receive(:execute_hooks)
 
@@ -35,5 +35,14 @@ describe MergeRequests::MergeService do
         expect(note.note).to include 'Status changed to merged'
       end
     end
+
+    context "something goes wrong" do
+      it "mark merge request as open" do
+        allow(service).to receive(:commit).and_return(false)
+        merge_request.merging
+        service.execute(merge_request, 'Awesome message')
+        expect(merge_request.state).to eq("opened")
+      end
+    end
   end
 end