Merge branch 'feature/project-import' of gitlab.com:gitlab-org/gitlab-ce into feature/project-export-ui-experimental

4 files changed, 40 insertions, 34 deletions

diff --git a/lib/gitlab/import_export/members_mapper.rb b/lib/gitlab/import_export/members_mapper.rb
index 13fe68d5408..c569a35a48b 100644
--- a/lib/gitlab/import_export/members_mapper.rb
+++ b/lib/gitlab/import_export/members_mapper.rb
@@ -2,37 +2,26 @@ module Gitlab
   module ImportExport
     class MembersMapper
 
-      attr_reader :note_member_list
+      attr_reader :missing_author_ids
 
       def initialize(exported_members:, user:, project:)
         @exported_members = exported_members
         @user = user
         @project = project
-        @note_member_list = []
+        @missing_author_ids = []
 
-        # This needs to run first, as second call would be from generate_map
+        # This needs to run first, as second call would be from #map
         # which means project members already exist.
         ensure_default_member!
       end
 
       def map
-        @map ||= generate_map
-      end
-
-      def default_user_id
-        @user.id
-      end
-
-      private
-
-
-      def generate_map
         @map ||=
           begin
             @exported_members.inject(missing_keys_tracking_hash) do |hash, member|
               existing_user = User.where(find_project_user_query(member)).first
               old_user_id = member['user']['id']
-              if existing_user && add_user_as_team_member(existing_user, member).persisted?
+              if existing_user && add_user_as_team_member(existing_user, member)
                 hash[old_user_id] = existing_user.id
               end
               hash
@@ -40,10 +29,16 @@ module Gitlab
           end
       end
 
+      def default_user_id
+        @user.id
+      end
+
+      private
+
       def missing_keys_tracking_hash
         Hash.new do |_, key|
-          @note_member_list << key
-          @user.id
+          @missing_author_ids << key
+          default_user_id
         end
       end
 
@@ -54,7 +49,7 @@ module Gitlab
       def add_user_as_team_member(existing_user, member)
         member['user'] = existing_user
 
-        ProjectMember.create(member_hash(member))
+        ProjectMember.create(member_hash(member)).persisted?
       end
 
       def member_hash(member)
diff --git a/lib/gitlab/import_export/project_tree_restorer.rb b/lib/gitlab/import_export/project_tree_restorer.rb
index 935bc2d7df9..43f033ac49c 100644
--- a/lib/gitlab/import_export/project_tree_restorer.rb
+++ b/lib/gitlab/import_export/project_tree_restorer.rb
@@ -67,29 +67,38 @@ module Gitlab
         project
       end
 
+      # Given a relation hash containing one or more models and its relationships,
+      # loops through each model and each object from a model type and
+      # and assigns its correspondent attributes hash from +tree_hash+
+      # Example:
+      # +relation_key+ issues, loops through the list of *issues* and for each individual
+      # issue, finds any subrelations such as notes, creates them and assign them back to the hash
       def create_sub_relations(relation, tree_hash)
         relation_key = relation.keys.first.to_s
         tree_hash[relation_key].each do |relation_item|
           relation.values.flatten.each do |sub_relation|
-
-            if sub_relation.is_a?(Hash)
-              relation_hash = relation_item[sub_relation.keys.first.to_s]
-              sub_relation = sub_relation.keys.first
-            else
-              relation_hash = relation_item[sub_relation.to_s]
-            end
-
+            relation_hash, sub_relation = assign_relation_hash(relation_item, sub_relation)
             relation_item[sub_relation.to_s] = create_relation(sub_relation, relation_hash) unless relation_hash.blank?
           end
         end
       end
 
+      def assign_relation_hash(relation_item, sub_relation)
+        if sub_relation.is_a?(Hash)
+          relation_hash = relation_item[sub_relation.keys.first.to_s]
+          sub_relation = sub_relation.keys.first
+        else
+          relation_hash = relation_item[sub_relation.to_s]
+        end
+        return relation_hash, sub_relation
+      end
+
       def create_relation(relation, relation_hash_list)
         relation_array = [relation_hash_list].flatten.map do |relation_hash|
           Gitlab::ImportExport::RelationFactory.create(relation_sym: relation.to_sym,
                                                        relation_hash: relation_hash.merge('project_id' => project.id),
                                                        members_mapper: members_mapper,
-                                                       user_admin: @user.is_admin?)
+                                                       user: @user)
         end
 
         relation_hash_list.is_a?(Array) ? relation_array : relation_array.first
diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb
index 486e09c9a39..5f12caa8981 100644
--- a/lib/gitlab/import_export/relation_factory.rb
+++ b/lib/gitlab/import_export/relation_factory.rb
@@ -16,11 +16,11 @@ module Gitlab
         new(*args).create
       end
 
-      def initialize(relation_sym:, relation_hash:, members_mapper:, user_admin:)
+      def initialize(relation_sym:, relation_hash:, members_mapper:, user:)
         @relation_name = OVERRIDES[relation_sym] || relation_sym
         @relation_hash = relation_hash.except('id', 'noteable_id')
         @members_mapper = members_mapper
-        @user_admin = user_admin
+        @user = user
       end
 
       # Creates an object from an actual model with name "relation_sym" with params from
@@ -57,9 +57,11 @@ module Gitlab
 
         author = @relation_hash.delete('author')
 
-        if admin_user? && @members_mapper.note_member_list.include?(old_author_id)
-          update_note_for_missing_author(author['name'])
-        end
+        update_note_for_missing_author(author['name']) if can_update_notes?
+      end
+
+      def can_update_notes?
+        (admin_user? && @members_mapper.missing_author_ids.include?(old_author_id)) || !admin_user?
       end
 
       def missing_author_note(updated_at, author_name)
@@ -119,7 +121,7 @@ module Gitlab
       end
 
       def admin_user?
-        @user_admin
+        @user.is_admin?
       end
     end
   end
diff --git a/spec/lib/gitlab/import_export/members_mapper_spec.rb b/spec/lib/gitlab/import_export/members_mapper_spec.rb
index adaadbff6f8..f135a285dfb 100644
--- a/spec/lib/gitlab/import_export/members_mapper_spec.rb
+++ b/spec/lib/gitlab/import_export/members_mapper_spec.rb
@@ -46,7 +46,7 @@ describe Gitlab::ImportExport::MembersMapper, services: true do
     it 'updates missing author IDs on missing project member' do
       members_mapper.map[-1]
 
-      expect(members_mapper.note_member_list.first).to eq(-1)
+      expect(members_mapper.missing_author_ids.first).to eq(-1)
     end
   end
 end