From 18a71c47603d703de73c46fef4889887f685bebe Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Mon, 7 Nov 2016 23:44:11 +0800
Subject: [PATCH 01/15] Show commit status from latest pipeline

Rather than compound status from all pipelines.

Closes #20560
---
 app/models/ci/pipeline.rb                     |  6 ++++-
 app/models/commit.rb                          | 16 ++++++-----
 ...how-commit-status-from-latest-pipeline.yml |  4 +++
 spec/models/commit_spec.rb                    | 27 ++++++++-----------
 4 files changed, 29 insertions(+), 24 deletions(-)
 create mode 100644 changelogs/unreleased/show-commit-status-from-latest-pipeline.yml

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index d3432632899..3ab19938c0f 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -83,9 +83,13 @@ module Ci
       end
     end
 
+    scope :latest, -> { order(id: :desc) }
+
     # ref can't be HEAD or SHA, can only be branch/tag name
+    scope :latest_for, ->(ref) { where(ref: ref).latest }
+
     def self.latest_successful_for(ref)
-      where(ref: ref).order(id: :desc).success.first
+      latest_for(ref).success.first
     end
 
     def self.truncate_sha(sha)
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 9e7fde9503d..2134ba2d75f 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -232,13 +232,15 @@ class Commit
   def status(ref = nil)
     @statuses ||= {}
 
-    if @statuses.key?(ref)
-      @statuses[ref]
-    elsif ref
-      @statuses[ref] = pipelines.where(ref: ref).status
-    else
-      @statuses[ref] = pipelines.status
-    end
+    return @statuses[ref] if @statuses.key?(ref)
+
+    latest_pipeline = if ref
+                        pipelines.latest_for(ref)
+                      else
+                        pipelines.latest
+                      end.first
+
+    @statuses[ref] = latest_pipeline.try(:status)
   end
 
   def revert_branch_name
diff --git a/changelogs/unreleased/show-commit-status-from-latest-pipeline.yml b/changelogs/unreleased/show-commit-status-from-latest-pipeline.yml
new file mode 100644
index 00000000000..bbd7a217493
--- /dev/null
+++ b/changelogs/unreleased/show-commit-status-from-latest-pipeline.yml
@@ -0,0 +1,4 @@
+---
+title: Show commit status from latest pipeline
+merge_request: 7333
+author:
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index e3bb3482d67..ca277601970 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -206,23 +206,18 @@ eos
   end
 
   describe '#status' do
-    context 'without arguments for compound status' do
-      shared_examples 'giving the status from pipeline' do
-        it do
-          expect(commit.status).to eq(Ci::Pipeline.status)
+    context 'without arguments' do
+      before do
+        5.times do
+          create(:ci_empty_pipeline,
+                 project: project,
+                 sha: commit.sha,
+                 status: Ci::Pipeline.all_state_names.sample)
         end
       end
 
-      context 'with pipelines' do
-        let!(:pipeline) do
-          create(:ci_empty_pipeline, project: project, sha: commit.sha)
-        end
-
-        it_behaves_like 'giving the status from pipeline'
-      end
-
-      context 'without pipelines' do
-        it_behaves_like 'giving the status from pipeline'
+      it 'gives the status from latest pipeline' do
+        expect(commit.status).to eq(Ci::Pipeline.latest.first.status)
       end
     end
 
@@ -248,8 +243,8 @@ eos
         expect(commit.status('fix')).to eq(pipeline_from_fix.status)
       end
 
-      it 'gives compound status if ref is nil' do
-        expect(commit.status(nil)).to eq(commit.status)
+      it 'gives status from latest pipeline for whatever branch' do
+        expect(commit.status(nil)).to eq(Ci::Pipeline.latest.first.status)
       end
     end
   end

From 590d61a2de453b2359c81f7070caae09df6e788d Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 8 Nov 2016 02:33:04 +0800
Subject: [PATCH 02/15] Also show latest pipeline for ImageForBuildService

