Merge branch 'instance-level-integrations' into 'master'

Find or initialize instance-level integration

See merge request gitlab-org/gitlab!28592

app/controllers/admin/integrations_controller.rb

@@ -5,6 +5,12 @@ class Admin::IntegrationsController < Admin::ApplicationController
 
   private
 
+  def find_or_initialize_integration(name)
+    if name.in?(Service.available_services_names)
+      "#{name}_service".camelize.constantize.find_or_initialize_by(instance: true) # rubocop:disable CodeReuse/ActiveRecord
+    end
+  end
+
   def integrations_enabled?
     Feature.enabled?(:instance_level_integrations)
   end

app/controllers/concerns/integrations_actions.rb

@@ -37,11 +37,7 @@ module IntegrationsActions
   end
 
   def test
-    if integration.can_test?
-      render json: service_test_response, status: :ok
-    else
-      render json: {}, status: :not_found
-    end
+    render json: {}, status: :ok
   end
 
   private
@@ -50,17 +46,11 @@ module IntegrationsActions
     false
   end
 
-  # TODO: Use actual integrations on the group / instance level
-  # To be completed in https://gitlab.com/groups/gitlab-org/-/epics/2430
-  def project
-    Project.first
-  end
-
   def integration
     # Using instance variable `@service` still required as it's used in ServiceParams
     # and app/views/shared/_service_settings.html.haml. Should be removed once
     # those 2 are refactored to use `@integration`.
-    @integration = @service ||= project.find_or_initialize_service(params[:id]) # rubocop:disable Gitlab/ModuleWithInstanceVariables
+    @integration = @service ||= find_or_initialize_integration(params[:id]) # rubocop:disable Gitlab/ModuleWithInstanceVariables
   end
 
   def success_message
@@ -74,21 +64,4 @@ module IntegrationsActions
       .as_json(only: integration.json_fields)
       .merge(errors: integration.errors.as_json)
   end
-
-  def service_test_response
-    unless integration.update(service_params[:service])
-      return { error: true, message: _('Validations failed.'), service_response: integration.errors.full_messages.join(','), test_failed: false }
-    end
-
-    data = integration.test_data(project, current_user)
-    outcome = integration.test(data)
-
-    unless outcome[:success]
-      return { error: true, message: _('Test failed.'), service_response: outcome[:result].to_s, test_failed: true }
-    end
-
-    {}
-  rescue Gitlab::HTTP::BlockedUrlError => e
-    { error: true, message: _('Test failed.'), service_response: e.message, test_failed: true }
-  end
 end

app/controllers/groups/settings/integrations_controller.rb

@@ -9,6 +9,12 @@ module Groups
 
       private
 
+      # TODO: Make this compatible with group-level integration
+      # https://gitlab.com/groups/gitlab-org/-/epics/2543
+      def find_or_initialize_integration(name)
+        Project.first.find_or_initialize_service(name)
+      end
+
       def integrations_enabled?
         Feature.enabled?(:group_level_integrations, group)
       end

app/models/project_services/jira_service.rb

@@ -201,8 +201,7 @@ class JiraService < IssueTrackerService
   end
 
   # Jira does not need test data.
-  # We are requesting the project that belongs to the project key.
-  def test_data(user = nil, project = nil)
+  def test_data(_, _)
     nil
   end
 
@@ -221,7 +220,6 @@ class JiraService < IssueTrackerService
   def test_settings
     return unless client_url.present?
 
-    # Test settings by getting the project
     jira_request { client.ServerInfo.all.attrs }
   end
 

app/models/project_services/pipelines_email_service.rb

@@ -53,7 +53,7 @@ class PipelinesEmailService < Service
   end
 
   def can_test?
-    project.ci_pipelines.any?
+    project&.ci_pipelines&.any?
   end
 
   def test_data(project, user)

app/models/service.rb

@@ -184,8 +184,10 @@ class Service < ApplicationRecord
     { success: result.present?, result: result }
   end
 
+  # Disable test for instance-level services.
+  # https://gitlab.com/gitlab-org/gitlab/-/issues/213138
   def can_test?
-    true
+    !instance?
   end
 
   # Provide convenient accessor methods

