Merge branch '35580-cannot-import-project-with-milestones' into 'master'

fix the import :milestone from adding the group_id

Closes #35580

See merge request gitlab-org/gitlab-ce!14657

changelogs/unreleased/35580-cannot-import-project-with-milestones.yml

+---
+title: Fix the project import with issues and milestones
+merge_request: 14657
+author:
+type: fixed

lib/gitlab/import_export/project_tree_restorer.rb

@@ -2,7 +2,7 @@ module Gitlab
   module ImportExport
     class ProjectTreeRestorer
       # Relations which cannot have both group_id and project_id at the same time
-      RESTRICT_PROJECT_AND_GROUP = %i(milestones).freeze
+      RESTRICT_PROJECT_AND_GROUP = %i(milestone milestones).freeze
 
       def initialize(user:, shared:, project:)
         @path = File.join(shared.export_path, 'project.json')

lib/gitlab/import_export/relation_factory.rb

@@ -37,7 +37,7 @@ module Gitlab
 
       def initialize(relation_sym:, relation_hash:, members_mapper:, user:, project:)
         @relation_name = OVERRIDES[relation_sym] || relation_sym
-        @relation_hash = relation_hash.except('noteable_id').merge('project_id' => project.id)
+        @relation_hash = relation_hash.except('noteable_id')
         @members_mapper = members_mapper
         @user = user
         @project = project
@@ -58,22 +58,21 @@ module Gitlab
       private
 
       def setup_models
-        if @relation_name == :notes
-          set_note_author
-
-          # attachment is deprecated and note uploads are handled by Markdown uploader
-          @relation_hash['attachment'] = nil
+        case @relation_name
+        when :merge_request_diff             then setup_st_diff_commits
+        when :merge_request_diff_files       then setup_diff
+        when :notes                          then setup_note
+        when :project_label, :project_labels then setup_label
+        when :milestone, :milestones         then setup_milestone
+        else
+          @relation_hash['project_id'] = @project.id
         end
 
         update_user_references
         update_project_references
 
-        handle_group_label if group_label?
         reset_tokens!
         remove_encrypted_attributes!
-
-        set_st_diff_commits if @relation_name == :merge_request_diff
-        set_diff if @relation_name == :merge_request_diff_files
       end
 
       def update_user_references
@@ -84,6 +83,12 @@ module Gitlab
         end
       end
 
+      def setup_note
+        set_note_author
+        # attachment is deprecated and note uploads are handled by Markdown uploader
+        @relation_hash['attachment'] = nil
+      end
+
       # Sets the author for a note. If the user importing the project
       # has admin access, an actual mapping with new project members
       # will be used. Otherwise, a note stating the original author name
@@ -136,11 +141,9 @@ module Gitlab
         @relation_hash['target_project_id'] && @relation_hash['target_project_id'] == @relation_hash['source_project_id']
       end
 
-      def group_label?
-        @relation_hash['type'] == 'GroupLabel'
-      end
+      def setup_label
+        return unless @relation_hash['type'] == 'GroupLabel'
 
-      def handle_group_label
         # If there's no group, move the label to a project label
         if @relation_hash['group_id']
           @relation_hash['project_id'] = nil
@@ -150,6 +153,14 @@ module Gitlab
         end
       end
 
+      def setup_milestone
+        if @relation_hash['group_id']
+          @relation_hash['group_id'] = @project.group.id
+        else
+          @relation_hash['project_id'] = @project.id
+        end
+      end
+
       def reset_tokens!
         return unless Gitlab::ImportExport.reset_tokens? && TOKEN_RESET_MODELS.include?(@relation_name.to_s)
 
@@ -198,14 +209,14 @@ module Gitlab
                                                                                relation_class: relation_class)
       end
 
-      def set_st_diff_commits
+      def setup_st_diff_commits
         @relation_hash['st_diffs'] = @relation_hash.delete('utf8_st_diffs')
 
         HashUtil.deep_symbolize_array!(@relation_hash['st_diffs'])
         HashUtil.deep_symbolize_array_with_date!(@relation_hash['st_commits'])
       end
 
