From b46b9d5e89d54cf1e374a014f0d523735c82ab8c Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Sat, 17 Aug 2019 00:42:23 -0700
Subject: [PATCH] Fix pipelines not always being created after a push

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/31741 introduced
a regression where not all the right parameters would be passed into
`Ci::CreatePipelineService`. We fix this by breaking out the pipeline
parameters and reusing a method from `Gitlab::DataBuilder::Push`.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/66196
---
 app/services/git/base_hooks_service.rb        | 19 ++++++++++++-------
 .../sh-fix-pipelines-not-being-created.yml    |  5 +++++
 lib/gitlab/data_builder/push.rb               |  2 --
 spec/services/git/branch_push_service_spec.rb | 16 ++++++++++++++++
 4 files changed, 33 insertions(+), 9 deletions(-)
 create mode 100644 changelogs/unreleased/sh-fix-pipelines-not-being-created.yml

diff --git a/app/services/git/base_hooks_service.rb b/app/services/git/base_hooks_service.rb
index 3fd38444196..47c308c8280 100644
--- a/app/services/git/base_hooks_service.rb
+++ b/app/services/git/base_hooks_service.rb
@@ -56,7 +56,7 @@ module Git
       return unless params.fetch(:create_pipelines, true)
 
       Ci::CreatePipelineService
-        .new(project, current_user, base_params)
+        .new(project, current_user, pipeline_params)
         .execute(:push, pipeline_options)
     end
 
@@ -75,24 +75,29 @@ module Git
       ProjectCacheWorker.perform_async(project.id, file_types, [], false)
     end
 
-    def base_params
+    def pipeline_params
       {
-        oldrev: params[:oldrev],
-        newrev: params[:newrev],
+        before: params[:oldrev],
+        after: params[:newrev],
         ref: params[:ref],
-        push_options: params[:push_options] || {}
+        push_options: params[:push_options] || {},
+        checkout_sha: Gitlab::DataBuilder::Push.checkout_sha(
+          project.repository, params[:newrev], params[:ref])
       }
     end
 
     def push_data_params(commits:, with_changed_files: true)
-      base_params.merge(
+      {
+        oldrev: params[:oldrev],
+        newrev: params[:newrev],
+        ref: params[:ref],
         project: project,
         user: current_user,
         commits: commits,
         message: event_message,
         commits_count: commits_count,
         with_changed_files: with_changed_files
-      )
+      }
     end
 
     def event_push_data
diff --git a/changelogs/unreleased/sh-fix-pipelines-not-being-created.yml b/changelogs/unreleased/sh-fix-pipelines-not-being-created.yml
new file mode 100644
index 00000000000..a6937eae588
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-pipelines-not-being-created.yml
@@ -0,0 +1,5 @@
+---
+title: Fix pipelines not always being created after a push
+merge_request: 31927
+author:
+type: fixed
diff --git a/lib/gitlab/data_builder/push.rb b/lib/gitlab/data_builder/push.rb
index 37fadb47736..75d9a2d55b9 100644
--- a/lib/gitlab/data_builder/push.rb
+++ b/lib/gitlab/data_builder/push.rb
@@ -129,8 +129,6 @@ module Gitlab
         SAMPLE_DATA
       end
 
-      private
-
       def checkout_sha(repository, newrev, ref)
         # Checkout sha is nil when we remove branch or tag
         return if Gitlab::Git.blank_ref?(newrev)
diff --git a/spec/services/git/branch_push_service_spec.rb b/spec/services/git/branch_push_service_spec.rb
index ad5d296f5c1..d9e607cd251 100644
--- a/spec/services/git/branch_push_service_spec.rb
+++ b/spec/services/git/branch_push_service_spec.rb
@@ -76,6 +76,22 @@ describe Git::BranchPushService, services: true do
       stub_ci_pipeline_to_return_yaml_file
     end
 
+    it 'creates a pipeline with the right parameters' do
+      expect(Ci::CreatePipelineService)
+        .to receive(:new)
+        .with(project,
+              user,
+              {
+                before: oldrev,
+                after: newrev,
+                ref: ref,
+                checkout_sha: SeedRepo::Commit::ID,
+                push_options: {}
+              }).and_call_original
+
+      subject
+    end
+
     it "creates a new pipeline" do
       expect { subject }.to change { Ci::Pipeline.count }