Merge request, better error handlingmerge_request_error

9 files changed, 38 insertions, 5 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 458758b3205..854152fec0b 100644
--- a/CHANGELOG
+++ b/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
diff --git a/app/assets/javascripts/merge_request_widget.js.coffee b/app/assets/javascripts/merge_request_widget.js.coffee
index 995a2f24093..40ee7735ee6 100644
--- a/app/assets/javascripts/merge_request_widget.js.coffee
+++ b/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'
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 7574842cd43..989e363f1a2 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/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
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index eb468c6cd53..5a364ac05f2 100644
--- a/app/models/merge_request.rb
+++ b/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
diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb
index 98a67c0bc99..9605b68e5df 100644
--- a/app/services/merge_requests/merge_service.rb
+++ b/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
diff --git a/app/views/projects/merge_requests/widget/_open.html.haml b/app/views/projects/merge_requests/widget/_open.html.haml
index 0aad9bb3e88..3b82ec4db53 100644
--- a/app/views/projects/merge_requests/widget/_open.html.haml
+++ b/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
diff --git a/app/views/projects/merge_requests/widget/_show.html.haml b/app/views/projects/merge_requests/widget/_show.html.haml
index a489d4f9b24..4f930b5817b 100644
--- a/app/views/projects/merge_requests/widget/_show.html.haml
+++ b/app/views/projects/merge_requests/widget/_show.html.haml
@@ -1,4 +1,4 @@
-- 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'
diff --git a/app/views/projects/merge_requests/widget/open/_in_progress.html.haml b/app/views/projects/merge_requests/widget/open/_in_progress.html.haml
new file mode 100644
index 00000000000..366e1071854
--- /dev/null
+++ b/app/views/projects/merge_requests/widget/open/_in_progress.html.haml
@@ -0,0 +1,7 @@
+.accept-merge-holder.clearfix.js-toggle-container
+  %i.fa.fa-spinner.fa-spin
+  Merge in progress
+
+:coffeescript
+  $ -> 
+    merge_request_widget.mergeInProgress()
diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb
index 7b564d34d7b..ac3d700b6a1 100644
--- a/spec/services/merge_requests/merge_service_spec.rb
+++ b/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