Store MergeWorker JID on merge request, and clean up stuck merges

11 files changed, 109 insertions, 20 deletions

diff --git a/spec/features/merge_requests/widget_spec.rb b/spec/features/merge_requests/widget_spec.rb
index 69e31c7481f..fd991293ee9 100644
--- a/spec/features/merge_requests/widget_spec.rb
+++ b/spec/features/merge_requests/widget_spec.rb
@@ -219,4 +219,17 @@ describe 'Merge request', :js do
       expect(page).to have_field('remove-source-branch-input', disabled: true)
     end
   end
+
+  context 'ongoing merge process' do
+    it 'shows Merging state' do
+      allow_any_instance_of(MergeRequest).to receive(:merge_ongoing?).and_return(true)
+
+      visit project_merge_request_path(project, merge_request)
+
+      wait_for_requests
+
+      expect(page).not_to have_button('Merge')
+      expect(page).to have_content('This merge request is in the process of being merged')
+    end
+  end
 end
diff --git a/spec/fixtures/api/schemas/entities/merge_request.json b/spec/fixtures/api/schemas/entities/merge_request.json
index 7ffa82fc4bd..2f12b671dec 100644
--- a/spec/fixtures/api/schemas/entities/merge_request.json
+++ b/spec/fixtures/api/schemas/entities/merge_request.json
@@ -19,7 +19,6 @@
     "human_time_estimate": { "type": ["integer", "null"] },
     "human_total_time_spent": { "type": ["integer", "null"] },
     "in_progress_merge_commit_sha": { "type": ["string", "null"] },
-    "locked_at": { "type": ["string", "null"] },
     "merge_error": { "type": ["string", "null"] },
     "merge_commit_sha": { "type": ["string", "null"] },
     "merge_params": { "type": ["object", "null"] },
@@ -94,7 +93,8 @@
     "commit_change_content_path": { "type": "string" },
     "remove_wip_path": { "type": "string" },
     "commits_count": { "type": "integer" },
-    "remove_source_branch": { "type": ["boolean", "null"] }
+    "remove_source_branch": { "type": ["boolean", "null"] },
+    "merge_ongoing": { "type": "boolean" }
   },
   "additionalProperties": false
 }
diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_locked_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_locked_spec.js
index fb2ef606604..237035648cf 100644
--- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_locked_spec.js
+++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_locked_spec.js
@@ -1,10 +1,10 @@
 import Vue from 'vue';
-import lockedComponent from '~/vue_merge_request_widget/components/states/mr_widget_locked';
+import mergingComponent from '~/vue_merge_request_widget/components/states/mr_widget_merging';
 
