More reliable manifest parser and group-only selector

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

4 files changed, 26 insertions, 10 deletions

diff --git a/app/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb
index 9be93fa69ae..9008db1b300 100644
--- a/app/helpers/namespaces_helper.rb
+++ b/app/helpers/namespaces_helper.rb
@@ -3,7 +3,7 @@ module NamespacesHelper
     params.dig(:project, :namespace_id) || params[:namespace_id]
   end
 
-  def namespaces_options(selected = :current_user, display_path: false, extra_group: nil)
+  def namespaces_options(selected = :current_user, display_path: false, extra_group: nil, groups_only: false)
     groups = current_user.manageable_groups
                .joins(:route)
                .includes(:route)
@@ -20,10 +20,13 @@ module NamespacesHelper
 
     options = []
     options << options_for_group(groups, display_path: display_path, type: 'group')
-    options << options_for_group(users, display_path: display_path, type: 'user')
 
-    if selected == :current_user && current_user.namespace
-      selected = current_user.namespace.id
+    unless groups_only
+      options << options_for_group(users, display_path: display_path, type: 'user')
+
+      if selected == :current_user && current_user.namespace
+        selected = current_user.namespace.id
+      end
     end
 
     grouped_options_for_select(options, selected)
diff --git a/app/views/import/manifest/_form.html.haml b/app/views/import/manifest/_form.html.haml
index 57980cfd58a..6261a2b6864 100644
--- a/app/views/import/manifest/_form.html.haml
+++ b/app/views/import/manifest/_form.html.haml
@@ -1,21 +1,21 @@
 = form_tag upload_import_manifest_path, multipart: true do
   .form-group
     = label_tag :manifest, class: 'label-light' do
-      Manifest
+      = _('Manifest')
     = file_field_tag :manifest, class: 'form-control-file', required: true
     .form-text.text-muted
-      Import multiple repositories by uploading a manifest file.
+      = _('Import multiple repositories by uploading a manifest file.')
 
   .form-group
     = label_tag :group_id, nil, class: 'label-light' do
-      Namespace
+      = _('Group')
     .input-group
       .input-group-prepend.has-tooltip{ title: root_url }
         .input-group-text
           = root_url
-      = select_tag :group_id, namespaces_options(nil, display_path: true), { class: 'select2 js-select-namespace' }
+      = select_tag :group_id, namespaces_options(nil, display_path: true, groups_only: true), { class: 'select2 js-select-namespace' }
     .form-text.text-muted
-      Choose the top-level namespace for your repository imports.
+      = _('Choose the top-level group for your repository imports.')
 
   .append-bottom-10
     = submit_tag 'Import projects', class: 'btn btn-success'
diff --git a/lib/gitlab/manifest_import/manifest.rb b/lib/gitlab/manifest_import/manifest.rb
index 87959d4ae7f..7b1e6a22c3a 100644
--- a/lib/gitlab/manifest_import/manifest.rb
+++ b/lib/gitlab/manifest_import/manifest.rb
@@ -66,7 +66,10 @@ module Gitlab
       end
 
       def remote
-        @remote ||= parsed_xml.css('manifest > remote').first['review']
+        return @remote if defined?(@remote)
+
+        remote_tag = parsed_xml.css('manifest > remote').first
+        @remote = remote_tag['review'] if remote_tag
       end
 
       def raw_projects
diff --git a/spec/helpers/namespaces_helper_spec.rb b/spec/helpers/namespaces_helper_spec.rb
index 460d3b6a7e4..343e140f5fb 100644
--- a/spec/helpers/namespaces_helper_spec.rb
+++ b/spec/helpers/namespaces_helper_spec.rb
@@ -28,6 +28,16 @@ describe NamespacesHelper do
 
       expect(options).not_to include(admin_group.name)
       expect(options).to include(user_group.name)
+      expect(options).to include(user.name)
+    end
+
+    it 'returns only groups if groups_only option is true' do
+      allow(helper).to receive(:current_user).and_return(user)
+
+      options = helper.namespaces_options(nil, groups_only: true)
+
+      expect(options).not_to include(user.name)
+      expect(options).to include(user_group.name)
     end
 
     context 'when nested groups are available', :nested_groups do