Commit ab81251d authored by Arturo Herrero's avatar Arturo Herrero

Find or initialize instance-level integration

https://gitlab.com/gitlab-org/gitlab/-/merge_requests/26454 implemented
the basic feature under a feature flag, this implements the backend
changes.

This is similar to the service templates feature that we are going to
remove https://gitlab.com/groups/gitlab-org/-/epics/2430. As for service
templates, we are not using the testing settings functionality because
is couple to project that we don't have in this context.

We will enable the testing settings functionality one-by-one
https://gitlab.com/gitlab-org/gitlab/-/issues/213138.
parent e68bf142
......@@ -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
......
......@@ -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
......@@ -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
......
......@@ -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
......
......@@ -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)
......
......@@ -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? ? false : true
end
# Provide convenient accessor methods
......
......@@ -60,7 +60,7 @@ class GithubService < Service
end
def can_test?
project.ci_pipelines.any?
project&.ci_pipelines&.any?
end
def disabled_title
......
......@@ -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
......
......@@ -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
......
......@@ -121,6 +121,18 @@ describe Service do
expect(service.can_test?).to be true
end
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 'returns false' do
expect(service.can_test?).to be_falsey
end
end
end
end
describe '#test' 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