---
 app/services/ci/image_for_build_service.rb | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/app/services/ci/image_for_build_service.rb b/app/services/ci/image_for_build_service.rb
index 75d847d5bee..17ce95073cb 100644
--- a/app/services/ci/image_for_build_service.rb
+++ b/app/services/ci/image_for_build_service.rb
@@ -1,11 +1,18 @@
 module Ci
   class ImageForBuildService
     def execute(project, opts)
-      sha = opts[:sha] || ref_sha(project, opts[:ref])
+      ref = opts[:ref]
+      sha = opts[:sha] || ref_sha(project, ref)
 
       pipelines = project.pipelines.where(sha: sha)
-      pipelines = pipelines.where(ref: opts[:ref]) if opts[:ref]
-      image_name = image_for_status(pipelines.status)
+
+      latest_pipeline = if ref
+                          pipelines.latest_for(ref)
+                        else
+                          pipelines.latest
+                        end.first
+
+      image_name = image_for_status(latest_pipeline.status)
 
       image_path = Rails.root.join('public/ci', image_name)
       OpenStruct.new(path: image_path, name: image_name)

From f593abbc70ab02823cd99d2db11598b629cbb3a0 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 8 Nov 2016 03:44:25 +0800
Subject: [PATCH 03/15] There's not always a pipeline

---
 app/services/ci/image_for_build_service.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/services/ci/image_for_build_service.rb b/app/services/ci/image_for_build_service.rb
index 17ce95073cb..026a727a8f9 100644
--- a/app/services/ci/image_for_build_service.rb
+++ b/app/services/ci/image_for_build_service.rb
@@ -12,7 +12,7 @@ module Ci
                           pipelines.latest
                         end.first
 
-      image_name = image_for_status(latest_pipeline.status)
+      image_name = image_for_status(latest_pipeline.try(:status))
 
       image_path = Rails.root.join('public/ci', image_name)
       OpenStruct.new(path: image_path, name: image_name)

From 721f2d3788ae5e8374f357014bd9e20d62de0a81 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Mon, 21 Nov 2016 22:19:16 +0800
Subject: [PATCH 04/15] Still use compound pipeline status, but group by

ref and sha so that it would show latest pipeline
if ref and sha are both specified, otherwise still
the same as before.
---
 app/models/ci/pipeline.rb                  | 16 +++++++++++++---
 app/models/commit.rb                       |  8 +-------
 app/services/ci/image_for_build_service.rb | 11 ++---------
 spec/models/commit_spec.rb                 |  8 ++++----
 4 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index cd7d8fd3af7..e566503bb18 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -89,13 +89,23 @@ module Ci
       end
     end
 
-    scope :latest, -> { order(id: :desc) }
+    scope :latest, -> do
+      max_id = unscope(:select).select("max(#{quoted_table_name}.id)")
+
+      where(id: max_id.group(:ref, :sha))
+    end
 
     # ref can't be HEAD or SHA, can only be branch/tag name
-    scope :latest_for, ->(ref) { where(ref: ref).latest }
+    scope :latest_for, ->(ref) do
+      if ref
+        where(ref: ref)
+      else
+        self
+      end.latest
+    end
 
     def self.latest_successful_for(ref)
-      latest_for(ref).success.first
+      where(ref: ref).order(id: :desc).success.first
     end
 
     def self.truncate_sha(sha)
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 2134ba2d75f..b588b93b158 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -234,13 +234,7 @@ class Commit
 
     return @statuses[ref] if @statuses.key?(ref)
 
-    latest_pipeline = if ref
-                        pipelines.latest_for(ref)
-                      else
-                        pipelines.latest
-                      end.first
-
-    @statuses[ref] = latest_pipeline.try(:status)
+    @statuses[ref] = pipelines.latest_for(ref).status
   end
 
   def revert_branch_name
