Merge branch '39032-improve-merge-ongoing-check-consistency' into 'master'

Make "merge ongoing" check more consistent

Closes #39032

See merge request gitlab-org/gitlab-ce!14825

5 files changed, 46 insertions, 2 deletions

diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 972a35dde4d..c3fae16d109 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -396,7 +396,7 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def merge_ongoing?
-    !!merge_jid && !merged?
+    !!merge_jid && !merged? && Gitlab::SidekiqStatus.running?(merge_jid)
   end
 
   def closed_without_fork?
diff --git a/changelogs/unreleased/39032-improve-merge-ongoing-check-consistency.yml b/changelogs/unreleased/39032-improve-merge-ongoing-check-consistency.yml
new file mode 100644
index 00000000000..361b6af196a
--- /dev/null
+++ b/changelogs/unreleased/39032-improve-merge-ongoing-check-consistency.yml
@@ -0,0 +1,5 @@
+---
+title: Make "merge ongoing" check more consistent
+merge_request:
+author:
+type: fixed
diff --git a/lib/gitlab/sidekiq_status.rb b/lib/gitlab/sidekiq_status.rb
index a0a2769cf9e..a1f689d94d9 100644
--- a/lib/gitlab/sidekiq_status.rb
+++ b/lib/gitlab/sidekiq_status.rb
@@ -51,6 +51,13 @@ module Gitlab
       self.num_running(job_ids).zero?
     end
 
+    # Returns true if the given job is running
+    #
+    # job_id - The Sidekiq job ID to check.
+    def self.running?(job_id)
+      num_running([job_id]) > 0
+    end
+
     # Returns the number of jobs that are running.
     #
     # job_ids - The Sidekiq job IDs to check.
diff --git a/spec/lib/gitlab/sidekiq_status_spec.rb b/spec/lib/gitlab/sidekiq_status_spec.rb
index c2e77ef6b6c..884f27b212c 100644
--- a/spec/lib/gitlab/sidekiq_status_spec.rb
+++ b/spec/lib/gitlab/sidekiq_status_spec.rb
@@ -39,6 +39,18 @@ describe Gitlab::SidekiqStatus do
     end
   end
 
+  describe '.running?', :clean_gitlab_redis_shared_state do
+    it 'returns true if job is running' do
+      described_class.set('123')
+
+      expect(described_class.running?('123')).to be(true)
+    end
+
+    it 'returns false if job is not found' do
+      expect(described_class.running?('123')).to be(false)
+    end
+  end
+
   describe '.num_running', :clean_gitlab_redis_shared_state do
     it 'returns 0 if all jobs have been completed' do
       expect(described_class.num_running(%w(123))).to eq(0)
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 17c9f15b021..73e038b61ed 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1460,11 +1460,31 @@ describe MergeRequest do
   end
 
   describe '#merge_ongoing?' do
-    it 'returns true when merge_id is present and MR is not merged' do
+    it 'returns true when merge_id, MR is not merged and it has no running job' do
       merge_request = build_stubbed(:merge_request, state: :open, merge_jid: 'foo')
+      allow(Gitlab::SidekiqStatus).to receive(:running?).with('foo') { true }
 
       expect(merge_request.merge_ongoing?).to be(true)
     end
+
+    it 'returns false when merge_jid is nil' do
+      merge_request = build_stubbed(:merge_request, state: :open, merge_jid: nil)
+
+      expect(merge_request.merge_ongoing?).to be(false)
+    end
+
+    it 'returns false if MR is merged' do
+      merge_request = build_stubbed(:merge_request, state: :merged, merge_jid: 'foo')
+
+      expect(merge_request.merge_ongoing?).to be(false)
+    end
+
+    it 'returns false if there is no merge job running' do
+      merge_request = build_stubbed(:merge_request, state: :open, merge_jid: 'foo')
+      allow(Gitlab::SidekiqStatus).to receive(:running?).with('foo') { false }
+
+      expect(merge_request.merge_ongoing?).to be(false)
+    end
   end
 
   describe "#closed_without_fork?" do