Use PipelinesFinder in Pipelines API

CHANGELOG

@@ -36,6 +36,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

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

app/finders/pipelines_finder.rb

 class PipelinesFinder
-  attr_reader :project
+  attr_reader :project, :pipelines
 
   def initialize(project)
     @project = project
+    @pipelines = project.pipelines
   end
 
-  def execute(pipelines, scope)
+  def execute(scope: nil)
+    scoped_pipelines =
       case scope
       when 'running'
         pipelines.running_or_pending
       when 'branches'
-      from_ids(pipelines, ids_for_ref(pipelines, branches))
+        from_ids(ids_for_ref(branches))
       when 'tags'
-      from_ids(pipelines, ids_for_ref(pipelines, 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
 

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

spec/finders/pipelines_finder_spec.rb

+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