Merge branch 'bvl-import-zip-multiple-assignees' into 'master'

Import multiple issue assignees from GitLab export

Closes #40275

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

4 files changed, 42 insertions, 15 deletions

diff --git a/changelogs/unreleased/bvl-import-zip-multiple-assignees.yml b/changelogs/unreleased/bvl-import-zip-multiple-assignees.yml
new file mode 100644
index 00000000000..86bd5faf8ed
--- /dev/null
+++ b/changelogs/unreleased/bvl-import-zip-multiple-assignees.yml
@@ -0,0 +1,5 @@
+---
+title: Fix importing multiple assignees from GitLab export
+merge_request: 17718
+author:
+type: fixed
diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb
index cf6b7e306dd..791a54e1b69 100644
--- a/lib/gitlab/import_export/relation_factory.rb
+++ b/lib/gitlab/import_export/relation_factory.rb
@@ -70,6 +70,7 @@ module Gitlab
 
         update_user_references
         update_project_references
+        remove_duplicate_assignees
 
         reset_tokens!
         remove_encrypted_attributes!
@@ -83,6 +84,14 @@ module Gitlab
         end
       end
 
+      def remove_duplicate_assignees
+        return unless @relation_hash['issue_assignees']
+
+        # When an assignee did not exist in the members mapper, the importer is
+        # assigned. We only need to assign each user once.
+        @relation_hash['issue_assignees'].uniq!(&:user_id)
+      end
+
       def setup_note
         set_note_author
         # attachment is deprecated and note uploads are handled by Markdown uploader
diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json
index 62ef93f847a..4a51777ba9b 100644
--- a/spec/lib/gitlab/import_export/project.json
+++ b/spec/lib/gitlab/import_export/project.json
@@ -43,7 +43,6 @@
     {
       "id": 40,
       "title": "Voluptatem",
-      "assignee_id": 1,
       "author_id": 22,
       "project_id": 5,
       "created_at": "2016-06-14T15:02:08.340Z",
@@ -61,7 +60,23 @@
       "issue_assignees": [
         {
           "user_id": 1,
-          "issue_id": 1
+          "issue_id": 40
+        },
+        {
+          "user_id": 15,
+          "issue_id": 40
+        },
+        {
+          "user_id": 16,
+          "issue_id": 40
+        },
+        {
+          "user_id": 16,
+          "issue_id": 40
+        },
+        {
+          "user_id": 6,
+          "issue_id": 40
         }
       ],
       "milestone": {
@@ -319,8 +334,7 @@
     },
     {
       "id": 39,
-      "title": "Delectus veniam ratione in eos culpa et natus molestiae earum aut.",
-      "assignee_id": 20,
+      "title": "Issue without assignees",
       "author_id": 22,
       "project_id": 5,
       "created_at": "2016-06-14T15:02:08.233Z",
@@ -334,6 +348,7 @@
       "confidential": false,
       "due_date": null,
       "moved_to_id": null,
+      "issue_assignees": [],
       "milestone": {
         "id": 1,
         "title": "test milestone",
@@ -539,7 +554,6 @@
     {
       "id": 38,
       "title": "Quasi adipisci non cupiditate dolorem quo qui earum sed.",
-      "assignee_id": 1,
       "author_id": 6,
       "project_id": 5,
       "created_at": "2016-06-14T15:02:08.154Z",
@@ -756,7 +770,6 @@
     {
       "id": 37,
       "title": "Cupiditate quo aut ducimus minima molestiae vero numquam possimus.",
-      "assignee_id": 15,
       "author_id": 20,
       "project_id": 5,
       "created_at": "2016-06-14T15:02:08.051Z",
@@ -952,7 +965,6 @@
     {
       "id": 36,
       "title": "Necessitatibus dolor est enim quia rem suscipit quidem voluptas ullam.",
-      "assignee_id": 20,
       "author_id": 16,
       "project_id": 5,
       "created_at": "2016-06-14T15:02:07.958Z",
@@ -1148,7 +1160,6 @@
     {
       "id": 35,
       "title": "Repellat praesentium deserunt maxime incidunt harum porro qui.",
-      "assignee_id": 6,
       "author_id": 20,
       "project_id": 5,
       "created_at": "2016-06-14T15:02:07.832Z",
@@ -1344,7 +1355,6 @@
     {
       "id": 34,
       "title": "Ullam expedita deserunt libero consequatur quia dolor harum perferendis facere quidem.",
-      "assignee_id": 20,
       "author_id": 1,
       "project_id": 5,
       "created_at": "2016-06-14T15:02:07.717Z",
@@ -1540,7 +1550,6 @@
     {
       "id": 33,
       "title": "Numquam accusamus eos iste exercitationem magni non inventore.",
-      "assignee_id": 15,
       "author_id": 26,
       "project_id": 5,
       "created_at": "2016-06-14T15:02:07.611Z",
@@ -1736,7 +1745,6 @@
     {
       "id": 32,
       "title": "Necessitatibus magnam qui at velit consequatur perspiciatis.",
-      "assignee_id": 22,
       "author_id": 15,
       "project_id": 5,
       "created_at": "2016-06-14T15:02:07.431Z",
@@ -1932,7 +1940,6 @@
     {
       "id": 31,
       "title": "Libero nam magnam incidunt eaque placeat error et.",
-      "assignee_id": 1,
       "author_id": 16,
       "project_id": 5,
       "created_at": "2016-06-14T15:02:07.280Z",
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 f4e466d1296..13e930bafe3 100644
--- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
@@ -4,7 +4,12 @@ include ImportExport::CommonUtil
 describe Gitlab::ImportExport::ProjectTreeRestorer do
   describe 'restore project tree' do
     before(:context) do
-      @user = create(:user)
+      # Using an admin for import, so we can check assignment of existing members
+      @user = create(:admin)
+      @existing_members = [
+        create(:user, username: 'bernard_willms'),
+        create(:user, username: 'saul_will')
+      ]
 
       RSpec::Mocks.with_temporary_scope do
         @project = create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project')
@@ -63,8 +68,9 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
         expect(issue.reload.updated_at.to_s).to eq('2016-06-14 15:02:47 UTC')
       end
 
-      it 'has issue assignees' do
-        expect(Issue.where(title: 'Voluptatem').first.issue_assignees).not_to be_empty
+      it 'has multiple issue assignees' do
+        expect(Issue.find_by(title: 'Voluptatem').assignees).to contain_exactly(@user, *@existing_members)
+        expect(Issue.find_by(title: 'Issue without assignees').assignees).to be_empty
       end
 
       it 'contains the merge access levels on a protected branch' do