-      def set_diff
+      def setup_diff
         @relation_hash['diff'] = @relation_hash.delete('utf8_diff')
       end
 
@@ -250,7 +261,13 @@ module Gitlab
       end
 
       def find_or_create_object!
-        finder_attributes = @relation_name == :group_label ? %w[title group_id] : %w[title project_id]
+        finder_attributes = if @relation_name == :group_label
+                              %w[title group_id]
+                            elsif parsed_relation_hash['project_id']
+                              %w[title project_id]
+                            else
+                              %w[title group_id]
+                            end
         finder_hash = parsed_relation_hash.slice(*finder_attributes)
 
         if label?

spec/lib/gitlab/import_export/project.group.json

+{
+  "description": "Nisi et repellendus ut enim quo accusamus vel magnam.",
+  "visibility_level": 10,
+  "archived": false,
+  "milestones": [
+    {
+      "id": 1,
+      "title": "Project milestone",
+      "project_id": 8,
+      "description": "Project-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": null
+    }
+  ],
+  "labels": [
+    {
+      "id": 2,
+      "title": "project label",
+      "color": "#428bca",
+      "project_id": 8,
+      "created_at": "2016-07-22T08:55:44.161Z",
+      "updated_at": "2016-07-22T08:55:44.161Z",
+      "template": false,
+      "description": "",
+      "type": "ProjectLabel",
+      "priorities": [
+        {
+          "id": 1,
+          "project_id": 5,
+          "label_id": 1,
+          "priority": 1,
+          "created_at": "2016-10-18T09:35:43.338Z",
+          "updated_at": "2016-10-18T09:35:43.338Z"
+        }
+      ]
+    }
+  ],
+  "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": 1,
+      "updated_by_id": 1,
+      "confidential": false,
+      "deleted_at": null,
+      "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": "Project milestone",
+        "project_id": 8,
+        "description": "Project-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": null
+      },
+      "label_links": [
+        {
+          "id": 11,
+          "label_id": 6,
+          "target_id": 1,
+          "target_type": "Issue",
+          "created_at": "2017-08-15T18:37:40.795Z",
+          "updated_at": "2017-08-15T18:37:40.795Z",
+          "label": {
+            "id": 6,
+            "title": "group label",
+            "color": "#A8D695",
+            "project_id": null,
+            "created_at": "2017-08-15T18:37:19.698Z",
+            "updated_at": "2017-08-15T18:37:19.698Z",
+            "template": false,
+            "description": "",
+            "group_id": 5,
+            "type": "GroupLabel",
+            "priorities": []
+          }
+        },
+        {
+          "id": 11,
+          "label_id": 2,
+          "target_id": 1,
+          "target_type": "Issue",
+          "created_at": "2017-08-15T18:37:40.795Z",
+          "updated_at": "2017-08-15T18:37:40.795Z",
+          "label": {
+            "id": 6,
+            "title": "project label",
+            "color": "#A8D695",
+            "project_id": null,
+            "created_at": "2017-08-15T18:37:19.698Z",
+            "updated_at": "2017-08-15T18:37:19.698Z",
+            "template": false,
+            "description": "",
+            "group_id": 5,
+            "type": "ProjectLabel",
+            "priorities": []
+          }
+        }
+      ]
+    },
+    {
+      "id": 2,
+      "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": 2,
+      "updated_by_id": 1,
+      "confidential": false,
+      "deleted_at": null,
+      "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": "A group 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": 100
+      },
+      "label_links": [
+        {
+          "id": 11,
+          "label_id": 2,
+          "target_id": 1,
+          "target_type": "Issue",
+          "created_at": "2017-08-15T18:37:40.795Z",
+          "updated_at": "2017-08-15T18:37:40.795Z",
+          "label": {
+            "id": 2,
+            "title": "project label",
+            "color": "#A8D695",
+            "project_id": null,
+            "created_at": "2017-08-15T18:37:19.698Z",
+            "updated_at": "2017-08-15T18:37:19.698Z",
+            "template": false,
+            "description": "",
+            "group_id": 5,
+            "type": "ProjectLabel",
+            "priorities": []
+          }
+        }
+      ]
+    }
+  ],
+  "snippets": [
+
+  ],
+  "hooks": [
+
+  ]
+}

