Fix group name conflict when migrating groups via BulkImport

Modify BulkImport to make group names unique as a namespace can not
have subgroups with the same name.

Changelog: fixed

lib/bulk_imports/groups/transformers/group_attributes_transformer.rb

@@ -7,10 +7,15 @@ class GroupAttributesTransformer
         def transform(context, data)
           import_entity = context.entity
 
+          if import_entity.destination_namespace.present?
+            namespace = Namespace.find_by_full_path(import_entity.destination_namespace)
+          end
+
           data
+            .then { |data| transform_name(import_entity, namespace, data) }
             .then { |data| transform_path(import_entity, data) }
             .then { |data| transform_full_path(data) }
-            .then { |data| transform_parent(context, import_entity, data) }
+            .then { |data| transform_parent(context, import_entity, namespace, data) }
             .then { |data| transform_visibility_level(data) }
             .then { |data| transform_project_creation_level(data) }
             .then { |data| transform_subgroup_creation_level(data) }
@@ -18,6 +23,20 @@ def transform(context, data)
 
         private
 
+        def transform_name(import_entity, namespace, data)
+          if namespace.present?
+            namespace_children_names = namespace.children.pluck(:name) # rubocop: disable CodeReuse/ActiveRecord
+
+            if namespace_children_names.include?(data['name'])
+              data['name'] = Uniquify.new(1).string(-> (counter) { "#{data['name']}(#{counter})" }) do |base|
+                namespace_children_names.include?(base)
+              end
+            end
+          end
+
+          data
+        end
+
         def transform_path(import_entity, data)
           data['path'] = import_entity.destination_name.parameterize
           data
@@ -28,11 +47,8 @@ def transform_full_path(data)
           data
         end
 
-        def transform_parent(context, import_entity, data)
-          unless import_entity.destination_namespace.blank?
-            namespace = Namespace.find_by_full_path(import_entity.destination_namespace)
-            data['parent_id'] = namespace.id
-          end
+        def transform_parent(context, import_entity, namespace, data)
+          data['parent_id'] = namespace.id if namespace.present?
 
           data
         end

spec/lib/bulk_imports/groups/transformers/group_attributes_transformer_spec.rb

@@ -4,12 +4,12 @@
 
 RSpec.describe BulkImports::Groups::Transformers::GroupAttributesTransformer do
   describe '#transform' do
-    let_it_be(:user) { create(:user) }
     let_it_be(:parent) { create(:group) }
-    let_it_be(:bulk_import) { create(:bulk_import, user: user) }
 
-    let_it_be(:entity) do
-      create(
+    let(:bulk_import) { build_stubbed(:bulk_import) }
+
+    let(:entity) do
+      build_stubbed(
         :bulk_import_entity,
         bulk_import: bulk_import,
         source_full_path: 'source/full/path',
@@ -18,8 +18,8 @@
       )
     end
 
-    let_it_be(:tracker) { create(:bulk_import_tracker, entity: entity) }
-    let_it_be(:context) { BulkImports::Pipeline::Context.new(tracker) }
+    let(:tracker) { build_stubbed(:bulk_import_tracker, entity: entity) }
+    let(:context) { BulkImports::Pipeline::Context.new(tracker) }
 
     let(:data) do
       {
@@ -87,14 +87,63 @@
       end
 
       context 'when destination namespace is empty' do
-        it 'does not set parent id' do
-          entity.update!(destination_namespace: '')
+        before do
+          entity.destination_namespace = ''
+        end
 
+        it 'does not set parent id' do
           transformed_data = subject.transform(context, data)
 
           expect(transformed_data).not_to have_key('parent_id')
         end
       end
     end
+
+    describe 'group name transformation' do
+      context 'when destination namespace is empty' do
+        before do
+          entity.destination_namespace = ''
+        end
+
+        it 'does not transform name' do
+          transformed_data = subject.transform(context, data)
+
+          expect(transformed_data['name']).to eq('Source Group Name')
+        end
+      end
+
+      context 'when destination namespace is present' do
+        context 'when destination namespace does not have a group with same name' do
+          it 'does not transform name' do
+            transformed_data = subject.transform(context, data)
+
+            expect(transformed_data['name']).to eq('Source Group Name')
+          end
+        end
+
+        context 'when destination namespace already have a group with the same name' do
+          before do
+            create(:group, parent: parent, name: 'Source Group Name', path: 'group_1')
+            create(:group, parent: parent, name: 'Source Group Name(1)', path: 'group_2')
+            create(:group, parent: parent, name: 'Source Group Name(2)', path: 'group_3')
+            create(:group, parent: parent, name: 'Source Group Name(1)(1)', path: 'group_4')
+          end
+
+          it 'makes the name unique by appeding a counter', :aggregate_failures do
+            transformed_data = subject.transform(context, data.merge('name' => 'Source Group Name'))
+            expect(transformed_data['name']).to eq('Source Group Name(3)')
+
+            transformed_data = subject.transform(context, data.merge('name' => 'Source Group Name(2)'))
+            expect(transformed_data['name']).to eq('Source Group Name(2)(1)')
+
+            transformed_data = subject.transform(context, data.merge('name' => 'Source Group Name(1)'))
+            expect(transformed_data['name']).to eq('Source Group Name(1)(2)')
+
+            transformed_data = subject.transform(context, data.merge('name' => 'Source Group Name(1)(1)'))
+            expect(transformed_data['name']).to eq('Source Group Name(1)(1)(1)')
+          end
+        end
+      end
+    end
   end
 end