From ab65f7d0d61f59f8656e76e0341b00d528aee5ce Mon Sep 17 00:00:00 2001
From: Josianne Hyson <jhyson@gitlab.com>
Date: Mon, 2 Mar 2020 09:55:18 +0000
Subject: [PATCH] Fix issue import to accept export format

We want to be able to export issues and then use that export file to
import the issues to another GitLab project.

Prior to this change, the format of the export file did not match the
structure of the file that the issue importer expects. This change
expands the importer to first look for the know export column names and
then fallback to positional values if they are not present.

Addresses:
- https://gitlab.com/gitlab-org/gitlab/issues/199038
- https://gitlab.com/gitlab-org/gitlab/issues/30931
---
 app/services/issues/import_csv_service.rb     | 15 +++++-
 ...jhyson-issue-import-export-consistency.yml |  5 ++
 doc/user/project/issues/csv_import.md         |  8 +--
 .../issues/export_csv_service_spec.rb         |  4 ++
 spec/fixtures/csv_gitlab_export.csv           |  5 ++
 .../issues/import_csv_service_spec.rb         | 50 +++++++++++++++++++
 6 files changed, 81 insertions(+), 6 deletions(-)
 create mode 100644 changelogs/unreleased/jhyson-issue-import-export-consistency.yml
 create mode 100644 spec/fixtures/csv_gitlab_export.csv

diff --git a/app/services/issues/import_csv_service.rb b/app/services/issues/import_csv_service.rb
index ef08fafa7ccd2..c01db5fcfe62c 100644
--- a/app/services/issues/import_csv_service.rb
+++ b/app/services/issues/import_csv_service.rb
@@ -21,8 +21,19 @@ def execute
     def process_csv
       csv_data = @csv_io.open(&:read).force_encoding(Encoding::UTF_8)
 
-      CSV.new(csv_data, col_sep: detect_col_sep(csv_data.lines.first), headers: true).each.with_index(2) do |row, line_no|
-        issue = Issues::CreateService.new(@project, @user, title: row[0], description: row[1]).execute
+      csv_parsing_params = {
+        col_sep: detect_col_sep(csv_data.lines.first),
+        headers: true,
+        header_converters: :symbol
+      }
+
+      CSV.new(csv_data, csv_parsing_params).each.with_index(2) do |row, line_no|
+        issue_attributes = {
+          title:       row[:title],
+          description: row[:description]
+        }
+
+        issue = Issues::CreateService.new(@project, @user, issue_attributes).execute
 
         if issue.persisted?
           @results[:success] += 1
diff --git a/changelogs/unreleased/jhyson-issue-import-export-consistency.yml b/changelogs/unreleased/jhyson-issue-import-export-consistency.yml
new file mode 100644
index 0000000000000..0d2b8167d9427
--- /dev/null
+++ b/changelogs/unreleased/jhyson-issue-import-export-consistency.yml
@@ -0,0 +1,5 @@
+---
+title: Fix issue importer so it matches issue export format
+merge_request: 25896
+author:
+type: fixed
diff --git a/doc/user/project/issues/csv_import.md b/doc/user/project/issues/csv_import.md
index 56643dcee537c..d67b135186f62 100644
--- a/doc/user/project/issues/csv_import.md
+++ b/doc/user/project/issues/csv_import.md
@@ -3,7 +3,7 @@
 > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/23532) in GitLab 11.7.
 
 Issues can be imported to a project by uploading a CSV file with the columns
-`title` and `description`, in that order.
+`title` and `description`.
 
 The user uploading the CSV file will be set as the author of the imported issues.
 
@@ -31,9 +31,9 @@ to you once the import is complete.
 
 When importing issues from a CSV file, it must be formatted in a certain way:
 
-- **header row:** CSV files must contain a header row where the first column header
-  is `title` and the second is `description`. If additional columns are present, they
-  will be ignored.
+- **header row:** CSV files must include the following headers:
+`title` and `description`. The case of the headers does not matter.
+- **columns:** Data from columns beyond `title` and `description` are not imported.
 - **separators:** The column separator is automatically detected from the header row.
   Supported separator characters are: commas (`,`), semicolons (`;`), and tabs (`\t`).
   The row separator can be either `CRLF` or `LF`.
diff --git a/ee/spec/services/issues/export_csv_service_spec.rb b/ee/spec/services/issues/export_csv_service_spec.rb
index 5594e1a648d04..a664a0a73427e 100644
--- a/ee/spec/services/issues/export_csv_service_spec.rb
+++ b/ee/spec/services/issues/export_csv_service_spec.rb
@@ -54,6 +54,10 @@ def csv
                     time_estimate: 72000)
     end
 
