From dba2d6ee7f94c5657903fb20e9ef4fdac667df74 Mon Sep 17 00:00:00 2001
From: Oswaldo Ferreira <oswaldo@gitlab.com>
Date: Wed, 27 Jun 2018 12:04:54 -0300
Subject: [PATCH] Mark MR as merged regardless of errors when closing issues

We should mark the MR as merged as first thing on PostMergeService as in practice it is already merged on the repository. Happens that errors may happen when executing external services such as Issues::CloseService, and we do not want a MR hanging as opened because of that.
---
 app/services/merge_requests/post_merge_service.rb   |  2 +-
 ...sw-mark-as-merged-as-first-post-merge-action.yml |  5 +++++
 .../merge_requests/post_merge_service_spec.rb       | 13 +++++++++++++
 3 files changed, 19 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/unreleased/osw-mark-as-merged-as-first-post-merge-action.yml

diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb
index 5b160ffba67..7606d68ff29 100644
--- a/app/services/merge_requests/post_merge_service.rb
+++ b/app/services/merge_requests/post_merge_service.rb
@@ -6,9 +6,9 @@ module MergeRequests
   #
   class PostMergeService < MergeRequests::BaseService
     def execute(merge_request)
+      merge_request.mark_as_merged
       close_issues(merge_request)
       todo_service.merge_merge_request(merge_request, current_user)
-      merge_request.mark_as_merged
       create_event(merge_request)
       create_note(merge_request)
       notification_service.merge_mr(merge_request, current_user)
diff --git a/changelogs/unreleased/osw-mark-as-merged-as-first-post-merge-action.yml b/changelogs/unreleased/osw-mark-as-merged-as-first-post-merge-action.yml
new file mode 100644
index 00000000000..2049afc3d44
--- /dev/null
+++ b/changelogs/unreleased/osw-mark-as-merged-as-first-post-merge-action.yml
@@ -0,0 +1,5 @@
+---
+title: Mark MR as merged regardless of errors when closing issues
+merge_request:
+author:
+type: fixed
diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb
index 790ecce8ded..46e4e3559dc 100644
--- a/spec/services/merge_requests/post_merge_service_spec.rb
+++ b/spec/services/merge_requests/post_merge_service_spec.rb
@@ -47,5 +47,18 @@ describe MergeRequests::PostMergeService do
 
       expect(diff_removal_service).to have_received(:execute)
     end
+
+    it 'marks MR as merged regardless of errors when closing issues' do
+      merge_request.update(target_branch: 'foo')
+      allow(project).to receive(:default_branch).and_return('foo')
+
+      issue = create(:issue, project: project)
+      allow(merge_request).to receive(:closes_issues).and_return([issue])
+      allow_any_instance_of(Issues::CloseService).to receive(:execute).with(issue, commit: merge_request).and_raise
+
+      expect { described_class.new(project, user, {}).execute(merge_request) }.to raise_error
+
+      expect(merge_request.reload).to be_merged
+    end
   end
 end