Commit 85dd05b5 authored by Felipe Artur's avatar Felipe Artur

Add JIRA remotelinks and prevent duplicated closing messages

parent 18f96e26
...@@ -70,7 +70,7 @@ class JiraService < IssueTrackerService ...@@ -70,7 +70,7 @@ class JiraService < IssueTrackerService
end end
def jira_project def jira_project
@jira_project ||= client.Project.find(project_key) @jira_project ||= jira_request { client.Project.find(project_key) }
end end
def help def help
...@@ -128,12 +128,19 @@ class JiraService < IssueTrackerService ...@@ -128,12 +128,19 @@ class JiraService < IssueTrackerService
# we just want to test settings # we just want to test settings
test_settings test_settings
else else
close_issue(push, issue) jira_issue = jira_request { client.Issue.find(issue.iid) }
return false unless jira_issue.present?
close_issue(push, jira_issue)
end end
end end
def create_cross_reference_note(mentioned, noteable, author) def create_cross_reference_note(mentioned, noteable, author)
issue_key = mentioned.id jira_issue = jira_request { client.Issue.find(mentioned.id) }
return false unless jira_issue.present?
project = self.project project = self.project
noteable_name = noteable.class.name.underscore.downcase noteable_name = noteable.class.name.underscore.downcase
noteable_id = if noteable.is_a?(Commit) noteable_id = if noteable.is_a?(Commit)
...@@ -160,7 +167,7 @@ class JiraService < IssueTrackerService ...@@ -160,7 +167,7 @@ class JiraService < IssueTrackerService
} }
} }
add_comment(data, issue_key) add_comment(data, jira_issue)
end end
# reason why service cannot be tested # reason why service cannot be tested
...@@ -181,16 +188,14 @@ class JiraService < IssueTrackerService ...@@ -181,16 +188,14 @@ class JiraService < IssueTrackerService
def test_settings def test_settings
return unless url.present? return unless url.present?
# Test settings by getting the project # Test settings by getting the project
jira_project jira_request { jira_project.present? }
rescue Errno::ECONNREFUSED, JIRA::HTTPError => e
Rails.logger.info "#{self.class.name} ERROR: #{e.message}. API URL: #{url}."
false
end end
private private
def close_issue(entity, issue) def close_issue(entity, issue)
return if issue.nil? || issue.resolution.present?
commit_id = if entity.is_a?(Commit) commit_id = if entity.is_a?(Commit)
entity.id entity.id
elsif entity.is_a?(MergeRequest) elsif entity.is_a?(MergeRequest)
...@@ -200,55 +205,85 @@ class JiraService < IssueTrackerService ...@@ -200,55 +205,85 @@ class JiraService < IssueTrackerService
commit_url = build_entity_url(:commit, commit_id) commit_url = build_entity_url(:commit, commit_id)
# Depending on the JIRA project's workflow, a comment during transition # Depending on the JIRA project's workflow, a comment during transition
# may or may not be allowed. Split the operation in to two calls so the # may or may not be allowed. Refresh the issue after transition and check
# comment always works. # if it is closed, so we don't have one comment for every commit.
transition_issue(issue) issue = jira_request { client.Issue.find(issue.key) } if transition_issue(issue)
add_issue_solved_comment(issue, commit_id, commit_url) add_issue_solved_comment(issue, commit_id, commit_url) if issue.resolution
end end
def transition_issue(issue) def transition_issue(issue)
issue = client.Issue.find(issue.iid)
issue.transitions.build.save(transition: { id: jira_issue_transition_id }) issue.transitions.build.save(transition: { id: jira_issue_transition_id })
end end
def add_issue_solved_comment(issue, commit_id, commit_url) def add_issue_solved_comment(issue, commit_id, commit_url)
comment = "Issue solved with [#{commit_id}|#{commit_url}]." link_title = "GitLab: Solved by commit #{commit_id}."
send_message(issue.iid, comment) comment = "Issue solved with [#{commit_id}|#{commit_url}]."
link_props = build_remote_link_props(url: commit_url, title: link_title, resolved: true)
send_message(issue, comment, link_props)
end end
def add_comment(data, issue_key) def add_comment(data, issue)
user_name = data[:user][:name] user_name = data[:user][:name]
user_url = data[:user][:url] user_url = data[:user][:url]
entity_name = data[:entity][:name] entity_name = data[:entity][:name]
entity_url = data[:entity][:url] entity_url = data[:entity][:url]
entity_title = data[:entity][:title] entity_title = data[:entity][:title]
project_name = data[:project][:name] project_name = data[:project][:name]
message = "[#{user_name}|#{user_url}] mentioned this issue in [a #{entity_name} of #{project_name}|#{entity_url}]:\n'#{entity_title}'" message = "[#{user_name}|#{user_url}] mentioned this issue in [a #{entity_name} of #{project_name}|#{entity_url}]:\n'#{entity_title}'"
link_title = "GitLab: Mentioned on #{entity_name} - #{entity_title}"
link_props = build_remote_link_props(url: entity_url, title: link_title)
unless comment_exists?(issue_key, message) unless comment_exists?(issue, message)
send_message(issue_key, message) send_message(issue, message, link_props)
end end
end end
def comment_exists?(issue_key, message) def comment_exists?(issue, message)
comments = client.Issue.find(issue_key).comments comments = jira_request { issue.comments }
comments.map { |comment| comment.body.include?(message) }.any?
comments.present? && comments.any? { |comment| comment.body.include?(message) }
end end
def send_message(issue_key, message) def send_message(issue, message, remote_link_props)
return unless url.present? return unless url.present?
issue = client.Issue.find(issue_key) jira_request do
if issue.comments.build.save!(body: message)
remote_link = issue.remotelink.build
remote_link.save!(remote_link_props)
result_message = "#{self.class.name} SUCCESS: Successfully posted to #{url}."
end
if issue.comments.build.save!(body: message) Rails.logger.info(result_message)
result_message = "#{self.class.name} SUCCESS: Successfully posted to #{url}." result_message
end end
end
Rails.logger.info(result_message) # Build remote link on JIRA properties
result_message # Icons here must be available on WEB so JIRA can read the URL
rescue URI::InvalidURIError, Errno::ECONNREFUSED, JIRA::HTTPError => e # We are using a open word graphics icon which have LGPL license
Rails.logger.info "#{self.class.name} Send message ERROR: #{url} - #{e.message}" def build_remote_link_props(url:, title:, resolved: false)
status = {
resolved: resolved
}
if resolved
status[:icon] = {
title: 'Closed',
url16x16: 'http://www.openwebgraphics.com/resources/data/1768/16x16_apply.png'
}
end
{
GlobalID: 'GitLab',
object: {
url: url,
title: title,
status: status,
icon: { title: 'GitLab', url16x16: 'https://gitlab.com/favicon.ico' }
}
}
end end
def resource_url(resource) def resource_url(resource)
...@@ -268,4 +303,13 @@ class JiraService < IssueTrackerService ...@@ -268,4 +303,13 @@ class JiraService < IssueTrackerService
) )
) )
end end
# Handle errors when doing JIRA API calls
def jira_request
yield
rescue Timeout::Error, Errno::EINVAL, Errno::ECONNRESET, Errno::ECONNREFUSED, URI::InvalidURIError, JIRA::HTTPError => e
Rails.logger.info "#{self.class.name} Send message ERROR: #{url} - #{e.message}"
nil
end
end end
---
title: Add JIRA remotelinks and prevent duplicated closing messages
merge_request:
author:
...@@ -85,17 +85,30 @@ describe JiraService, models: true do ...@@ -85,17 +85,30 @@ describe JiraService, models: true do
project_key: 'GitLabProject' project_key: 'GitLabProject'
) )
# These stubs are needed to test JiraService#close_issue.
# We close the issue then do another request to API to check if it got closed.
# Here is stubbed the API return with a closed and an opened issues.
open_issue = JIRA::Resource::Issue.new(@jira_service.client, attrs: { "id" => "JIRA-123" })
closed_issue = open_issue.dup
allow(open_issue).to receive(:resolution).and_return(false)
allow(closed_issue).to receive(:resolution).and_return(true)
allow(JIRA::Resource::Issue).to receive(:find).and_return(open_issue, closed_issue)
allow_any_instance_of(JIRA::Resource::Issue).to receive(:key).and_return("JIRA-123")
@jira_service.save @jira_service.save
project_issues_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123' project_issues_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123'
@project_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/project/GitLabProject' @project_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/project/GitLabProject'
@transitions_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/transitions' @transitions_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/transitions'
@comment_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/comment' @comment_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/comment'
@remote_link_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/remotelink'
WebMock.stub_request(:get, @project_url) WebMock.stub_request(:get, @project_url)
WebMock.stub_request(:get, project_issues_url) WebMock.stub_request(:get, project_issues_url)
WebMock.stub_request(:post, @transitions_url) WebMock.stub_request(:post, @transitions_url)
WebMock.stub_request(:post, @comment_url) WebMock.stub_request(:post, @comment_url)
WebMock.stub_request(:post, @remote_link_url)
end end
it "calls JIRA API" do it "calls JIRA API" do
...@@ -106,6 +119,37 @@ describe JiraService, models: true do ...@@ -106,6 +119,37 @@ describe JiraService, models: true do
).once ).once
end end
# Check https://developer.atlassian.com/jiradev/jira-platform/guides/other/guide-jira-remote-issue-links/fields-in-remote-issue-links
# for more information
it "creates Remote Link reference in JIRA for comment" do
@jira_service.execute(merge_request, ExternalIssue.new("JIRA-123", project))
# Creates comment
expect(WebMock).to have_requested(:post, @comment_url)
# Creates Remote Link in JIRA issue fields
expect(WebMock).to have_requested(:post, @remote_link_url).with(
body: hash_including(
GlobalID: "GitLab",
object: {
url: "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/commit/#{merge_request.diff_head_sha}",
title: "GitLab: Solved by commit #{merge_request.diff_head_sha}.",
icon: { title: "GitLab", url16x16: "https://gitlab.com/favicon.ico" },
status: { resolved: true, icon: { url16x16: "http://www.openwebgraphics.com/resources/data/1768/16x16_apply.png", title: "Closed" } }
}
)
).once
end
it "does not send comment or remote links to issues already closed" do
allow_any_instance_of(JIRA::Resource::Issue).to receive(:resolution).and_return(true)
@jira_service.execute(merge_request, ExternalIssue.new("JIRA-123", project))
expect(WebMock).not_to have_requested(:post, @comment_url)
expect(WebMock).not_to have_requested(:post, @remote_link_url)
end
it "references the GitLab commit/merge request" do it "references the GitLab commit/merge request" do
@jira_service.execute(merge_request, ExternalIssue.new("JIRA-123", project)) @jira_service.execute(merge_request, ExternalIssue.new("JIRA-123", project))
......
...@@ -492,6 +492,16 @@ describe GitPushService, services: true do ...@@ -492,6 +492,16 @@ describe GitPushService, services: true do
let(:message) { "this is some work.\n\ncloses JIRA-1" } let(:message) { "this is some work.\n\ncloses JIRA-1" }
let(:comment_body) { { body: "Issue solved with [#{closing_commit.id}|http://localhost/#{project.path_with_namespace}/commit/#{closing_commit.id}]." }.to_json } let(:comment_body) { { body: "Issue solved with [#{closing_commit.id}|http://localhost/#{project.path_with_namespace}/commit/#{closing_commit.id}]." }.to_json }
before do
open_issue = JIRA::Resource::Issue.new(jira_tracker.client, attrs: { "id" => "JIRA-1" })
closed_issue = open_issue.dup
allow(open_issue).to receive(:resolution).and_return(false)
allow(closed_issue).to receive(:resolution).and_return(true)
allow(JIRA::Resource::Issue).to receive(:find).and_return(open_issue, closed_issue)
allow_any_instance_of(JIRA::Resource::Issue).to receive(:key).and_return("JIRA-1")
end
context "using right markdown" do context "using right markdown" do
it "initiates one api call to jira server to close the issue" do it "initiates one api call to jira server to close the issue" do
execute_service(project, commit_author, @oldrev, @newrev, @ref ) execute_service(project, commit_author, @oldrev, @newrev, @ref )
......
...@@ -67,17 +67,19 @@ describe MergeRequests::MergeService, services: true do ...@@ -67,17 +67,19 @@ describe MergeRequests::MergeService, services: true do
it 'closes issues on JIRA issue tracker' do it 'closes issues on JIRA issue tracker' do
jira_issue = ExternalIssue.new('JIRA-123', project) jira_issue = ExternalIssue.new('JIRA-123', project)
stub_jira_urls(jira_issue)
commit = double('commit', safe_message: "Fixes #{jira_issue.to_reference}") commit = double('commit', safe_message: "Fixes #{jira_issue.to_reference}")
allow(merge_request).to receive(:commits).and_return([commit]) allow(merge_request).to receive(:commits).and_return([commit])
expect_any_instance_of(JiraService).to receive(:close_issue).with(merge_request, jira_issue).once expect_any_instance_of(JiraService).to receive(:close_issue).with(merge_request, an_instance_of(JIRA::Resource::Issue)).once
service.execute(merge_request) service.execute(merge_request)
end end
context "wrong issue markdown" do context "wrong issue markdown" do
it 'does not close issues on JIRA issue tracker' do it 'does not close issues on JIRA issue tracker' do
jira_issue = ExternalIssue.new('#123', project) jira_issue = ExternalIssue.new('#JIRA-123', project)
stub_jira_urls(jira_issue)
commit = double('commit', safe_message: "Fixes #{jira_issue.to_reference}") commit = double('commit', safe_message: "Fixes #{jira_issue.to_reference}")
allow(merge_request).to receive(:commits).and_return([commit]) allow(merge_request).to receive(:commits).and_return([commit])
......
require 'spec_helper' require 'spec_helper'
describe SystemNoteService, services: true do describe SystemNoteService, services: true do
include Gitlab::Routing.url_helpers
let(:project) { create(:project) } let(:project) { create(:project) }
let(:author) { create(:user) } let(:author) { create(:user) }
let(:noteable) { create(:issue, project: project) } let(:noteable) { create(:issue, project: project) }
...@@ -543,23 +545,55 @@ describe SystemNoteService, services: true do ...@@ -543,23 +545,55 @@ describe SystemNoteService, services: true do
before { stub_jira_urls(jira_issue.id) } before { stub_jira_urls(jira_issue.id) }
context 'in JIRA issue tracker' do context 'in issue' do
before { jira_service_settings } before { jira_service_settings }
describe "new reference" do describe "new reference" do
subject { described_class.cross_reference(jira_issue, commit, author) } subject { described_class.cross_reference(jira_issue, commit, author) }
it { is_expected.to eq(success_message) } it { is_expected.to eq(success_message) }
it "creates remote link" do
subject
expect(WebMock).to have_requested(:post, jira_api_remote_link_url(jira_issue)).with(
body: hash_including(
GlobalID: "GitLab",
object: {
url: namespace_project_commit_url(project.namespace, project, commit),
title: "GitLab: Mentioned on commit - #{commit.title}",
icon: { title: "GitLab", url16x16: "https://gitlab.com/favicon.ico" },
status: { resolved: false }
}
)
).once
end
end end
end end
context 'issue from an issue' do context 'in commit' do
context 'in JIRA issue tracker' do context 'in JIRA issue tracker' do
before { jira_service_settings } before { jira_service_settings }
subject { described_class.cross_reference(jira_issue, issue, author) } subject { described_class.cross_reference(jira_issue, issue, author) }
it { is_expected.to eq(success_message) } it { is_expected.to eq(success_message) }
it "creates remote link" do
subject
expect(WebMock).to have_requested(:post, jira_api_remote_link_url(jira_issue)).with(
body: hash_including(
GlobalID: "GitLab",
object: {
url: namespace_project_issue_url(project.namespace, project, issue),
title: "GitLab: Mentioned on issue - #{issue.title}",
icon: { title: "GitLab", url16x16: "https://gitlab.com/favicon.ico" },
status: { resolved: false }
}
)
).once
end
end end
end end
...@@ -572,6 +606,13 @@ describe SystemNoteService, services: true do ...@@ -572,6 +606,13 @@ describe SystemNoteService, services: true do
subject { described_class.cross_reference(jira_issue, commit, author) } subject { described_class.cross_reference(jira_issue, commit, author) }
it { is_expected.not_to eq(success_message) } it { is_expected.not_to eq(success_message) }
it 'does not try to create comment and remote link' do
subject
expect(WebMock).not_to have_requested(:post, jira_api_comment_url(jira_issue))
expect(WebMock).not_to have_requested(:post, jira_api_remote_link_url(jira_issue))
end
end end
end end
end end
...@@ -57,6 +57,10 @@ module JiraServiceHelper ...@@ -57,6 +57,10 @@ module JiraServiceHelper
JIRA_API + "/issue/#{issue_id}/comment" JIRA_API + "/issue/#{issue_id}/comment"
end end
def jira_api_remote_link_url(issue_id)
JIRA_API + "/issue/#{issue_id}/remotelink"
end
def jira_api_transition_url(issue_id) def jira_api_transition_url(issue_id)
JIRA_API + "/issue/#{issue_id}/transitions" JIRA_API + "/issue/#{issue_id}/transitions"
end end
...@@ -75,6 +79,7 @@ module JiraServiceHelper ...@@ -75,6 +79,7 @@ module JiraServiceHelper
WebMock.stub_request(:get, jira_issue_url(issue_id)) WebMock.stub_request(:get, jira_issue_url(issue_id))
WebMock.stub_request(:get, jira_api_test_url) WebMock.stub_request(:get, jira_api_test_url)
WebMock.stub_request(:post, jira_api_comment_url(issue_id)) WebMock.stub_request(:post, jira_api_comment_url(issue_id))
WebMock.stub_request(:post, jira_api_remote_link_url(issue_id))
WebMock.stub_request(:post, jira_api_transition_url(issue_id)) WebMock.stub_request(:post, jira_api_transition_url(issue_id))
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment