update code based on feedback

lib/gitlab/import_export/group_project_object_builder.rb

@@ -19,46 +19,35 @@ module Gitlab
       def initialize(klass, attributes)
         @klass = klass < Label ? Label : klass
         @attributes = attributes
-        @group = @attributes[:group]
+        @group = @attributes['group']
-        @project = @attributes[:project]
+        @project = @attributes['project']
       end
 
       def find
-        find_or_action do
+        find_object || @klass.create(project_attributes)
-          label? ? @klass.new(project_attributes) : @klass.create(project_attributes)
-        end
       end
 
       private
 
-      def find_or_action(&block)
+      def find_object
-        @klass.where(where_clause).first || yield
+        @klass.where(where_clause).first
       end
 
       def where_clause
         @attributes.slice('title').map do |key, value|
-          if @group
+          scope_clause = table[:project_id].eq(@project.id)
-            project_group_clause(key, value)
+          scope_clause = scope_clause.or(table[:group_id].eq(@group.id)) if @group
-          else
+
-            project_clause(key, value)
+          table[key].eq(value).and(scope_clause)
-          end
         end.reduce(:or)
       end
 
-      def project_group_clause(key, value)
-        table[key].eq(value).and(table[:project_id].eq(@project.id).or(table[:group_id].eq(@group.id)))
-      end
-
-      def project_clause(key, value)
-        table[key].eq(value).and(table[:project_id].eq(@project.id))
-      end
-
       def table
         @table ||= @klass.arel_table
       end
 
       def project_attributes
-        @attributes.except(:group).tap do |atts|
+        @attributes.except('group').tap do |atts|
           if label?
             atts['type'] = 'ProjectLabel' # Always create project labels
           elsif milestone?
@@ -80,27 +69,21 @@ module Gitlab
         @klass == Milestone
       end
 
-      # If an existing group milesone used the IID
+      # If an existing group milestone used the IID
       # claim the IID back and set the group milestone to use one available
-      # This is neccessary to fix situations like the following:
+      # This is necessary to fix situations like the following:
       #  - Importing into a user namespace project with exported group milestones
       #    where the IID of the Group milestone could conflict with a project one.
       def claim_iid
         # The milestone has to be a group milestone, as it's the only case where
         # we set the IID as the maximum. The rest of them are fixed.
-        group_milestone = @project.milestones.find_by(iid: @attributes['iid'])
+        milestone = @project.milestones.find_by(iid: @attributes['iid'])
 
-        group_milestone.update!(iid:  max_milestone_iid(group_milestone)) if group_milestone
+        return unless milestone
-      end
 
-      def max_milestone_iid(group_milestone)
+        milestone.iid = nil
-        init_iid = [@attributes['iid'], @project.milestones.maximum(:iid)].compact.max + 1
+        milestone.ensure_project_iid!
-
+        milestone.save!
-        InternalId::InternalIdGenerator.new(group_milestone,
-                                            { project: @project },
-                                            :milestones,
-                                            init_iid
-                                           ).generate
       end
     end
   end

lib/gitlab/import_export/relation_factory.rb

@@ -267,8 +267,8 @@ module Gitlab
 
         # Can't use IDs as validation exists calling `group` or `project` attributes
         finder_hash = parsed_relation_hash.tap do |hash|
-          hash[:group] = @project.group if relation_class.attribute_method?('group_id')
+          hash['group'] = @project.group if relation_class.attribute_method?('group_id')
-          hash[:project] = @project
+          hash['project'] = @project
           hash.delete('project_id')
         end
 

spec/lib/gitlab/import_export/group_project_object_builder_spec.rb

@@ -15,18 +15,18 @@ describe Gitlab::ImportExport::GroupProjectObjectBuilder do
       group_label = create(:group_label, 'name': 'group label', 'group': project.group)
 
       expect(described_class.build(Label,
-                                  title: 'group label',
+                                  'title' => 'group label',
-                                  project: project,
+                                  'project' => project,
-                                  group: project.group)).to eq(group_label)
+                                  'group' => project.group)).to eq(group_label)
     end
 
-    it 'initializes a new label' do
+    it 'creates a new label' do
       label = described_class.build(Label,
-                                   title: 'group label',
+                                   'title' => 'group label',
-                                   project: project,
+                                   'project' => project,
-                                   group: project.group)
+                                   'group' => project.group)
 
-      expect(label.persisted?).to be false
+      expect(label.persisted?).to be true
     end
   end
 
@@ -35,16 +35,16 @@ describe Gitlab::ImportExport::GroupProjectObjectBuilder do
       milestone = create(:milestone, 'name' => 'group milestone', 'group' => project.group)
 
       expect(described_class.build(Milestone,
-                                  title: 'group milestone',
+                                  'title' => 'group milestone',
-                                  project: project,
+                                  'project' => project,
-                                  group: project.group)).to eq(milestone)
+                                  'group' => project.group)).to eq(milestone)
     end
 
     it 'creates a new milestone' do
       milestone = described_class.build(Milestone,
-                                       title: 'group milestone',
+                                       'title' => 'group milestone',
-                                       project: project,
+                                       'project' => project,
-                                       group: project.group)
+                                       'group' => project.group)
 
       expect(milestone.persisted?).to be true
     end