Merge branch 'jira_cross_reference' into 'master'

Add option for Jira cross reference mentions and fix Jira issues closing

See merge request !304

app/helpers/projects_helper.rb

@@ -265,4 +265,14 @@ module ProjectsHelper
       false
     end
   end
+
+  def service_field_value(type, value)
+    return value unless type == 'password'
+
+    if value.present?
+      "***********"
+    else
+      nil
+    end
+  end
 end

app/models/concerns/mentionable.rb

@@ -64,9 +64,10 @@ module Mentionable
     return [] if text.blank?
     ext = Gitlab::ReferenceExtractor.new
     ext.analyze(text, p)
-    (ext.issues_for +
-     ext.merge_requests_for +
-     ext.commits_for).uniq - [local_reference]
+    (ext.issues_for(p) +
+     ext.merge_requests_for(p) +
+     ext.commits_for(p)
+     ).uniq - [local_reference]
   end
 
   # Create a cross-reference Note for each GFM reference to another Mentionable found in +mentionable_text+.

app/models/external_issue.rb

+class ExternalIssue
+  def initialize(issue_identifier, project)
+    @issue_identifier, @project = issue_identifier, project
+  end
+
+  def to_s
+    @issue_identifier.to_s
+  end
+
+  def id
+    @issue_identifier.to_s
+  end
+
+  def iid
+    @issue_identifier.to_s
+  end
+
+  def ==(other)
+    other.is_a?(self.class) && (to_s == other.to_s)
+  end
+
+  def project
+    @project
+  end
+end

app/models/jira_issue.rb

-class JiraIssue
-  def initialize(issue_identifier)
-    @issue_identifier = issue_identifier
-  end
-
-  def to_s
-    @issue_identifier.to_s
-  end
-
-  def id
-    @issue_identifier.to_s
-  end
-
-  def iid
-    @issue_identifier.to_s
-  end
-
-  def ==(other)
-    other.is_a?(self.class) && (to_s == other.to_s)
-  end
+class JiraIssue < ExternalIssue
 end

app/models/note.rb

@@ -90,7 +90,12 @@ class Note < ActiveRecord::Base
         note_options.merge!(noteable: noteable)
       end
 
-      create(note_options) unless cross_reference_disallowed?(noteable, mentioner)
+
+      if noteable.is_a?(ExternalIssue)
+        project.issues_tracker.create_cross_reference_note(noteable, mentioner, author)
+      else
+        create(note_options) unless cross_reference_disallowed?(noteable, mentioner)
+      end
     end
 
     def create_milestone_change_note(noteable, project, author, milestone)

app/models/project_services/issue_tracker_service.rb

@@ -36,6 +36,10 @@ class IssueTrackerService < Service
     # implement inside child
   end
 
+  def create_cross_reference_note
+    # implement inside child
+  end
+
   def issue_url(iid)
     self.issues_url.gsub(':id', iid.to_s)
   end

app/models/project_services/jira_service.rb

@@ -14,11 +14,12 @@
 
 class JiraService < IssueTrackerService
   include HTTParty
+  include Rails.application.routes.url_helpers
 
   prop_accessor :username, :password, :api_version, :jira_issue_transition_id,
                 :title, :description, :project_url, :issues_url, :new_issue_url
 
-  before_validation :set_api_version
+  before_validation :set_api_version, :set_jira_issue_transition_id
 
   def title
     if self.properties && self.properties['title'].present?
@@ -49,53 +50,147 @@ class JiraService < IssueTrackerService
     )
   end
 
+  def execute(push, issue = nil)
+    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
+    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,
+        url: resource_url(user_path(author)),
+      },
+      project: {
+        name: project.path_with_namespace,
+        url: resource_url(project_path(project))
+      },
+      entity: {
+        name: noteable_name.humanize,
+        url: entity_url
+      }
+    }
+
+    add_comment(data, issue_name)
+  end
+
+  private
+
+
   def set_api_version
     self.api_version ||= "2"
   end
 
-  def execute(push, issue = nil)
-    close_issue(push, issue) if issue
+  def set_jira_issue_transition_id
+    self.jira_issue_transition_id ||= "2"
   end
 
-  private
+  def close_issue(commit, issue)
+    url = close_issue_url(issue.iid)
 
