From 833dc3204d9573dfb1a8f0a433f21a0ff709969c Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Mon, 13 Jun 2016 16:55:51 +0200
Subject: [PATCH] few more changes based on feedback

---
 lib/gitlab/import_export/members_mapper.rb    |  6 ++---
 .../import_export/project_tree_restorer.rb    | 27 ++++++++++++-------
 lib/gitlab/import_export/relation_factory.rb  |  8 +++---
 .../import_export/members_mapper_spec.rb      |  2 +-
 4 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/lib/gitlab/import_export/members_mapper.rb b/lib/gitlab/import_export/members_mapper.rb
index d2748ee5cb4..c569a35a48b 100644
--- a/lib/gitlab/import_export/members_mapper.rb
+++ b/lib/gitlab/import_export/members_mapper.rb
@@ -2,13 +2,13 @@ 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 #map
         # which means project members already exist.
@@ -37,7 +37,7 @@ module Gitlab
 
       def missing_keys_tracking_hash
         Hash.new do |_, key|
-          @note_member_list << key
+          @missing_author_ids << key
           default_user_id
         end
       end
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..df05d6a6f67 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,7 +57,7 @@ module Gitlab
 
         author = @relation_hash.delete('author')
 
-        if admin_user? && @members_mapper.note_member_list.include?(old_author_id)
+        if admin_user? && @members_mapper.missing_author_ids.include?(old_author_id)
           update_note_for_missing_author(author['name'])
         end
       end
@@ -119,7 +119,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