WIP: Clean up import controllersdm-cleanup-import-controllers

12 files changed, 42 insertions, 55 deletions

diff --git a/app/controllers/import/base_controller.rb b/app/controllers/import/base_controller.rb
index 9de0297ecfd..e89d76d25c5 100644
--- a/app/controllers/import/base_controller.rb
+++ b/app/controllers/import/base_controller.rb
@@ -1,27 +1,16 @@
 class Import::BaseController < ApplicationController
   private
 
-  def find_or_create_namespace(names, owner)
-    return current_user.namespace if names == owner
-    return current_user.namespace unless current_user.can_create_group?
-
-    names = params[:target_namespace].presence || names
-    full_path_namespace = Namespace.find_by_full_path(names)
-
-    return full_path_namespace if full_path_namespace
+  def find_or_create_namespace(namespace_path, client_username)
+    if params[:target_namespace].present?
+      namespace_path = params[:target_namespace]
+    elsif namespace_path == client_username
+      return current_user.namespace
+    end
 
-    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 = Namespace.find_by_full_path(namespace_path)
+    return namespace if namespace
 
-        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: namespace_path).execute
   end
 end
diff --git a/app/controllers/import/bitbucket_controller.rb b/app/controllers/import/bitbucket_controller.rb
index 5ad1e116e4e..762f69593f9 100644
--- a/app/controllers/import/bitbucket_controller.rb
+++ b/app/controllers/import/bitbucket_controller.rb
@@ -17,7 +17,6 @@ class Import::BitbucketController < Import::BaseController
   end
 
   def status
-    bitbucket_client = Bitbucket::Client.new(credentials)
     repos = bitbucket_client.repos
 
     @repos, @incompatible_repos = repos.partition { |repo| repo.valid? }
@@ -35,20 +34,13 @@ class Import::BitbucketController < Import::BaseController
   end
 
   def create
-    bitbucket_client = Bitbucket::Client.new(credentials)
-
     @repo_id = params[:repo_id].to_s
     name = @repo_id.gsub('___', '/')
     repo = bitbucket_client.repo(name)
     @project_name = params[:new_name].presence || repo.name
+    @target_namespace = find_or_create_namespace(repo.owner, bitbucket_client.user.username)
 
-    repo_owner = repo.owner
-    repo_owner = current_user.username if repo_owner == bitbucket_client.user.username
-    namespace_path = params[:new_namespace].presence || repo_owner
-
-    @target_namespace = find_or_create_namespace(namespace_path, current_user)
-
-    if current_user.can?(:create_projects, @target_namespace)
+    if can?(current_user, :create_projects, @target_namespace)
       # The token in a session can be expired, we need to get most recent one because
       # Bitbucket::Connection class refreshes it.
       session[:bitbucket_token] = bitbucket_client.connection.token
@@ -64,6 +56,10 @@ class Import::BitbucketController < Import::BaseController
     @client ||= OAuth2::Client.new(provider.app_id, provider.app_secret, options)
   end
 
+  def bitbucket_client
+    @bitbucket_client ||= Bitbucket::Client.new(credentials)
+  end
+
   def provider
     Gitlab::OAuth::Provider.config_for('bitbucket')
   end
diff --git a/app/controllers/import/fogbugz_controller.rb b/app/controllers/import/fogbugz_controller.rb
index 5df6bd34185..027db55127a 100644
--- a/app/controllers/import/fogbugz_controller.rb
+++ b/app/controllers/import/fogbugz_controller.rb
@@ -64,11 +64,9 @@ class Import::FogbugzController < Import::BaseController
     @target_namespace = current_user.namespace
     @project_name = repo.name
 
-    namespace = @target_namespace
+    user_map = session[:fogbugz_user_map] || client.user_map
 
-    umap = session[:fogbugz_user_map] || client.user_map
-
-    @project = Gitlab::FogbugzImport::ProjectCreator.new(repo, fb_session, namespace, current_user, umap).execute
+    @project = Gitlab::FogbugzImport::ProjectCreator.new(repo, fb_session, @target_namespace, current_user, user_map).execute
   end
 
   private
diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb
index b8ba7921613..1a7b5cf6f28 100644
--- a/app/controllers/import/github_controller.rb
+++ b/app/controllers/import/github_controller.rb
@@ -24,6 +24,7 @@ class Import::GithubController < Import::BaseController
 
   def status
     @repos = client.repos
+
     @already_added_projects = current_user.created_projects.where(import_type: provider)
     already_added_projects_names = @already_added_projects.pluck(:import_source)
 
