Merge branch '300984-jira-entities-base-web-url' into 'master'

Jira Issues List page shows Web URL links instead of API URL

See merge request gitlab-org/gitlab!54434

ee/app/serializers/integrations/jira/issue_entity.rb

@@ -5,8 +5,8 @@ module Integrations
     class IssueEntity < Grape::Entity
       include RequestAwareEntity
 
-      expose :project_id do |_jira_issue, options|
-        options[:project].id
+      expose :project_id do |_jira_issue|
+        project.id
       end
 
       expose :title do |jira_issue|
@@ -55,12 +55,12 @@ module Integrations
       end
 
       expose :web_url do |jira_issue|
-        "#{base_web_url(jira_issue)}/browse/#{jira_issue.key}"
+        "#{base_web_url}/browse/#{jira_issue.key}"
       end
 
       expose :gitlab_web_url do |jira_issue|
-        if ::Feature.enabled?(:jira_issues_show_integration, options[:project], default_enabled: :yaml)
-          project_integrations_jira_issue_path(options[:project], jira_issue.key)
+        if ::Feature.enabled?(:jira_issues_show_integration, project, default_enabled: :yaml)
+          project_integrations_jira_issue_path(project, jira_issue.key)
         else
           nil
         end
@@ -92,20 +92,19 @@ module Integrations
         # 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
         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
-          "#{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
       # rubocop:enable GitlabSecurity/PublicSend
 
-      def base_web_url(jira_issue)
-        site_url = jira_issue.client.options[:site].delete_suffix('/')
-        context_path = jira_issue.client.options[:context_path].to_s.delete_prefix('/')
-
-        return site_url if context_path.empty?
+      def base_web_url
+        @base_web_url ||= project.jira_service.url
+      end
 
-        [site_url, context_path].join('/')
+      def project
+        @project ||= options[:project]
       end
     end
   end

ee/changelogs/unreleased/300984-jira-entities-base-web-url.yml

+---
+title: Jira Issues List page shows Web URL links instead of API URL
+merge_request: 54434
+author:
+type: fixed

ee/spec/graphql/resolvers/external_issue_resolver_spec.rb

@@ -8,7 +8,9 @@ RSpec.describe Resolvers::ExternalIssueResolver do
   let_it_be(:current_user) { create(:user) }
 
   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
       double(
@@ -17,8 +19,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do
         created: Time.at(1606348800).utc,
         updated: Time.at(1606348800).utc,
         status: double(name: 'To Do'),
-        key: 'GV-1',
-        client: double(options: { site: 'http://jira.com/' })
+        key: 'GV-1'
       )
     end
 
@@ -28,7 +29,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do
         'created_at' => '2020-11-26T00:00:00.000Z',
         'updated_at' => '2020-11-26T00:00:00.000Z',
         'status' => 'To Do',
-        'web_url' => 'http://jira.com/browse/GV-1',
+        'web_url' => 'https://jira.example.com/browse/GV-1',
         'references' => {
           'relative' => 'GV-1'
         },
@@ -45,6 +46,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do
 
       it 'sends request to Jira to fetch issues' do
         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(issues_finder).to receive(:execute).and_return(nil)
         end
@@ -54,6 +56,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do
 
       it 'returns nil' do
         result = batch_sync { resolve_external_issue({}) }
+
         expect(result).to be_nil
       end
     end
@@ -67,6 +70,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do
 
       it 'sends request to Jira to fetch issues' do
         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(issues_finder).to receive(:execute).and_return(issues: [jira_issue])
         end
@@ -76,6 +80,7 @@ RSpec.describe Resolvers::ExternalIssueResolver do
 
       it 'returns serialized Jira issues' do
         result = batch_sync { resolve_external_issue({}) }
+
         expect(result.as_json).to eq(expected_result)
       end
     end

ee/spec/serializers/integrations/jira/issue_detail_entity_spec.rb

@@ -3,8 +3,11 @@
 require 'spec_helper'
 
 RSpec.describe Integrations::Jira::IssueDetailEntity do
-  let(:project) { build(:project) }
-  let(:jira_client) { double(options: { site: 'http://jira.com/' }) }
+  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
     double(
       'displayName' => 'reporter',
@@ -34,7 +37,6 @@ RSpec.describe Integrations::Jira::IssueDetailEntity do
       assignee: assignee,
       project: double(key: 'GL'),
       key: 'GL-5',
-      client: jira_client,
       status: double(name: 'To Do')
     )
   end
@@ -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')
     end
 
-    context 'and context_path' do
-      let(:jira_client) { double(options: { site: 'http://jira.com/', context_path: '/jira-sub-path' }) }
+    context 'with only url' do
+      before do
+        stub_jira_service_test
+        jira_service.update!(api_url: nil)
+      end
 
-      it 'returns URLs including context path' do
-        expect(subject[:author]).to include(web_url: 'http://jira.com/jira-sub-path/secure/ViewProfile.jspa?name=reporter@reporter.com')
-        expect(subject[:web_url]).to eq('http://jira.com/jira-sub-path/browse/GL-5')
+      it 'returns URLs with the web url' do
+        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/browse/GL-5')
       end
     end
   end

ee/spec/serializers/integrations/jira/issue_entity_spec.rb

@@ -3,7 +3,10 @@
 require 'spec_helper'
 
 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
     double(
@@ -32,13 +35,10 @@ RSpec.describe Integrations::Jira::IssueEntity do
       assignee: assignee,
       project: double(key: 'GL'),
       key: 'GL-5',
-      client: jira_client,
       status: double(name: 'To Do')
     )
   end
 
-  let(:jira_client) { double(options: { site: 'http://jira.com/' }) }
-
   subject { described_class.new(jira_issue, project: project).as_json }
 
   it 'returns the Jira issues attributes' 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')
     end
 
-    context 'and context_path' do
-      let(:jira_client) { double(options: { site: 'http://jira.com/', context_path: '/jira-sub-path' }) }
+    context 'with only url' do
+      before do
+        stub_jira_service_test
+        jira_service.update!(api_url: nil)
+      end
 
-      it 'returns URLs including context path' do
-        expect(subject[:author]).to include(web_url: 'http://jira.com/jira-sub-path/secure/ViewProfile.jspa?name=reporter@reporter.com')
-        expect(subject[:web_url]).to eq('http://jira.com/jira-sub-path/browse/GL-5')
+      it 'returns URLs with the web url' do
+        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/browse/GL-5')
       end
     end
   end

ee/spec/serializers/integrations/jira/issue_serializer_spec.rb

@@ -3,10 +3,10 @@
 require 'spec_helper'
 
 RSpec.describe Integrations::Jira::IssueSerializer do
-  let(:serializer) { described_class.new }
-  let(:project) { build(:project) }
+  let_it_be(:project) { create(: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
     context 'when an empty array is being serialized' do