Merge branch '229223-save-jira-server-type-on-test' into 'master'

Store Jira server type in `jira_tracking_data` table

Closes #229223

See merge request gitlab-org/gitlab!37003

app/models/project_services/jira_service.rb

@@ -5,6 +5,7 @@ class JiraService < IssueTrackerService
   include Gitlab::Routing
   include ApplicationHelper
   include ActionView::Helpers::AssetUrlHelper
+  include Gitlab::Utils::StrongMemoize
 
   PROJECTS_PER_PAGE = 50
 
@@ -32,6 +33,7 @@ class JiraService < IssueTrackerService
   data_field :username, :password, :url, :api_url, :jira_issue_transition_id, :project_key, :issues_enabled
 
   before_update :reset_password
+  after_commit :update_deployment_type, on: [:create, :update], if: :update_deployment_type?
 
   enum comment_detail: {
     standard: 1,
@@ -212,7 +214,7 @@ class JiraService < IssueTrackerService
   end
 
   def test(_)
-    result = test_settings
+    result = server_info
     success = result.present?
     result = @error&.message unless success
 
@@ -231,10 +233,10 @@ class JiraService < IssueTrackerService
 
   private
 
-  def test_settings
-    return unless client_url.present?
-
-    jira_request { client.ServerInfo.all.attrs }
+  def server_info
+    strong_memoize(:server_info) do
+      client_url.present? ? jira_request { client.ServerInfo.all.attrs } : nil
+    end
   end
 
   def can_cross_reference?(noteable)
@@ -436,6 +438,25 @@ class JiraService < IssueTrackerService
     url_changed?
   end
 
+  def update_deployment_type?
+    api_url_changed? || url_changed? || username_changed? || password_changed?
+  end
+
+  def update_deployment_type
+    clear_memoization(:server_info) # ensure we run the request when we try to update deployment type
+    results = server_info
+    return data_fields.deployment_unknown! unless results.present?
+
+    case results['deploymentType']
+    when 'Server'
+      data_fields.deployment_server!
+    when 'Cloud'
+      data_fields.deployment_cloud!
+    else
+      data_fields.deployment_unknown!
+    end
+  end
+
   def self.event_description(event)
     case event
     when "merge_request", "merge_request_events"

changelogs/unreleased/229223-save-jira-server-type-on-test.yml

+---
+title: Store deployment_type of Jira server in jira_tracker_data table
+merge_request: 37003
+author:
+type: changed

spec/controllers/admin/integrations_controller_spec.rb

@@ -23,9 +23,12 @@ RSpec.describe Admin::IntegrationsController do
   end
 
   describe '#update' do
+    include JiraServiceHelper
+
     let(:integration) { create(:jira_service, :instance) }
 
     before do
+      stub_jira_service_test
       allow(PropagateIntegrationWorker).to receive(:perform_async)
 
       put :update, params: { id: integration.class.to_param, service: { url: url } }

spec/controllers/groups/settings/integrations_controller_spec.rb

@@ -81,10 +81,13 @@ RSpec.describe Groups::Settings::IntegrationsController do
   end
 
   describe '#update' do
+    include JiraServiceHelper
+
     let(:integration) { create(:jira_service, project: nil, group_id: group.id) }
 
     before do
       group.add_owner(user)
+      stub_jira_service_test
 
       put :update, params: { group_id: group, id: integration.class.to_param, service: { url: url } }
     end

spec/controllers/projects/services_controller_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Projects::ServicesController do
+  include JiraServiceHelper
+
   let(:project) { create(:project, :repository) }
   let(:user)    { create(:user) }
   let(:service) { create(:jira_service, project: project) }
@@ -54,8 +56,7 @@ RSpec.describe Projects::ServicesController do
         end
 
         it 'returns success' do
-          stub_request(:get, 'http://example.com/rest/api/2/serverInfo')
-            .to_return(status: 200, body: '{}')
+          stub_jira_service_test
 
           expect(Gitlab::HTTP).to receive(:get).with('/rest/api/2/serverInfo', any_args).and_call_original
 
@@ -66,8 +67,7 @@ RSpec.describe Projects::ServicesController do
       end
 
       it 'returns success' do
-        stub_request(:get, 'http://example.com/rest/api/2/serverInfo')
-          .to_return(status: 200, body: '{}')
+        stub_jira_service_test
 
         expect(Gitlab::HTTP).to receive(:get).with('/rest/api/2/serverInfo', any_args).and_call_original
 
@@ -200,6 +200,7 @@ RSpec.describe Projects::ServicesController do
 
     describe 'as JSON' do
       before do
+        stub_jira_service_test
         put :update, params: project_params(service: service_params, format: :json)
       end
 

spec/features/projects/services/user_activates_jira_spec.rb

@@ -61,7 +61,10 @@ RSpec.describe 'User activates Jira', :js do
   end
 
   describe 'user disables the Jira Service' do
+    include JiraServiceHelper
+
     before do
+      stub_jira_service_test
       visit_project_integration('Jira')
       fill_form(disable: true)
       click_button('Save changes')

spec/models/project_services/jira_service_spec.rb

@@ -10,6 +10,11 @@ RSpec.describe JiraService do
   let(:username) { 'jira-username' }
   let(:password) { 'jira-password' }
   let(:transition_id) { 'test27' }
+  let(:server_info_results) { { 'deploymentType' => 'Cloud' } }
+
+  before do
+    WebMock.stub_request(:get, /serverInfo/).to_return(body: server_info_results.to_json )
+  end
 
   describe '#options' do
     let(:options) do
@@ -103,7 +108,7 @@ RSpec.describe JiraService do
       expect(subject.properties).to be_nil
     end
 
-    it 'stores data in data_fields correcty' do
+    it 'stores data in data_fields correctly' do
       service = subject
 
       expect(service.jira_tracker_data.url).to eq(url)
@@ -111,6 +116,35 @@ RSpec.describe JiraService do
       expect(service.jira_tracker_data.username).to eq(username)
       expect(service.jira_tracker_data.password).to eq(password)
       expect(service.jira_tracker_data.jira_issue_transition_id).to eq(transition_id)
+      expect(service.jira_tracker_data.deployment_cloud?).to be_truthy
+    end
+
+    context 'when loading serverInfo' do
+      let!(:jira_service) { subject }
+
+      context 'Cloud instance' do
+        let(:server_info_results) { { 'deploymentType' => 'Cloud' } }
+
+        it 'is detected' do
+          expect(jira_service.jira_tracker_data.deployment_cloud?).to be_truthy
+        end
+      end
+
+      context 'Server instance' do
+        let(:server_info_results) { { 'deploymentType' => 'Server' } }
+
+        it 'is detected' do
+          expect(jira_service.jira_tracker_data.deployment_server?).to be_truthy
+        end
+      end
+
+      context 'Unknown instance' do
+        let(:server_info_results) { { 'deploymentType' => 'FutureCloud' } }
+
+        it 'is detected' do
+          expect(jira_service.jira_tracker_data.deployment_unknown?).to be_truthy
+        end
+      end
     end
   end
 
@@ -151,8 +185,8 @@ RSpec.describe JiraService do
 
       describe '#update' do
         context 'basic update' do
-          let(:new_username) { 'new_username' }
-          let(:new_url) { 'http://jira-new.example.com' }
+          let_it_be(:new_username) { 'new_username' }
+          let_it_be(:new_url) { 'http://jira-new.example.com' }
 
           before do
             service.update(username: new_username, url: new_url)
@@ -173,6 +207,53 @@ RSpec.describe JiraService do
           end
         end
 
+        context 'when updating the url, api_url, username, or password' do
+          it 'updates deployment type' do
+            service.update(url: 'http://first.url')
+            service.jira_tracker_data.update(deployment_type: 'server')
+
+            expect(service.jira_tracker_data.deployment_server?).to be_truthy
+
+            service.update(api_url: 'http://another.url')
+            service.jira_tracker_data.reload
+
+            expect(service.jira_tracker_data.deployment_cloud?).to be_truthy
+            expect(WebMock).to have_requested(:get, /serverInfo/).twice
+          end
+
+          it 'calls serverInfo for url' do
+            service.update(url: 'http://first.url')
+
+            expect(WebMock).to have_requested(:get, /serverInfo/)
+          end
+
+          it 'calls serverInfo for api_url' do
+            service.update(api_url: 'http://another.url')
+
+            expect(WebMock).to have_requested(:get, /serverInfo/)
+          end
+
+          it 'calls serverInfo for username' do
+            service.update(username: 'test-user')
+
+            expect(WebMock).to have_requested(:get, /serverInfo/)
+          end
+
+          it 'calls serverInfo for password' do
+            service.update(password: 'test-password')
+
+            expect(WebMock).to have_requested(:get, /serverInfo/)
+          end
+        end
+
+        context 'when not updating the url, api_url, username, or password' do
+          it 'does not update deployment type' do
+            service.update(jira_issue_transition_id: 'jira_issue_transition_id')
+
+            expect(WebMock).not_to have_requested(:get, /serverInfo/)
+          end
+        end
+
         context 'stored password invalidation' do
           context 'when a password was previously set' do
             context 'when only web url present' do
@@ -627,6 +708,7 @@ RSpec.describe JiraService do
   end
 
   describe '#test' do
+    let(:server_info_results) { { 'url' => 'http://url', 'deploymentType' => 'Cloud' } }
     let(:jira_service) do
       described_class.new(
         url: url,
@@ -635,24 +717,21 @@ RSpec.describe JiraService do
       )
     end
 
-    def test_settings(url = 'jira.example.com')
-      test_url = "http://#{url}/rest/api/2/serverInfo"
-
-      WebMock.stub_request(:get, test_url).with(basic_auth: [username, password])
-        .to_return(body: { url: 'http://url' }.to_json )
-
+    def server_info
       jira_service.test(nil)
     end
 
     context 'when the test succeeds' do
       it 'gets Jira project with URL when API URL not set' do
-        expect(test_settings).to eq(success: true, result: { 'url' => 'http://url' })
+        expect(server_info).to eq(success: true, result: server_info_results)
+        expect(WebMock).to have_requested(:get, /jira.example.com/)
       end
 
       it 'gets Jira project with API URL if set' do
         jira_service.update(api_url: 'http://jira.api.com')
 
-        expect(test_settings('jira.api.com')).to eq(success: true, result: { 'url' => 'http://url' })
+        expect(server_info).to eq(success: true, result: server_info_results)
+        expect(WebMock).to have_requested(:get, /jira.api.com/)
       end
     end
 

spec/services/admin/propagate_integration_service_spec.rb

@@ -4,6 +4,12 @@ require 'spec_helper'
 
 RSpec.describe Admin::PropagateIntegrationService do
   describe '.propagate' do
+    include JiraServiceHelper
+
+    before do
+      stub_jira_service_test
+    end
+
     let(:excluded_attributes) { %w[id project_id inherit_from_id instance created_at updated_at default] }
     let!(:project) { create(:project) }
     let!(:instance_integration) do

spec/services/git/branch_push_service_spec.rb

@@ -416,6 +416,7 @@ RSpec.describe Git::BranchPushService, services: true do
       before do
         # project.create_jira_service doesn't seem to invalidate the cache here
         project.has_external_issue_tracker = true
+        stub_jira_service_test
         jira_service_settings
         stub_jira_urls("JIRA-1")
 

spec/services/merge_requests/merge_service_spec.rb

@@ -152,6 +152,7 @@ RSpec.describe MergeRequests::MergeService do
         let(:commit)       { double('commit', safe_message: "Fixes #{jira_issue.to_reference}") }
 
         before do
+          stub_jira_service_test
           project.update!(has_external_issue_tracker: true)
           jira_service_settings
           stub_jira_urls(jira_issue.id)

spec/services/projects/propagate_service_template_spec.rb

@@ -79,7 +79,11 @@ RSpec.describe Projects::PropagateServiceTemplate do
     end
 
     context 'service with data fields' do
+      include JiraServiceHelper
+
       let(:service_template) do
+        stub_jira_service_test
+
         JiraService.create!(
           template: true,
           active: true,

spec/services/system_note_service_spec.rb

@@ -347,6 +347,7 @@ RSpec.describe SystemNoteService do
     let(:success_message) { "SUCCESS: Successfully posted to http://jira.example.net." }
 
     before do
+      stub_jira_service_test
       stub_jira_urls(jira_issue.id)
       jira_service_settings
     end

spec/support/helpers/jira_service_helper.rb

@@ -78,8 +78,7 @@ module JiraServiceHelper
   end
 
   def stub_jira_service_test
-    WebMock.stub_request(:get, 'https://jira.example.com/rest/api/2/serverInfo')
-      .to_return(body: { url: 'http://url' }.to_json)
+    WebMock.stub_request(:get, /serverInfo/).to_return(body: { url: 'http://url' }.to_json)
   end
 
   def stub_jira_urls(issue_id)

spec/support/shared_contexts/services_shared_context.rb

@@ -2,6 +2,8 @@
 
 Service.available_services_names.each do |service|
   RSpec.shared_context service do
+    include JiraServiceHelper if service == 'jira'
+
     let(:dashed_service) { service.dasherize }
     let(:service_method) { "#{service}_service".to_sym }
     let(:service_klass) { "#{service}_service".classify.constantize }
@@ -39,6 +41,7 @@ Service.available_services_names.each do |service|
 
     before do
       enable_license_for_service(service)
+      stub_jira_service_test if service == 'jira'
     end
 
     def initialize_service(service)