Merge branch '255923-transfer-project-group-with-integrations' into 'master'

Transfer a project/group to a new namespace inheriting integrations

See merge request gitlab-org/gitlab!48621

app/models/service.rb

@@ -70,6 +70,7 @@ class Service < ApplicationRecord
   scope :by_type, -> (type) { where(type: type) }
   scope :by_active_flag, -> (flag) { where(active: flag) }
   scope :inherit_from_id, -> (id) { where(inherit_from_id: id) }
+  scope :inherit, -> { where.not(inherit_from_id: nil) }
   scope :for_group, -> (group) { where(group_id: group, type: available_services_types(include_project_specific: false)) }
   scope :for_template, -> { where(template: true, type: available_services_types(include_project_specific: false)) }
   scope :for_instance, -> { where(instance: true, type: available_services_types(include_project_specific: false)) }
@@ -278,7 +279,7 @@ class Service < ApplicationRecord
       active.where(instance: true),
       active.where(group_id: group_ids, inherit_from_id: nil)
     ]).order(Arel.sql("type ASC, array_position(#{array}::bigint[], services.group_id), instance DESC")).group_by(&:type).each do |type, records|
-      build_from_integration(records.first, association => scope.id).save!
+      build_from_integration(records.first, association => scope.id).save
     end
   end
 

app/services/groups/transfer_service.rb

@@ -28,9 +28,11 @@ module Groups
       Group.transaction do
         update_group_attributes
         ensure_ownership
+        update_integrations
       end
 
       post_update_hooks(@updated_project_ids)
+      propagate_integrations
 
       true
     end
@@ -196,6 +198,17 @@ module Groups
         raise TransferError, result[:message] unless result[:status] == :success
       end
     end
+
+    def update_integrations
+      @group.services.inherit.delete_all
+      Service.create_from_active_default_integrations(@group, :group_id)
+    end
+
+    def propagate_integrations
+      @group.services.inherit.each do |integration|
+        PropagateIntegrationWorker.perform_async(integration.id)
+      end
+    end
   end
 end
 

app/services/projects/transfer_service.rb

@@ -59,7 +59,7 @@ module Projects
         raise TransferError.new(s_("TransferProject|Root namespace can't be updated if project has NPM packages"))
       end
 
-      attempt_transfer_transaction
+      proceed_to_transfer
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
@@ -67,7 +67,7 @@ module Projects
       new_namespace.root_ancestor == project.namespace.root_ancestor
     end
 
-    def attempt_transfer_transaction
+    def proceed_to_transfer
       Project.transaction do
         project.expire_caches_before_rename(@old_path)
 
@@ -87,6 +87,8 @@ module Projects
         # Move uploads
         move_project_uploads(project)
 
+        update_integrations
+
         project.old_path_with_namespace = @old_path
 
         update_repository_configuration(@new_path)
@@ -214,6 +216,11 @@ module Projects
         project.shared_runners_enabled = false
       end
     end
+
+    def update_integrations
+      project.services.inherit.delete_all
+      Service.create_from_active_default_integrations(project, :project_id)
+    end
   end
 end
 

changelogs/unreleased/255923-transfer-project-group-with-integrations.yml

+---
+title: Transfer a project/group to a new namespace inheriting integrations
+merge_request: 48621
+author:
+type: changed

spec/services/groups/transfer_service_spec.rb

@@ -3,15 +3,15 @@
 require 'spec_helper'
 
 RSpec.describe Groups::TransferService do
-  let(:user) { create(:user) }
-  let(:new_parent_group) { create(:group, :public) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:new_parent_group) { create(:group, :public) }
   let!(:group_member) { create(:group_member, :owner, group: group, user: user) }
   let(:transfer_service) { described_class.new(group, user) }
 
   context 'handling packages' do
     let_it_be(:group) { create(:group, :public) }
+    let_it_be(:new_group) { create(:group, :public) }
     let(:project) { create(:project, :public, namespace: group) }
-    let(:new_group) { create(:group, :public) }
 
     before do
       group.add_owner(user)
@@ -35,8 +35,8 @@ RSpec.describe Groups::TransferService do
       it_behaves_like 'transfer not allowed'
 
       context 'with a project within subgroup' do
-        let(:root_group) { create(:group) }
-        let(:group) { create(:group, parent: root_group) }
+        let_it_be(:root_group) { create(:group) }
+        let_it_be(:group) { create(:group, parent: root_group) }
 
         before do
           root_group.add_owner(user)
