Commit 513a636c authored by Marc Shaw's avatar Marc Shaw

UI Bug Fix: Expand the early checks when using /merge quick action

At the moment we do a check of the `mergable_state?` method to see
if it can be merged, and later when we try to merge, the
`mergeable?` method is called. But sometimes this means we return
that the merge request was merged, when it actually isn't.

Issue: gitlab.com/gitlab-org/gitlab/-/issues/354453
MR: gitlab.com/gitlab-org/gitlab/-/merge_requests/82170

Changelog: fixed
EE: true
parent d14b5c3f
......@@ -26,7 +26,7 @@ module MergeRequests
def can_merge_immediately?(merge_request)
merge_request.can_be_merged_by?(current_user) &&
merge_request.mergeable_state?
merge_request.mergeable?
end
def can_merge_automatically?(merge_request)
......
......@@ -8,6 +8,12 @@ FactoryBot.modify do
end
end
trait :blocked do
after :create do |merge_request, _|
create(:merge_request_block, blocked_merge_request: merge_request)
end
end
trait :on_train do
transient do
train_creator { author }
......
......@@ -87,6 +87,22 @@ RSpec.describe QuickActions::InterpretService do
end
end
shared_examples 'failed command' do |error_msg|
let(:match_msg) { error_msg ? eq(error_msg) : be_empty }
it 'populates {} if content contains an unsupported command' do
_, updates, _ = service.execute(content, issuable)
expect(updates).to be_empty
end
it "returns #{error_msg || 'an empty'} message" do
_, _, message = service.execute(content, issuable)
expect(message).to match_msg
end
end
describe '#execute' do
let(:merge_request) { create(:merge_request, source_project: project) }
......@@ -1227,6 +1243,36 @@ RSpec.describe QuickActions::InterpretService do
end
end
context 'when the merge request is not approved' do
it_behaves_like 'failed command', 'Could not apply merge command.' do
let(:content) { '/merge' }
let(:issuable) { merge_request }
let!(:rule) { create(:any_approver_rule, merge_request: merge_request, approvals_required: 1) }
end
end
context 'when the merge request is blocked' do
it_behaves_like 'failed command', 'Could not apply merge command.' do
before do
stub_licensed_features(blocking_merge_requests: true)
end
let(:content) { '/merge' }
let(:issuable) { create(:merge_request, :blocked, source_project: project) }
end
end
context 'when merge request has denied policies' do
it_behaves_like 'failed command', 'Could not apply merge command.' do
before do
allow(merge_request).to receive(:has_denied_policies?).and_return(true)
end
let(:content) { '/merge' }
let(:issuable) { merge_request }
end
end
context 'approved merge request can be merged' do
before do
merge_request.update!(approvals_before_merge: 1)
......
......@@ -64,7 +64,7 @@ RSpec.describe MergeRequests::MergeOrchestrationService do
context 'when merge request is not mergeable' do
before do
allow(merge_request).to receive(:mergeable_state?) { false }
allow(merge_request).to receive(:mergeable?) { false }
end
it 'does nothing' do
......@@ -87,7 +87,7 @@ RSpec.describe MergeRequests::MergeOrchestrationService do
context 'when merge request is not mergeable' do
before do
allow(merge_request).to receive(:mergeable_state?) { false }
allow(merge_request).to receive(:mergeable?) { false }
end
it { is_expected.to eq(false) }
......
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