-  def close_issue(push_data, issue_name)
-    url = close_issue_url(issue_name)
-    commit_url = push_data[:commits].first[:url]
+    commit_url = build_entity_url(:commit, commit.id)
 
     message = {
-      'update' => {
-        'comment' => [{
-          'add' => {
-            'body' => "Issue solved with #{commit_url}"
+      update: {
+        comment: [{
+          add: {
+            body: "Issue solved with [#{commit.id}|#{commit_url}]."
           }
         }]
       },
-      'transition' => {
-        'id' => jira_issue_transition_id
+      transition: {
+        id: jira_issue_transition_id
       }
-    }
+    }.to_json
+
+    send_message(url, message)
+  end
+
+  def add_comment(data, issue_name)
+    url = add_comment_url(issue_name)
+    user_name = data[:user][:name]
+    user_url = data[:user][:url]
+    entity_name = data[:entity][:name]
+    entity_url = data[:entity][:url]
+    entity_iid = data[:entity][:iid]
+    project_name = data[:project][:name]
+    project_url = data[:project][:url]
 
-    json_body = message.to_json
-    Rails.logger.info("#{self.class.name}: sending POST with body #{json_body} to #{url}")
+    message = {
+      body: "[#{user_name}|#{user_url}] mentioned #{issue_name} in #{entity_name} of [#{project_name}|#{entity_url}]."
+    }.to_json
+
+    send_message(url, message)
+  end
 
-    JiraService.post(
+
+  def auth
+    require 'base64'
+    Base64.urlsafe_encode64("#{self.username}:#{self.password}")
+  end
+
+  def send_message(url, message)
+    result = JiraService.post(
       url,
-      body: json_body,
+      body: message,
       headers: {
         'Content-Type' => 'application/json',
         'Authorization' => "Basic #{auth}"
       }
     )
+
+    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
+                "#{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 close_issue_url(issue_name)
-    "#{self.project_url.chomp("/")}/rest/api/#{self.api_version}/issue/#{issue_name}/transitions"
+  def server_url
+    server = URI(project_url)
+    default_ports = [80, 443].include?(server.port)
+    server_url = "#{server.scheme}://#{server.host}"
+    server_url.concat(":#{server.port}") unless default_ports
+    return server_url
   end
 
+  def resource_url(resource)
+    "#{Settings.gitlab['url'].chomp("/")}#{resource}"
+  end
 
-  def auth
-    require 'base64'
-    Base64.urlsafe_encode64("#{self.username}:#{self.password}")
+  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"
+  end
+
+  def add_comment_url(issue_name)
+    "#{server_url}/rest/api/#{self.api_version}/issue/#{issue_name}/comment"
   end
 end

app/models/service.rb

@@ -92,8 +92,4 @@ class Service < ActiveRecord::Base
   def issue_tracker?
     self.category == :issue_tracker
   end
-
-  def self.issue_tracker_service_list
-    Service.select(&:issue_tracker?).map{ |s| s.to_param }
-  end
 end

app/services/git_push_service.rb

@@ -53,6 +53,7 @@ class GitPushService
 
       @push_data = post_receive_data(oldrev, newrev, ref)
       create_push_event(@push_data)
+
       project.execute_hooks(@push_data.dup, :push_hooks)
       project.execute_services(@push_data.dup)
     end
@@ -89,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

app/views/projects/services/_form.html.haml

@@ -28,7 +28,7 @@
 
   - @service.fields.each do |field|
     - name = field[:name]
-    - value = @service.send(name) unless field[:type] == 'password'
+    - value = service_field_value(field[:type], @service.send(name))
     - type = field[:type]
     - placeholder = field[:placeholder]
     - choices = field[:choices]
@@ -43,12 +43,10 @@
           = f.text_area name, rows: 5, class: "form-control", placeholder: placeholder
         - elsif type == 'checkbox'
           = f.check_box name
-        - elsif type == 'password'
-          = f.password_field name, class: "form-control"
         - elsif type == 'select'
           = f.select name, options_for_select(choices, value ? value : default_choice), {}, { class: "form-control" }
         - elsif type == 'password'
-          = f.password_field name, class: 'form-control'
+          = f.password_field name, placeholder: value, class: 'form-control'
 
   .form-actions
     = f.submit 'Save', class: 'btn btn-save'

config/gitlab.yml.example

@@ -77,7 +77,7 @@ production: &base
     # This happens when the commit is pushed or merged into the default branch of a project.
     # When not specified the default issue_closing_pattern as specified below will be used.
     # Tip: you can test your closing pattern at http://rubular.com
-    # issue_closing_pattern: '((?:[Cc]los(?:e[sd]|ing)|[Ff]ix(?:e[sd]|ing)?) +(?:(?:issues? +)?#\d+(?:(?:, *| +and +)?))+)'
+    # issue_closing_pattern: '((?:[Cc]los(?:e[sd]|ing)|[Ff]ix(?:e[sd]|ing)?) +(?:(?:issues? +)?#\d+(?:(?:, *| +and +)?)|([A-Z]*-\d*))+)'
 
     ## Default project features settings
     default_projects_features:

config/initializers/1_settings.rb

@@ -121,7 +121,7 @@ Settings.gitlab['signup_enabled'] ||= true if Settings.gitlab['signup_enabled'].
 Settings.gitlab['signin_enabled'] ||= true if Settings.gitlab['signin_enabled'].nil?
 Settings.gitlab['restricted_visibility_levels'] = Settings.send(:verify_constant_array, Gitlab::VisibilityLevel, Settings.gitlab['restricted_visibility_levels'], [])
 Settings.gitlab['username_changing_enabled'] = true if Settings.gitlab['username_changing_enabled'].nil?
-Settings.gitlab['issue_closing_pattern'] = '((?:[Cc]los(?:e[sd]|ing)|[Ff]ix(?:e[sd]|ing)?) +(?:(?:issues? +)?#\d+(?:(?:, *| +and +)?))+)' if Settings.gitlab['issue_closing_pattern'].nil?
+Settings.gitlab['issue_closing_pattern'] = '((?:[Cc]los(?:e[sd]|ing)|[Ff]ix(?:e[sd]|ing)?) +(?:(?:issues? +)?#\d+(?:(?:, *| +and +)?)|([A-Z]*-\d*))+)' if Settings.gitlab['issue_closing_pattern'].nil?
 Settings.gitlab['default_projects_features'] ||= {}
 Settings.gitlab['webhook_timeout'] ||= 10
 Settings.gitlab.default_projects_features['issues']         = true if Settings.gitlab.default_projects_features['issues'].nil?

lib/gitlab/reference_extractor.rb

@@ -23,17 +23,13 @@ module Gitlab
     end
 
     def issues_for(project = nil)
-      if project && project.jira_tracker?
-        issues.uniq.map do |jira_identifier|
-          JiraIssue.new(jira_identifier[:id])
+      issues.uniq.map do |entry|
+        if should_lookup?(project, entry[:project])
+          entry[:project].issues.where(iid: entry[:id]).first
+        elsif entry[:project] && entry[:project].jira_tracker?
+          JiraIssue.new(entry[:id], entry[:project])
         end
-      else
-        issues.map do |entry|
-          if should_lookup?(project, entry[:project])
-            entry[:project].issues.where(iid: entry[:id]).first
-          end
-        end.reject(&:nil?)
-      end
+      end.reject(&:nil?)
     end
 
     def merge_requests_for(project = nil)
@@ -70,7 +66,7 @@ module Gitlab
       if entry_project.nil?
         false
       else
-        project.nil? || project.id == entry_project.id
+        project.nil? || entry_project.default_issues_tracker?
       end
     end
   end

spec/lib/gitlab/reference_extractor_spec.rb

@@ -75,10 +75,11 @@ describe Gitlab::ReferenceExtractor do
 
     it 'returns JIRA issues for a JIRA-integrated project' do
       project.stub(jira_tracker?: true)
+      project.stub(default_issues_tracker?: false)
 
       subject.analyze('JIRA-123 and FOOBAR-4567', project)
       subject.issues_for(project).should eq(
-        [JiraIssue.new('JIRA-123'), JiraIssue.new('FOOBAR-4567')]
+        [JiraIssue.new('JIRA-123', project), JiraIssue.new('FOOBAR-4567', project)]
       )
     end
 

spec/models/jira_issue_spec.rb

 require 'spec_helper'
 
 describe JiraIssue do
-  subject { JiraIssue.new('JIRA-123') }
+  let(:project) { create(:project) }
+  subject { JiraIssue.new('JIRA-123', project) }
 
   its(:id) { should eq('JIRA-123') }
   its(:iid) { should eq('JIRA-123') }
   its(:to_s) { should eq('JIRA-123') }
 
   describe :== do
-    specify { subject.should eq(JiraIssue.new('JIRA-123')) }
-    specify { subject.should_not eq(JiraIssue.new('JIRA-124')) }
+    specify { subject.should eq(JiraIssue.new('JIRA-123', project)) }
+    specify { subject.should_not eq(JiraIssue.new('JIRA-124', project)) }
 
     it 'only compares with JiraIssues' do
       subject.should_not eq('JIRA-123')

spec/models/merge_request_spec.rb

@@ -115,8 +115,8 @@ describe MergeRequest do
     end
 
     context 'for a project with JIRA integration' do
-      let(:issue0) { JiraIssue.new('JIRA-123') }
-      let(:issue1) { JiraIssue.new('FOOBAR-4567') }
+      let(:issue0) { JiraIssue.new('JIRA-123', subject.project) }
+      let(:issue1) { JiraIssue.new('FOOBAR-4567', subject.project) }
 
       it 'returns sorted JiraIssues' do
         subject.project.stub(default_branch: subject.target_branch)

spec/models/note_spec.rb

@@ -209,26 +209,80 @@ describe Note do
     let(:issue)      { create(:issue, project: project) }
     let(:mergereq)   { create(:merge_request, :simple, target_project: project, source_project: project) }
     let(:commit)     { project.repository.commit }
+    let(:jira_issue) { JiraIssue.new("JIRA-1", project)}
+    let(:jira_tracker) { project.create_jira_service if project.jira_service.nil? }
+    let(:api_mention_url) { 'http://jira.example/rest/api/2/issue/JIRA-1/comment' }
 
     # Test all of {issue, merge request, commit} in both the referenced and referencing
     # roles, to ensure that the correct information can be inferred from any argument.
 
     context 'issue from a merge request' do
-      subject { Note.create_cross_reference_note(issue, mergereq, author, project) }
+      context 'in default issue tracker' do
+        subject { Note.create_cross_reference_note(issue, mergereq, author, project) }
+
+        it { should be_valid }
+        its(:noteable) { should == issue }
+        its(:project)  { should == issue.project }
+        its(:author)   { should == author }
+        its(:note) { should == "_mentioned in merge request !#{mergereq.iid}_" }
+      end
 
-      it { should be_valid }
-      its(:noteable) { should == issue }
-      its(:project)  { should == issue.project }
-      its(:author)   { should == author }
-      its(:note) { should == "_mentioned in merge request !#{mergereq.iid}_" }
+      context 'in JIRA issue tracker' do
+        before do
+          jira_service_settings
+          WebMock.stub_request(:post, api_mention_url)
+        end
+
+        after do
+          jira_tracker.destroy!
+        end
+
+        subject { Note.create_cross_reference_note(jira_issue, mergereq, author, project) }
+
+        it { should == jira_status_message }
+      end
     end
 
     context 'issue from a commit' do
-      subject { Note.create_cross_reference_note(issue, commit, author, project) }
+      context 'in default issue tracker' do
+        subject { Note.create_cross_reference_note(issue, commit, author, project) }
 
-      it { should be_valid }
-      its(:noteable) { should == issue }
-      its(:note) { should == "_mentioned in commit #{commit.sha}_" }
+        it { should be_valid }
+        its(:noteable) { should == issue }
+        its(:note) { should == "_mentioned in commit #{commit.sha}_" }
+      end
+
+      context 'in JIRA issue tracker' do
+        before do
+          jira_service_settings
+          WebMock.stub_request(:post, api_mention_url)
+        end
+
+        after do
+          jira_tracker.destroy!
+        end
+
+        subject { Note.create_cross_reference_note(jira_issue, commit, author, project) }
+
+        it { should == jira_status_message }
+      end
+    end
+
+    context 'issue from an isue' do
+      context 'in JIRA issue tracker' do
+        before do
+          jira_service_settings
+          WebMock.stub_request(:post, api_mention_url)
+        end
+
+        after do
+          jira_tracker.destroy!
+        end
+
+        subject { Note.create_cross_reference_note(jira_issue, issue, author, project) }
+
+        it { should == jira_status_message }
+      end
     end
 
     context 'merge request from an issue' do
@@ -386,4 +440,19 @@ describe Note do
     let(:backref_text) { issue.gfm_reference }
     let(:set_mentionable_text) { ->(txt) { subject.note = txt } }
   end
+
+  def jira_service_settings
+    properties = {
+      "title"=>"JIRA tracker",
+      "project_url"=>"http://jira.example/issues/?jql=project=A",
+      "issues_url"=>"http://jira.example/browse/JIRA-1",
+      "new_issue_url"=>"http://jira.example/secure/CreateIssue.jspa"
+    }
+
+    jira_tracker.update_attributes(properties: properties, active: true)
+  end
+
+  def jira_status_message
+    "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"))
+      @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"))
+      @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

spec/services/git_push_service_spec.rb

@@ -201,38 +201,101 @@ describe GitPushService do
     let(:commit_author) { create :user }
     let(:closing_commit) { project.repository.commit }
 
-    before do
-      closing_commit.stub({
-        issue_closing_regex: /^([Cc]loses|[Ff]ixes) #\d+/,
-        safe_message: "this is some work.\n\ncloses ##{issue.iid}",
-        author_name: commit_author.name,
-        author_email: commit_author.email
-      })
+    context "for default gitlab issue tracker" do
+      before do
+        closing_commit.stub({
+          issue_closing_regex: Regexp.new(Gitlab.config.gitlab.issue_closing_pattern),
+          safe_message: "this is some work.\n\ncloses ##{issue.iid}",
+          author_name: commit_author.name,
+          author_email: commit_author.email
+        })
+
+        project.repository.stub(commits_between: [closing_commit])
+      end
 
-      project.repository.stub(commits_between: [closing_commit])
-    end
+      it "closes issues with commit messages" do
+        service.execute(project, user, @oldrev, @newrev, @ref)
 
-    it "closes issues with commit messages" do
-      service.execute(project, user, @oldrev, @newrev, @ref)
+        Issue.find(issue.id).should be_closed
+      end
+
+      it "doesn't create cross-reference notes for a closing reference" do
+        expect {
+          service.execute(project, user, @oldrev, @newrev, @ref)
+        }.not_to change { Note.where(project_id: project.id, system: true, commit_id: closing_commit.id).count }
+      end
+
+      it "doesn't close issues when pushed to non-default branches" do
+        project.stub(default_branch: 'durf')
 
-      Issue.find(issue.id).should be_closed
+        # The push still shouldn't create cross-reference notes.
+        expect {
+          service.execute(project, user, @oldrev, @newrev, 'refs/heads/hurf')
+        }.not_to change { Note.where(project_id: project.id, system: true).count }
+
+        Issue.find(issue.id).should be_opened
+      end
     end
 
-    it "doesn't create cross-reference notes for a closing reference" do
-      expect {
+    context "for jira issue tracker" do
+      let(:api_transition_url) { 'http://jira.example/rest/api/2/issue/JIRA-1/transitions' }
+      let(:api_mention_url) { 'http://jira.example/rest/api/2/issue/JIRA-1/comment' }
+      let(:jira_tracker) { project.create_jira_service if project.jira_service.nil? }
+
+      before do
+        properties = {
+          "title"=>"JIRA tracker",
+          "project_url"=>"http://jira.example/issues/?jql=project=A",
+          "issues_url"=>"http://jira.example/browse/JIRA-1",
+          "new_issue_url"=>"http://jira.example/secure/CreateIssue.jspa"
+        }
+        jira_tracker.update_attributes(properties: properties, active: true)
+
+        WebMock.stub_request(:post, api_transition_url)
+        WebMock.stub_request(:post, api_mention_url)
+
+        closing_commit.stub({
+          issue_closing_regex: Regexp.new(Gitlab.config.gitlab.issue_closing_pattern),
+          safe_message: "this is some work.\n\ncloses JIRA-1",
+          author_name: commit_author.name,
+          author_email: commit_author.email
+        })
+
+        project.repository.stub(commits_between: [closing_commit])
+      end
+
+      after do
+        jira_tracker.destroy!
+      end
+
+      it "should initiate one api call to jira server to close the issue" do
+         message = {
+          update: {
+            comment: [{
+              add: {
+                body: "Issue solved with [#{closing_commit.id}|http://localhost/#{project.path_with_namespace}/commit/#{closing_commit.id}]."
+              }
+            }]
+          },
+          transition: {
+            id: '2'
+          }
+        }.to_json
+
         service.execute(project, user, @oldrev, @newrev, @ref)
-      }.not_to change { Note.where(project_id: project.id, system: true, commit_id: closing_commit.id).count }
-    end
+        WebMock.should have_requested(:post, api_transition_url).with(
+          body: message
+        ).once
+      end
 
-    it "doesn't close issues when pushed to non-default branches" do
-      project.stub(default_branch: 'durf')
+      it "should initiate one api call to jira server to mention the issue" do
+        service.execute(project, user, @oldrev, @newrev, @ref)
 
-      # The push still shouldn't create cross-reference notes.
-      expect {
-        service.execute(project, user, @oldrev, @newrev, 'refs/heads/hurf')
-      }.not_to change { Note.where(project_id: project.id, system: true).count }
+        WebMock.should have_requested(:post, api_mention_url).with(
+          body: /mentioned JIRA-1 in/
+        ).once
+      end
 
-      Issue.find(issue.id).should be_opened
     end
   end
 end