@@ -39,8 +40,7 @@ class Import::GithubController < Import::BaseController
     @repo_id = params[:repo_id].to_i
     repo = client.repo(@repo_id)
     @project_name = params[:new_name].presence || repo.name
-    namespace_path = params[:target_namespace].presence || current_user.namespace_path
-    @target_namespace = find_or_create_namespace(namespace_path, current_user.namespace_path)
+    @target_namespace = find_or_create_namespace(repo.owner.login, client.user.login)
 
     if can?(current_user, :create_projects, @target_namespace)
       @project = Gitlab::LegacyGithubImport::ProjectCreator.new(repo, @project_name, @target_namespace, current_user, access_params, type: provider).execute
diff --git a/app/controllers/import/gitlab_controller.rb b/app/controllers/import/gitlab_controller.rb
index 407154e59a0..38276ed226b 100644
--- a/app/controllers/import/gitlab_controller.rb
+++ b/app/controllers/import/gitlab_controller.rb
@@ -27,9 +27,10 @@ class Import::GitlabController < Import::BaseController
     @repo_id = params[:repo_id].to_i
     repo = client.project(@repo_id)
     @project_name = repo['name']
-    @target_namespace = find_or_create_namespace(repo['namespace']['path'], client.user['username'])
+    namespace_path = repo['namespace']['full_path'] || repo['namespace']['path']
+    @target_namespace = find_or_create_namespace(namespace_path, client.user['username'])
 
-    if current_user.can?(:create_projects, @target_namespace)
+    if can?(current_user, :create_projects, @target_namespace)
       @project = Gitlab::GitlabImport::ProjectCreator.new(repo, @target_namespace, current_user, access_params).execute
     else
       render 'unauthorized'
diff --git a/app/controllers/import/gitlab_projects_controller.rb b/app/controllers/import/gitlab_projects_controller.rb
index 510813846a4..3a0514bd8c5 100644
--- a/app/controllers/import/gitlab_projects_controller.rb
+++ b/app/controllers/import/gitlab_projects_controller.rb
@@ -3,7 +3,7 @@ class Import::GitlabProjectsController < Import::BaseController
 
   def new
     @namespace = Namespace.find(project_params[:namespace_id])
-    return render_404 unless current_user.can?(:create_projects, @namespace)
+    return render_404 unless can?(current_user, :create_projects, @namespace)
     @path = project_params[:path]
   end
 
diff --git a/app/controllers/import/google_code_controller.rb b/app/controllers/import/google_code_controller.rb
index 7d7f13ce5d5..e77e4f0653b 100644
--- a/app/controllers/import/google_code_controller.rb
+++ b/app/controllers/import/google_code_controller.rb
@@ -90,11 +90,9 @@ class Import::GoogleCodeController < Import::BaseController
     @target_namespace = current_user.namespace
     @project_name = repo.name
 
-    namespace = @target_namespace
-
     user_map = session[:google_code_user_map]
 
-    @project = Gitlab::GoogleCodeImport::ProjectCreator.new(repo, namespace, current_user, user_map).execute
+    @project = Gitlab::GoogleCodeImport::ProjectCreator.new(repo, @target_namespace, current_user, user_map).execute
   end
 
   private
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 4d401e7ba18..1b56b3cbacd 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -21,8 +21,8 @@ class Namespace < ActiveRecord::Base
   has_many :project_statistics
   belongs_to :owner, class_name: "User"
 
-  belongs_to :parent, class_name: "Namespace"
-  has_many :children, class_name: "Namespace", foreign_key: :parent_id
+  belongs_to :parent, class_name: "Group"
+  has_many :children, class_name: "Group", foreign_key: :parent_id
   has_one :chat_team, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
 
   validates :owner, presence: true, unless: ->(n) { n.type == "Group" }
diff --git a/app/services/groups/nested_create_service.rb b/app/services/groups/nested_create_service.rb
index d6f08fc3cce..cbd94ea3e4e 100644
--- a/app/services/groups/nested_create_service.rb
+++ b/app/services/groups/nested_create_service.rb
@@ -11,10 +11,12 @@ module Groups
     def execute
       return nil unless group_path
 
+      # TODO: What if already exists as user namespace?
       if group = Group.find_by_full_path(group_path)
         return group
       end
 
+      # TODO: Should already be handled by validation
       if group_path.include?('/') && !Group.supports_nested_groups?
         raise 'Nested groups are not supported on MySQL'
       end