spec/lib/gitlab/import_export/project.light.json

@@ -5,9 +5,9 @@
   "milestones": [
     {
       "id": 1,
-      "title": "test milestone",
+      "title": "Project milestone",
       "project_id": 8,
-      "description": "test milestone",
+      "description": "Project-level milestone",
       "due_date": null,
       "created_at": "2016-06-14T15:02:04.415Z",
       "updated_at": "2016-06-14T15:02:04.415Z",
@@ -19,7 +19,7 @@
   "labels": [
     {
       "id": 2,
-      "title": "test2",
+      "title": "A project label",
       "color": "#428bca",
       "project_id": 8,
       "created_at": "2016-07-22T08:55:44.161Z",
@@ -63,28 +63,19 @@
       "last_edited_at": null,
       "last_edited_by_id": null,
       "group_milestone_id": null,
+      "milestone": {
+        "id": 1,
+        "title": "Project milestone",
+        "project_id": 8,
+        "description": "Project-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": null
+      },
       "label_links": [
-        {
-          "id": 11,
-          "label_id": 6,
-          "target_id": 1,
-          "target_type": "Issue",
-          "created_at": "2017-08-15T18:37:40.795Z",
-          "updated_at": "2017-08-15T18:37:40.795Z",
-          "label": {
-            "id": 6,
-            "title": "group label",
-            "color": "#A8D695",
-            "project_id": null,
-            "created_at": "2017-08-15T18:37:19.698Z",
-            "updated_at": "2017-08-15T18:37:19.698Z",
-            "template": false,
-            "description": "",
-            "group_id": 5,
-            "type": "GroupLabel",
-            "priorities": []
-          }
-        },
         {
           "id": 11,
           "label_id": 2,
@@ -94,14 +85,14 @@
           "updated_at": "2017-08-15T18:37:40.795Z",
           "label": {
             "id": 6,
-            "title": "project label",
+            "title": "Another project label",
             "color": "#A8D695",
             "project_id": null,
             "created_at": "2017-08-15T18:37:19.698Z",
             "updated_at": "2017-08-15T18:37:19.698Z",
             "template": false,
             "description": "",
-            "group_id": 5,
+            "group_id": null,
             "type": "ProjectLabel",
             "priorities": []
           }
@@ -109,10 +100,6 @@
       ]
     }
   ],
-  "snippets": [
-
-  ],
-  "hooks": [
-
-  ]
+  "snippets": [],
+  "hooks": []
 }

spec/lib/gitlab/import_export/project_tree_restorer_spec.rb

@@ -24,7 +24,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
 
     context 'JSON' do
       it 'restores models based on JSON' do
-        expect(@restored_project_json).to be true
+        expect(@restored_project_json).to be_truthy
       end
 
       it 'restore correct project features' do
@@ -182,6 +182,53 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
     end
   end
 
+  shared_examples 'restores project successfully' do
+    it 'correctly restores project' do
+      expect(shared.errors).to be_empty
+      expect(restored_project_json).to be_truthy
+    end
+  end
+
+  shared_examples 'restores project correctly' do |**results|
+    it 'has labels' do
+      expect(project.labels.size).to eq(results.fetch(:labels, 0))
+    end
+
+    it 'has label priorities' do
+      expect(project.labels.first.priorities).not_to be_empty
+    end
+
+    it 'has milestones' do
+      expect(project.milestones.size).to eq(results.fetch(:milestones, 0))
+    end
+
+    it 'has issues' do
+      expect(project.issues.size).to eq(results.fetch(:issues, 0))
+    end
+
+    it 'has issue with group label and project label' do
+      labels = project.issues.first.labels
+
+      expect(labels.where(type: "ProjectLabel").count).to eq(results.fetch(:first_issue_labels, 0))
+    end
+  end
+
+  shared_examples 'restores group correctly' do |**results|
+    it 'has group label' do
+      expect(project.group.labels.size).to eq(results.fetch(:labels, 0))
+    end
+
+    it 'has group milestone' do
+      expect(project.group.milestones.size).to eq(results.fetch(:milestones, 0))
+    end
+
+    it 'has issue with group label' do
+      labels = project.issues.first.labels
+
+      expect(labels.where(type: "GroupLabel").count).to eq(results.fetch(:first_issue_labels, 0))
+    end
+  end
+
   context 'Light JSON' do
     let(:user) { create(:user) }
     let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: "", project_path: 'path') }
