Ci::Pipeline.latest order by id DESC

The name latest implies that it's reverse chronological,
and we did expect it that way.

https://gitlab.com/gitlab-org/gitlab-ce/issues/25993#note_20429761

>>>
ok, I think markglenfletchera is correct in
https://gitlab.com/gitlab-com/support-forum/issues/1394#note_20399939
that `Project#latest_successful_builds_for` is giving oldest pipeline
rather than latest pipeline. This is a ~regression introduced by !7333
where `order(id: :desc)` was removed causing this. The offending change
was:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7333/diffs#b22732e5f39e176c7c719fe485847d0fb0564275_92_108

The confusion was caused by the `latest` name implication, which
actually didn't order anything, and I think we should add `order(id:
:desc)` to `Ci::Pipeline.latest` otherwise it's confusing that it's not
actually ordered.
>>>

Closes #25993

4 files changed, 18 insertions, 14 deletions

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index f2f6453b3b9..5494a8da0d9 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -93,11 +93,13 @@ module Ci
         .select("max(#{quoted_table_name}.id)")
         .group(:ref, :sha)
 
-      if ref
-        where(id: max_id, ref: ref)
-      else
-        where(id: max_id)
-      end
+      query = if ref
+                where(id: max_id, ref: ref)
+              else
+                where(id: max_id)
+              end
+
+      query.order(id: :desc)
     end
 
     def self.latest_status(ref = nil)
diff --git a/app/models/project.rb b/app/models/project.rb
index 26fa20f856d..72fdd4514c4 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -418,7 +418,7 @@ class Project < ActiveRecord::Base
     repository.commit(ref)
   end
 
-  # ref can't be HEAD, can only be branch/tag name or SHA
+  # ref can't be HEAD or SHA, can only be branch/tag name
   def latest_successful_builds_for(ref = default_branch)
     latest_pipeline = pipelines.latest_successful_for(ref)
 
diff --git a/changelogs/unreleased/fix-latest-pipeine-ordering.yml b/changelogs/unreleased/fix-latest-pipeine-ordering.yml
new file mode 100644
index 00000000000..c7c9885d55a
--- /dev/null
+++ b/changelogs/unreleased/fix-latest-pipeine-ordering.yml
@@ -0,0 +1,4 @@
+---
+title: Fix downloading latest artifact
+merge_request: 8286
+author:
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index dc377d15f15..f5d206e0a3e 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -424,20 +424,18 @@ describe Ci::Pipeline, models: true do
     context 'when no ref is specified' do
       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')
-        expect(pipelines.map(&:status)).
-          to contain_exactly('success', 'failed', 'skipped')
+      it 'returns the latest pipelines for the same ref and different sha' do
+        expect(pipelines.map(&:sha)).to eq(%w[C B A])
+        expect(pipelines.map(&:status)).to eq(%w[skipped failed success])
       end
     end
 
     context 'when ref is specified' do
       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')
-        expect(pipelines.map(&:status)).
-          to contain_exactly('success', 'failed')
+      it 'returns the latest pipelines for ref and different sha' do
+        expect(pipelines.map(&:sha)).to eq(%w[B A])
+        expect(pipelines.map(&:status)).to eq(%w[failed success])
       end
     end
   end