Commit 609f4048 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'make-merge-jid-handling-less-stateful' into 'master'

Fix widget of locked merge requests not being presented

See merge request gitlab-org/gitlab-ce!15069
parents 2573818f c4124fce
...@@ -396,6 +396,10 @@ class MergeRequest < ActiveRecord::Base ...@@ -396,6 +396,10 @@ class MergeRequest < ActiveRecord::Base
end end
def merge_ongoing? def merge_ongoing?
# While the MergeRequest is locked, it should present itself as 'merge ongoing'.
# The unlocking process is handled by StuckMergeJobsWorker scheduled in Cron.
return true if locked?
!!merge_jid && !merged? && Gitlab::SidekiqStatus.running?(merge_jid) !!merge_jid && !merged? && Gitlab::SidekiqStatus.running?(merge_jid)
end end
......
...@@ -82,16 +82,9 @@ module MergeRequests ...@@ -82,16 +82,9 @@ module MergeRequests
@merge_request.can_remove_source_branch?(branch_deletion_user) @merge_request.can_remove_source_branch?(branch_deletion_user)
end end
# Logs merge error message and cleans `MergeRequest#merge_jid`.
#
def handle_merge_error(log_message:, save_message_on_model: false) def handle_merge_error(log_message:, save_message_on_model: false)
Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{log_message}") Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{log_message}")
@merge_request.update(merge_error: log_message) if save_message_on_model
if save_message_on_model
@merge_request.update(merge_error: log_message, merge_jid: nil)
else
clean_merge_jid
end
end end
def merge_request_info def merge_request_info
......
...@@ -23,7 +23,7 @@ class StuckMergeJobsWorker ...@@ -23,7 +23,7 @@ class StuckMergeJobsWorker
merge_requests = MergeRequest.where(id: completed_ids) merge_requests = MergeRequest.where(id: completed_ids)
merge_requests.where.not(merge_commit_sha: nil).update_all(state: :merged) merge_requests.where.not(merge_commit_sha: nil).update_all(state: :merged)
merge_requests.where(merge_commit_sha: nil).update_all(state: :opened) merge_requests.where(merge_commit_sha: nil).update_all(state: :opened, merge_jid: nil)
Rails.logger.info("Updated state of locked merge jobs. JIDs: #{completed_jids.join(', ')}") Rails.logger.info("Updated state of locked merge jobs. JIDs: #{completed_jids.join(', ')}")
end end
......
---
title: Fix widget of locked merge requests not being presented
merge_request:
author:
type: fixed
...@@ -1460,6 +1460,12 @@ describe MergeRequest do ...@@ -1460,6 +1460,12 @@ describe MergeRequest do
end end
describe '#merge_ongoing?' do describe '#merge_ongoing?' do
it 'returns true when the merge request is locked' do
merge_request = build_stubbed(:merge_request, state: :locked)
expect(merge_request.merge_ongoing?).to be(true)
end
it 'returns true when merge_id, MR is not merged and it has no running job' 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') merge_request = build_stubbed(:merge_request, state: :open, merge_jid: 'foo')
allow(Gitlab::SidekiqStatus).to receive(:running?).with('foo') { true } allow(Gitlab::SidekiqStatus).to receive(:running?).with('foo') { true }
......
...@@ -12,55 +12,6 @@ describe MergeRequests::MergeService do ...@@ -12,55 +12,6 @@ describe MergeRequests::MergeService do
end end
describe '#execute' do describe '#execute' do
context 'MergeRequest#merge_jid' do
let(:service) do
described_class.new(project, user, commit_message: 'Awesome message')
end
before do
merge_request.update_column(:merge_jid, 'hash-123')
end
it 'is cleaned when no error is raised' do
service.execute(merge_request)
expect(merge_request.reload.merge_jid).to be_nil
end
it 'is cleaned when expected error is raised' do
allow(service).to receive(:commit).and_raise(described_class::MergeError)
service.execute(merge_request)
expect(merge_request.reload.merge_jid).to be_nil
end
it 'is cleaned when merge request is not mergeable' do
allow(merge_request).to receive(:mergeable?).and_return(false)
service.execute(merge_request)
expect(merge_request.reload.merge_jid).to be_nil
end
it 'is cleaned when no source is found' do
allow(merge_request).to receive(:diff_head_sha).and_return(nil)
service.execute(merge_request)
expect(merge_request.reload.merge_jid).to be_nil
end
it 'is not cleaned when unexpected error is raised' do
service = described_class.new(project, user, commit_message: 'Awesome message')
allow(service).to receive(:commit).and_raise(StandardError)
expect { service.execute(merge_request) }.to raise_error(StandardError)
expect(merge_request.reload.merge_jid).to be_present
end
end
context 'valid params' do context 'valid params' do
let(:service) { described_class.new(project, user, commit_message: 'Awesome message') } let(:service) { described_class.new(project, user, commit_message: 'Awesome message') }
......
...@@ -12,8 +12,13 @@ describe StuckMergeJobsWorker do ...@@ -12,8 +12,13 @@ describe StuckMergeJobsWorker do
worker.perform worker.perform
expect(mr_with_sha.reload).to be_merged mr_with_sha.reload
expect(mr_without_sha.reload).to be_opened mr_without_sha.reload
expect(mr_with_sha).to be_merged
expect(mr_without_sha).to be_opened
expect(mr_with_sha.merge_jid).to be_present
expect(mr_without_sha.merge_jid).to be_nil
end end
it 'updates merge request to opened when locked but has not been merged' do it 'updates merge request to opened when locked but has not been merged' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment