diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-03-01 16:50:23 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-03-01 16:50:23 +0000 |
commit | 9b3c1a8c4f53d97ef46e82706dc159756b354ad3 (patch) | |
tree | e935245d7132d15c33d47725fe8ce26b960d986d | |
parent | 6b0e37b05fae14530b10b9357ebaef87920222e3 (diff) | |
parent | 278f4423d1bbec596d3556582439e09b2376367b (diff) | |
download | gitlab-ce-9b3c1a8c4f53d97ef46e82706dc159756b354ad3.tar.gz |
Merge branch 'fix/commit-status-api-improvement' into 'master'
Return empty array when commit has no statuses in API
This makes API endpoint for Commit Statuses return empty array instead
of 404 when commit exists, but has no statuses.
Closes #3080
See merge request !3010
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | lib/api/commit_statuses.rb | 10 | ||||
-rw-r--r-- | spec/requests/api/commit_status_spec.rb | 164 |
3 files changed, 117 insertions, 58 deletions
diff --git a/CHANGELOG b/CHANGELOG index 088e31f7d48..4dc38117eb7 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -6,6 +6,7 @@ v 8.6.0 (unreleased) - Fix issue when pushing to projects ending in .wiki - Fix avatar stretching by providing a cropping feature (Johann Pardanaud) - Strip leading and trailing spaces in URL validator (evuez) + - Return empty array instead of 404 when commit has no statuses in commit status API - Update documentation to reflect Guest role not being enforced on internal projects v 8.5.2 diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index 9422d438d21..8e74e177ea0 100644 --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -18,10 +18,12 @@ module API # Examples: # GET /projects/:id/repository/commits/:sha/statuses get ':id/repository/commits/:sha/statuses' do - authorize! :read_commit_status, user_project - sha = params[:sha] - ci_commit = user_project.ci_commit(sha) - not_found! 'Commit' unless ci_commit + authorize!(:read_commit_status, user_project) + + not_found!('Commit') unless user_project.commit(params[:sha]) + ci_commit = user_project.ci_commit(params[:sha]) + return [] unless ci_commit + statuses = ci_commit.statuses statuses = statuses.latest unless parse_boolean(params[:all]) statuses = statuses.where(ref: params[:ref]) if params[:ref].present? diff --git a/spec/requests/api/commit_status_spec.rb b/spec/requests/api/commit_status_spec.rb index 89b554622ef..8017ed97d88 100644 --- a/spec/requests/api/commit_status_spec.rb +++ b/spec/requests/api/commit_status_spec.rb @@ -2,88 +2,125 @@ require 'spec_helper' describe API::CommitStatus, api: true do include ApiHelpers + let!(:project) { create(:project) } let(:commit) { project.repository.commit } - let!(:ci_commit) { project.ensure_ci_commit(commit.id) } let(:commit_status) { create(:commit_status, commit: ci_commit) } let(:guest) { create_user(ProjectMember::GUEST) } let(:reporter) { create_user(ProjectMember::REPORTER) } let(:developer) { create_user(ProjectMember::DEVELOPER) } + let(:sha) { commit.id } + describe "GET /projects/:id/repository/commits/:sha/statuses" do - it_behaves_like 'a paginated resources' do - let(:request) { get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses", reporter) } - end + let(:get_url) { "/projects/#{project.id}/repository/commits/#{sha}/statuses" } - context "reporter user" do - let(:statuses_id) { json_response.map { |status| status['id'] } } + context 'ci commit exists' do + let!(:ci_commit) { project.ensure_ci_commit(commit.id) } - before do - @status1 = create(:commit_status, commit: ci_commit, status: 'running') - @status2 = create(:commit_status, commit: ci_commit, name: 'coverage', status: 'pending') - @status3 = create(:commit_status, commit: ci_commit, name: 'coverage', ref: 'develop', status: 'running', allow_failure: true) - @status4 = create(:commit_status, commit: ci_commit, name: 'coverage', status: 'success') - @status5 = create(:commit_status, commit: ci_commit, ref: 'develop', status: 'success') - @status6 = create(:commit_status, commit: ci_commit, status: 'success') + it_behaves_like 'a paginated resources' do + let(:request) { get api(get_url, reporter) } end - it "should return latest commit statuses" do - get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses", reporter) - expect(response.status).to eq(200) + context "reporter user" do + let(:statuses_id) { json_response.map { |status| status['id'] } } - expect(json_response).to be_an Array - expect(statuses_id).to contain_exactly(@status3.id, @status4.id, @status5.id, @status6.id) - json_response.sort_by!{ |status| status['id'] } - expect(json_response.map{ |status| status['allow_failure'] }).to eq([true, false, false, false]) - end + def create_status(opts = {}) + create(:commit_status, { commit: ci_commit }.merge(opts)) + end - it "should return all commit statuses" do - get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses?all=1", reporter) - expect(response.status).to eq(200) + let!(:status1) { create_status(status: 'running') } + let!(:status2) { create_status(name: 'coverage', status: 'pending') } + let!(:status3) { create_status(ref: 'develop', status: 'running', allow_failure: true) } + let!(:status4) { create_status(name: 'coverage', status: 'success') } + let!(:status5) { create_status(name: 'coverage', ref: 'develop', status: 'success') } + let!(:status6) { create_status(status: 'success') } - 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) - end + context 'latest commit statuses' do + before { get api(get_url, reporter) } - it "should return latest commit statuses for specific ref" do - get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses?ref=develop", reporter) - expect(response.status).to eq(200) + it 'returns latest commit statuses' do + expect(response.status).to eq(200) - expect(json_response).to be_an Array - expect(statuses_id).to contain_exactly(@status3.id, @status5.id) + expect(json_response).to be_an Array + expect(statuses_id).to contain_exactly(status3.id, status4.id, status5.id, status6.id) + json_response.sort_by!{ |status| status['id'] } + expect(json_response.map{ |status| status['allow_failure'] }).to eq([true, false, false, false]) + end + end + + context 'all commit statuses' do + before { get api(get_url, reporter), all: 1 } + + it 'returns all commit statuses' do + expect(response.status).to eq(200) + + 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) + end + end + + context 'latest commit statuses for specific ref' do + before { get api(get_url, reporter), ref: 'develop' } + + it 'returns latest commit statuses for specific ref' do + expect(response.status).to eq(200) + + expect(json_response).to be_an Array + expect(statuses_id).to contain_exactly(status3.id, status5.id) + end + end + + context 'latest commit statues for specific name' do + before { get api(get_url, reporter), name: 'coverage' } + + it 'return latest commit statuses for specific name' do + expect(response.status).to eq(200) + + expect(json_response).to be_an Array + expect(statuses_id).to contain_exactly(status4.id, status5.id) + end + end end + end - it "should return latest commit statuses for specific name" do - get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses?name=coverage", reporter) - expect(response.status).to eq(200) + context 'ci commit does not exist' do + before { get api(get_url, reporter) } + it 'returns empty array' do + expect(response.status).to eq 200 expect(json_response).to be_an Array - expect(statuses_id).to contain_exactly(@status3.id, @status4.id) + expect(json_response).to be_empty end end context "guest user" do + before { get api(get_url, guest) } + it "should not return project commits" do - get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses", guest) expect(response.status).to eq(403) end end context "unauthorized user" do + before { get api(get_url) } + it "should not return project commits" do - get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses") expect(response.status).to eq(401) end end end describe 'POST /projects/:id/statuses/:sha' do - let(:post_url) { "/projects/#{project.id}/statuses/#{commit.id}" } + let(:post_url) { "/projects/#{project.id}/statuses/#{sha}" } context 'developer user' do - context 'should create commit status' do - it 'with only required parameters' do - post api(post_url, developer), state: 'success' + context 'only required parameters' do + before { post api(post_url, developer), state: 'success' } + + it 'creates commit status' do expect(response.status).to eq(201) expect(json_response['sha']).to eq(commit.id) expect(json_response['status']).to eq('success') @@ -92,9 +129,17 @@ describe API::CommitStatus, api: true do expect(json_response['target_url']).to be_nil expect(json_response['description']).to be_nil end + end - it 'with all optional parameters' do - post api(post_url, developer), state: 'success', context: 'coverage', ref: 'develop', target_url: 'url', description: 'test' + context 'with all optional parameters' do + before do + optional_params = { state: 'success', context: 'coverage', + ref: 'develop', target_url: 'url', description: 'test' } + + post api(post_url, developer), optional_params + end + + it 'creates commit status' do expect(response.status).to eq(201) expect(json_response['sha']).to eq(commit.id) expect(json_response['status']).to eq('success') @@ -105,41 +150,52 @@ describe API::CommitStatus, api: true do end end - context 'should not create commit status' do - it 'with invalid state' do - post api(post_url, developer), state: 'invalid' + context 'invalid status' do + before { post api(post_url, developer), state: 'invalid' } + + it 'does not create commit status' do expect(response.status).to eq(400) end + end - it 'without state' do - post api(post_url, developer) + context 'request without state' do + before { post api(post_url, developer) } + + it 'does not create commit status' do expect(response.status).to eq(400) end + end - it 'invalid commit' do - post api("/projects/#{project.id}/statuses/invalid_sha", developer), state: 'running' + context 'invalid commit' do + let(:sha) { 'invalid_sha' } + before { post api(post_url, developer), state: 'running' } + + it 'returns not found error' do expect(response.status).to eq(404) end end end context 'reporter user' do + before { post api(post_url, reporter) } + it 'should not create commit status' do - post api(post_url, reporter) expect(response.status).to eq(403) end end context 'guest user' do + before { post api(post_url, guest) } + it 'should not create commit status' do - post api(post_url, guest) expect(response.status).to eq(403) end end context 'unauthorized user' do + before { post api(post_url) } + it 'should not create commit status' do - post api(post_url) expect(response.status).to eq(401) end end |