Use PipelinesFinder in Pipelines APIzj-use-project-finder-pipelines-api

5 files changed, 78 insertions, 20 deletions

diff --git a/CHANGELOG b/CHANGELOG
index f54a17e1ea2..b587dd8cbca 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -32,6 +32,7 @@ v 8.12.0 (unreleased)
   - Add Sentry logging to API calls
   - Add BroadcastMessage API
   - Use 'git update-ref' for safer web commits !6130
+  - Sort pipelines requested through the API
   - Automatically expand hidden discussions when accessed by a permalink !5585 (Mike Greiling)
   - Remove unused mixins (ClemMakesApps)
   - Add search to all issue board lists
diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb
index b0c72cfe4b4..371cc3787fb 100644
--- a/app/controllers/projects/pipelines_controller.rb
+++ b/app/controllers/projects/pipelines_controller.rb
@@ -7,11 +7,10 @@ class Projects::PipelinesController < Projects::ApplicationController
 
   def index
     @scope = params[:scope]
-    all_pipelines = project.pipelines
-    @pipelines_count = all_pipelines.count
-    @running_or_pending_count = all_pipelines.running_or_pending.count
-    @pipelines = PipelinesFinder.new(project).execute(all_pipelines, @scope)
-    @pipelines = @pipelines.order(id: :desc).page(params[:page]).per(30)
+    @pipelines = PipelinesFinder.new(project).execute(scope: @scope).page(params[:page]).per(30)
+
+    @running_or_pending_count = PipelinesFinder.new(project).execute(scope: 'running').count
+    @pipelines_count = PipelinesFinder.new(project).execute.count
   end
 
   def new
diff --git a/app/finders/pipelines_finder.rb b/app/finders/pipelines_finder.rb
index 641fbf838f1..32aea75486d 100644
--- a/app/finders/pipelines_finder.rb
+++ b/app/finders/pipelines_finder.rb
@@ -1,30 +1,34 @@
 class PipelinesFinder
-  attr_reader :project
+  attr_reader :project, :pipelines
 
   def initialize(project)
     @project = project
+    @pipelines = project.pipelines
   end
 
-  def execute(pipelines, scope)
-    case scope
-    when 'running'
-      pipelines.running_or_pending
-    when 'branches'
-      from_ids(pipelines, ids_for_ref(pipelines, branches))
-    when 'tags'
-      from_ids(pipelines, ids_for_ref(pipelines, tags))
-    else
-      pipelines
-    end
+  def execute(scope: nil)
+    scoped_pipelines =
+      case scope
+      when 'running'
+        pipelines.running_or_pending
+      when 'branches'
+        from_ids(ids_for_ref(branches))
+      when 'tags'
+        from_ids(ids_for_ref(tags))
+      else
+        pipelines
+      end
+
+    scoped_pipelines.order(id: :desc)
   end
 
   private
 
-  def ids_for_ref(pipelines, refs)
+  def ids_for_ref(refs)
     pipelines.where(ref: refs).group(:ref).select('max(id)')
   end
 
-  def from_ids(pipelines, ids)
+  def from_ids(ids)
     pipelines.unscoped.where(id: ids)
   end
 
diff --git a/lib/api/pipelines.rb b/lib/api/pipelines.rb
index 2aae75c471d..2a0c8e1f2c0 100644
--- a/lib/api/pipelines.rb
+++ b/lib/api/pipelines.rb
@@ -13,11 +13,14 @@ module API
       params do
         optional :page,     type: Integer, desc: 'Page number of the current request'
         optional :per_page, type: Integer, desc: 'Number of items per page'
+        optional :scope,    type: String, values: ['running', 'branches', 'tags'],
+                            desc: 'Either running, branches, or tags'
       end
       get ':id/pipelines' do
         authorize! :read_pipeline, user_project
 
-        present paginate(user_project.pipelines), with: Entities::Pipeline
+        pipelines = PipelinesFinder.new(user_project).execute(scope: params[:scope])
+        present paginate(pipelines), with: Entities::Pipeline
       end
 
       desc 'Gets a specific pipeline for the project' do
diff --git a/spec/finders/pipelines_finder_spec.rb b/spec/finders/pipelines_finder_spec.rb
new file mode 100644
index 00000000000..7100266ab55
--- /dev/null
+++ b/spec/finders/pipelines_finder_spec.rb
@@ -0,0 +1,51 @@
+require 'spec_helper'
+
+describe PipelinesFinder do
+  let(:project) { create(:project) }
+
+  let!(:tag_pipeline)    { create(:ci_pipeline, project: project, ref: 'v1.0.0') }
+  let!(:branch_pipeline) { create(:ci_pipeline, project: project) }
+
+  subject { described_class.new(project).execute(params) }
+
+  describe "#execute" do
+    context 'when a scope is passed' do
+      context 'when scope is nil' do
+        let(:params) { { scope: nil } }
+
+        it 'selects all pipelines' do
+          expect(subject.count).to be 2
+          expect(subject).to include tag_pipeline
+          expect(subject).to include branch_pipeline
+        end
+      end
+
+      context 'when selecting branches' do
+        let(:params) { { scope: 'branches' } }
+
+        it 'excludes tags' do
+          expect(subject).not_to include tag_pipeline
+          expect(subject).to     include branch_pipeline
+        end
+      end
+
+      context 'when selecting tags' do
+        let(:params) { { scope: 'tags' } }
+
+        it 'excludes branches' do
+          expect(subject).to     include tag_pipeline
+          expect(subject).not_to include branch_pipeline
+        end
+      end
+    end
+
+    # Scoping to running will speed up the test as it doesn't hit the FS
+    let(:params) { { scope: 'running' } }
+
+    it 'orders in descending order on ID' do
+      create(:ci_pipeline, project: project, ref: 'feature')
+
+      expect(subject.map(&:id)).to eq [3, 2, 1]
+    end
+  end
+end