diff --git a/app/services/ci/image_for_build_service.rb b/app/services/ci/image_for_build_service.rb
index 026a727a8f9..d5a07ef630b 100644
--- a/app/services/ci/image_for_build_service.rb
+++ b/app/services/ci/image_for_build_service.rb
@@ -3,18 +3,11 @@ module Ci
     def execute(project, opts)
       ref = opts[:ref]
       sha = opts[:sha] || ref_sha(project, ref)
-
       pipelines = project.pipelines.where(sha: sha)
 
-      latest_pipeline = if ref
-                          pipelines.latest_for(ref)
-                        else
-                          pipelines.latest
-                        end.first
-
-      image_name = image_for_status(latest_pipeline.try(:status))
-
+      image_name = image_for_status(pipelines.latest_for(ref).status)
       image_path = Rails.root.join('public/ci', image_name)
+
       OpenStruct.new(path: image_path, name: image_name)
     end
 
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index ca277601970..21590cd4ff1 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -216,8 +216,8 @@ eos
         end
       end
 
-      it 'gives the status from latest pipeline' do
-        expect(commit.status).to eq(Ci::Pipeline.latest.first.status)
+      it 'gives compound status' do
+        expect(commit.status).to eq(Ci::Pipeline.latest.status)
       end
     end
 
@@ -243,8 +243,8 @@ eos
         expect(commit.status('fix')).to eq(pipeline_from_fix.status)
       end
 
-      it 'gives status from latest pipeline for whatever branch' do
-        expect(commit.status(nil)).to eq(Ci::Pipeline.latest.first.status)
+      it 'gives compound status if ref is nil' do
+        expect(commit.status(nil)).to eq(Ci::Pipeline.latest.status)
       end
     end
   end

From dd6b16a111a6e6cce0be322be8ddba17a2a30534 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Mon, 21 Nov 2016 22:24:09 +0800
Subject: [PATCH 05/15] Use latest_for in latest_successful_for

---
 app/models/ci/pipeline.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index e566503bb18..95af8c72309 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -105,7 +105,7 @@ module Ci
     end
 
     def self.latest_successful_for(ref)
-      where(ref: ref).order(id: :desc).success.first
+      latest_for(ref).success.first
     end
 
     def self.truncate_sha(sha)

From 66301ce274b42cefe76e343a3d545c8f12d847b2 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 22 Nov 2016 01:48:41 +0800
Subject: [PATCH 06/15] Filter against status first, otherwise we can't

find the latest successful one if the last one is
failed and we already exclude the others.
---
 app/models/ci/pipeline.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 95af8c72309..61d9316a5d3 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -105,7 +105,7 @@ module Ci
     end
 
     def self.latest_successful_for(ref)
-      latest_for(ref).success.first
+      success.latest_for(ref).first
     end
 
     def self.truncate_sha(sha)

From 5ea2628921325bf60abdf9b192abec1f5bcc129e Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 23 Nov 2016 18:25:32 +0800
Subject: [PATCH 07/15] Fix test description to mention latest pipeline,

feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_18819886
---
 spec/models/commit_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 21590cd4ff1..a16b2e73dd7 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -216,7 +216,7 @@ eos
         end
       end
 
-      it 'gives compound status' do
+      it 'gives compound status from latest pipelines' do
         expect(commit.status).to eq(Ci::Pipeline.latest.status)
       end
     end
@@ -243,7 +243,7 @@ eos
         expect(commit.status('fix')).to eq(pipeline_from_fix.status)
       end
 
-      it 'gives compound status if ref is nil' do
+      it 'gives compound status from latest pipelines if ref is nil' do
         expect(commit.status(nil)).to eq(Ci::Pipeline.latest.status)
       end
     end

