Merge branch '28408-feature-proposal-include-search-options-to-pipelines-api' into 'master'

Resolve "Feature Proposal: Include search options to pipelines API"

Closes #28408

See merge request !9367

app/controllers/projects/pipelines_controller.rb

@@ -9,19 +9,19 @@ class Projects::PipelinesController < Projects::ApplicationController
   def index
     @scope = params[:scope]
     @pipelines = PipelinesFinder
-      .new(project)
-      .execute(scope: @scope)
+      .new(project, scope: @scope)
+      .execute
       .page(params[:page])
       .per(30)
 
     @running_count = PipelinesFinder
-      .new(project).execute(scope: 'running').count
+      .new(project, scope: 'running').execute.count
 
     @pending_count = PipelinesFinder
-      .new(project).execute(scope: 'pending').count
+      .new(project, scope: 'pending').execute.count
 
     @finished_count = PipelinesFinder
-      .new(project).execute(scope: 'finished').count
+      .new(project, scope: 'finished').execute.count
 
     @pipelines_count = PipelinesFinder
       .new(project).execute.count

app/finders/pipelines_finder.rb

@@ -1,29 +1,23 @@
 class PipelinesFinder
-  attr_reader :project, :pipelines
+  attr_reader :project, :pipelines, :params
 
-  def initialize(project)
+  ALLOWED_INDEXED_COLUMNS = %w[id status ref user_id].freeze
+
+  def initialize(project, params = {})
     @project = project
     @pipelines = project.pipelines
+    @params = params
   end
 
-  def execute(scope: nil)
-    scoped_pipelines =
-      case scope
-      when 'running'
-        pipelines.running
-      when 'pending'
-        pipelines.pending
-      when 'finished'
-        pipelines.finished
-      when 'branches'
-        from_ids(ids_for_ref(branches))
-      when 'tags'
-        from_ids(ids_for_ref(tags))
-      else
-        pipelines
-      end
-
-    scoped_pipelines.order(id: :desc)
+  def execute
+    items = pipelines
+    items = by_scope(items)
+    items = by_status(items)
+    items = by_ref(items)
+    items = by_name(items)
+    items = by_username(items)
+    items = by_yaml_errors(items)
+    sort_items(items)
   end
 
   private
@@ -43,4 +37,78 @@ class PipelinesFinder
   def tags
     project.repository.tag_names
   end
+
+  def by_scope(items)
+    case params[:scope]
+    when 'running'
+      items.running
+    when 'pending'
+      items.pending
+    when 'finished'
+      items.finished
+    when 'branches'
+      from_ids(ids_for_ref(branches))
+    when 'tags'
+      from_ids(ids_for_ref(tags))
+    else
+      items
+    end
+  end
+
+  def by_status(items)
+    return items unless HasStatus::AVAILABLE_STATUSES.include?(params[:status])
+
+    items.where(status: params[:status])
+  end
+
+  def by_ref(items)
+    if params[:ref].present?
+      items.where(ref: params[:ref])
+    else
+      items
+    end
+  end
+
+  def by_name(items)
+    if params[:name].present?
+      items.joins(:user).where(users: { name: params[:name] })
+    else
+      items
+    end
+  end
+
+  def by_username(items)
+    if params[:username].present?
+      items.joins(:user).where(users: { username: params[:username] })
+    else
+      items
+    end
+  end
+
+  def by_yaml_errors(items)
+    case Gitlab::Utils.to_boolean(params[:yaml_errors])
+    when true
+      items.where("yaml_errors IS NOT NULL")
+    when false
+      items.where("yaml_errors IS NULL")
+    else
+      items
+    end
+  end
+
+  def sort_items(items)
+    order_by = if ALLOWED_INDEXED_COLUMNS.include?(params[:order_by])
+                 params[:order_by]
+               else
+                 :id
+               end
+
+    sort = if params[:sort] =~ /\A(ASC|DESC)\z/i
+             params[:sort]
+           else
+             :desc
+           end
+
+    items.order(order_by => sort)
+  end
 end

changelogs/unreleased/28408-feature-proposal-include-search-options-to-pipelines-api.yml

@@ -0,0 +1,4 @@
+---
+title: 'API: Add parameters to allow filtering project pipelines'
+merge_request: 9367
+author: dosuken123

doc/api/pipelines.md

