Migrate creation of nested groups into a service

app/services/groups/nested_create_service.rb

+module Groups
+  class NestedCreateService < Groups::BaseService
+    attr_reader :group_path
+
+    def initialize(user, params)
+      @current_user, @params = user, params.dup
+
+      @group_path = @params.delete(:group_path)
+    end
+
+    def execute
+      return nil unless group_path
+
+      if group = Group.find_by_full_path(group_path)
+        return group
+      end
+
+      create_group_path
+    end
+
+    private
+
+    def create_group_path
+      group_path_segments = group_path.split('/')
+
+      last_group = nil
+      partial_path_segments = []
+      while subgroup_name = group_path_segments.shift
+        partial_path_segments << subgroup_name
+        partial_path = partial_path_segments.join('/')
+
+        new_params = params.reverse_merge(
+          path: subgroup_name,
+          name: subgroup_name,
+          parent: last_group
+        )
+        new_params[:visibility_level] ||= Gitlab::CurrentSettings.current_application_settings.default_group_visibility
+
+        last_group = Group.find_by_full_path(partial_path) || Groups::CreateService.new(current_user, new_params).execute
+      end
+
+      last_group
+    end
+  end
+end

lib/gitlab/bare_repository_importer.rb

@@ -80,39 +80,8 @@ module Gitlab
         return namespace
       end
 
-      create_group_path
-    end
-
-    def create_group_path
-      group_path_segments = group_path.split('/')
-
-      new_group, parent_group = nil
-      partial_path_segments = []
-      while subgroup_name = group_path_segments.shift
-        partial_path_segments << subgroup_name
-        partial_path = partial_path_segments.join('/')
-
-        unless new_group = Group.find_by_full_path(partial_path)
-          log " * Creating group #{partial_path}.".color(:green)
-          params = {
-            path: subgroup_name,
-            name: subgroup_name,
-            parent: parent_group,
-            visibility_level: Gitlab::CurrentSettings.current_application_settings.default_group_visibility
-          }
-          new_group = Groups::CreateService.new(user, params).execute
-        end
-
-        if new_group.persisted?
-          log " * Group #{partial_path} (#{new_group.id}) available".color(:green)
-        else
-          log " * Failed trying to create group #{partial_path}.".color(:red)
-          log " * Errors:  #{new_group.errors.messages}".color(:red)
-        end
-        parent_group = new_group
-      end
-
-      new_group
+      log " * Creating Group: #{group_path}"
+      Groups::NestedCreateService.new(user, group_path: group_path).execute
     end
 
     # This is called from within a rake task only used by Admins, so allow writing

lib/tasks/import.rake

@@ -72,23 +72,7 @@ class GithubImport
     return @current_user.namespace if names == @current_user.namespace_path
     return @current_user.namespace unless @current_user.can_create_group?
 
-    full_path_namespace = Namespace.find_by_full_path(names)
-
-    return full_path_namespace if full_path_namespace
-
-    names.split('/').inject(nil) do |parent, name|
-      begin
-        namespace = Group.create!(name: name,
-                                  path: name,
-                                  owner: @current_user,
-                                  parent: parent)
-        namespace.add_owner(@current_user)
-
-        namespace
-      rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid
-        Namespace.where(parent: parent).find_by_path_or_name(name)
-      end
-    end
+    Groups::NestedCreateService.new(@current_user, group_path: names).execute
   end
 
   def full_path_namespace(names)

spec/lib/gitlab/bare_repository_importer_spec.rb

@@ -64,25 +64,5 @@ describe Gitlab::BareRepositoryImporter, repository: true do
 
       expect(Project.find_by_full_path(project_path)).not_to be_nil
     end
-
-    it 'creates group and subgroup in the database' do
-      importer.create_project_if_needed
-
-      parent = Group.find_by_full_path('a-group')
-      child = parent.children.find_by(path: 'a-sub-group')
-
-      expect(parent).not_to be_nil
-      expect(child).not_to be_nil
-    end
-
-    it 'creates the group with correct visibility level' do
-      allow(Gitlab::CurrentSettings.current_application_settings)
-        .to receive(:default_group_visibility) { Gitlab::VisibilityLevel::INTERNAL }
-      project = importer.create_project_if_needed
-
-      group = project.namespace
-
-      expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL)
-    end
   end
 end

spec/services/groups/nested_create_service_spec.rb

+require 'spec_helper'
+
+describe Groups::NestedCreateService do
+  let(:user) { create(:user) }
+  let(:params) { { group_path: 'a-group/a-sub-group' } }
+
+  subject(:service) { described_class.new(user, params) }
+
+  describe "#execute" do
+    it 'returns the group if it already existed' do
+      parent = create(:group, path: 'a-group', owner: user)
+      child = create(:group, path: 'a-sub-group', parent: parent, owner: user)
+
+      expect(service.execute).to eq(child)
+    end
+
+    it 'reuses a parent if it already existed' do
+      parent = create(:group, path: 'a-group')
+      parent.add_owner(user)
+
+      expect(service.execute.parent).to eq(parent)
+    end
+
+    it 'creates group and subgroup in the database' do
+      service.execute
+
+      parent = Group.find_by_full_path('a-group')
+      child = parent.children.find_by(path: 'a-sub-group')
+
+      expect(parent).not_to be_nil
+      expect(child).not_to be_nil
+    end
+
+    it 'creates the group with correct visibility level' do
+      allow(Gitlab::CurrentSettings.current_application_settings)
+        .to receive(:default_group_visibility) { Gitlab::VisibilityLevel::INTERNAL }
+
+      group = service.execute
+
+      expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL)
+    end
+
+    context 'adding a visibility level ' do
+      let(:params) { { group_path: 'a-group/a-sub-group', visibility_level: Gitlab::VisibilityLevel::PRIVATE } }
+
+      it 'overwrites the visibility level' do
+        group = service.execute
+
+        expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE)
+      end
+    end
+  end
+end