@@ -38,6 +40,7 @@ module Groups
           name: subgroup_name,
           parent: last_group
         )
+        # TODO: What if existing group has lower visibility?
         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
diff --git a/app/views/import/_githubish_status.html.haml b/app/views/import/_githubish_status.html.haml
index e9a04e6c122..626eb336b58 100644
--- a/app/views/import/_githubish_status.html.haml
+++ b/app/views/import/_githubish_status.html.haml
@@ -45,11 +45,13 @@
             %fieldset.row
             .input-group
               .project-path.input-group-btn
+                -# TODO: The user should be able to choose create a new group if they don't currently have any
                 - if current_user.can_select_namespace?
                   - selected = params[:namespace_id] || :current_user
                   - opts = current_user.can_create_group? ? { extra_group: Group.new(name: repo.owner.login, path: repo.owner.login) } : {}
                   = select_tag :namespace_id, namespaces_options(selected, opts.merge({ display_path: true })), { class: 'select2 js-select-namespace', tabindex: 1 }
                 - else
+                  -# This is likely wrong, see https://gitlab.com/gitlab-org/gitlab-ce/issues/28744
                   = text_field_tag :path, current_user.namespace_path, class: "input-large form-control", tabindex: 1, disabled: true
               %span.input-group-addon /
               = text_field_tag :path, repo.name, class: "input-mini form-control", tabindex: 2, autofocus: true, required: true
diff --git a/lib/gitlab/bare_repository_import/importer.rb b/lib/gitlab/bare_repository_import/importer.rb
index 196de667805..b77044cfdba 100644
--- a/lib/gitlab/bare_repository_import/importer.rb
+++ b/lib/gitlab/bare_repository_import/importer.rb
@@ -29,7 +29,7 @@ module Gitlab
       attr_reader :user, :project_name, :bare_repo
 
       delegate :log, to: :class
-      delegate :project_name, :project_full_path, :group_path, :repo_path, :wiki_path, to: :bare_repo
+      delegate :project_name, :project_full_path, :namespace_path, :repo_path, :wiki_path, to: :bare_repo
 
       def initialize(user, bare_repo)
         @user = user
@@ -49,13 +49,13 @@ module Gitlab
       private
 
       def create_project
-        group = find_or_create_groups
+        namespace = find_or_create_namespace
 
         project = Projects::CreateService.new(user,
                                               name: project_name,
                                               path: project_name,
                                               skip_disk_validation: true,
-                                              namespace_id: group&.id).execute
+                                              namespace_id: namespace&.id).execute
 
         if project.persisted? && mv_repo(project)
           log " * Created #{project.name} (#{project_full_path})".color(:green)
@@ -83,12 +83,12 @@ module Gitlab
         false
       end
 
-      def find_or_create_groups
-        return nil unless group_path.present?
+      def find_or_create_namespace
+        return nil unless namespace_path.present?
 
-        log " * Using namespace: #{group_path}"
+        log " * Using namespace: #{namespace_path}"
 
-        Groups::NestedCreateService.new(user, group_path: group_path).execute
+        Groups::NestedCreateService.new(user, group_path: namespace_path).execute
       end
 
       # This is called from within a rake task only used by Admins, so allow writing
diff --git a/lib/gitlab/bare_repository_import/repository.rb b/lib/gitlab/bare_repository_import/repository.rb
index 8574ac6eb30..14de732c326 100644
--- a/lib/gitlab/bare_repository_import/repository.rb
+++ b/lib/gitlab/bare_repository_import/repository.rb
@@ -1,14 +1,14 @@
 module Gitlab
   module BareRepositoryImport
     class Repository
-      attr_reader :group_path, :project_name, :repo_path
+      attr_reader :namespace_path, :project_name, :repo_path
 
       def initialize(root_path, repo_path)
         @root_path = root_path
         @repo_path = repo_path
 
         # Split path into 'all/the/namespaces' and 'project_name'
-        @group_path, _, @project_name = repo_relative_path.rpartition('/')
+        @namespace_path, _, @project_name = repo_relative_path.rpartition('/')
       end
 
       def wiki_exists?
@@ -24,11 +24,11 @@ module Gitlab
       end
 
       def hashed?
-        @hashed ||= group_path.start_with?('@hashed')
+        @hashed ||= namespace_path.start_with?('@hashed')
       end
 
       def project_full_path
-        @project_full_path ||= "#{group_path}/#{project_name}"
+        @project_full_path ||= "#{namespace_path}/#{project_name}"
       end
 
       private