+    it 'includes the columns required for import' do
+      expect(csv.headers).to include('Title', 'Description')
+    end
+
     specify 'iid' do
       expect(csv[0]['Issue ID']).to eq issue.iid.to_s
     end
diff --git a/spec/fixtures/csv_gitlab_export.csv b/spec/fixtures/csv_gitlab_export.csv
new file mode 100644
index 0000000000000..65422509eef4a
--- /dev/null
+++ b/spec/fixtures/csv_gitlab_export.csv
@@ -0,0 +1,5 @@
+Issue ID,URL,Title,State,Description,Author,Author Username,Assignee,Assignee Username,Confidential,Locked,Due Date,Created At (UTC),Updated At (UTC),Closed At (UTC),Milestone,Weight,Labels,Time Estimate,Time Spent,Epic ID,Epic Title
+1,http://localhost:3000/jashkenas/underscore/issues/1,Title,Open,,Elva Jerde,jamel,Tierra Effertz,aurora_hahn,No,No,,2020-01-17 10:36:26,2020-02-19 10:36:26,,v1.0,,"Brene,Cutlass,Escort,GVM",0,0,,
+3,http://localhost:3000/jashkenas/underscore/issues/3,Nihil impedit neque quos totam ut aut enim cupiditate doloribus molestiae.,Open,Omnis aliquid sint laudantium quam.,Marybeth Goodwin,rocio.blanda,Annemarie Von,reynalda_howe,No,No,,2020-01-23 10:36:26,2020-02-19 10:36:27,,v1.0,,"Brene,Cutlass,Escort,GVM",0,0,,
+34,http://localhost:3000/jashkenas/underscore/issues/34,Dismiss Cipher with no integrity,Open,,Marybeth Goodwin,rocio.blanda,"","",No,No,,2020-02-19 10:38:49,2020-02-19 10:38:49,,,,,0,0,,
+35,http://localhost:3000/jashkenas/underscore/issues/35,Test Title,Open,Test Description,Marybeth Goodwin,rocio.blanda,"","",No,No,,2020-02-19 10:38:49,2020-02-19 10:38:49,,,,,0,0,,
diff --git a/spec/services/issues/import_csv_service_spec.rb b/spec/services/issues/import_csv_service_spec.rb
index e7370407d4c02..aa43892a36d37 100644
--- a/spec/services/issues/import_csv_service_spec.rb
+++ b/spec/services/issues/import_csv_service_spec.rb
@@ -27,6 +27,29 @@
       end
     end
 
+    context 'with a file generated by Gitlab CSV export' do
+      let(:file) { fixture_file_upload('spec/fixtures/csv_gitlab_export.csv') }
+
+      it 'imports the CSV without errors' do
+        expect_next_instance_of(Notify) do |instance|
+          expect(instance).to receive(:import_issues_csv_email)
+        end
+
+        expect(subject[:success]).to eq(4)
+        expect(subject[:error_lines]).to eq([])
+        expect(subject[:parse_error]).to eq(false)
+      end
+
+      it 'correctly sets the issue attributes' do
+        expect { subject }.to change { project.issues.count }.by 4
+
+        expect(project.issues.reload.last).to have_attributes(
+          title: 'Test Title',
+          description: 'Test Description'
+        )
+      end
+    end
+
     context 'comma delimited file' do
       let(:file) { fixture_file_upload('spec/fixtures/csv_comma.csv') }
 
@@ -39,6 +62,15 @@
         expect(subject[:error_lines]).to eq([])
         expect(subject[:parse_error]).to eq(false)
       end
+
+      it 'correctly sets the issue attributes' do
+        expect { subject }.to change { project.issues.count }.by 3
+
+        expect(project.issues.reload.last).to have_attributes(
+          title: 'Title with quote"',
+          description: 'Description'
+        )
+      end
     end
 
     context 'tab delimited file with error row' do
@@ -53,6 +85,15 @@
         expect(subject[:error_lines]).to eq([3])
         expect(subject[:parse_error]).to eq(false)
       end
+
+      it 'correctly sets the issue attributes' do
+        expect { subject }.to change { project.issues.count }.by 2
+
+        expect(project.issues.reload.last).to have_attributes(
+          title: 'Hello',
+          description: 'World'
+        )
+      end
     end
 
     context 'semicolon delimited file with CRLF' do
@@ -67,6 +108,15 @@
         expect(subject[:error_lines]).to eq([4])
         expect(subject[:parse_error]).to eq(false)
       end
+
+      it 'correctly sets the issue attributes' do
+        expect { subject }.to change { project.issues.count }.by 3
+
+        expect(project.issues.reload.last).to have_attributes(
+          title: 'Hello',
+          description: 'World'
+        )
+      end
     end
   end
 end
-- 
GitLab