From 3a99e36e4448409bf21a0258dde1a82f6494922e Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 24 Nov 2016 16:27:57 +0800
Subject: [PATCH 08/15] Avoid using random in the tests, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_18860042
---
 spec/models/commit_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index a16b2e73dd7..62b77ef86c5 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -208,11 +208,11 @@ eos
   describe '#status' do
     context 'without arguments' do
       before do
-        5.times do
+        %w[success failed created pending].each do |status|
           create(:ci_empty_pipeline,
                  project: project,
                  sha: commit.sha,
-                 status: Ci::Pipeline.all_state_names.sample)
+                 status: status)
         end
       end
 

From 6192ea53fad0ea04e356e5a79a5a0e5359ba46ce Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 24 Nov 2016 16:50:37 +0800
Subject: [PATCH 09/15] Rename latest_for to latest, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333/diffs#note_18819292
---
 app/models/ci/pipeline.rb                  | 12 ++++--------
 app/models/commit.rb                       |  2 +-
 app/services/ci/image_for_build_service.rb |  2 +-
 3 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 61d9316a5d3..d1ce43570ac 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -89,23 +89,19 @@ module Ci
       end
     end
 
-    scope :latest, -> do
+    # ref can't be HEAD or SHA, can only be branch/tag name
+    scope :latest, ->(ref = nil) do
       max_id = unscope(:select).select("max(#{quoted_table_name}.id)")
 
-      where(id: max_id.group(:ref, :sha))
-    end
-
-    # ref can't be HEAD or SHA, can only be branch/tag name
-    scope :latest_for, ->(ref) do
       if ref
         where(ref: ref)
       else
         self
-      end.latest
+      end.where(id: max_id.group(:ref, :sha))
     end
 
     def self.latest_successful_for(ref)
-      success.latest_for(ref).first
+      success.latest(ref).first
     end
 
     def self.truncate_sha(sha)
diff --git a/app/models/commit.rb b/app/models/commit.rb
index b588b93b158..946bfc4712c 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -234,7 +234,7 @@ class Commit
 
     return @statuses[ref] if @statuses.key?(ref)
 
-    @statuses[ref] = pipelines.latest_for(ref).status
+    @statuses[ref] = pipelines.latest(ref).status
   end
 
   def revert_branch_name
diff --git a/app/services/ci/image_for_build_service.rb b/app/services/ci/image_for_build_service.rb
index d5a07ef630b..1eeb0e2363a 100644
--- a/app/services/ci/image_for_build_service.rb
+++ b/app/services/ci/image_for_build_service.rb
@@ -5,7 +5,7 @@ module Ci
       sha = opts[:sha] || ref_sha(project, ref)
       pipelines = project.pipelines.where(sha: sha)
 
-      image_name = image_for_status(pipelines.latest_for(ref).status)
+      image_name = image_for_status(pipelines.latest(ref).status)
       image_path = Rails.root.join('public/ci', image_name)
 
       OpenStruct.new(path: image_path, name: image_name)

From 101cde38cf6d5506ea37c5f912fb4c37af50c541 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 24 Nov 2016 17:00:37 +0800
Subject: [PATCH 10/15] Use Ci::Pipeline#latest for finding pipelines

Feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_18861407
---
 lib/gitlab/badge/build/status.rb           | 3 ++-
 spec/lib/gitlab/badge/build/status_spec.rb | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/gitlab/badge/build/status.rb b/lib/gitlab/badge/build/status.rb
index 50aa45e5406..f78dfd5b83a 100644
--- a/lib/gitlab/badge/build/status.rb
+++ b/lib/gitlab/badge/build/status.rb
@@ -20,7 +20,8 @@ module Gitlab
 
         def status
           @project.pipelines
-            .where(sha: @sha, ref: @ref)
+            .where(sha: @sha)
+            .latest(@ref)
             .status || 'unknown'
         end
 
