Remove unused methods in reference_extractor and update jira service.

app/models/project_services/jira_service.rb

@@ -51,19 +51,21 @@ class JiraService < IssueTrackerService
   end
 
   def execute(push, issue = nil)
-    close_issue(push, issue.id) if issue
+    close_issue(push, issue) if issue
   end
 
   def create_cross_reference_note(mentioned, noteable, author)
     issue_name = mentioned.id
     project = self.project
-    noteable_name = noteable.class.name.underscore.downcase.to_sym
+    noteable_name = noteable.class.name.underscore.downcase
     noteable_id = if noteable.is_a?(Commit)
                     noteable.id
                   else
                     noteable.iid
                   end
 
+    entity_url = build_entity_url(noteable_name.to_sym, noteable_id)
+
     data = {
       user: {
         name: author.name,
@@ -74,8 +76,8 @@ class JiraService < IssueTrackerService
         url: resource_url(project_path(project))
       },
       entity: {
-        name: noteable.class.name.underscore.humanize.downcase,
-        url: resource_url(polymorphic_url([project, noteable_name], id: noteable_id, routing_type: :path))
+        name: noteable_name.humanize,
+        url: entity_url
       }
     }
 
@@ -93,15 +95,16 @@ class JiraService < IssueTrackerService
     self.jira_issue_transition_id ||= "2"
   end
 
-  def close_issue(push_data, issue_name)
-    url = close_issue_url(issue_name)
-    commit = push_data[:commits].first
+  def close_issue(commit, issue)
+    url = close_issue_url(issue.iid)
+
+    commit_url = build_entity_url(:commit, commit.id)
 
     message = {
       update: {
         comment: [{
           add: {
-            body: "Issue solved with [#{commit[:id]}|#{commit[:url]}]."
+            body: "Issue solved with [#{commit.id}|#{commit_url}]."
           }
         }]
       },
@@ -137,34 +140,28 @@ class JiraService < IssueTrackerService
   end
 
   def send_message(url, message)
-    begin
-      result = JiraService.post(
-        url,
-        body: message,
-        headers: {
-          'Content-Type' => 'application/json',
-          'Authorization' => "Basic #{auth}"
-        }
-      )
-    rescue URI::InvalidURIError => e
-      result = e.message
-    end
+    result = JiraService.post(
+      url,
+      body: message,
+      headers: {
+        'Content-Type' => 'application/json',
+        'Authorization' => "Basic #{auth}"
+      }
+    )
 
-    message = if result.is_a?(String)
-                "#{self.class.name} ERROR: #{result}. Hostname: #{url}."
+    message = case result.code
+              when 201, 200
+                "#{self.class.name} SUCCESS #{result.code}: Sucessfully posted to #{url}."
+              when 401
+                "#{self.class.name} ERROR 401: Unauthorized. Check the #{self.username} credentials and JIRA access permissions and try again."
               else
-                case result.code
-                when 201, 200
-                  "#{self.class.name} SUCCESS 201: Sucessfully posted to #{url}."
-                when 401
-                  "#{self.class.name} ERROR 401: Unauthorized. Check the #{self.username} credentials and JIRA access permissions and try again."
-                else
-                  "#{self.class.name} ERROR #{result.code}: #{result.parsed_response}"
-                end
+                "#{self.class.name} ERROR #{result.code}: #{result.parsed_response}"
               end
 
     Rails.logger.info(message)
     message
+  rescue URI::InvalidURIError => e
+    Rails.logger.info "#{self.class.name} ERROR: #{e.message}. Hostname: #{url}."
   end
 
   def server_url
@@ -179,6 +176,15 @@ class JiraService < IssueTrackerService
     "#{Settings.gitlab['url'].chomp("/")}#{resource}"
   end
 
+  def build_entity_url(entity_name, entity_id)
+    resource_url(
+      polymorphic_url(
+        [self.project, entity_name],
+        id: entity_id,
+        routing_type: :path
+      )
+    )
+  end
 
   def close_issue_url(issue_name)
     "#{server_url}/rest/api/#{self.api_version}/issue/#{issue_name}/transitions"

app/services/git_push_service.rb

@@ -43,17 +43,17 @@ class GitPushService
           # that shouldn't matter because we check for existing cross-references later.
           @push_commits = project.repository.commits_between(project.default_branch, newrev)
         end
+        process_commit_messages(ref)
       elsif push_to_existing_branch?(ref, oldrev)
         # Collect data for this git push
         @push_commits = project.repository.commits_between(oldrev, newrev)
         project.update_merge_requests(oldrev, newrev, ref, @user)
+        process_commit_messages(ref)
       end
 
       @push_data = post_receive_data(oldrev, newrev, ref)
       create_push_event(@push_data)
 
-      process_commit_messages(ref)
-
       project.execute_hooks(@push_data.dup, :push_hooks)
       project.execute_services(@push_data.dup)
     end
@@ -90,7 +90,7 @@ class GitPushService
 
         issues_to_close.each do |issue|
           if project.jira_tracker? && project.jira_service.active
-            project.jira_service.execute(push_data, issue)
+            project.jira_service.execute(commit, issue)
           else
             Issues::CloseService.new(project, author, {}).execute(issue, commit)
           end

lib/gitlab/reference_extractor.rb

@@ -26,7 +26,7 @@ module Gitlab
       issues.uniq.map do |entry|
         if should_lookup?(project, entry[:project])
           entry[:project].issues.where(iid: entry[:id]).first
-        elsif external_jira_reference?(project, entry[:project])
+        elsif entry[:project] && entry[:project].jira_tracker?
           JiraIssue.new(entry[:id], entry[:project])
         end
       end.reject(&:nil?)
@@ -69,13 +69,5 @@ module Gitlab
         project.nil? || entry_project.default_issues_tracker?
       end
     end
-
-    def external_jira_reference?(project, entry_project)
-      if project.id == entry_project.id
-        project && project.jira_tracker?
-      else
-        entry_project && entry_project.jira_tracker?
-      end
-    end
   end
 end

spec/models/note_spec.rb

@@ -453,6 +453,6 @@ describe Note do
   end
 
   def jira_status_message
-    "JiraService SUCCESS 201: Sucessfully posted to #{api_mention_url}."
+    "JiraService SUCCESS 200: Sucessfully posted to #{api_mention_url}."
   end
 end

spec/models/project_services/jira_service_spec.rb

@@ -15,6 +15,8 @@
 require 'spec_helper'
 
 describe JiraService do
+  include RepoHelpers
+
   describe "Associations" do
     it { should belong_to :project }
     it { should have_one :service_hook }
@@ -43,7 +45,7 @@ describe JiraService do
     end
 
     it "should call JIRA API" do
-      @jira_service.execute(@sample_data, JiraIssue.new("JIRA-123", project))
+      @jira_service.execute(sample_commit, JiraIssue.new("JIRA-123", project))
       WebMock.should have_requested(:post, @api_url).with(
         body: /Issue solved with/
       ).once
@@ -51,7 +53,7 @@ describe JiraService do
 
     it "calls the api with jira_issue_transition_id" do
       @jira_service.jira_issue_transition_id = 'this-is-a-custom-id'
-      @jira_service.execute(@sample_data, JiraIssue.new("JIRA-123", project))
+      @jira_service.execute(sample_commit, JiraIssue.new("JIRA-123", project))
       WebMock.should have_requested(:post, @api_url).with(
         body: /this-is-a-custom-id/
       ).once