@@ -79,8 +79,6 @@ RSpec.describe Groups::TransferService do
 
   shared_examples 'ensuring allowed transfer for a group' do
     context "when there's an exception on GitLab shell directories" do
-      let(:new_parent_group) { create(:group, :public) }
-
       before do
         allow_next_instance_of(described_class) do |instance|
           allow(instance).to receive(:update_group_attributes).and_raise(Gitlab::UpdatePathError, 'namespace directory cannot be moved')
@@ -101,7 +99,7 @@ RSpec.describe Groups::TransferService do
 
   describe '#execute' do
     context 'when transforming a group into a root group' do
-      let!(:group) { create(:group, :public, :nested) }
+      let_it_be_with_reload(:group) { create(:group, :public, :nested) }
 
       it_behaves_like 'ensuring allowed transfer for a group'
 
@@ -115,7 +113,7 @@ RSpec.describe Groups::TransferService do
       end
 
       context 'when the user does not have the right policies' do
-        let!(:group_member) { create(:group_member, :guest, group: group, user: user) }
+        let_it_be(:group_member) { create(:group_member, :guest, group: group, user: user) }
 
         it "returns false" do
           expect(transfer_service.execute(nil)).to be_falsy
@@ -128,7 +126,7 @@ RSpec.describe Groups::TransferService do
       end
 
       context 'when there is a group with the same path' do
-        let!(:group) { create(:group, :public, :nested, path: 'not-unique') }
+        let_it_be(:group) { create(:group, :public, :nested, path: 'not-unique') }
 
         before do
           create(:group, path: 'not-unique')
@@ -145,9 +143,9 @@ RSpec.describe Groups::TransferService do
       end
 
       context 'when the group is a subgroup and the transfer is valid' do
-        let!(:subgroup1) { create(:group, :private, parent: group) }
-        let!(:subgroup2) { create(:group, :internal, parent: group) }
-        let!(:project1) { create(:project, :repository, :private, namespace: group) }
+        let_it_be(:subgroup1) { create(:group, :private, parent: group) }
+        let_it_be(:subgroup2) { create(:group, :internal, parent: group) }
+        let_it_be(:project1) { create(:project, :repository, :private, namespace: group) }
 
         before do
           transfer_service.execute(nil)
@@ -173,12 +171,12 @@ RSpec.describe Groups::TransferService do
     end
 
     context 'when transferring a subgroup into another group' do
-      let(:group) { create(:group, :public, :nested) }
+      let_it_be_with_reload(:group) { create(:group, :public, :nested) }
 
       it_behaves_like 'ensuring allowed transfer for a group'
 
       context 'when the new parent group is the same as the previous parent group' do
-        let(:group) { create(:group, :public, :nested, parent: new_parent_group) }
+        let_it_be(:group) { create(:group, :public, :nested, parent: new_parent_group) }
 
         it 'returns false' do
           expect(transfer_service.execute(new_parent_group)).to be_falsy
@@ -191,7 +189,7 @@ RSpec.describe Groups::TransferService do
       end
 
       context 'when the user does not have the right policies' do
-        let!(:group_member) { create(:group_member, :guest, group: group, user: user) }
+        let_it_be(:group_member) { create(:group_member, :guest, group: group, user: user) }
 
         it "returns false" do
           expect(transfer_service.execute(new_parent_group)).to be_falsy
@@ -221,7 +219,7 @@ RSpec.describe Groups::TransferService do
       end
 
       context 'when the parent group has a project with the same path' do
-        let!(:group) { create(:group, :public, :nested, path: 'foo') }
+        let_it_be_with_reload(:group) { create(:group, :public, :nested, path: 'foo') }
 
         before do
           create(:group_member, :owner, group: new_parent_group, user: user)
@@ -240,8 +238,13 @@ RSpec.describe Groups::TransferService do
       end
 
       context 'when the group is allowed to be transferred' do
+        let_it_be(:new_parent_group_integration) { create(:slack_service, group: new_parent_group, project: nil, webhook: 'http://new-group.slack.com') }
+
         before do
+          allow(PropagateIntegrationWorker).to receive(:perform_async)
+
           create(:group_member, :owner, group: new_parent_group, user: user)
+
           transfer_service.execute(new_parent_group)
         end
 