diff --git a/spec/lib/gitlab/badge/build/status_spec.rb b/spec/lib/gitlab/badge/build/status_spec.rb
index 38eebb2a176..70f03021d36 100644
--- a/spec/lib/gitlab/badge/build/status_spec.rb
+++ b/spec/lib/gitlab/badge/build/status_spec.rb
@@ -69,8 +69,8 @@ describe Gitlab::Badge::Build::Status do
         new_build.success!
       end
 
-      it 'reports the compound status' do
-        expect(badge.status).to eq 'failed'
+      it 'does not take outdated pipeline into account' do
+        expect(badge.status).to eq 'success'
       end
     end
   end

From 7cced60069c248156decf6ceabc4d1f447e47ff7 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 14 Dec 2016 21:00:06 +0800
Subject: [PATCH 11/15] Introduce latest_status and add a few tests

Feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20003268
---
 app/models/ci/pipeline.rb                  |  4 ++
 app/models/commit.rb                       |  4 +-
 app/services/ci/image_for_build_service.rb |  2 +-
 spec/models/ci/pipeline_spec.rb            | 57 ++++++++++++++++++++++
 spec/models/commit_spec.rb                 |  4 +-
 5 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index aa0367227a8..9edfc75eac7 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -98,6 +98,10 @@ module Ci
       end.where(id: max_id.group(:ref, :sha))
     end
 
+    def self.latest_status(ref = nil)
+      latest(ref).status
+    end
+
     def self.latest_successful_for(ref)
       success.latest(ref).first
     end
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 91c7970fca6..69cfc47f5bf 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -230,7 +230,7 @@ class Commit
 
     return @statuses[ref] if @statuses.key?(ref)
 
-    @statuses[ref] = pipelines.latest(ref).status
+    @statuses[ref] = pipelines.latest_status(ref)
   end
 
   def revert_branch_name
@@ -266,7 +266,7 @@ class Commit
     @merged_merge_request_hash ||= Hash.new do |hash, user|
       hash[user] = merged_merge_request_no_cache(user)
     end
-    
+
     @merged_merge_request_hash[current_user]
   end
 
diff --git a/app/services/ci/image_for_build_service.rb b/app/services/ci/image_for_build_service.rb
index 1eeb0e2363a..240ddabec36 100644
--- a/app/services/ci/image_for_build_service.rb
+++ b/app/services/ci/image_for_build_service.rb
@@ -5,7 +5,7 @@ module Ci
       sha = opts[:sha] || ref_sha(project, ref)
       pipelines = project.pipelines.where(sha: sha)
 
-      image_name = image_for_status(pipelines.latest(ref).status)
+      image_name = image_for_status(pipelines.latest_status(ref))
       image_path = Rails.root.join('public/ci', image_name)
 
       OpenStruct.new(path: image_path, name: image_name)
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index e78ae14b737..21df5df1b76 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -381,6 +381,63 @@ describe Ci::Pipeline, models: true do
     end
   end
 
