Ignore predefined relations on Import

Currently our Milestone model has some predefined instances that are not
actually persisted to the database (only their ID is persisted on the
related model).

We don't want to import these as actual relations in the database as
their predefined IDs imply some additional functionality in the system -
which is lost if they are imported as a new record.

For now, ignore these records on import as we do not have a clear way to
handle them - but we still need to import the user generated milestones.

app/models/milestone.rb

@@ -134,11 +134,12 @@ class Milestone < ApplicationRecord
       reorder(nil).group(:state).count
     end
 
+    def predefined_id?(id)
+      [Any.id, None.id, Upcoming.id, Started.id].include?(id)
+    end
+
     def predefined?(milestone)
-      milestone == Any ||
-        milestone == None ||
-        milestone == Upcoming ||
-        milestone == Started
+      predefined_id?(milestone&.id)
     end
   end
 

ee/spec/fixtures/lib/gitlab/import_export/group_exports/light/group.json

@@ -87,7 +87,8 @@
       "created_at": "2019-11-20T17:27:41.118Z",
       "updated_at": "2019-11-20T17:27:41.118Z",
       "name": "first board",
-      "milestone_id": 7638,
+      "milestone_id": -2,
+      "milestone": {"id": -2, "name":  "#upcoming", "title":  "Upcoming"},
       "group_id": 4351,
       "weight": null,
       "labels": [],
@@ -153,6 +154,31 @@
           }
         }
       ]
+    },
+    {
+      "id": 57,
+      "project_id": null,
+      "created_at": "2019-11-20T17:27:41.118Z",
+      "updated_at": "2019-11-20T17:27:41.118Z",
+      "name": "second board",
+      "milestone_id": 7642,
+      "milestone": {
+        "id": 7642,
+        "title": "v4.0",
+        "project_id": null,
+        "description": "Et laudantium enim omnis ea reprehenderit iure.",
+        "due_date": null,
+        "created_at": "2019-11-20T17:02:14.336Z",
+        "updated_at": "2019-11-20T17:02:14.336Z",
+        "state": "closed",
+        "iid": 5,
+        "start_date": null,
+        "group_id": 4351
+      },
+      "group_id": 4351,
+      "weight": null,
+      "labels": [],
+      "lists": []
     }
   ],
   "members": [

ee/spec/lib/ee/gitlab/import_export/group/tree_restorer_spec.rb

@@ -46,5 +46,17 @@ describe Gitlab::ImportExport::Group::TreeRestorer do
         expect(lists.map(&:list_type)).to contain_exactly('assignee', 'milestone')
       end
     end
+
+    context 'boards' do
+      it 'has user generated milestones' do
+        board = group.boards.find_by(name: 'second board')
+        expect(board.milestone.title).to eq 'v4.0'
+      end
+
+      it 'does not have predefined milestones' do
+        board = group.boards.find_by(name: 'first board')
+        expect(board.milestone).to be_nil
+      end
+    end
   end
 end

lib/gitlab/import_export/base/relation_factory.rb

@@ -67,7 +67,7 @@ module Gitlab
         # the relation_hash, updating references with new object IDs, mapping users using
         # the "members_mapper" object, also updating notes if required.
         def create
-          return if invalid_relation?
+          return if invalid_relation? || predefined_relation?
 
           setup_base_models
           setup_models
@@ -89,6 +89,10 @@ module Gitlab
           false
         end
 
+        def predefined_relation?
+          relation_class.try(:predefined_id?, @relation_hash['id'])
+        end
+
         def setup_models
           raise NotImplementedError
         end

spec/lib/gitlab/import_export/base/relation_factory_spec.rb

@@ -33,6 +33,15 @@ describe Gitlab::ImportExport::Base::RelationFactory do
       end
     end
 
+    context 'when the relation is predefined' do
+      let(:relation_sym) { :milestone }
+      let(:relation_hash) { { 'name' => '#upcoming', 'title' => 'Upcoming', 'id' => -2 } }
+
+      it 'returns without creating a new relation' do
+        expect(subject).to be_nil
+      end
+    end
+
     context 'when #setup_models is not implemented' do
       it 'raises NotImplementedError' do
         expect { subject }.to raise_error(NotImplementedError)

spec/models/milestone_spec.rb

@@ -191,6 +191,16 @@ describe Milestone do
     end
   end
 
+  describe '.predefined_id?' do
+    it 'returns true for a predefined Milestone ID' do
+      expect(Milestone.predefined_id?(described_class::Upcoming.id)).to be true
+    end
+
+    it 'returns false for a Milestone ID that is not predefined' do
+      expect(Milestone.predefined_id?(milestone.id)).to be false
+    end
+  end
+
   describe '.order_by_name_asc' do
     it 'sorts by name ascending' do
       milestone1 = create(:milestone, title: 'Foo')