-describe('MRWidgetLocked', () => {
+describe('MRWidgetMerging', () => {
   describe('props', () => {
     it('should have props', () => {
-      const { mr } = lockedComponent.props;
+      const { mr } = mergingComponent.props;
 
       expect(mr.type instanceof Object).toBeTruthy();
       expect(mr.required).toBeTruthy();
@@ -13,7 +13,7 @@ describe('MRWidgetLocked', () => {
 
   describe('template', () => {
     it('should have correct elements', () => {
-      const Component = Vue.extend(lockedComponent);
+      const Component = Vue.extend(mergingComponent);
       const mr = {
         targetBranchPath: '/branch-path',
         targetBranch: 'branch',
@@ -24,7 +24,7 @@ describe('MRWidgetLocked', () => {
       }).$el;
 
       expect(el.classList.contains('mr-widget-body')).toBeTruthy();
-      expect(el.innerText).toContain('it is locked');
+      expect(el.innerText).toContain('This merge request is in the process of being merged');
       expect(el.innerText).toContain('changes will be merged into');
       expect(el.querySelector('.label-branch a').getAttribute('href')).toEqual(mr.targetBranchPath);
       expect(el.querySelector('.label-branch a').textContent).toContain(mr.targetBranch);
diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js
index ad2f28b24f0..0795d0aaa82 100644
--- a/spec/javascripts/vue_mr_widget/mock_data.js
+++ b/spec/javascripts/vue_mr_widget/mock_data.js
@@ -20,7 +20,6 @@ export default {
   "human_time_estimate": null,
   "human_total_time_spent": null,
   "in_progress_merge_commit_sha": null,
-  "locked_at": null,
   "merge_commit_sha": "53027d060246c8f47e4a9310fb332aa52f221775",
   "merge_error": null,
   "merge_params": {
diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
index 3a0c50b750f..669ee248bf1 100644
--- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
+++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
@@ -342,7 +342,7 @@ describe('mrWidgetOptions', () => {
       expect(comps['mr-widget-related-links']).toBeDefined();
       expect(comps['mr-widget-merged']).toBeDefined();
       expect(comps['mr-widget-closed']).toBeDefined();
-      expect(comps['mr-widget-locked']).toBeDefined();
+      expect(comps['mr-widget-merging']).toBeDefined();
       expect(comps['mr-widget-failed-to-merge']).toBeDefined();
       expect(comps['mr-widget-wip']).toBeDefined();
       expect(comps['mr-widget-archived']).toBeDefined();
diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json
index 469a014e4d2..4e631e13410 100644
--- a/spec/lib/gitlab/import_export/project.json
+++ b/spec/lib/gitlab/import_export/project.json
@@ -2534,7 +2534,6 @@
       "iid": 9,
       "description": null,
       "position": 0,
-      "locked_at": null,
       "updated_by_id": null,
       "merge_error": null,
       "merge_params": {
@@ -2983,7 +2982,6 @@
       "iid": 8,
       "description": null,
       "position": 0,
-      "locked_at": null,
       "updated_by_id": null,
       "merge_error": null,
       "merge_params": {
@@ -3267,7 +3265,6 @@
       "iid": 7,
       "description": "Et commodi deserunt aspernatur vero rerum. Ut non dolorum alias in odit est libero. Voluptatibus eos in et vitae repudiandae facilis ex mollitia.",
       "position": 0,
-      "locked_at": null,
       "updated_by_id": null,
       "merge_error": null,
       "merge_params": {
@@ -3551,7 +3548,6 @@
       "iid": 6,
       "description": "Dicta magnam non voluptates nam dignissimos nostrum deserunt. Dolorum et suscipit iure quae doloremque. Necessitatibus saepe aut labore sed.",
       "position": 0,
-      "locked_at": null,
       "updated_by_id": null,
       "merge_error": null,
       "merge_params": {
@@ -4241,7 +4237,6 @@
       "iid": 5,
       "description": "Est eaque quasi qui qui. Similique voluptatem impedit iusto ratione reprehenderit. Itaque est illum ut nulla aut.",
       "position": 0,
-      "locked_at": null,
       "updated_by_id": null,
       "merge_error": null,
       "merge_params": {
@@ -4789,7 +4784,6 @@
       "iid": 4,
       "description": "Nam magnam odit velit rerum. Sapiente dolore sunt saepe debitis. Culpa maiores ut ad dolores dolorem et.",
       "position": 0,
-      "locked_at": null,
       "updated_by_id": null,
       "merge_error": null,
       "merge_params": {
@@ -5288,7 +5282,6 @@
       "iid": 3,
       "description": "Libero nesciunt mollitia quis odit eos vero quasi. Iure voluptatem ut sint pariatur voluptates ut aut. Laborum possimus unde illum ipsum eum.",
       "position": 0,
-      "locked_at": null,
       "updated_by_id": null,
       "merge_error": null,
       "merge_params": {
@@ -5548,7 +5541,6 @@
       "iid": 2,
       "description": "Ut dolor quia aliquid dolore et nisi. Est minus suscipit enim quaerat sapiente consequatur rerum. Eveniet provident consequatur dolor accusantium reiciendis.",
       "position": 0,
-      "locked_at": null,
       "updated_by_id": null,
       "merge_error": null,
       "merge_params": {
@@ -6238,7 +6230,6 @@
       "iid": 1,
       "description": "Eveniet nihil ratione veniam similique qui aut sapiente tempora. Sed praesentium iusto dignissimos possimus id repudiandae quo nihil. Qui doloremque autem et iure fugit.",
       "position": 0,
-      "locked_at": null,
       "updated_by_id": null,
       "merge_error": null,
       "merge_params": {
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 11f4c16ff96..4dce48f8079 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -145,7 +145,6 @@ MergeRequest:
 - iid
 - description
 - position
-- locked_at
 - updated_by_id
 - merge_error
 - merge_params
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 3402c260f27..a1a3e70a7d2 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1369,6 +1369,32 @@ describe MergeRequest do
     end
   end
 
+  describe '#merge_ongoing?' do
+    it 'returns true when merge process is ongoing for merge_jid' do
+      merge_request = create(:merge_request, merge_jid: 'foo')
+
+      allow(Gitlab::SidekiqStatus).to receive(:num_running).with(['foo']).and_return(1)
+
+      expect(merge_request.merge_ongoing?).to be(true)
+    end
+
+    it 'returns false when no merge process running for merge_jid' do
+      merge_request = build(:merge_request, merge_jid: 'foo')
+
+      allow(Gitlab::SidekiqStatus).to receive(:num_running).with(['foo']).and_return(0)
+
+      expect(merge_request.merge_ongoing?).to be(false)
+    end
+
+    it 'returns false when merge_jid is nil' do
+      merge_request = build(:merge_request, merge_jid: nil)
+
+      expect(Gitlab::SidekiqStatus).not_to receive(:num_running)
+
+      expect(merge_request.merge_ongoing?).to be(false)
+    end
+  end
+
   describe "#closed_without_fork?" do
     let(:project)      { create(:project) }
     let(:fork_project) { create(:project, forked_from_project: project) }
diff --git a/spec/serializers/merge_request_entity_spec.rb b/spec/serializers/merge_request_entity_spec.rb
index 18cd9e9c006..a2fd5b7daae 100644
--- a/spec/serializers/merge_request_entity_spec.rb
+++ b/spec/serializers/merge_request_entity_spec.rb
@@ -47,7 +47,7 @@ describe MergeRequestEntity do
                                :cancel_merge_when_pipeline_succeeds_path,
                                :create_issue_to_resolve_discussions_path,
                                :source_branch_path, :target_branch_commits_path,
-                               :target_branch_tree_path, :commits_count)
+                               :target_branch_tree_path, :commits_count, :merge_ongoing)
   end
 
   it 'has email_patches_path' do
diff --git a/spec/workers/merge_worker_spec.rb b/spec/workers/merge_worker_spec.rb
index 303193bab9b..ee51000161a 100644
--- a/spec/workers/merge_worker_spec.rb
+++ b/spec/workers/merge_worker_spec.rb
@@ -27,4 +27,15 @@ describe MergeWorker do
       expect(source_project.repository.branch_names).not_to include('markdown')
     end
   end
+
+  it 'persists merge_jid' do
+    merge_request = create(:merge_request, merge_jid: nil)
+    user = create(:user)
+    worker = described_class.new
+
+    allow(worker).to receive(:jid) { '999' }
+
+    expect { worker.perform(merge_request.id, user.id, {}) }
+      .to change { merge_request.reload.merge_jid }.from(nil).to('999')
+  end
 end
diff --git a/spec/workers/stuck_merge_jobs_worker_spec.rb b/spec/workers/stuck_merge_jobs_worker_spec.rb
new file mode 100644
index 00000000000..a5ad78393c9
--- /dev/null
+++ b/spec/workers/stuck_merge_jobs_worker_spec.rb
@@ -0,0 +1,50 @@
+require 'spec_helper'
+
+describe StuckMergeJobsWorker do
+  describe 'perform' do
+    let(:worker) { described_class.new }
+
+    context 'merge job identified as completed' do
+      it 'updates merge request to merged when locked but has merge_commit_sha' do
+        allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return(%w(123 456))
+        mr_with_sha = create(:merge_request, :locked, merge_jid: '123', state: :locked, merge_commit_sha: 'foo-bar-baz')
+        mr_without_sha = create(:merge_request, :locked, merge_jid: '123', state: :locked, merge_commit_sha: nil)
+
+        worker.perform
+
+        expect(mr_with_sha.reload).to be_merged
+        expect(mr_without_sha.reload).to be_opened
+      end
+
+      it 'updates merge request to opened when locked but has not been merged' do
+        allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return(%w(123))
+        merge_request = create(:merge_request, :locked, merge_jid: '123', state: :locked)
+
+        worker.perform
+
+        expect(merge_request.reload).to be_opened
+      end
+
+      it 'logs updated stuck merge job ids' do
+        allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return(%w(123 456))
+
+        create(:merge_request, :locked, merge_jid: '123')
+        create(:merge_request, :locked, merge_jid: '456')
+
+        expect(Rails).to receive_message_chain(:logger, :info).with('Updated state of locked merge jobs. JIDs: 123, 456')
+
+        worker.perform
+      end
+    end
+
+    context 'merge job not identified as completed' do
+      it 'does not change merge request state when job is not completed yet' do
+        allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return([])
+
+        merge_request = create(:merge_request, :locked, merge_jid: '123')
+
+        expect { worker.perform }.not_to change { merge_request.reload.state }.from('locked')
+      end
+    end
+  end
+end