@@ -190,33 +237,45 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
     let(:restored_project_json) { project_tree_restorer.restore }
 
     before do
-      project_tree_restorer.instance_variable_set(:@path, "spec/lib/gitlab/import_export/project.light.json")
-
       allow(shared).to receive(:export_path).and_return('spec/lib/gitlab/import_export/')
     end
 
-    context 'project.json file access check' do
-      it 'does not read a symlink' do
-        Dir.mktmpdir do |tmpdir|
-          setup_symlink(tmpdir, 'project.json')
-          allow(shared).to receive(:export_path).and_call_original
+    context 'with a simple project' do
+      before do
+        project_tree_restorer.instance_variable_set(:@path, "spec/lib/gitlab/import_export/project.light.json")
+
+        restored_project_json
+      end
+
+      it_behaves_like 'restores project correctly',
+                      issues: 1,
+                      labels: 1,
+                      milestones: 1,
+                      first_issue_labels: 1
 
-          restored_project_json
+      context 'project.json file access check' do
+        it 'does not read a symlink' do
+          Dir.mktmpdir do |tmpdir|
+            setup_symlink(tmpdir, 'project.json')
+            allow(shared).to receive(:export_path).and_call_original
 
-          expect(shared.errors.first).to be_nil
+            restored_project_json
+
+            expect(shared.errors).to be_empty
+          end
         end
       end
-    end
 
-    context 'when there is an existing build with build token' do
-      it 'restores project json correctly' do
-        create(:ci_build, token: 'abcd')
+      context 'when there is an existing build with build token' do
+        before do
+          create(:ci_build, token: 'abcd')
+        end
 
-        expect(restored_project_json).to be true
+        it_behaves_like 'restores project successfully'
       end
     end
 
-    context 'with group' do
+    context 'with a project that has a group' do
       let!(:project) do
         create(:project,
                :builds_disabled,
@@ -227,43 +286,22 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
       end
 
       before do
-        project_tree_restorer.instance_variable_set(:@path, "spec/lib/gitlab/import_export/project.light.json")
+        project_tree_restorer.instance_variable_set(:@path, "spec/lib/gitlab/import_export/project.group.json")
 
         restored_project_json
       end
 
-      it 'correctly restores project' do
-        expect(restored_project_json).to be_truthy
-        expect(shared.errors).to be_empty
-      end
+      it_behaves_like 'restores project successfully'
+      it_behaves_like 'restores project correctly',
+                      issues: 2,
+                      labels: 1,
+                      milestones: 1,
+                      first_issue_labels: 1
 
-      it 'has labels' do
-        expect(project.labels.count).to eq(2)
-      end
-
-      it 'creates group label' do
-        expect(project.group.labels.count).to eq(1)
-      end
-
-      it 'has label priorities' do
-        expect(project.labels.first.priorities).not_to be_empty
-      end
-
-      it 'has milestones' do
-        expect(project.milestones.count).to eq(1)
-      end
-
-      it 'has issue' do
-        expect(project.issues.count).to eq(1)
-        expect(project.issues.first.labels.count).to eq(2)
-      end
-
-      it 'has issue with group label and project label' do
-        labels = project.issues.first.labels
-
-        expect(labels.where(type: "GroupLabel").count).to eq(1)
-        expect(labels.where(type: "ProjectLabel").count).to eq(1)
-      end
+      it_behaves_like 'restores group correctly',
+                      labels: 1,
+                      milestones: 1,
+                      first_issue_labels: 1
     end
   end
 end