From e0efa97c7f24bc5816ecdcb6a633150e7a6a43f6 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Wed, 3 Apr 2019 18:33:52 +0700
Subject: Prevent triggering pipelines when target branch is updated

Currently, pipelines for merge requests are triggered when source or
target branch is updated. However, we should create only when source
branch is updated, because it runs unexpected pipelines.
---
 app/services/merge_requests/refresh_service.rb     | 10 +++++--
 ...nt-running-mr-pipelines-when-target-updated.yml |  5 ++++
 .../merge_requests/refresh_service_spec.rb         | 34 +++++++++++++++++-----
 3 files changed, 40 insertions(+), 9 deletions(-)
 create mode 100644 changelogs/unreleased/prevent-running-mr-pipelines-when-target-updated.yml

diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index 51d27673787..e0460f7081c 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -21,6 +21,7 @@ module MergeRequests
       post_merge_manually_merged
       reload_merge_requests
       outdate_suggestions
+      refresh_pipelines_on_merge_requests
       reset_merge_when_pipeline_succeeds
       mark_pending_todos_done
       cache_merge_requests_closing_issues
@@ -107,8 +108,6 @@ module MergeRequests
         end
 
         merge_request.mark_as_unchecked
-        create_pipeline_for(merge_request, current_user)
-        UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id)
       end
 
       # Upcoming method calls need the refreshed version of
@@ -134,6 +133,13 @@ module MergeRequests
       end
     end
 
+    def refresh_pipelines_on_merge_requests
+      merge_requests_for_source_branch.each do |merge_request|
+        create_pipeline_for(merge_request, current_user)
+        UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id)
+      end
+    end
+
     def reset_merge_when_pipeline_succeeds
       merge_requests_for_source_branch.each(&:reset_merge_when_pipeline_succeeds)
     end
diff --git a/changelogs/unreleased/prevent-running-mr-pipelines-when-target-updated.yml b/changelogs/unreleased/prevent-running-mr-pipelines-when-target-updated.yml
new file mode 100644
index 00000000000..d003ca55feb
--- /dev/null
+++ b/changelogs/unreleased/prevent-running-mr-pipelines-when-target-updated.yml
@@ -0,0 +1,5 @@
+---
+title: Create pipelines for merge requests only when source branch is updated
+merge_request: 26921
+author:
+type: fixed
diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb
index bd10523bc94..5ed06df7072 100644
--- a/spec/services/merge_requests/refresh_service_spec.rb
+++ b/spec/services/merge_requests/refresh_service_spec.rb
@@ -146,7 +146,10 @@ describe MergeRequests::RefreshService do
         stub_ci_pipeline_yaml_file(YAML.dump(config))
       end
 
-      subject { service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/master') }
+      subject { service.new(project, @user).execute(@oldrev, @newrev, ref) }
+
+      let(:ref) { 'refs/heads/master' }
+      let(:project) { @project }
 
       context "when .gitlab-ci.yml has merge_requests keywords" do
         let(:config) do
@@ -162,14 +165,17 @@ describe MergeRequests::RefreshService do
         it 'create detached merge request pipeline with commits' do
           expect { subject }
             .to change { @merge_request.merge_request_pipelines.count }.by(1)
-            .and change { @fork_merge_request.merge_request_pipelines.count }.by(1)
             .and change { @another_merge_request.merge_request_pipelines.count }.by(0)
 
           expect(@merge_request.has_commits?).to be_truthy
-          expect(@fork_merge_request.has_commits?).to be_truthy
           expect(@another_merge_request.has_commits?).to be_falsy
         end
 
+        it 'does not create detached merge request pipeline for forked project' do
+          expect { subject }
+            .not_to change { @fork_merge_request.merge_request_pipelines.count }
+        end
+
         it 'create detached merge request pipeline for non-fork merge request' do
           subject
 
@@ -177,11 +183,25 @@ describe MergeRequests::RefreshService do
             .to be_detached_merge_request_pipeline
         end
 
-        it 'create legacy detached merge request pipeline for fork merge request' do
-          subject
+        context 'when service is hooked by target branch' do
+          let(:ref) { 'refs/heads/feature' }
 
-          expect(@fork_merge_request.merge_request_pipelines.first)
-            .to be_legacy_detached_merge_request_pipeline
+          it 'does not create detached merge request pipeline' do
+            expect { subject }
+              .not_to change { @merge_request.merge_request_pipelines.count }
+          end
+        end
+
+        context 'when service runs on forked project' do
+          let(:project) { @fork_project }
+
+          it 'creates legacy detached merge request pipeline for fork merge request' do
+            expect { subject }
+              .to change { @fork_merge_request.merge_request_pipelines.count }.by(1)
+
+            expect(@fork_merge_request.merge_request_pipelines.first)
+              .to be_legacy_detached_merge_request_pipeline
+          end
         end
 
         context 'when ci_use_merge_request_ref feature flag is false' do
-- 
cgit v1.2.3