From ecb0cb563e4339b4fc807ced7ce1a6d57d82fc85 Mon Sep 17 00:00:00 2001
From: Gary Holtz <gholtz@gitlab.com>
Date: Wed, 3 Aug 2022 16:22:27 +0000
Subject: [PATCH] This adds a dependency condition to the base auto merge
service
* This will do a check for any MR dependencies before allowing the
appropriate auto merge strategy.
* Adds backend specs for the possible scenarios as well
Changelog: fixed
EE: true
---
app/models/merge_request.rb | 4 ++
app/services/auto_merge/base_service.rb | 1 +
ee/app/models/ee/merge_request.rb | 1 +
...ain_when_pipeline_succeeds_service_spec.rb | 9 ++++
.../auto_merge/merge_train_service_spec.rb | 9 ++++
...rge_when_pipeline_succeeds_service_spec.rb | 52 +++++++++++++++++++
spec/models/merge_request_spec.rb | 6 +++
7 files changed, 82 insertions(+)
create mode 100644 ee/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 3d789035b9a42..a4d9bed70df68 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -2002,6 +2002,10 @@ def target_default_branch?
target_branch == project.default_branch
end
+ def merge_blocked_by_other_mrs?
+ false # Overridden in EE
+ end
+
private
attr_accessor :skip_fetch_ref
diff --git a/app/services/auto_merge/base_service.rb b/app/services/auto_merge/base_service.rb
index 9e49bd86ec092..1660ddb934faa 100644
--- a/app/services/auto_merge/base_service.rb
+++ b/app/services/auto_merge/base_service.rb
@@ -59,6 +59,7 @@ def available_for?(merge_request)
!merge_request.broken? &&
!merge_request.draft? &&
merge_request.mergeable_discussions_state? &&
+ !merge_request.merge_blocked_by_other_mrs? &&
yield
end
end
diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb
index 35eb48dc3ab4b..6b7e5a75e99a8 100644
--- a/ee/app/models/ee/merge_request.rb
+++ b/ee/app/models/ee/merge_request.rb
@@ -151,6 +151,7 @@ def mergeability_checks
] + super
end
+ override :merge_blocked_by_other_mrs?
def merge_blocked_by_other_mrs?
strong_memoize(:merge_blocked_by_other_mrs) do
project.feature_available?(:blocking_merge_requests) &&
diff --git a/ee/spec/services/auto_merge/add_to_merge_train_when_pipeline_succeeds_service_spec.rb b/ee/spec/services/auto_merge/add_to_merge_train_when_pipeline_succeeds_service_spec.rb
index 30c36c0109752..704a5d8912f8f 100644
--- a/ee/spec/services/auto_merge/add_to_merge_train_when_pipeline_succeeds_service_spec.rb
+++ b/ee/spec/services/auto_merge/add_to_merge_train_when_pipeline_succeeds_service_spec.rb
@@ -154,6 +154,15 @@
it { is_expected.to eq(false) }
end
+ context 'when there is an open MR dependency' do
+ before do
+ stub_licensed_features(blocking_merge_requests: true)
+ create(:merge_request_block, blocked_merge_request: merge_request)
+ end
+
+ it { is_expected.to be_falsy }
+ end
+
context 'when the user does not have permission to merge' do
before do
allow(merge_request).to receive(:can_be_merged_by?) { false }
diff --git a/ee/spec/services/auto_merge/merge_train_service_spec.rb b/ee/spec/services/auto_merge/merge_train_service_spec.rb
index 29d6bf486f743..5fb7e498c878c 100644
--- a/ee/spec/services/auto_merge/merge_train_service_spec.rb
+++ b/ee/spec/services/auto_merge/merge_train_service_spec.rb
@@ -357,6 +357,15 @@
it { is_expected.to be_falsy }
end
+ context 'when there is an open MR dependency' do
+ before do
+ stub_licensed_features(blocking_merge_requests: true)
+ create(:merge_request_block, blocked_merge_request: merge_request)
+ end
+
+ it { is_expected.to be_falsy }
+ end
+
context 'when merge train ci setting is disabled' do
before do
stub_feature_flags(disable_merge_trains: true)
diff --git a/ee/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb b/ee/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb
new file mode 100644
index 0000000000000..dc2c6ee5a0797
--- /dev/null
+++ b/ee/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe AutoMerge::MergeWhenPipelineSucceedsService do
+ let_it_be(:user) { create(:user) }
+ let_it_be(:project) { create(:project, :repository) }
+
+ let(:mr_merge_if_green_enabled) do
+ create(:merge_request, merge_when_pipeline_succeeds: true, merge_user: user,
+ source_branch: "master", target_branch: 'feature',
+ source_project: project, target_project: project, state: "opened")
+ end
+
+ let(:pipeline) do
+ create(:ci_pipeline, ref: mr_merge_if_green_enabled.source_branch, project: project)
+ end
+
+ let(:service) do
+ described_class.new(project, user, commit_message: 'Awesome message')
+ end
+
+ before_all do
+ project.add_maintainer(user)
+ end
+
+ before do
+ allow(MergeWorker).to receive(:with_status).and_return(MergeWorker)
+ end
+
+ describe "#available_for?" do
+ subject { service.available_for?(mr_merge_if_green_enabled) }
+
+ let(:pipeline_status) { :running }
+
+ before do
+ create(:ci_pipeline, pipeline_status, ref: mr_merge_if_green_enabled.source_branch,
+ sha: mr_merge_if_green_enabled.diff_head_sha,
+ project: mr_merge_if_green_enabled.source_project)
+ mr_merge_if_green_enabled.update_head_pipeline
+ end
+
+ context 'when there is an open MR dependency' do
+ before do
+ stub_licensed_features(blocking_merge_requests: true)
+ create(:merge_request_block, blocked_merge_request: mr_merge_if_green_enabled)
+ end
+
+ it { is_expected.to be_falsy }
+ end
+ end
+end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index bd35b51409651..d15f0bd410364 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -5078,6 +5078,12 @@ def create_build(pipeline, coverage, name)
end
end
+ describe '#merge_blocked_by_other_mrs?' do
+ it 'returns false when there is no blocking merge requests' do
+ expect(subject.merge_blocked_by_other_mrs?).to be_falsy
+ end
+ end
+
describe '#merge_request_reviewers_with' do
let_it_be(:reviewer1) { create(:user) }
let_it_be(:reviewer2) { create(:user) }
--
GitLab