Commit 51462058 authored by Arturo Herrero's avatar Arturo Herrero

Jira entity uses url instead of api_url if possible

When configuring Jira, users can provide web url and api url. When
displaying the Jira Issue List page the calls to Jira must be made with
the api url, but the links we display in the frontend must use the web
url.
parent 65cd1cc5
...@@ -5,8 +5,8 @@ module Integrations ...@@ -5,8 +5,8 @@ module Integrations
class IssueEntity < Grape::Entity class IssueEntity < Grape::Entity
include RequestAwareEntity include RequestAwareEntity
expose :project_id do |_jira_issue, options| expose :project_id do |_jira_issue|
options[:project].id project.id
end end
expose :title do |jira_issue| expose :title do |jira_issue|
...@@ -55,12 +55,12 @@ module Integrations ...@@ -55,12 +55,12 @@ module Integrations
end end
expose :web_url do |jira_issue| expose :web_url do |jira_issue|
"#{base_web_url(jira_issue)}/browse/#{jira_issue.key}" "#{base_web_url}/browse/#{jira_issue.key}"
end end
expose :gitlab_web_url do |jira_issue| expose :gitlab_web_url do |jira_issue|
if ::Feature.enabled?(:jira_issues_show_integration, options[:project], default_enabled: :yaml) if ::Feature.enabled?(:jira_issues_show_integration, project, default_enabled: :yaml)
project_integrations_jira_issue_path(options[:project], jira_issue.key) project_integrations_jira_issue_path(project, jira_issue.key)
else else
nil nil
end end
...@@ -92,20 +92,19 @@ module Integrations ...@@ -92,20 +92,19 @@ module Integrations
# accountId is only available on Jira Cloud. # accountId is only available on Jira Cloud.
# https://community.atlassian.com/t5/Jira-Questions/How-to-find-account-id-on-jira-on-premise/qaq-p/1168652 # https://community.atlassian.com/t5/Jira-Questions/How-to-find-account-id-on-jira-on-premise/qaq-p/1168652
if jira_issue.public_send(user_type).try(:accountId) if jira_issue.public_send(user_type).try(:accountId)
"#{base_web_url(jira_issue)}/people/#{jira_issue.public_send(user_type).accountId}" "#{base_web_url}/people/#{jira_issue.public_send(user_type).accountId}"
else else
"#{base_web_url(jira_issue)}/secure/ViewProfile.jspa?name=#{jira_issue.public_send(user_type).name}" "#{base_web_url}/secure/ViewProfile.jspa?name=#{jira_issue.public_send(user_type).name}"
end end
end end
# rubocop:enable GitlabSecurity/PublicSend # rubocop:enable GitlabSecurity/PublicSend
def base_web_url(jira_issue) def base_web_url
site_url = jira_issue.client.options[:site].delete_suffix('/') @base_web_url ||= project.jira_service.url
context_path = jira_issue.client.options[:context_path].to_s.delete_prefix('/') end
return site_url if context_path.empty?
[site_url, context_path].join('/') def project
@project ||= options[:project]
end end
end end
end end
......
---
title: Jira Issues List page shows Web URL links instead of API URL
merge_request: 54434
author:
type: fixed
...@@ -8,7 +8,9 @@ RSpec.describe Resolvers::ExternalIssueResolver do ...@@ -8,7 +8,9 @@ RSpec.describe Resolvers::ExternalIssueResolver do
let_it_be(:current_user) { create(:user) } let_it_be(:current_user) { create(:user) }
context 'when Jira issues are requested' do context 'when Jira issues are requested' do
let_it_be(:vulnerability_external_issue_link) { create(:vulnerabilities_external_issue_link) } let_it_be(:project) { create(:project) }
let_it_be(:jira_service) { create(:jira_service, project: project) }
let_it_be(:vulnerability_external_issue_link) { create(:vulnerabilities_external_issue_link, project: project) }
let(:jira_issue) do let(:jira_issue) do
double( double(
...@@ -17,8 +19,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do ...@@ -17,8 +19,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do
created: Time.at(1606348800).utc, created: Time.at(1606348800).utc,
updated: Time.at(1606348800).utc, updated: Time.at(1606348800).utc,
status: double(name: 'To Do'), status: double(name: 'To Do'),
key: 'GV-1', key: 'GV-1'
client: double(options: { site: 'http://jira.com/' })
) )
end end
...@@ -28,7 +29,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do ...@@ -28,7 +29,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do
'created_at' => '2020-11-26T00:00:00.000Z', 'created_at' => '2020-11-26T00:00:00.000Z',
'updated_at' => '2020-11-26T00:00:00.000Z', 'updated_at' => '2020-11-26T00:00:00.000Z',
'status' => 'To Do', 'status' => 'To Do',
'web_url' => 'http://jira.com/browse/GV-1', 'web_url' => 'https://jira.example.com/browse/GV-1',
'references' => { 'references' => {
'relative' => 'GV-1' 'relative' => 'GV-1'
}, },
...@@ -45,6 +46,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do ...@@ -45,6 +46,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do
it 'sends request to Jira to fetch issues' do it 'sends request to Jira to fetch issues' do
params = [vulnerability_external_issue_link.vulnerability.project, [vulnerability_external_issue_link.external_issue_key]] params = [vulnerability_external_issue_link.vulnerability.project, [vulnerability_external_issue_link.external_issue_key]]
expect_next_instance_of(::Projects::Integrations::Jira::ByIdsFinder, *params) do |issues_finder| expect_next_instance_of(::Projects::Integrations::Jira::ByIdsFinder, *params) do |issues_finder|
expect(issues_finder).to receive(:execute).and_return(nil) expect(issues_finder).to receive(:execute).and_return(nil)
end end
...@@ -54,6 +56,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do ...@@ -54,6 +56,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do
it 'returns nil' do it 'returns nil' do
result = batch_sync { resolve_external_issue({}) } result = batch_sync { resolve_external_issue({}) }
expect(result).to be_nil expect(result).to be_nil
end end
end end
...@@ -67,6 +70,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do ...@@ -67,6 +70,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do
it 'sends request to Jira to fetch issues' do it 'sends request to Jira to fetch issues' do
params = [vulnerability_external_issue_link.vulnerability.project, [vulnerability_external_issue_link.external_issue_key]] params = [vulnerability_external_issue_link.vulnerability.project, [vulnerability_external_issue_link.external_issue_key]]
expect_next_instance_of(::Projects::Integrations::Jira::ByIdsFinder, *params) do |issues_finder| expect_next_instance_of(::Projects::Integrations::Jira::ByIdsFinder, *params) do |issues_finder|
expect(issues_finder).to receive(:execute).and_return(issues: [jira_issue]) expect(issues_finder).to receive(:execute).and_return(issues: [jira_issue])
end end
...@@ -76,6 +80,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do ...@@ -76,6 +80,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do
it 'returns serialized Jira issues' do it 'returns serialized Jira issues' do
result = batch_sync { resolve_external_issue({}) } result = batch_sync { resolve_external_issue({}) }
expect(result.as_json).to eq(expected_result) expect(result.as_json).to eq(expected_result)
end end
end end
......
...@@ -3,8 +3,11 @@ ...@@ -3,8 +3,11 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Integrations::Jira::IssueDetailEntity do RSpec.describe Integrations::Jira::IssueDetailEntity do
let(:project) { build(:project) } include JiraServiceHelper
let(:jira_client) { double(options: { site: 'http://jira.com/' }) }
let_it_be(:project) { create(:project) }
let_it_be(:jira_service) { create(:jira_service, project: project, url: 'http://jira.com', api_url: 'http://api.jira.com') }
let(:reporter) do let(:reporter) do
double( double(
'displayName' => 'reporter', 'displayName' => 'reporter',
...@@ -34,7 +37,6 @@ RSpec.describe Integrations::Jira::IssueDetailEntity do ...@@ -34,7 +37,6 @@ RSpec.describe Integrations::Jira::IssueDetailEntity do
assignee: assignee, assignee: assignee,
project: double(key: 'GL'), project: double(key: 'GL'),
key: 'GL-5', key: 'GL-5',
client: jira_client,
status: double(name: 'To Do') status: double(name: 'To Do')
) )
end end
...@@ -87,12 +89,15 @@ RSpec.describe Integrations::Jira::IssueDetailEntity do ...@@ -87,12 +89,15 @@ RSpec.describe Integrations::Jira::IssueDetailEntity do
expect(subject[:assignees].first).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=assignee@assignee.com') expect(subject[:assignees].first).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=assignee@assignee.com')
end end
context 'and context_path' do context 'with only url' do
let(:jira_client) { double(options: { site: 'http://jira.com/', context_path: '/jira-sub-path' }) } before do
stub_jira_service_test
jira_service.update!(api_url: nil)
end
it 'returns URLs including context path' do it 'returns URLs with the web url' do
expect(subject[:author]).to include(web_url: 'http://jira.com/jira-sub-path/secure/ViewProfile.jspa?name=reporter@reporter.com') expect(subject[:author]).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=reporter@reporter.com')
expect(subject[:web_url]).to eq('http://jira.com/jira-sub-path/browse/GL-5') expect(subject[:web_url]).to eq('http://jira.com/browse/GL-5')
end end
end end
end end
......
...@@ -3,7 +3,10 @@ ...@@ -3,7 +3,10 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Integrations::Jira::IssueEntity do RSpec.describe Integrations::Jira::IssueEntity do
let(:project) { build(:project) } include JiraServiceHelper
let_it_be(:project) { create(:project) }
let_it_be(:jira_service) { create(:jira_service, project: project, url: 'http://jira.com', api_url: 'http://api.jira.com') }
let(:reporter) do let(:reporter) do
double( double(
...@@ -32,13 +35,10 @@ RSpec.describe Integrations::Jira::IssueEntity do ...@@ -32,13 +35,10 @@ RSpec.describe Integrations::Jira::IssueEntity do
assignee: assignee, assignee: assignee,
project: double(key: 'GL'), project: double(key: 'GL'),
key: 'GL-5', key: 'GL-5',
client: jira_client,
status: double(name: 'To Do') status: double(name: 'To Do')
) )
end end
let(:jira_client) { double(options: { site: 'http://jira.com/' }) }
subject { described_class.new(jira_issue, project: project).as_json } subject { described_class.new(jira_issue, project: project).as_json }
it 'returns the Jira issues attributes' do it 'returns the Jira issues attributes' do
...@@ -85,12 +85,15 @@ RSpec.describe Integrations::Jira::IssueEntity do ...@@ -85,12 +85,15 @@ RSpec.describe Integrations::Jira::IssueEntity do
expect(subject[:assignees].first).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=assignee@assignee.com') expect(subject[:assignees].first).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=assignee@assignee.com')
end end
context 'and context_path' do context 'with only url' do
let(:jira_client) { double(options: { site: 'http://jira.com/', context_path: '/jira-sub-path' }) } before do
stub_jira_service_test
jira_service.update!(api_url: nil)
end
it 'returns URLs including context path' do it 'returns URLs with the web url' do
expect(subject[:author]).to include(web_url: 'http://jira.com/jira-sub-path/secure/ViewProfile.jspa?name=reporter@reporter.com') expect(subject[:author]).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=reporter@reporter.com')
expect(subject[:web_url]).to eq('http://jira.com/jira-sub-path/browse/GL-5') expect(subject[:web_url]).to eq('http://jira.com/browse/GL-5')
end end
end end
end end
......
...@@ -3,10 +3,10 @@ ...@@ -3,10 +3,10 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Integrations::Jira::IssueSerializer do RSpec.describe Integrations::Jira::IssueSerializer do
let(:serializer) { described_class.new } let_it_be(:project) { create(:project) }
let(:project) { build(:project) } let_it_be(:jira_service) { create(:jira_service, project: project) }
subject { serializer.represent(jira_issues, project: project) } subject { described_class.new.represent(jira_issues, project: project) }
describe '#represent' do describe '#represent' do
context 'when an empty array is being serialized' do context 'when an empty array is being serialized' do
......
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