Merge branch '233932-crud-group-level-integrations' into 'master'

Complete CRUD for group-level integrations

See merge request gitlab-org/gitlab!39959

app/controllers/admin/application_settings_controller.rb

@@ -32,7 +32,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
   end
 
   def integrations
-    @integrations = Service.find_or_initialize_instances.sort_by(&:title)
+    @integrations = Service.find_or_initialize_all(Service.instances).sort_by(&:title)
   end
 
   def update

app/controllers/admin/integrations_controller.rb

@@ -6,9 +6,7 @@ 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
+    Service.find_or_initialize_integration(name, instance: true)
   end
 
   def integrations_enabled?

app/controllers/groups/settings/integrations_controller.rb

@@ -8,15 +8,13 @@ module Groups
       before_action :authorize_admin_group!
 
       def index
-        @integrations = []
+        @integrations = Service.find_or_initialize_all(Service.by_group(group)).sort_by(&:title)
       end
 
       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)
+        Service.find_or_initialize_integration(name, group_id: group.id)
       end
 
       def integrations_enabled?

app/models/service.rb

@@ -63,6 +63,7 @@ class Service < ApplicationRecord
   scope :active, -> { where(active: true) }
   scope :by_type, -> (type) { where(type: type) }
   scope :by_active_flag, -> (flag) { where(active: flag) }
+  scope :by_group, -> (group) { where(group_id: group, type: available_services_types) }
   scope :templates, -> { where(template: true, type: available_services_types) }
   scope :instances, -> { where(instance: true, type: available_services_types) }
 
@@ -305,12 +306,18 @@ class Service < ApplicationRecord
     end
   end
 
-  def self.find_or_initialize_instances
-    instances + build_nonexistent_instances
+  def self.find_or_initialize_integration(name, instance: false, group_id: nil)
+    if name.in?(available_services_names)
+      "#{name}_service".camelize.constantize.find_or_initialize_by(instance: instance, group_id: group_id)
+    end
+  end
+
+  def self.find_or_initialize_all(scope)
+    scope + build_nonexistent_services_for(scope)
   end
 
-  private_class_method def self.build_nonexistent_instances
-    list_nonexistent_services_for(instances).map do |service_type|
+  private_class_method def self.build_nonexistent_services_for(scope)
+    list_nonexistent_services_for(scope).map do |service_type|
       service_type.constantize.new
     end
   end

spec/controllers/groups/settings/integrations_controller_spec.rb

@@ -3,7 +3,6 @@
 require 'spec_helper'
 
 RSpec.describe Groups::Settings::IntegrationsController do
-  let_it_be(:project) { create(:project) }
   let(:user) { create(:user) }
   let(:group) { create(:group) }
 
@@ -82,7 +81,7 @@ RSpec.describe Groups::Settings::IntegrationsController do
   end
 
   describe '#update' do
-    let(:integration) { create(:jira_service, project: project) }
+    let(:integration) { create(:jira_service, project: nil, group_id: group.id) }
 
     before do
       group.add_owner(user)

spec/models/service_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Service do
+  let_it_be(:group) { create(:group) }
+
   describe "Associations" do
     it { is_expected.to belong_to :project }
     it { is_expected.to have_one :service_hook }
@@ -13,7 +15,6 @@ RSpec.describe Service do
   describe 'validations' do
     using RSpec::Parameterized::TableSyntax
 
-    let(:group) { create(:group) }
     let(:project) { create(:project) }
 
     it { is_expected.to validate_presence_of(:type) }
@@ -91,17 +92,12 @@ RSpec.describe Service do
       end
     end
 
-    describe '#operating?' do
-      it 'is false when the service is not active' do
-        expect(build(:service).operating?).to eq(false)
-      end
-
-      it 'is false when the service is not persisted' do
-        expect(build(:service, active: true).operating?).to eq(false)
-      end
+    describe '.by_group' do
+      let!(:service1) { create(:jira_service, project_id: nil, group_id: group.id) }
+      let!(:service2) { create(:jira_service) }
 
-      it 'is true when the service is active and persisted' do
-        expect(create(:service, active: true).operating?).to eq(true)
+      it 'returns the right group service' do
+        expect(described_class.by_group(group)).to match_array([service1])
       end
     end
 
@@ -134,6 +130,20 @@ RSpec.describe Service do
     end
   end
 
+  describe '#operating?' do
+    it 'is false when the service is not active' do
+      expect(build(:service).operating?).to eq(false)
+    end
+
+    it 'is false when the service is not persisted' do
+      expect(build(:service, active: true).operating?).to eq(false)
+    end
+
+    it 'is true when the service is active and persisted' do
+      expect(create(:service, active: true).operating?).to eq(true)
+    end
+  end
+
   describe "Test Button" do
     describe '#can_test?' do
       subject { service.can_test? }
@@ -189,14 +199,27 @@ RSpec.describe Service do
     end
   end
 
-  describe '.find_or_initialize_instances' do
+  describe '.find_or_initialize_integration' do
+    let!(:service1) { create(:jira_service, project_id: nil, group_id: group.id) }
+    let!(:service2) { create(:jira_service) }
+
+    it 'returns the right service' do
+      expect(Service.find_or_initialize_integration('jira', group_id: group)).to eq(service1)
+    end
+
+    it 'does not create a new service' do
+      expect { Service.find_or_initialize_integration('redmine', group_id: group) }.not_to change { Service.count }
+    end
+  end
+
+  describe '.find_or_initialize_all' do
     shared_examples 'service instances' do
       it 'returns the available service instances' do
-        expect(Service.find_or_initialize_instances.pluck(:type)).to match_array(Service.available_services_types)
+        expect(Service.find_or_initialize_all(Service.instances).pluck(:type)).to match_array(Service.available_services_types)
       end
 
       it 'does not create service instances' do
-        expect { Service.find_or_initialize_instances }.not_to change { Service.count }
+        expect { Service.find_or_initialize_all(Service.instances) }.not_to change { Service.count }
       end
     end
 
@@ -211,9 +234,9 @@ RSpec.describe Service do
 
       it_behaves_like 'service instances'
 
-      context 'with a previous existing service (Previous) and a new service (Asana)' do
+      context 'with a previous existing service (MockCiService) and a new service (Asana)' do
         before do
-          Service.insert(type: 'PreviousService', instance: true)
+          Service.insert(type: 'MockCiService', instance: true)
           Service.delete_by(type: 'AsanaService', instance: true)
         end