+  shared_context 'with some empty pipelines' do
+    before do
+      create_pipeline(:canceled, 'ref', 'A')
+      create_pipeline(:success, 'ref', 'A')
+      create_pipeline(:failed, 'ref', 'B')
+      create_pipeline(:skipped, 'feature', 'C')
+    end
+
+    def create_pipeline(status, ref, sha)
+      create(:ci_empty_pipeline, status: status, ref: ref, sha: sha)
+    end
+  end
+
+  describe '.latest' do
+    include_context 'with some empty pipelines'
+
+    context 'when no ref is specified' do
+      let(:pipelines) { Ci::Pipeline.latest.all }
+
+      it 'returns the latest pipeline for the same ref and different sha' do
+        expect(pipelines.map(&:sha)).to contain_exactly('A', 'B', 'C')
+        expect(pipelines.map(&:status)).
+          to contain_exactly('success', 'failed', 'skipped')
+      end
+    end
+
+    context 'when ref is specified' do
+      let(:pipelines) { Ci::Pipeline.latest('ref').all }
+
+      it 'returns the latest pipeline for ref and different sha' do
+        expect(pipelines.map(&:sha)).to contain_exactly('A', 'B')
+        expect(pipelines.map(&:status)).
+          to contain_exactly('success', 'failed')
+      end
+    end
+  end
+
+  describe '.latest_status' do
+    include_context 'with some empty pipelines'
+
+    context 'when no ref is specified' do
+      let(:latest_status) { Ci::Pipeline.latest_status }
+
+      it 'returns the latest status for the same ref and different sha' do
+        expect(latest_status).to eq(Ci::Pipeline.latest.status)
+      end
+    end
+
+    context 'when ref is specified' do
+      let(:latest_status) { Ci::Pipeline.latest_status('ref') }
+
+      it 'returns the latest status for ref and different sha' do
+        expect(latest_status).to eq(Ci::Pipeline.latest_status('ref'))
+      end
+    end
+  end
+
   describe '#status' do
     let!(:build) { create(:ci_build, :created, pipeline: pipeline, name: 'test') }
 
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index b81fab0372a..a2a8392699e 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -224,7 +224,7 @@ eos
       end
 
       it 'gives compound status from latest pipelines' do
-        expect(commit.status).to eq(Ci::Pipeline.latest.status)
+        expect(commit.status).to eq(Ci::Pipeline.latest_status)
       end
     end
 
@@ -251,7 +251,7 @@ eos
       end
 
       it 'gives compound status from latest pipelines if ref is nil' do
-        expect(commit.status(nil)).to eq(Ci::Pipeline.latest.status)
+        expect(commit.status(nil)).to eq(Ci::Pipeline.latest_status)
       end
     end
   end

From f463ef5ec58fd26ad5368f57a29d758756dafc3f Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 15 Dec 2016 18:12:33 +0800
Subject: [PATCH 12/15] Also use latest_status, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20058857
---
 lib/gitlab/badge/build/status.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/gitlab/badge/build/status.rb b/lib/gitlab/badge/build/status.rb
index f78dfd5b83a..b762d85b6e5 100644
--- a/lib/gitlab/badge/build/status.rb
+++ b/lib/gitlab/badge/build/status.rb
@@ -21,8 +21,7 @@ module Gitlab
         def status
           @project.pipelines
             .where(sha: @sha)
-            .latest(@ref)
-            .status || 'unknown'
+            .latest_status(@ref) || 'unknown'
         end
 
         def metadata

From cc6f578d5fc45f9c3d4cc7df5af72b15ce47b3a8 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 15 Dec 2016 18:14:48 +0800
Subject: [PATCH 13/15] Use described_class and update description

Feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20059124

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20059187

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20059322
---
 spec/models/ci/pipeline_spec.rb | 18 +++++++++---------
 spec/models/commit_spec.rb      |  2 +-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 21df5df1b76..d06c30a891c 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -381,7 +381,7 @@ describe Ci::Pipeline, models: true do
     end
   end
 
-  shared_context 'with some empty pipelines' do
+  shared_context 'with some outdated pipelines' do
     before do
       create_pipeline(:canceled, 'ref', 'A')
       create_pipeline(:success, 'ref', 'A')
@@ -395,10 +395,10 @@ describe Ci::Pipeline, models: true do
   end
 
   describe '.latest' do
-    include_context 'with some empty pipelines'
+    include_context 'with some outdated pipelines'
 
     context 'when no ref is specified' do
-      let(:pipelines) { Ci::Pipeline.latest.all }
+      let(:pipelines) { described_class.latest.all }
 
       it 'returns the latest pipeline for the same ref and different sha' do
         expect(pipelines.map(&:sha)).to contain_exactly('A', 'B', 'C')
@@ -408,7 +408,7 @@ describe Ci::Pipeline, models: true do
     end
 
     context 'when ref is specified' do
