From af86b8c2c2b6fb08ea55eb89f1dd20aba81862ae Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Mon, 18 Jul 2016 21:11:53 +0800
Subject: [PATCH] Latest success pipelines (rather than builds)

---
 app/models/ci/pipeline.rb                 |  8 ++++++
 app/models/commit_status.rb               |  4 +--
 app/models/project.rb                     |  7 ++---
 spec/models/build_spec.rb                 | 35 +++++++++++++++++------
 spec/requests/shared/artifacts_context.rb | 19 +++++++-----
 5 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index fa4071e2482..431a91004cd 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -18,6 +18,14 @@ module Ci
     after_touch :update_state
     after_save :keep_around_commits
 
+    scope :latest, -> do
+      max_id = unscope(:select).
+                 select("max(#{table_name}.id)").
+                 group(:ref)
+
+      where(id: max_id)
+    end
+
     def self.truncate_sha(sha)
       sha[0...8]
     end
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index baabbd785cc..3e97fe68d4b 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -18,8 +18,8 @@ class CommitStatus < ActiveRecord::Base
 
   scope :latest, -> do
     max_id = unscope(:select).
-           select("max(#{table_name}.id)").
-           group(:name, :commit_id)
+               select("max(#{table_name}.id)").
+               group(:name, :commit_id)
 
     where(id: max_id)
   end
diff --git a/app/models/project.rb b/app/models/project.rb
index 770ec1c8a68..1578fe67581 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -430,12 +430,9 @@ class Project < ActiveRecord::Base
   end
 
   def latest_success_builds_for(ref = 'HEAD')
-    builds_for(ref).success.latest
-  end
-
-  def builds_for(ref = 'HEAD')
     Ci::Build.joins(:pipeline).
-      merge(Ci::Pipeline.where(ref: ref, project: self))
+      merge(pipelines.where(ref: ref).success.latest).
+      with_artifacts
   end
 
   def merge_base_commit(first_commit_id, second_commit_id)
diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index 53064138a50..8a8a4b46f08 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -6,7 +6,8 @@ describe Ci::Build, models: true do
   let(:pipeline) do
     create(:ci_pipeline, project: project,
                          sha: project.commit.id,
-                         ref: 'fix')
+                         ref: 'fix',
+                         status: 'success')
   end
 
   let(:build) { create(:ci_build, pipeline: pipeline) }
@@ -694,20 +695,38 @@ describe Ci::Build, models: true do
   end
 
   describe 'Project#latest_success_builds_for' do
+    let(:build) do
+      create(:ci_build, :artifacts, :success, pipeline: pipeline)
+    end
+
     before do
-      build.update(status: 'success')
+      build
     end
 
-    it 'returns builds from ref' do
-      builds = project.latest_success_builds_for('fix')
+    context 'with succeed pipeline' do
+      it 'returns builds from ref' do
+        builds = project.latest_success_builds_for('fix')
+
+        expect(builds).to contain_exactly(build)
+      end
+
+      it 'returns empty relation if the build cannot be found' do
+        builds = project.latest_success_builds_for('TAIL').all
 
-      expect(builds).to contain_exactly(build)
+        expect(builds).to be_empty
+      end
     end
 
-    it 'returns empty relation if the build cannot be found' do
-      builds = project.latest_success_builds_for('TAIL').all
+    context 'with pending pipeline' do
+      before do
+        pipeline.update(status: 'pending')
+      end
 
-      expect(builds).to be_empty
+      it 'returns empty relation' do
+        builds = project.latest_success_builds_for('fix').all
+
+        expect(builds).to be_empty
+      end
     end
   end
 end
diff --git a/spec/requests/shared/artifacts_context.rb b/spec/requests/shared/artifacts_context.rb
index ff74b72a0b3..635c5646f91 100644
--- a/spec/requests/shared/artifacts_context.rb
+++ b/spec/requests/shared/artifacts_context.rb
@@ -25,7 +25,7 @@ shared_examples 'artifacts from ref with 404' do
 
   context 'has no such build' do
     before do
-      get path_from_ref(pipeline.sha, 'NOBUILD')
+      get path_from_ref(pipeline.ref, 'NOBUILD')
     end
 
     it('gives 404') { verify }
@@ -33,6 +33,11 @@ shared_examples 'artifacts from ref with 404' do
 end
 
 shared_examples 'artifacts from ref successfully' do
+  def create_new_pipeline(status)
+    new_pipeline = create(:ci_pipeline, status: 'success')
+    create(:ci_build, status, :artifacts, pipeline: new_pipeline)
+  end
+
   context 'with regular branch' do
     before do
       pipeline.update(ref: 'master',
@@ -59,10 +64,10 @@ shared_examples 'artifacts from ref successfully' do
     it('gives the file') { verify }
   end
 
-  context 'with latest build' do
+  context 'with latest pipeline' do
     before do
-      3.times do # creating some old builds
-        create(:ci_build, :success, :artifacts, pipeline: pipeline)
+      3.times do # creating some old pipelines
+        create_new_pipeline(:success)
       end
     end
 
@@ -73,10 +78,10 @@ shared_examples 'artifacts from ref successfully' do
     it('gives the file') { verify }
   end
 
-  context 'with success build' do
+  context 'with success pipeline' do
     before do
-      build # make sure build was old, but still the latest success one
-      create(:ci_build, :pending, :artifacts, pipeline: pipeline)
+      build # make sure pipeline was old, but still the latest success one
+      create_new_pipeline(:pending)
     end
 
     before do
-- 
GitLab