Refactoring Import::BaseController#find_or_create_namespace

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_namespace = $namespace_input.prop('value');
-            $target_field.empty().append(new_namespace + "/" + ($target_field.data('project_name')));
+            $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'
           });

app/controllers/import/base_controller.rb

@@ -2,21 +2,16 @@ class Import::BaseController < ApplicationController
   private
 
   def find_or_create_namespace(name, owner)
+    return current_user.namespace if name == owner
+    return current_user.namespace unless current_user.can_create_group?
+
     begin
-      @target_namespace = params[:new_namespace].presence || name
+      name = params[:target_namespace].presence || name
-      @target_namespace = current_user.namespace_path if name == owner || !current_user.can_create_group?
+      namespace = Group.create!(name: name, path: name, owner: current_user)
-
-      namespace = Group.create!(name: @target_namespace, path: @target_namespace, owner: current_user)
       namespace.add_owner(current_user)
-    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
-    end
-
       namespace
+    rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid
+      Namespace.find_by_path_or_name(name)
+    end
   end
 end

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 'deploy_key' and return
-      render
-      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

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

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

app/views/import/base/create.js.haml

@@ -1,23 +1,4 @@
-- if @already_been_taken
+- if @project.persisted?
-  :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?
   :plain
     job = $("tr#repo_#{@repo_id}")
     job.attr("id", "project_#{@project.id}")

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')

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>")