Merge branch '42696-gitlab-import-leaves-group_id-on-projectlabel' into 'master'

Resolve "GitLab import leaves `group_id` on ProjectLabel"

Closes #42696

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

6 files changed, 50 insertions, 4 deletions

diff --git a/changelogs/unreleased/42696-gitlab-import-leaves-group_id-on-projectlabel.yml b/changelogs/unreleased/42696-gitlab-import-leaves-group_id-on-projectlabel.yml
new file mode 100644
index 00000000000..c46cc86c99b
--- /dev/null
+++ b/changelogs/unreleased/42696-gitlab-import-leaves-group_id-on-projectlabel.yml
@@ -0,0 +1,5 @@
+---
+title: Fix GitLab import leaving group_id on ProjectLabel
+merge_request: 16877
+author:
+type: fixed
diff --git a/db/post_migrate/20180202111106_remove_project_labels_group_id.rb b/db/post_migrate/20180202111106_remove_project_labels_group_id.rb
new file mode 100644
index 00000000000..db7fd0d167d
--- /dev/null
+++ b/db/post_migrate/20180202111106_remove_project_labels_group_id.rb
@@ -0,0 +1,19 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RemoveProjectLabelsGroupId < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    update_column_in_batches(:labels, :group_id, nil) do |table, query|
+      query.where(table[:type].eq('ProjectLabel').and(table[:group_id].not_eq(nil)))
+    end
+  end
+
+  def down
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 01a2df13dd3..f72989ff391 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20180201145907) do
+ActiveRecord::Schema.define(version: 20180202111106) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb
index cb711a83433..759833a5ee5 100644
--- a/lib/gitlab/import_export/relation_factory.rb
+++ b/lib/gitlab/import_export/relation_factory.rb
@@ -139,13 +139,12 @@ module Gitlab
       end
 
       def setup_label
-        return unless @relation_hash['type'] == 'GroupLabel'
-
         # If there's no group, move the label to a project label
-        if @relation_hash['group_id']
+        if @relation_hash['type'] == 'GroupLabel' && @relation_hash['group_id']
           @relation_hash['project_id'] = nil
           @relation_name = :group_label
         else
+          @relation_hash['group_id'] = nil
           @relation_hash['type'] = 'ProjectLabel'
         end
       end
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 9dfd879a1bc..d076007e4bc 100644
--- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
@@ -236,12 +236,14 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
       labels = project.issues.first.labels
 
       expect(labels.where(type: "ProjectLabel").count).to eq(results.fetch(:first_issue_labels, 0))
+      expect(labels.where(type: "ProjectLabel").where.not(group_id: nil).count).to eq(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))
+      expect(project.group.labels.where(type: "GroupLabel").where.not(project_id: nil).count).to eq(0)
     end
 
     it 'has group milestone' do
diff --git a/spec/migrations/remove_project_labels_group_id_spec.rb b/spec/migrations/remove_project_labels_group_id_spec.rb
new file mode 100644
index 00000000000..d80d61af20b
--- /dev/null
+++ b/spec/migrations/remove_project_labels_group_id_spec.rb
@@ -0,0 +1,21 @@
+# encoding: utf-8
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20180202111106_remove_project_labels_group_id.rb')
+
+describe RemoveProjectLabelsGroupId, :delete do
+  let(:migration) { described_class.new }
+  let(:group) { create(:group) }
+  let!(:project_label) { create(:label, group_id: group.id) }
+  let!(:group_label) { create(:group_label) }
+
+  describe '#up' do
+    it 'updates the project labels group ID' do
+      expect { migration.up }.to change { project_label.reload.group_id }.to(nil)
+    end
+
+    it 'keeps the group labels group ID' do
+      expect { migration.up }.not_to change { group_label.reload.group_id }
+    end
+  end
+end