add more specs and refactor more relation factory code

4 files changed, 118 insertions, 9 deletions

diff --git a/lib/gitlab/import_export/project_tree_restorer.rb b/lib/gitlab/import_export/project_tree_restorer.rb
index 25661a42ad5..8b30483a5e2 100644
--- a/lib/gitlab/import_export/project_tree_restorer.rb
+++ b/lib/gitlab/import_export/project_tree_restorer.rb
@@ -178,7 +178,7 @@ module Gitlab
       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: parsed_relation_hash(relation_hash, relation.to_sym),
+                                                       relation_hash: relation_hash,
                                                        members_mapper: members_mapper,
                                                        user: @user,
                                                        project: @restored_project,
@@ -188,12 +188,6 @@ module Gitlab
         relation_hash_list.is_a?(Array) ? relation_array : relation_array.first
       end
 
-      def parsed_relation_hash(relation_hash, relation_type)
-        params = { 'group_id' => restored_project.group.try(:id), 'project_id' => restored_project.id }
-
-        relation_hash.merge(params)
-      end
-
       def reader
         @reader ||= Gitlab::ImportExport::Reader.new(shared: @shared)
       end
diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb
index c5e8aa54f5d..f5a20a4d333 100644
--- a/lib/gitlab/import_export/relation_factory.rb
+++ b/lib/gitlab/import_export/relation_factory.rb
@@ -54,6 +54,8 @@ module Gitlab
         @project = project
         @imported_object_retries = 0
 
+        @relation_hash['project_id'] = @project.id
+
         # Remove excluded keys from relation_hash
         # We don't do this in the parsed_relation_hash because of the 'transformed attributes'
         # For example, MergeRequestDiffFiles exports its diff attribute as utf8_diff. Then,
@@ -81,12 +83,11 @@ module Gitlab
         when :merge_request_diff_files       then setup_diff
         when :notes                          then setup_note
         when 'Ci::Pipeline'                  then setup_pipeline
-        else
-          @relation_hash['project_id'] = @project.id
         end
 
         update_user_references
         update_project_references
+        update_group_references
         remove_duplicate_assignees
 
         reset_tokens!
@@ -161,6 +162,13 @@ module Gitlab
         @relation_hash['target_project_id'] = project_id if @relation_hash['target_project_id']
       end
 
+      def update_group_references
+        return unless EXISTING_OBJECT_CHECK.include?(@relation_name)
+        return unless @relation_hash['group_id']
+
+        @relation_hash['group_id'] = @project.group&.id
+      end
+
       def same_source_and_target?
         @relation_hash['target_project_id'] && @relation_hash['target_project_id'] == @relation_hash['source_project_id']
       end
diff --git a/spec/lib/gitlab/import_export/project.milestone-iid.json b/spec/lib/gitlab/import_export/project.milestone-iid.json
new file mode 100644
index 00000000000..b028147b5eb
--- /dev/null
+++ b/spec/lib/gitlab/import_export/project.milestone-iid.json
@@ -0,0 +1,80 @@
+{
+  "description": "Nisi et repellendus ut enim quo accusamus vel magnam.",
+  "import_type": "gitlab_project",
+  "creator_id": 123,
+  "visibility_level": 10,
+  "archived": false,
+  "issues": [
+    {
+      "id": 1,
+      "title": "Fugiat est minima quae maxime non similique.",
+      "assignee_id": null,
+      "project_id": 8,
+      "author_id": 1,
+      "created_at": "2017-07-07T18:13:01.138Z",
+      "updated_at": "2017-08-15T18:37:40.807Z",
+      "branch_name": null,
+      "description": "Quam totam fuga numquam in eveniet.",
+      "state": "opened",
+      "iid": 20,
+      "updated_by_id": 1,
+      "confidential": false,
+      "due_date": null,
+      "moved_to_id": null,
+      "lock_version": null,
+      "time_estimate": 0,
+      "closed_at": null,
+      "last_edited_at": null,
+      "last_edited_by_id": null,
+      "group_milestone_id": null,
+      "milestone": {
+        "id": 1,
+        "title": "Group-level milestone",
+        "description": "Group-level milestone",
+        "due_date": null,
+        "created_at": "2016-06-14T15:02:04.415Z",
+        "updated_at": "2016-06-14T15:02:04.415Z",
+        "state": "active",
+        "iid": 1,
+        "group_id": 8
+      }
+    },
+    {
+      "id": 2,
+      "title": "est minima quae maxime non similique.",
+      "assignee_id": null,
+      "project_id": 8,
+      "author_id": 1,
+      "created_at": "2017-07-07T18:13:01.138Z",
+      "updated_at": "2017-08-15T18:37:40.807Z",
+      "branch_name": null,
+      "description": "Quam totam fuga numquam in eveniet.",
+      "state": "opened",
+      "iid": 21,
+      "updated_by_id": 1,
+      "confidential": false,
+      "due_date": null,
+      "moved_to_id": null,
+      "lock_version": null,
+      "time_estimate": 0,
+      "closed_at": null,
+      "last_edited_at": null,
+      "last_edited_by_id": null,
+      "group_milestone_id": null,
+      "milestone": {
+        "id": 2,
+        "title": "Another milestone",
+        "project_id": 8,
+        "description": "milestone",
+        "due_date": null,
+        "created_at": "2016-06-14T15:02:04.415Z",
+        "updated_at": "2016-06-14T15:02:04.415Z",
+        "state": "active",
+        "iid": 1,
+        "group_id": null
+      }
+    }
+  ],
+  "snippets": [],
+  "hooks": []
+}
diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
index 94251e2280b..1601b913b91 100644
--- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
@@ -391,5 +391,32 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
         expect(project.milestones.count).to eq(0)
       end
     end
+
+    context 'with clashing milestones on IID' do
+      let!(:project) do
+        create(:project,
+               :builds_disabled,
+               :issues_disabled,
+               name: 'project',
+               path: 'project',
+               group: create(:group))
+      end
+
+      before do
+        project_tree_restorer.instance_variable_set(:@path, "spec/lib/gitlab/import_export/project.milestone-iid.json")
+      end
+
+      it 'preserves the project milestone IID' do
+        create(:milestone, name: 'A milestone', group: project.group)
+
+        expect_any_instance_of(Gitlab::ImportExport::Shared).not_to receive(:error)
+
+        restored_project_json
+
+        expect(project.milestones.count).to eq(2)
+        expect(Milestone.find_by_title('Another milestone').iid).to eq(1)
+        expect(Milestone.find_by_title('Group-level milestone').iid).to eq(2)
+      end
+    end
   end
 end