@@ -11,6 +11,14 @@ GET /projects/:id/pipelines
 | Attribute | Type    | Required | Description         |
 |-----------|---------|----------|---------------------|
 | `id`      | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
+| `scope`   | string  | no       | The scope of pipelines, one of: `running`, `pending`, `finished`, `branches`, `tags` |
+| `status`  | string  | no       | The status of pipelines, one of: `running`, `pending`, `success`, `failed`, `canceled`, `skipped` |
+| `ref`     | string  | no       | The ref of pipelines |
+| `yaml_errors`| boolean  | no       | Returns pipelines with invalid configurations |
+| `name`| string  | no       | The name of the user who triggered pipelines |
+| `username`| string  | no       | The username of the user who triggered pipelines |
+| `order_by`| string  | no       | Order pipelines by `id`, `status`, `ref`, or `user_id` (default: `id`) |
+| `sort`    | string  | no       | Sort pipelines in `asc` or `desc` order (default: `desc`) |
 
 ```
 curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/pipelines"

lib/api/pipelines.rb

@@ -14,13 +14,23 @@ module API
       end
       params do
         use :pagination
-        optional :scope,    type: String, values: %w(running branches tags),
-                            desc: 'Either running, branches, or tags'
+        optional :scope,    type: String, values: %w[running pending finished branches tags],
+                            desc: 'The scope of pipelines'
+        optional :status,   type: String, values: HasStatus::AVAILABLE_STATUSES,
+                            desc: 'The status of pipelines'
+        optional :ref,      type: String, desc: 'The ref of pipelines'
+        optional :yaml_errors, type: Boolean, desc: 'Returns pipelines with invalid configurations'
+        optional :name,     type: String, desc: 'The name of the user who triggered pipelines'
+        optional :username, type: String, desc: 'The username of the user who triggered pipelines'
+        optional :order_by, type: String, values: PipelinesFinder::ALLOWED_INDEXED_COLUMNS, default: 'id',
+                            desc: 'Order pipelines'
+        optional :sort,     type: String, values: %w[asc desc], default: 'desc',
+                            desc: 'Sort pipelines'
       end
       get ':id/pipelines' do
         authorize! :read_pipeline, user_project
 
-        pipelines = PipelinesFinder.new(user_project).execute(scope: params[:scope])
+        pipelines = PipelinesFinder.new(user_project, params).execute
         present paginate(pipelines), with: Entities::PipelineBasic
       end
 

lib/api/v3/pipelines.rb

@@ -21,7 +21,7 @@ module API
         get ':id/pipelines' do
           authorize! :read_pipeline, user_project
 
-          pipelines = PipelinesFinder.new(user_project).execute(scope: params[:scope])
+          pipelines = PipelinesFinder.new(user_project, scope: params[:scope]).execute
           present paginate(pipelines), with: ::API::Entities::Pipeline
         end
       end

spec/finders/pipelines_finder_spec.rb

@@ -3,50 +3,205 @@ require 'spec_helper'
 describe PipelinesFinder do
   let(:project) { create(:project, :repository) }
 
-  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) }
+  subject { described_class.new(project, params).execute }
 
   describe "#execute" do
-    context 'when a scope is passed' do
-      context 'when scope is nil' do
-        let(:params) { { scope: nil } }
+    context 'when params is empty' do
+      let(:params) { {} }
+      let!(:pipelines) { create_list(:ci_pipeline, 2, project: project) }
 
-        it 'selects all pipelines' do
-          expect(subject.count).to be 2
-          expect(subject).to include tag_pipeline
-          expect(subject).to include branch_pipeline
-        end
+      it 'returns all pipelines' do
+        is_expected.to match_array(pipelines)
       end
+    end
 
-      context 'when selecting branches' do
-        let(:params) { { scope: 'branches' } }
+    %w[running pending].each do |target|
+      context "when scope is #{target}" do
+        let(:params) { { scope: target } }
+        let!(:pipeline) { create(:ci_pipeline, project: project, status: target) }
 
-        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
+        it 'returns matched pipelines' do
+          is_expected.to eq([pipeline])
         end
       end
     end
 
-    # Scoping to pending will speed up the test as it doesn't hit the FS
-    let(:params) { { scope: 'pending' } }
+    context 'when scope is finished' do
+      let(:params) { { scope: 'finished' } }
+      let!(:pipelines) do
+        [create(:ci_pipeline, project: project, status: 'success'),
+         create(:ci_pipeline, project: project, status: 'failed'),
+         create(:ci_pipeline, project: project, status: 'canceled')]
+      end
 
-    it 'orders in descending order on ID' do
-      feature_pipeline = create(:ci_pipeline, project: project, ref: 'feature')
+      it 'returns matched pipelines' do
+        is_expected.to match_array(pipelines)
+      end
+    end
 
-      expected_ids = [feature_pipeline.id, branch_pipeline.id, tag_pipeline.id].sort.reverse
-      expect(subject.map(&:id)).to eq expected_ids
+    context 'when scope is branches or tags' do
+      let!(:pipeline_branch) { create(:ci_pipeline, project: project) }
+      let!(:pipeline_tag) { create(:ci_pipeline, project: project, ref: 'v1.0.0', tag: true) }
+
+      context 'when scope is branches' do
+        let(:params) { { scope: 'branches' } }
+
+        it 'returns matched pipelines' do
+          is_expected.to eq([pipeline_branch])
+        end
+      end
+
+      context 'when scope is tags' do
+        let(:params) { { scope: 'tags' } }
+
+        it 'returns matched pipelines' do
+          is_expected.to eq([pipeline_tag])
+        end
+      end
+    end
+
+    HasStatus::AVAILABLE_STATUSES.each do |target|
+      context "when status is #{target}" do
+        let(:params) { { status: target } }
+        let!(:pipeline) { create(:ci_pipeline, project: project, status: target) }
+
+        before do
+          exception_status = HasStatus::AVAILABLE_STATUSES - [target]
+          create(:ci_pipeline, project: project, status: exception_status.first)
+        end
+
+        it 'returns matched pipelines' do
+          is_expected.to eq([pipeline])
+        end
+      end
+    end
+
+    context 'when ref is specified' do
+      let!(:pipeline) { create(:ci_pipeline, project: project) }
+
+      context 'when ref exists' do
+        let(:params) { { ref: 'master' } }
+
+        it 'returns matched pipelines' do
+          is_expected.to eq([pipeline])
+        end
+      end
+
+      context 'when ref does not exist' do
+        let(:params) { { ref: 'invalid-ref' } }
+
+        it 'returns empty' do
+          is_expected.to be_empty
+        end
+      end
+    end
+
+    context 'when name is specified' do
+      let(:user) { create(:user) }
+      let!(:pipeline) { create(:ci_pipeline, project: project, user: user) }
+
+      context 'when name exists' do
+        let(:params) { { name: user.name } }
+
+        it 'returns matched pipelines' do
+          is_expected.to eq([pipeline])
+        end
+      end
+
+      context 'when name does not exist' do
+        let(:params) { { name: 'invalid-name' } }
+
+        it 'returns empty' do
+          is_expected.to be_empty
+        end
+      end
+    end
+
+    context 'when username is specified' do
+      let(:user) { create(:user) }
+      let!(:pipeline) { create(:ci_pipeline, project: project, user: user) }
+
+      context 'when username exists' do
+        let(:params) { { username: user.username } }
+
+        it 'returns matched pipelines' do
+          is_expected.to eq([pipeline])
+        end
+      end
+
+      context 'when username does not exist' do
+        let(:params) { { username: 'invalid-username' } }
+
+        it 'returns empty' do
+          is_expected.to be_empty
+        end
+      end
+    end
+
+    context 'when yaml_errors is specified' do
+      let!(:pipeline1) { create(:ci_pipeline, project: project, yaml_errors: 'Syntax error') }
+      let!(:pipeline2) { create(:ci_pipeline, project: project) }
+
+      context 'when yaml_errors is true' do
+        let(:params) { { yaml_errors: true } }
+
+        it 'returns matched pipelines' do
+          is_expected.to eq([pipeline1])
+        end
+      end
+
+      context 'when yaml_errors is false' do
+        let(:params) { { yaml_errors: false } }
+
+        it 'returns matched pipelines' do
+          is_expected.to eq([pipeline2])
+        end
+      end
+
+      context 'when yaml_errors is invalid' do
+        let(:params) { { yaml_errors: "invalid-yaml_errors" } }
+
+        it 'returns all pipelines' do
+          is_expected.to match_array([pipeline1, pipeline2])
+        end
+      end
+    end
+
+    context 'when order_by and sort are specified' do
+      context 'when order_by user_id' do
+        let(:params) { { order_by: 'user_id', sort: 'asc' } }
+        let!(:pipelines) { create_list(:ci_pipeline, 2, project: project, user: create(:user)) }
+
+        it 'sorts as user_id: :asc' do
+          is_expected.to match_array(pipelines)
+        end
+
+        context 'when sort is invalid' do
+          let(:params) { { order_by: 'user_id', sort: 'invalid_sort' } }
+
+          it 'sorts as user_id: :desc' do
+            is_expected.to eq(pipelines.sort_by { |p| -p.user.id })
+          end
+        end
+      end
+
+      context 'when order_by is invalid' do
+        let(:params) { { order_by: 'invalid_column', sort: 'asc' } }
+        let!(:pipelines) { create_list(:ci_pipeline, 2, project: project) }
+
+        it 'sorts as id: :asc' do
+          is_expected.to eq(pipelines.sort_by { |p| p.id })
+        end
+      end
+
+      context 'when both are nil' do
+        let(:params) { { order_by: nil, sort: nil } }
+        let!(:pipelines) { create_list(:ci_pipeline, 2, project: project) }
+
+        it 'sorts as id: :desc' do
+          is_expected.to eq(pipelines.sort_by { |p| -p.id })
+        end
+      end
     end
   end
 end

spec/requests/api/pipelines_spec.rb

@@ -24,6 +24,245 @@ describe API::Pipelines do
         expect(json_response.first['id']).to eq pipeline.id
         expect(json_response.first.keys).to contain_exactly(*%w[id sha ref status])
       end
+
+      context 'when parameter is passed' do
+        %w[running pending].each do |target|
+          context "when scope is #{target}" do
+            before do
+              create(:ci_pipeline, project: project, status: target)
+            end
+
+            it 'returns matched pipelines' do
+              get api("/projects/#{project.id}/pipelines", user), scope: target
+
+              expect(response).to have_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(json_response).not_to be_empty
+              json_response.each { |r| expect(r['status']).to eq(target) }
+            end
+          end
+        end
+
+        context 'when scope is finished' do
+          before do
+            create(:ci_pipeline, project: project, status: 'success')
+            create(:ci_pipeline, project: project, status: 'failed')
+            create(:ci_pipeline, project: project, status: 'canceled')
+          end
+
+          it 'returns matched pipelines' do
+            get api("/projects/#{project.id}/pipelines", user), scope: 'finished'
+
+            expect(response).to have_http_status(:ok)
+            expect(response).to include_pagination_headers
+            expect(json_response).not_to be_empty
+            json_response.each { |r| expect(r['status']).to be_in(%w[success failed canceled]) }
+          end
+        end
+
+        context 'when scope is branches or tags' do
+          let!(:pipeline_branch) { create(:ci_pipeline, project: project) }
+          let!(:pipeline_tag) { create(:ci_pipeline, project: project, ref: 'v1.0.0', tag: true) }
+
+          context 'when scope is branches' do
+            it 'returns matched pipelines' do
+              get api("/projects/#{project.id}/pipelines", user), scope: 'branches'
+
+              expect(response).to have_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(json_response).not_to be_empty
+              expect(json_response.last['id']).to eq(pipeline_branch.id)
+            end
+          end
+
+          context 'when scope is tags' do
+            it 'returns matched pipelines' do
+              get api("/projects/#{project.id}/pipelines", user), scope: 'tags'
+
+              expect(response).to have_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(json_response).not_to be_empty
+              expect(json_response.last['id']).to eq(pipeline_tag.id)
+            end
+          end
+        end
+
+        context 'when scope is invalid' do
+          it 'returns bad_request' do
+            get api("/projects/#{project.id}/pipelines", user), scope: 'invalid-scope'
+
+            expect(response).to have_http_status(:bad_request)
+          end
+        end
+
+        HasStatus::AVAILABLE_STATUSES.each do |target|
+          context "when status is #{target}" do
+            before do
+              create(:ci_pipeline, project: project, status: target)
+              exception_status = HasStatus::AVAILABLE_STATUSES - [target]
+              create(:ci_pipeline, project: project, status: exception_status.sample)
+            end
+
+            it 'returns matched pipelines' do
+              get api("/projects/#{project.id}/pipelines", user), status: target
+
+              expect(response).to have_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(json_response).not_to be_empty
+              json_response.each { |r| expect(r['status']).to eq(target) }
+            end
+          end
+        end
+
+        context 'when status is invalid' do
+          it 'returns bad_request' do
+            get api("/projects/#{project.id}/pipelines", user), status: 'invalid-status'
+
+            expect(response).to have_http_status(:bad_request)
+          end
+        end
+
+        context 'when ref is specified' do
+          before do
+            create(:ci_pipeline, project: project)
+          end
+
+          context 'when ref exists' do
+            it 'returns matched pipelines' do
+              get api("/projects/#{project.id}/pipelines", user), ref: 'master'
+
+              expect(response).to have_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(json_response).not_to be_empty
+              json_response.each { |r| expect(r['ref']).to eq('master') }
+            end
+          end
+
+          context 'when ref does not exist' do
+            it 'returns empty' do
+              get api("/projects/#{project.id}/pipelines", user), ref: 'invalid-ref'
+
+              expect(response).to have_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(json_response).to be_empty
+            end
+          end
+        end
+
+        context 'when name is specified' do
+          let!(:pipeline) { create(:ci_pipeline, project: project, user: user) }
+
+          context 'when name exists' do
+            it 'returns matched pipelines' do
+              get api("/projects/#{project.id}/pipelines", user), name: user.name
+
+              expect(response).to have_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(json_response.first['id']).to eq(pipeline.id)
+            end
+          end
+
+          context 'when name does not exist' do
+            it 'returns empty' do
+              get api("/projects/#{project.id}/pipelines", user), name: 'invalid-name'
+
+              expect(response).to have_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(json_response).to be_empty
+            end
+          end
+        end
+
+        context 'when username is specified' do
+          let!(:pipeline) { create(:ci_pipeline, project: project, user: user) }
+
+          context 'when username exists' do
+            it 'returns matched pipelines' do
+              get api("/projects/#{project.id}/pipelines", user), username: user.username
+
+              expect(response).to have_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(json_response.first['id']).to eq(pipeline.id)
+            end
+          end
+
+          context 'when username does not exist' do
+            it 'returns empty' do
+              get api("/projects/#{project.id}/pipelines", user), username: 'invalid-username'
+
+              expect(response).to have_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(json_response).to be_empty
+            end
+          end
+        end
+
+        context 'when yaml_errors is specified' do
+          let!(:pipeline1) { create(:ci_pipeline, project: project, yaml_errors: 'Syntax error') }
+          let!(:pipeline2) { create(:ci_pipeline, project: project) }
+
+          context 'when yaml_errors is true' do
+            it 'returns matched pipelines' do
+              get api("/projects/#{project.id}/pipelines", user), yaml_errors: true
+
+              expect(response).to have_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(json_response.first['id']).to eq(pipeline1.id)
+            end
+          end
+
+          context 'when yaml_errors is false' do
+            it 'returns matched pipelines' do
+              get api("/projects/#{project.id}/pipelines", user), yaml_errors: false
+
+              expect(response).to have_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(json_response.first['id']).to eq(pipeline2.id)
+            end
+          end
+
+          context 'when yaml_errors is invalid' do
+            it 'returns bad_request' do
+              get api("/projects/#{project.id}/pipelines", user), yaml_errors: 'invalid-yaml_errors'
+
+              expect(response).to have_http_status(:bad_request)
+            end
+          end
+        end
+
+        context 'when order_by and sort are specified' do
+          context 'when order_by user_id' do
+            let!(:pipeline) { create_list(:ci_pipeline, 2, project: project, user: create(:user)) }
+
+            it 'sorts as user_id: :asc' do
+              get api("/projects/#{project.id}/pipelines", user), order_by: 'user_id', sort: 'asc'
+
+              expect(response).to have_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(json_response).not_to be_empty
+              pipeline.sort_by { |p| p.user.id }.tap do |sorted_pipeline|
+                json_response.each_with_index { |r, i| expect(r['id']).to eq(sorted_pipeline[i].id) }
+              end
+            end
+
+            context 'when sort is invalid' do
+              it 'returns bad_request' do
+                get api("/projects/#{project.id}/pipelines", user), order_by: 'user_id', sort: 'invalid_sort'
+
+                expect(response).to have_http_status(:bad_request)
+              end
+            end
+          end
+
+          context 'when order_by is invalid' do
+            it 'returns bad_request' do
+              get api("/projects/#{project.id}/pipelines", user), order_by: 'lock_version', sort: 'asc'
+
+              expect(response).to have_http_status(:bad_request)
+            end
+          end
+        end
+      end
     end
 
     context 'unauthorized user' do