-      let(:pipelines) { Ci::Pipeline.latest('ref').all }
+      let(:pipelines) { described_class.latest('ref').all }
 
       it 'returns the latest pipeline for ref and different sha' do
         expect(pipelines.map(&:sha)).to contain_exactly('A', 'B')
@@ -419,21 +419,21 @@ describe Ci::Pipeline, models: true do
   end
 
   describe '.latest_status' do
-    include_context 'with some empty pipelines'
+    include_context 'with some outdated pipelines'
 
     context 'when no ref is specified' do
-      let(:latest_status) { Ci::Pipeline.latest_status }
+      let(:latest_status) { described_class.latest_status }
 
       it 'returns the latest status for the same ref and different sha' do
-        expect(latest_status).to eq(Ci::Pipeline.latest.status)
+        expect(latest_status).to eq(described_class.latest.status)
       end
     end
 
     context 'when ref is specified' do
-      let(:latest_status) { Ci::Pipeline.latest_status('ref') }
+      let(:latest_status) { described_class.latest_status('ref') }
 
       it 'returns the latest status for ref and different sha' do
-        expect(latest_status).to eq(Ci::Pipeline.latest_status('ref'))
+        expect(latest_status).to eq(described_class.latest_status('ref'))
       end
     end
   end
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index a2a8392699e..9aac2abfb27 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -213,7 +213,7 @@ eos
   end
 
   describe '#status' do
-    context 'without arguments' do
+    context 'without ref argument' do
       before do
         %w[success failed created pending].each do |status|
           create(:ci_empty_pipeline,

From b4a7e7cfbf2a79cf6ecdb8b251ad41c09a40e5bd Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 15 Dec 2016 18:38:26 +0800
Subject: [PATCH 14/15] Don't call anything on a block, use simple if

Feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20058743
---
 app/models/ci/pipeline.rb | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 9edfc75eac7..48354cdbefb 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -89,13 +89,15 @@ module Ci
 
     # ref can't be HEAD or SHA, can only be branch/tag name
     scope :latest, ->(ref = nil) do
-      max_id = unscope(:select).select("max(#{quoted_table_name}.id)")
+      max_id = unscope(:select)
+        .select("max(#{quoted_table_name}.id)")
+        .group(:ref, :sha)
 
       if ref
-        where(ref: ref)
+        where(id: max_id, ref: ref)
       else
-        self
-      end.where(id: max_id.group(:ref, :sha))
+        where(id: max_id)
+      end
     end
 
     def self.latest_status(ref = nil)

From d9000184e5e8a63e0c24fe264e864fc27f6515c4 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 15 Dec 2016 18:52:36 +0800
Subject: [PATCH 15/15] Add explicit status test, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20058993

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20059060

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333#note_20059357
---
 spec/models/ci/pipeline_spec.rb | 2 ++
 spec/models/commit_spec.rb      | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index d06c30a891c..52dd41065e9 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -426,6 +426,7 @@ describe Ci::Pipeline, models: true do
 
       it 'returns the latest status for the same ref and different sha' do
         expect(latest_status).to eq(described_class.latest.status)
+        expect(latest_status).to eq('failed')
       end
     end
 
@@ -434,6 +435,7 @@ describe Ci::Pipeline, models: true do
 
       it 'returns the latest status for ref and different sha' do
         expect(latest_status).to eq(described_class.latest_status('ref'))
+        expect(latest_status).to eq('failed')
       end
     end
   end
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 9aac2abfb27..74b50d2908d 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -225,6 +225,7 @@ eos
 
       it 'gives compound status from latest pipelines' do
         expect(commit.status).to eq(Ci::Pipeline.latest_status)
+        expect(commit.status).to eq('pending')
       end
     end
 
@@ -252,6 +253,7 @@ eos
 
       it 'gives compound status from latest pipelines if ref is nil' do
         expect(commit.status(nil)).to eq(Ci::Pipeline.latest_status)
+        expect(commit.status(nil)).to eq('failed')
       end
     end
   end