Merge branch 'merge_request_error' into 'master'

Fix CI status in the MR page

https://gitlab.com/gitlab-org/gitlab-ce/issues/2587

See merge request !1347

5 files changed, 13 insertions, 75 deletions

diff --git a/app/models/project_services/buildkite_service.rb b/app/models/project_services/buildkite_service.rb
index 9e5da6f45d2..40058b53df5 100644
--- a/app/models/project_services/buildkite_service.rb
+++ b/app/models/project_services/buildkite_service.rb
@@ -69,14 +69,6 @@ class BuildkiteService < CiService
     "#{project_url}/builds?commit=#{sha}"
   end
 
-  def builds_path
-    "#{project_url}/builds?branch=#{project.default_branch}"
-  end
-
-  def status_img_path
-    "#{buildkite_endpoint('badge')}/#{status_token}.svg"
-  end
-
   def title
     'Buildkite'
   end
diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb
index 3e2b7faecdb..b13fa3e9ecb 100644
--- a/app/models/project_services/drone_ci_service.rb
+++ b/app/models/project_services/drone_ci_service.rb
@@ -135,20 +135,6 @@ class DroneCiService < CiService
     commit_page(sha, ref)
   end
 
-  def builds_path
-    url = [drone_url, "#{project.namespace.path}/#{project.path}"]
-
-    URI.join(*url).to_s
-  end
-
-  def status_img_path
-    url = [drone_url, 
-           "api/badges/#{project.namespace.path}/#{project.path}/status.svg", 
-           "?branch=#{URI::encode(project.default_branch)}"]
-
-    URI.join(*url).to_s
-  end
-
   def title
     'Drone CI'
   end
diff --git a/app/models/project_services/gitlab_ci_service.rb b/app/models/project_services/gitlab_ci_service.rb
index acbbc9935b6..1bba6326949 100644
--- a/app/models/project_services/gitlab_ci_service.rb
+++ b/app/models/project_services/gitlab_ci_service.rb
@@ -58,24 +58,13 @@ class GitlabCiService < CiService
     service_hook.execute(data)
   end
 
-  def commit_status_path(sha, ref)
-    URI::encode(project_url + "/refs/#{ref}/commits/#{sha}/status.json?token=#{token}")
-  end
-
-  def get_ci_build(sha, ref)
-    @ci_builds ||= {}
-    @ci_builds[sha] ||= HTTParty.get(commit_status_path(sha, ref), verify: false)
+  def get_ci_commit(sha, ref)
+    Ci::Project.find(project.gitlab_ci_project).commits.find_by_sha_and_ref!(sha, ref)
   end
 
   def commit_status(sha, ref)
-    response = get_ci_build(sha, ref)
-
-    if response.code == 200 and response["status"]
-      response["status"]
-    else
-      :error
-    end
-  rescue Errno::ECONNREFUSED
+    get_ci_commit(sha, ref).status
+  rescue ActiveRecord::RecordNotFound
     :error
   end
 
@@ -101,25 +90,13 @@ class GitlabCiService < CiService
   end
 
   def commit_coverage(sha, ref)
-    response = get_ci_build(sha, ref)
-
-    if response.code == 200 and response["coverage"]
-      response["coverage"]
-    end
-  rescue Errno::ECONNREFUSED
-    nil
+    get_ci_commit(sha, ref).coverage
+  rescue ActiveRecord::RecordNotFound
+    :error
   end
 
   def build_page(sha, ref)
-    URI::encode(project_url + "/refs/#{ref}/commits/#{sha}")
-  end
-
-  def builds_path
-    project_url + "?ref=" + project.default_branch
-  end
-
-  def status_img_path
-    project_url + "/status.png?ref=" + project.default_branch
+    Ci::RoutesHelper.ci_project_ref_commits_path(project.gitlab_ci_project, ref, sha)
   end
 
   def title
diff --git a/spec/models/project_services/buildkite_service_spec.rb b/spec/models/project_services/buildkite_service_spec.rb
index 9445d96c337..230807ea672 100644
--- a/spec/models/project_services/buildkite_service_spec.rb
+++ b/spec/models/project_services/buildkite_service_spec.rb
@@ -63,19 +63,5 @@ describe BuildkiteService do
         )
       end
     end
-
-    describe :builds_page do
-      it 'returns the correct path to the builds page' do
-        expect(@service.builds_path).to eq(
-          'https://buildkite.com/account-name/example-project/builds?branch=default-brancho'
-        )
-      end
-    end
-
-    describe :status_img_path do
-      it 'returns the correct path to the status image' do
-        expect(@service.status_img_path).to eq('https://badge.buildkite.com/secret-sauce-status-token.svg')
-      end
-    end
   end
 end
diff --git a/spec/models/project_services/gitlab_ci_service_spec.rb b/spec/models/project_services/gitlab_ci_service_spec.rb
index a14384c87b4..516d1d4fc1d 100644
--- a/spec/models/project_services/gitlab_ci_service_spec.rb
+++ b/spec/models/project_services/gitlab_ci_service_spec.rb
@@ -55,22 +55,19 @@ describe GitlabCiService do
 
   describe 'commits methods' do
     before do
+      @ci_project = create(:ci_project)
       @service = GitlabCiService.new
       allow(@service).to receive_messages(
         service_hook: true,
         project_url: 'http://ci.gitlab.org/projects/2',
-        token: 'verySecret'
+        token: 'verySecret',
+        project: @ci_project.gl_project
       )
     end
 
-    describe :commit_status_path do
-      it { expect(@service.commit_status_path("2ab7834c", 'master')).to eq("http://ci.gitlab.org/projects/2/refs/master/commits/2ab7834c/status.json?token=verySecret")}
-      it { expect(@service.commit_status_path("issue#2", 'master')).to eq("http://ci.gitlab.org/projects/2/refs/master/commits/issue%232/status.json?token=verySecret")}
-    end
-
     describe :build_page do
-      it { expect(@service.build_page("2ab7834c", 'master')).to eq("http://ci.gitlab.org/projects/2/refs/master/commits/2ab7834c")}
-      it { expect(@service.build_page("issue#2", 'master')).to eq("http://ci.gitlab.org/projects/2/refs/master/commits/issue%232")}
+      it { expect(@service.build_page("2ab7834c", 'master')).to eq("/ci/projects/#{@ci_project.id}/refs/master/commits/2ab7834c")}
+      it { expect(@service.build_page("issue#2", 'master')).to eq("/ci/projects/#{@ci_project.id}/refs/master/commits/issue%232")}
     end
 
     describe "execute" do