From f0ee085e568ccdc4f12271e63f32b6d0a7cdebdc Mon Sep 17 00:00:00 2001 From: "guillaume.micouin-jorda" Date: Thu, 9 Jan 2025 14:58:18 +0100 Subject: [PATCH] Filter and sort commit statuses by pipeline id Signed-off-by: guillaume.micouin-jorda --- doc/api/commits.md | 21 ++++--- lib/api/commit_statuses.rb | 40 ++++++++++-- spec/requests/api/commit_statuses_spec.rb | 77 ++++++++++++++++------- 3 files changed, 100 insertions(+), 38 deletions(-) diff --git a/doc/api/commits.md b/doc/api/commits.md index 453542aead6275..6f0788ef588b1c 100644 --- a/doc/api/commits.md +++ b/doc/api/commits.md @@ -696,6 +696,8 @@ The commit status API for use with GitLab. ### List the statuses of a commit +> - `pipeline_id`, `order_by`, and `sort` fields [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/176142) in GitLab 17.9. + List the statuses of a commit in a project. The pagination parameters `page` and `per_page` can be used to restrict the list of references. @@ -703,14 +705,17 @@ The pagination parameters `page` and `per_page` can be used to restrict the list GET /projects/:id/repository/commits/:sha/statuses ``` -| Attribute | Type | Required | Description | -| --------- | ---- | -------- | ----------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](rest/index.md#namespaced-paths) | -| `sha` | string | yes | The commit SHA | -| `ref` | string | no | The name of a repository branch or tag or, if not given, the default branch | -| `stage` | string | no | Filter by [build stage](../ci/yaml/index.md#stages), for example, `test` | -| `name` | string | no | Filter by [job name](../ci/yaml/index.md#job-keywords), for example, `bundler:audit` | -| `all` | boolean | no | Return all statuses, not only the latest ones | +| Attribute | Type | Required | Description | +|---------------|----------------| -------- |--------------------------------------------------------------------------------------| +| `id` | integer/string | Yes | ID or [URL-encoded path of the project](rest/index.md#namespaced-paths). | +| `sha` | string | Yes | Hash of the commit. | +| `ref` | string | No | Name of the branch or tag. Default is the default branch. | +| `stage` | string | No | Filter statuses by [build stage](../ci/yaml/index.md#stages). For example, `test`. | +| `name` | string | No | Filter statuses by [job name](../ci/yaml/index.md#job-keywords). For example, `bundler:audit`. | +| `pipeline_id` | integer | No | Filter statuses by pipeline ID. For example, `1234`. | +| `order_by` | string | No | Values for sorting statuses. Valid values are `id` and `pipeline_id`. Default is `id`. | +| `sort` | string | No | Sort statuses in ascending or descending order. Valid values are `asc` and `desc`. Default is `asc`. | +| `all` | boolean | No | Include all statuses instead of latest only. Default is `false`. | ```shell curl --header "PRIVATE-TOKEN: " \ diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index b6ecd8347dc64d..b64c80becdd684 100644 --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -7,8 +7,14 @@ class CommitStatuses < ::API::Base feature_category :continuous_integration urgency :low + ALLOWED_SORT_VALUES = %w[id pipeline_id].freeze + DEFAULT_SORT_VALUE = 'id' + + ALLOWED_SORT_DIRECTIONS = %w[asc desc].freeze + DEFAULT_SORT_DIRECTION = 'asc' + params do - requires :id, types: [String, Integer], desc: 'The ID or URL-encoded path of the project' + requires :id, types: [String, Integer], desc: 'ID or URL-encoded path of the project.' end resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do include PaginationParams @@ -25,11 +31,23 @@ class CommitStatuses < ::API::Base is_array true end params do - requires :sha, type: String, desc: 'The commit hash', documentation: { example: '18f3e63d05582537db6d183d9d557be09e1f90c8' } - optional :ref, type: String, desc: 'The ref', documentation: { example: 'develop' } - optional :stage, type: String, desc: 'The stage', documentation: { example: 'test' } - optional :name, type: String, desc: 'The name', documentation: { example: 'bundler:audit' } - optional :all, type: Boolean, desc: 'Show all statuses', documentation: { default: false } + requires :sha, type: String, desc: 'Hash of the commit.', documentation: { example: '18f3e63d05582537db6d183d9d557be09e1f90c8' } + optional :ref, type: String, desc: 'Name of the branch or tag. Default is the default branch.', documentation: { example: 'develop' } + optional :stage, type: String, desc: 'Filter statuses by build stage.', documentation: { example: 'test' } + optional :name, type: String, desc: 'Filter statuses by job name.', documentation: { example: 'bundler:audit' } + optional :pipeline_id, type: Integer, desc: 'Filter statuses by pipeline ID.', documentation: { example: 1234 } + optional :all, type: Boolean, desc: 'Include all statuses instead of latest only. Default is `false`.', documentation: { default: false } + optional :order_by, + type: String, + values: ALLOWED_SORT_VALUES, + default: DEFAULT_SORT_VALUE, + desc: 'Values for sorting statuses. Valid values are `id` and `pipeline_id`. Default is `id`.', + documentation: { default: DEFAULT_SORT_VALUE } + optional :sort, + type: String, + values: ALLOWED_SORT_DIRECTIONS, + desc: 'Sort statuses in ascending or descending order. Valid values are `asc` and `desc`. Default is `asc`.', + documentation: { default: DEFAULT_SORT_DIRECTION } use :pagination end # rubocop: disable CodeReuse/ActiveRecord @@ -39,11 +57,13 @@ class CommitStatuses < ::API::Base not_found!('Commit') unless user_project.commit(params[:sha]) pipelines = user_project.ci_pipelines.where(sha: params[:sha]) + pipelines = pipelines.where(id: params[:pipeline_id]) if params[:pipeline_id].present? statuses = ::CommitStatus.where(pipeline: pipelines) statuses = statuses.latest unless to_boolean(params[:all]) statuses = statuses.where(ref: params[:ref]) if params[:ref].present? statuses = statuses.where(stage: params[:stage]) if params[:stage].present? statuses = statuses.where(name: params[:name]) if params[:name].present? + statuses = order_and_sort_statuses(statuses) present paginate(statuses), with: Entities::CommitStatus end # rubocop: enable CodeReuse/ActiveRecord @@ -95,6 +115,14 @@ def optional_commit_status_params updatable_optional_attributes = %w[target_url description coverage] attributes_for_keys(updatable_optional_attributes) end + + # rubocop: disable CodeReuse/ActiveRecord -- Better code maintainability here, this won't be reused anywhere + def order_and_sort_statuses(statuses) + sort_direction = params[:sort].presence || DEFAULT_SORT_DIRECTION + order_column = ALLOWED_SORT_VALUES.include?(params[:order_by]) ? params[:order_by] : DEFAULT_SORT_VALUE + statuses.order(order_column => sort_direction) + end + # rubocop: enable CodeReuse/ActiveRecord end end end diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index e62430a22d664c..d2579b047f4eb0 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -31,8 +31,8 @@ context "reporter user" do let(:statuses_id) { json_response.map { |status| status['id'] } } - def create_status(commit, opts = {}) - create(:commit_status, { pipeline: commit, ref: commit.ref }.merge(opts)) + def create_status(pipeline, opts = {}) + create(:commit_status, { pipeline: pipeline, ref: pipeline.ref }.merge(opts)) end let!(:status1) { create_status(master, status: 'running', retried: true) } @@ -58,44 +58,73 @@ def create_status(commit, opts = {}) end end - context 'all commit statuses' do + shared_examples_for 'get commit statuses' do before do - get api(get_url, reporter), params: { all: 1 } + get api(get_url, reporter), params: params end it 'returns all commit statuses' do expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(statuses_id).to contain_exactly( - status1.id, status2.id, status3.id, status4.id, status5.id, status6.id - ) + expect(statuses_id).to eq(expected_statuses) end end + context 'Get all commit statuses' do + let(:params) { { all: 1 } } + let(:expected_statuses) { [status1.id, status2.id, status3.id, status4.id, status5.id, status6.id] } + + it_behaves_like 'get commit statuses' + end + context 'latest commit statuses for specific ref' do - before do - get api(get_url, reporter), params: { ref: 'develop' } - end + let(:params) { { ref: 'develop' } } + let(:expected_statuses) { [status3.id, status5.id] } - it 'returns latest commit statuses for specific ref' do - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(statuses_id).to contain_exactly(status3.id, status5.id) - end + it_behaves_like 'get commit statuses' end context 'latest commit statues for specific name' do - before do - get api(get_url, reporter), params: { name: 'coverage' } - end + let(:params) { { name: 'coverage' } } + let(:expected_statuses) { [status4.id, status5.id] } - it 'return latest commit statuses for specific name' do - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(statuses_id).to contain_exactly(status4.id, status5.id) + it_behaves_like 'get commit statuses' + end + + context 'latest commit statuses for specific pipeline' do + let(:params) { { pipeline_id: develop.id } } + let(:expected_statuses) { [status3.id, status5.id] } + + it_behaves_like 'get commit statuses' + end + + context 'return commit statuses sort by desc id' do + let(:params) { { all: 1, sort: "desc" } } + let(:expected_statuses) { [status6.id, status5.id, status4.id, status3.id, status2.id, status1.id] } + + it_behaves_like 'get commit statuses' + end + + context 'return commit statuses sort by desc pipeline_id' do + let(:params) { { all: 1, order_by: "pipeline_id", sort: "desc" } } + let(:expected_statuses) { [status3.id, status5.id, status1.id, status2.id, status4.id, status6.id] } + + it_behaves_like 'get commit statuses' + end + + context 'return commit statuses sort by asc pipeline_id' do + let(:params) { { all: 1, order_by: "pipeline_id" } } + let(:expected_statuses) { [status1.id, status2.id, status4.id, status6.id, status3.id, status5.id] } + + it_behaves_like 'get commit statuses' + end + + context 'Bad filter commit statuses' do + it 'return commit statuses order by an unmanaged field' do + get api(get_url, reporter), params: { all: 1, order_by: "name" } + + expect(response).to have_gitlab_http_status(:bad_request) end end end -- GitLab