Refactoring Import::BaseController#find_or_create_namespace

8 files changed, 53 insertions, 47 deletions

diff --git a/app/assets/javascripts/importer_status.js b/app/assets/javascripts/importer_status.js
index 0f840821f53..9efad1ce943 100644
--- a/app/assets/javascripts/importer_status.js
+++ b/app/assets/javascripts/importer_status.js
@@ -10,21 +10,24 @@
     ImporterStatus.prototype.initStatusPage = function() {
       $('.js-add-to-import').off('click').on('click', (function(_this) {
         return function(e) {
-          var $btn, $namespace_input, $target_field, $tr, id, new_namespace;
+          var $btn, $namespace_input, $target_field, $tr, id, target_namespace;
           $btn = $(e.currentTarget);
           $tr = $btn.closest('tr');
           $target_field = $tr.find('.import-target');
           $namespace_input = $target_field.find('input');
           id = $tr.attr('id').replace('repo_', '');
-          new_namespace = null;
+          target_namespace = null;
+
           if ($namespace_input.length > 0) {
-            new_namespace = $namespace_input.prop('value');
-            $target_field.empty().append(new_namespace + "/" + ($target_field.data('project_name')));
+            target_namespace = $namespace_input.prop('value');
+            $target_field.empty().append(target_namespace + "/" + ($target_field.data('project_name')));
           }
+
           $btn.disable().addClass('is-loading');
+
           return $.post(_this.import_url, {
             repo_id: id,
-            new_namespace: new_namespace
+            target_namespace: target_namespace
           }, {
             dataType: 'script'
           });
@@ -70,7 +73,7 @@
     if ($('.js-importer-status').length) {
       var jobsImportPath = $('.js-importer-status').data('jobs-import-path');
       var importPath = $('.js-importer-status').data('import-path');
-      
+
       new ImporterStatus(jobsImportPath, importPath);
     }
   });
diff --git a/app/controllers/import/base_controller.rb b/app/controllers/import/base_controller.rb
index 1ca33bc5d22..256c41e6145 100644
--- a/app/controllers/import/base_controller.rb
+++ b/app/controllers/import/base_controller.rb
@@ -2,21 +2,16 @@ class Import::BaseController < ApplicationController
   private
 
   def find_or_create_namespace(name, owner)
-    begin
-      @target_namespace = params[:new_namespace].presence || name
-      @target_namespace = current_user.namespace_path if name == owner || !current_user.can_create_group?
+    return current_user.namespace if name == owner
+    return current_user.namespace unless current_user.can_create_group?
 
-      namespace = Group.create!(name: @target_namespace, path: @target_namespace, owner: current_user)
+    begin
+      name = params[:target_namespace].presence || name
+      namespace = Group.create!(name: name, path: name, owner: current_user)
       namespace.add_owner(current_user)
+      namespace
     rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid
-      namespace = Namespace.find_by_path_or_name(@target_namespace)
-
-      unless current_user.can?(:create_projects, namespace)
-        @already_been_taken = true
-        return false
-      end
+      Namespace.find_by_path_or_name(name)
     end
-
-    namespace
   end
 end
diff --git a/app/controllers/import/bitbucket_controller.rb b/app/controllers/import/bitbucket_controller.rb
index 94e213b8743..6ea54744da8 100644
--- a/app/controllers/import/bitbucket_controller.rb
+++ b/app/controllers/import/bitbucket_controller.rb
@@ -38,15 +38,17 @@ class Import::BitbucketController < Import::BaseController
     @repo_id = params[:repo_id].to_s
     repo = client.project(@repo_id.gsub('___', '/'))
     @project_name = repo['slug']
-    namespace = find_or_create_namespace(repo['owner'], client.user['user']['username']) || (render and return)
+    @target_namespace = find_or_create_namespace(repo['owner'], client.user['user']['username'])
 
     unless Gitlab::BitbucketImport::KeyAdder.new(repo, current_user, access_params).execute
-      @access_denied = true
-      render
-      return
+      render 'deploy_key' and return
     end
 
-    @project = Gitlab::BitbucketImport::ProjectCreator.new(repo, namespace, current_user, access_params).execute
+    if current_user.can?(:create_projects, @target_namespace)
+      @project = Gitlab::BitbucketImport::ProjectCreator.new(repo, @target_namespace, current_user, access_params).execute
+    else
+      render 'unauthorized'
+    end
   end
 
   private
diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb
index 4047e62efa2..8c6bdd16383 100644
--- a/app/controllers/import/github_controller.rb
+++ b/app/controllers/import/github_controller.rb
@@ -41,9 +41,13 @@ class Import::GithubController < Import::BaseController
     @repo_id = params[:repo_id].to_i
     repo = client.repo(@repo_id)
     @project_name = repo.name
-    namespace = find_or_create_namespace(repo.owner.login, client.user.login) || (render and return)
+    @target_namespace = find_or_create_namespace(repo.owner.login, client.user.login)
 
-    @project = Gitlab::GithubImport::ProjectCreator.new(repo, namespace, current_user, access_params).execute
+    if current_user.can?(:create_projects, @target_namespace)
+      @project = Gitlab::GithubImport::ProjectCreator.new(repo, @target_namespace, current_user, access_params).execute
+    else
+      render 'unauthorized'
+    end
   end
 
   private
diff --git a/app/controllers/import/gitlab_controller.rb b/app/controllers/import/gitlab_controller.rb
index bc967e55ab1..73837ffbe67 100644
--- a/app/controllers/import/gitlab_controller.rb
+++ b/app/controllers/import/gitlab_controller.rb
@@ -27,9 +27,13 @@ class Import::GitlabController < Import::BaseController
     @repo_id = params[:repo_id].to_i
     repo = client.project(@repo_id)
     @project_name = repo['name']
-    namespace = find_or_create_namespace(repo['namespace']['path'], client.user['username']) || (render and return)
+    @target_namespace = find_or_create_namespace(repo['namespace']['path'], client.user['username'])
 
-    @project = Gitlab::GitlabImport::ProjectCreator.new(repo, namespace, current_user, access_params).execute
+    if current_user.can?(:create_projects, @target_namespace)
+      @project = Gitlab::GitlabImport::ProjectCreator.new(repo, @target_namespace, current_user, access_params).execute
+    else
+      render 'unauthorized'
+    end
   end
 
   private
diff --git a/app/views/import/base/create.js.haml b/app/views/import/base/create.js.haml
index 804ad88468f..8e929538351 100644
--- a/app/views/import/base/create.js.haml
+++ b/app/views/import/base/create.js.haml
@@ -1,23 +1,4 @@
-- if @already_been_taken
-  :plain
-    tr = $("tr#repo_#{@repo_id}")
-    target_field = tr.find(".import-target")
-    import_button = tr.find(".btn-import")
-    origin_target = target_field.text()
-    project_name = "#{@project_name}"
-    origin_namespace = "#{@target_namespace}"
-    target_field.empty()
-    target_field.append("<p class='alert alert-danger'>This namespace already been taken! Please choose another one</p>")
-    target_field.append("<input type='text' name='target_namespace' />")
-    target_field.append("/" + project_name)
-    target_field.data("project_name", project_name)
-    target_field.find('input').prop("value", origin_namespace)
-    import_button.enable().removeClass('is-loading')
-- elsif @access_denied
-  :plain
-    job = $("tr#repo_#{@repo_id}")
-    job.find(".import-actions").html("<p class='alert alert-danger'>Access denied! Please verify you can add deploy keys to this repository.</p>")
-- elsif @project.persisted?
+- if @project.persisted?
   :plain
     job = $("tr#repo_#{@repo_id}")
     job.attr("id", "project_#{@project.id}")
diff --git a/app/views/import/base/unauthorized.js.haml b/app/views/import/base/unauthorized.js.haml
new file mode 100644
index 00000000000..36f8069c1f7
--- /dev/null
+++ b/app/views/import/base/unauthorized.js.haml
@@ -0,0 +1,14 @@
+:plain
+  tr = $("tr#repo_#{@repo_id}")
+  target_field = tr.find(".import-target")
+  import_button = tr.find(".btn-import")
+  origin_target = target_field.text()
+  project_name = "#{@project_name}"
+  origin_namespace = "#{@target_namespace.path}"
+  target_field.empty()
+  target_field.append("<p class='alert alert-danger'>This namespace has already been taken! Please choose another one.</p>")
+  target_field.append("<input type='text' name='target_namespace' />")
+  target_field.append("/" + project_name)
+  target_field.data("project_name", project_name)
+  target_field.find('input').prop("value", origin_namespace)
+  import_button.enable().removeClass('is-loading')
diff --git a/app/views/import/bitbucket/deploy_key.js.haml b/app/views/import/bitbucket/deploy_key.js.haml
new file mode 100644
index 00000000000..81b34ab5c9d
--- /dev/null
+++ b/app/views/import/bitbucket/deploy_key.js.haml
@@ -0,0 +1,3 @@
+:plain
+  job = $("tr#repo_#{@repo_id}")
+  job.find(".import-actions").html("<p class='alert alert-danger'>Access denied! Please verify you can add deploy keys to this repository.</p>")