ee/app/models/project_services/github_service.rb

@@ -60,7 +60,7 @@ class GithubService < Service
   end
 
   def can_test?
-    project.ci_pipelines.any?
+    project&.ci_pipelines&.any?
   end
 
   def disabled_title

spec/controllers/admin/integrations_controller_spec.rb

@@ -3,7 +3,6 @@
 require 'spec_helper'
 
 describe Admin::IntegrationsController do
-  let_it_be(:project) { create(:project) }
   let(:admin) { create(:admin) }
 
   before do
@@ -34,7 +33,7 @@ describe Admin::IntegrationsController do
   end
 
   describe '#update' do
-    let(:integration) { create(:jira_service, project: project) }
+    let(:integration) { create(:jira_service, :instance) }
 
     before do
       put :update, params: { id: integration.class.to_param, service: { url: url } }
@@ -52,34 +51,9 @@ describe Admin::IntegrationsController do
     context 'invalid params' do
       let(:url) { 'https://jira.localhost' }
 
-      it 'does not update the integration' do
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(response).to render_template(:edit)
-        expect(integration.reload.url).not_to eq(url)
-      end
-    end
-  end
-
-  describe '#test' do
-    context 'testable' do
-      let(:integration) { create(:jira_service, project: project) }
-
-      it 'returns ok' do
-        allow_any_instance_of(integration.class).to receive(:test) { { success: true } }
-
-        put :test, params: { id: integration.class.to_param }
-
-        expect(response).to have_gitlab_http_status(:ok)
-      end
-    end
-
-    context 'not testable' do
-      let(:integration) { create(:alerts_service, project: project) }
-
-      it 'returns not found' do
-        put :test, params: { id: integration.class.to_param }
-
-        expect(response).to have_gitlab_http_status(:not_found)
+      it 'updates the integration' do
+        expect(response).to have_gitlab_http_status(:found)
+        expect(integration.reload.url).to eq(url)
       end
     end
   end

spec/controllers/groups/settings/integrations_controller_spec.rb

@@ -67,40 +67,11 @@ describe Groups::Settings::IntegrationsController do
     end
 
     context 'invalid params' do
-      let(:url) { 'ftp://jira.localhost' }
+      let(:url) { 'https://jira.localhost' }
 
       it 'does not update the integration' do
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(response).to render_template(:edit)
-        expect(integration.reload.url).not_to eq(url)
-      end
-    end
-  end
-
-  describe '#test' do
-    context 'testable' do
-      let(:integration) { create(:jira_service, project: project) }
-
-      before do
-        group.add_owner(user)
-      end
-
-      it 'returns ok' do
-        allow_any_instance_of(integration.class).to receive(:test) { { success: true } }
-
-        put :test, params: { group_id: group, id: integration.class.to_param }
-
-        expect(response).to have_gitlab_http_status(:ok)
-      end
-    end
-
-    context 'not testable' do
-      let(:integration) { create(:alerts_service, project: project) }
-
-      it 'returns not found' do
-        put :test, params: { group_id: group, id: integration.class.to_param }
-
-        expect(response).to have_gitlab_http_status(:not_found)
+        expect(response).to have_gitlab_http_status(:found)
+        expect(integration.reload.url).to eq(url)
       end
     end
   end

spec/models/service_spec.rb

@@ -104,21 +104,29 @@ describe Service do
 
   describe "Test Button" do
     describe '#can_test?' do
+      subject { service.can_test? }
+
       let(:service) { create(:service, project: project) }
 
       context 'when repository is not empty' do
         let(:project) { create(:project, :repository) }
 
-        it 'returns true' do
-          expect(service.can_test?).to be true
-        end
+        it { is_expected.to be true }
       end
 
       context 'when repository is empty' do
         let(:project) { create(:project) }
 
-        it 'returns true' do
-          expect(service.can_test?).to be true
+        it { is_expected.to be true }
+      end
+
+      context 'when instance-level service' do
+        Service.available_services_types.each do |service_type|
+          let(:service) do
+            service_type.constantize.new(instance: true)
+          end
+
+          it { is_expected.to be_falsey }
         end
       end
     end