Improve manifest import logic

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

4 files changed, 41 insertions, 28 deletions

diff --git a/app/controllers/import/manifest_controller.rb b/app/controllers/import/manifest_controller.rb
index 639431690bb..c183bbe1ded 100644
--- a/app/controllers/import/manifest_controller.rb
+++ b/app/controllers/import/manifest_controller.rb
@@ -1,17 +1,16 @@
 class Import::ManifestController < Import::BaseController
-  before_action :ensure_session, only: [:create, :status, :jobs]
-  before_action :group, only: [:status, :create]
+  before_action :ensure_import_vars, only: [:create, :status]
 
   def new
   end
 
   def status
-    @repos = session[:projects]
+    @already_added_projects = find_already_added_projects
+    already_added_import_urls = @already_added_projects.pluck(:import_url)
 
-    @already_added_projects = find_already_added_projects('manifest').where(namespace_id: group)
-    already_added_projects_names = @already_added_projects.pluck(:import_url)
-
-    @repos = @repos.to_a.reject { |repo| already_added_projects_names.include? repo[:url] }
+    @pending_repositories = repositories.to_a.reject do |repository|
+      already_added_import_urls.include?(repository[:url])
+    end
   end
 
   def upload
@@ -26,11 +25,9 @@ class Import::ManifestController < Import::BaseController
     manifest = Gitlab::ManifestImport::Manifest.new(params[:manifest].tempfile)
 
     if manifest.valid?
-      session[:projects] = manifest.projects
+      session[:repositories] = manifest.projects
       session[:group_id] = group.id
 
-      flash[:notice] = "Import successfully started."
-
       redirect_to status_import_manifest_path
     else
       @errors = manifest.errors
@@ -40,11 +37,11 @@ class Import::ManifestController < Import::BaseController
   end
 
   def jobs
-    render json: find_jobs('manifest')
+    render json: find_jobs
   end
 
   def create
-    repository = session[:projects].find do |project|
+    repository = repositories.find do |project|
       project[:id] == params[:repo_id].to_i
     end
 
@@ -59,13 +56,28 @@ class Import::ManifestController < Import::BaseController
 
   private
 
-  def ensure_session
-    if session[:projects].blank? || session[:group_id].blank?
+  def ensure_import_vars
+    unless group && repositories.present?
       redirect_to(new_import_manifest_path)
     end
   end
 
   def group
-    @group ||= Group.find(session[:group_id])
+    @group ||= Group.find_by(id: session[:group_id])
+  end
+
+  def repositories
+    @repositories ||= session[:repositories]
+  end
+
+  def find_jobs
+    find_already_added_projects.to_json(only: [:id], methods: [:import_status])
+  end
+
+  def find_already_added_projects
+    group.all_projects
+      .where(import_type: 'manifest')
+      .where(creator_id: current_user)
+      .includes(:import_state)
   end
 end
diff --git a/app/views/import/manifest/new.html.haml b/app/views/import/manifest/new.html.haml
index 1f27dec1b2b..056e4922b9e 100644
--- a/app/views/import/manifest/new.html.haml
+++ b/app/views/import/manifest/new.html.haml
@@ -1,9 +1,8 @@
-- page_title "Manifest Import"
+- page_title "Manifest file import"
 - header_title "Projects", root_path
 
 %h3.page-title
-  = icon('git')
-  Import multiple repositories
+  = _('Manifest file import')
 
 - if @errors.present?
   .alert.alert-danger
diff --git a/app/views/import/manifest/status.html.haml b/app/views/import/manifest/status.html.haml
index 3355bed056b..39aba9234d3 100644
--- a/app/views/import/manifest/status.html.haml
+++ b/app/views/import/manifest/status.html.haml
@@ -3,8 +3,10 @@
 - provider = 'manifest'
 
 %h3.page-title
-  = icon('git')
-  = _('Import multiple repositories')
+  = _('Manifest file import')
+
+%p.light
+  = _('Import multiple repositories by uploading a manifest file.')
 
 %p
   = button_tag class: "btn btn-import btn-success js-import-all" do
@@ -25,9 +27,9 @@
       - @already_added_projects.each do |project|
         %tr{ id: "project_#{project.id}", class: "#{project_status_css_class(project.import_status)}" }
           %td
-            = link_to_project project
-          %td
             = project.import_url
+          %td
+            = link_to_project project
           %td.job-status
             - if project.import_status == 'finished'
               %span
@@ -41,15 +43,15 @@
             - else
               = project.human_import_status_name
 
-      - @repos.each do |repo|
-        %tr{ id: "repo_#{repo[:id]}" }
+      - @pending_repositories.each do |repository|
+        %tr{ id: "repo_#{repository[:id]}" }
           %td
-            = repo[:url]
+            = repository[:url]
           %td.import-target
-            = import_project_target(@group.path, repo[:path])
+            = import_project_target(@group.full_path, repository[:path])
           %td.import-actions.job-status
             = button_tag class: "btn btn-import js-add-to-import" do
-              Import
+              = _('Import')
               = icon("spinner spin", class: "loading-icon")
 
 .js-importer-status{ data: { jobs_import_path: "#{url_for([:jobs, :import, provider])}",
diff --git a/lib/gitlab/manifest_import/importer.rb b/lib/gitlab/manifest_import/importer.rb
index 35b3bb1e0ca..8ffeb0a4143 100644
--- a/lib/gitlab/manifest_import/importer.rb
+++ b/lib/gitlab/manifest_import/importer.rb
@@ -17,7 +17,7 @@ module Gitlab
 
       def import_project
         group_full_path, _, project_path = repository[:path].rpartition('/')
-        group_full_path = File.join(destination.path, group_full_path) if destination
+        group_full_path = File.join(destination.full_path, group_full_path) if destination
         group = Group.find_by_full_path(group_full_path) ||
           create_group_with_parents(group_full_path)