@@ -267,6 +270,30 @@ RSpec.describe Groups::TransferService do
           end
         end
 
+        context 'with a group integration' do
+          let_it_be(:instance_integration) { create(:slack_service, :instance, webhook: 'http://project.slack.com') }
+          let(:new_created_integration) { Service.find_by(group: group) }
+
+          context 'with an inherited integration' do
+            let_it_be(:group_integration) { create(:slack_service, group: group, project: nil, webhook: 'http://group.slack.com', inherit_from_id: instance_integration.id) }
+
+            it 'replaces inherited integrations', :aggregate_failures do
+              expect(new_created_integration.webhook).to eq(new_parent_group_integration.webhook)
+              expect(PropagateIntegrationWorker).to have_received(:perform_async).with(new_created_integration.id)
+              expect(Service.count).to eq(3)
+            end
+          end
+
+          context 'with a custom integration' do
+            let_it_be(:group_integration) { create(:slack_service, group: group, project: nil, webhook: 'http://group.slack.com') }
+
+            it 'does not updates the integrations', :aggregate_failures do
+              expect { transfer_service.execute(new_parent_group) }.not_to change { group_integration.webhook }
+              expect(PropagateIntegrationWorker).not_to have_received(:perform_async)
+            end
+          end
+        end
+
         it 'updates visibility for the group based on the parent group' do
           expect(group.visibility_level).to eq(new_parent_group.visibility_level)
         end
@@ -464,7 +491,7 @@ RSpec.describe Groups::TransferService do
         end
 
         context 'updated paths' do
-          let(:group) { create(:group, :public) }
+          let_it_be_with_reload(:group) { create(:group, :public) }
 
           before do
             transfer_service.execute(new_parent_group)
@@ -500,10 +527,10 @@ RSpec.describe Groups::TransferService do
         end
 
         context 'resets project authorizations' do
-          let(:old_parent_group) { create(:group) }
-          let(:group) { create(:group, :private, parent: old_parent_group) }
-          let(:new_group_member) { create(:user) }
-          let(:old_group_member) { create(:user) }
+          let_it_be(:old_parent_group) { create(:group) }
+          let_it_be_with_reload(:group) { create(:group, :private, parent: old_parent_group) }
+          let_it_be(:new_group_member) { create(:user) }
+          let_it_be(:old_group_member) { create(:user) }
 
           before do
             new_parent_group.add_maintainer(new_group_member)

spec/services/projects/transfer_service_spec.rb

@@ -7,6 +7,7 @@ RSpec.describe Projects::TransferService do
 
   let_it_be(:user) { create(:user) }
   let_it_be(:group) { create(:group) }
+  let_it_be(:group_integration) { create(:slack_service, group: group, project: nil, webhook: 'http://group.slack.com') }
   let(:project) { create(:project, :repository, :legacy_storage, namespace: user.namespace) }
 
   subject(:execute_transfer) { described_class.new(project, user).execute(group).tap { project.reload } }
@@ -117,6 +118,30 @@ RSpec.describe Projects::TransferService do
         shard_name: project.repository_storage
       )
     end
+
+    context 'with a project integration' do
+      let_it_be_with_reload(:project) { create(:project, namespace: user.namespace) }
+      let_it_be(:instance_integration) { create(:slack_service, :instance, webhook: 'http://project.slack.com') }
+
+      context 'with an inherited integration' do
+        let_it_be(:project_integration) { create(:slack_service, project: project, webhook: 'http://project.slack.com', inherit_from_id: instance_integration.id) }
+
+        it 'replaces inherited integrations', :aggregate_failures do
+          execute_transfer
+
+          expect(project.slack_service.webhook).to eq(group_integration.webhook)
+          expect(Service.count).to eq(3)
+        end
+      end
+
+      context 'with a custom integration' do
+        let_it_be(:project_integration) { create(:slack_service, project: project, webhook: 'http://project.slack.com') }
+
+        it 'does not updates the integrations' do
+          expect { execute_transfer }.not_to change { project.slack_service.webhook }
+        end
+      end
+    end
   end
 
   context 'when transfer fails' do
@@ -527,7 +552,7 @@ RSpec.describe Projects::TransferService do
       group.add_owner(user)
     end
 
-    it 'schedules a job  when pages are deployed' do
+    it 'schedules a job when pages are deployed' do
       project.mark_pages_as_deployed
 
       expect(PagesTransferWorker).to receive(:perform_async)