diff options
| -rw-r--r-- | app/services/groups/nested_create_service.rb | 45 | ||||
| -rw-r--r-- | lib/gitlab/bare_repository_importer.rb | 35 | ||||
| -rw-r--r-- | lib/tasks/import.rake | 18 | ||||
| -rw-r--r-- | spec/lib/gitlab/bare_repository_importer_spec.rb | 20 | ||||
| -rw-r--r-- | spec/services/groups/nested_create_service_spec.rb | 53 |
5 files changed, 101 insertions, 70 deletions
diff --git a/app/services/groups/nested_create_service.rb b/app/services/groups/nested_create_service.rb new file mode 100644 index 00000000000..8d793f5c02e --- /dev/null +++ b/app/services/groups/nested_create_service.rb @@ -0,0 +1,45 @@ +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 diff --git a/lib/gitlab/bare_repository_importer.rb b/lib/gitlab/bare_repository_importer.rb index 4bc3ccc3a1a..9323bfc7fb2 100644 --- a/lib/gitlab/bare_repository_importer.rb +++ b/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 diff --git a/lib/tasks/import.rake b/lib/tasks/import.rake index 96b8f59242c..1206302cb76 100644 --- a/lib/tasks/import.rake +++ b/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) diff --git a/spec/lib/gitlab/bare_repository_importer_spec.rb b/spec/lib/gitlab/bare_repository_importer_spec.rb index 88d30a2e855..892f2dafc96 100644 --- a/spec/lib/gitlab/bare_repository_importer_spec.rb +++ b/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 diff --git a/spec/services/groups/nested_create_service_spec.rb b/spec/services/groups/nested_create_service_spec.rb new file mode 100644 index 00000000000..c1526456bac --- /dev/null +++ b/spec/services/groups/nested_create_service_spec.rb @@ -0,0 +1,53 @@ +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 |