From dcc67ac1fdf21f3e9b301ba91acadf8adadc1cf9 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 29 Feb 2016 13:54:33 +0100
Subject: [PATCH] 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
---
 lib/api/commit_statuses.rb              |  10 ++-
 spec/requests/api/commit_status_spec.rb | 106 +++++++++++++++---------
 2 files changed, 75 insertions(+), 41 deletions(-)

diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb
index 9422d438d21..f98fdd4e159 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)
+
+        ci_commit = user_project.ci_commit(params[:sha])
+        not_found!('Commit') unless user_project.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..b03fc045a94 100644
--- a/spec/requests/api/commit_status_spec.rb
+++ b/spec/requests/api/commit_status_spec.rb
@@ -2,63 +2,95 @@ 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) }
 
   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
-
-    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("/projects/#{project.id}/repository/commits/#{commit.id}/statuses", 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
+        let!(:status1) do
+          create(:commit_status, commit: ci_commit, status: 'running')
+        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!(:status2) do
+          create(:commit_status, commit: ci_commit, name: 'coverage', status: 'pending')
+        end
 
-        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
+        let!(:status3) do
+          create(:commit_status, commit: ci_commit, name: 'coverage', ref: 'develop',
+                                 status: 'running', allow_failure: true)
+        end
+
+        let!(:status4) do
+          create(:commit_status, commit: ci_commit, name: 'coverage', status: 'success')
+        end
 
-      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)
+        let!(:status5) do
+          create(:commit_status, commit: ci_commit, ref: 'develop', status: 'success')
+        end
 
-        expect(json_response).to be_an Array
-        expect(statuses_id).to contain_exactly(@status3.id, @status5.id)
+        let!(:status6) do
+          create(:commit_status, commit: ci_commit, status: 'success')
+        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)
+
+          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
+
+        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)
+
+          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
+
+        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)
+
+          expect(json_response).to be_an Array
+          expect(statuses_id).to contain_exactly(status3.id, status5.id)
+        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)
+
+          expect(json_response).to be_an Array
+          expect(statuses_id).to contain_exactly(status3.id, status4.id)
+        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 do
+        get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses", reporter)
+      end
 
+      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
 
-- 
GitLab