Merge branch 'mc/bug/nil-take' into 'master'

Fix nil take regression Closes #65725 See merge request gitlab-org/gitlab-ce!31554

Merge branch 'mc/bug/nil-take' into 'master'
Fix nil take regression Closes #65725 See merge request gitlab-org/gitlab-ce!31554
ee028785 · Stan Hu · 2fd73269 · 56876070 · ee028785 · ee028785
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1487,6 +1487,9 @@ def allowed_to_share_with_group?
  end

  def pipeline_for(ref, sha = nil, id = nil)
+    sha ||= commit(ref).try(:sha)
+    return unless sha
+
    if id.present?
      pipelines_for(ref, sha).find_by(id: id)
    else
@@ -1494,11 +1497,7 @@ def pipeline_for(ref, sha = nil, id = nil)
    end
  end

-  def pipelines_for(ref, sha = nil)
-    sha ||= commit(ref).try(:sha)
-
-    return unless sha
-
+  def pipelines_for(ref, sha)
    ci_pipelines.order(id: :desc).where(sha: sha, ref: ref)
  end


--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1156,7 +1156,6 @@

  describe '#pipeline_for' do
    let(:project) { create(:project, :repository) }
-    let!(:pipeline) { create_pipeline(project) }

    shared_examples 'giving the correct pipeline' do
      it { is_expected.to eq(pipeline) }
@@ -1168,24 +1167,34 @@
      end
    end

-    context 'with explicit sha' do
-      subject { project.pipeline_for('master', pipeline.sha) }
+    context 'with a matching pipeline' do
+      let!(:pipeline) { create_pipeline(project) }
+
+      context 'with explicit sha' do
+        subject { project.pipeline_for('master', pipeline.sha) }
+
+        it_behaves_like 'giving the correct pipeline'
+
+        context 'with supplied id' do
+          let!(:other_pipeline) { create_pipeline(project) }

-      it_behaves_like 'giving the correct pipeline'
+          subject { project.pipeline_for('master', pipeline.sha, other_pipeline.id) }

-      context 'with supplied id' do
-        let!(:other_pipeline) { create_pipeline(project) }
+          it { is_expected.to eq(other_pipeline) }
+        end
+      end

-        subject { project.pipeline_for('master', pipeline.sha, other_pipeline.id) }
+      context 'with implicit sha' do
+        subject { project.pipeline_for('master') }

-        it { is_expected.to eq(other_pipeline) }
+        it_behaves_like 'giving the correct pipeline'
      end
    end

-    context 'with implicit sha' do
+    context 'when there is no matching pipeline' do
      subject { project.pipeline_for('master') }

-      it_behaves_like 'giving the correct pipeline'
+      it { is_expected.to be_nil }
    end
  end

@@ -1194,11 +1203,9 @@
    let!(:pipeline) { create_pipeline(project) }
    let!(:other_pipeline) { create_pipeline(project) }

-    context 'with implicit sha' do
-      subject { project.pipelines_for('master') }
+    subject { project.pipelines_for(project.default_branch, project.commit.sha) }

-      it { is_expected.to contain_exactly(pipeline, other_pipeline) }
-    end
+    it { is_expected.to contain_exactly(pipeline, other_pipeline) }
  end

  describe '#builds_enabled' do