From 67b69336a1334bd096b2c0c7c46f0cc03eb3507d Mon Sep 17 00:00:00 2001
From: charlie ablett <cablett@gitlab.com>
Date: Wed, 28 Sep 2022 18:55:20 +1300
Subject: [PATCH 1/2] First attempt at import csv

---
 app/services/import_csv/base_service.rb       | 103 ++++++++++++++++++
 .../issuable/import_csv/base_service.rb       |  87 ++-------------
 app/services/issues/import_csv_service.rb     |   4 +-
 app/services/work_items/import_csv_service.rb |  39 +++++++
 .../import_work_items_csv_worker.rb           |  30 +++++
 .../import_csv_service.rb                     |   6 +-
 6 files changed, 184 insertions(+), 85 deletions(-)
 create mode 100644 app/services/import_csv/base_service.rb
 create mode 100644 app/services/work_items/import_csv_service.rb
 create mode 100644 app/workers/work_items/import_work_items_csv_worker.rb

diff --git a/app/services/import_csv/base_service.rb b/app/services/import_csv/base_service.rb
new file mode 100644
index 00000000000000..51e91412986c68
--- /dev/null
+++ b/app/services/import_csv/base_service.rb
@@ -0,0 +1,103 @@
+# frozen_string_literal: true
+
+module ImportCsv
+  class BaseService
+    def initialize(user, project, csv_io)
+      @user = user
+      @project = project
+      @csv_io = csv_io
+      @results = { success: 0, error_lines: [], parse_error: false }
+    end
+
+    def execute
+      process_csv
+      email_results_to_user
+
+      @results
+    end
+
+    private
+
+    def process_csv
+      with_csv_lines.each do |row, line_no|
+        attributes = attributes_for(row)
+
+        if create_object(attributes)&.persisted?
+          @results[:success] += 1
+        else
+          @results[:error_lines].push(line_no)
+        end
+      end
+    rescue ArgumentError, CSV::MalformedCSVError
+      @results[:parse_error] = true
+    end
+
+    def attributes_for(row)
+      raise NotImplementedError
+    end
+
+    def headers_valid?
+      required_headers.all? { |header| headers.include?(header) }
+    end
+
+    def with_csv_lines
+      csv_data = @csv_io.open(&:read).force_encoding(Encoding::UTF_8)
+      validate_headers_presence!(csv_data.lines.first)
+
+      CSV.new(
+        csv_data,
+        col_sep: detect_col_sep(csv_data.lines.first),
+        headers: true,
+        header_converters: :symbol
+      ).each.with_index(2)
+    end
+
+    def validate_headers_presence!(headers)
+      headers.downcase! if headers
+      return if headers && headers_valid?
+
+      raise CSV::MalformedCSVError
+    end
+
+    def detect_col_sep(header)
+      if header.include?(",")
+        ","
+      elsif header.include?(";")
+        ";"
+      elsif header.include?("\t")
+        "\t"
+      else
+        raise CSV::MalformedCSVError
+      end
+    end
+
+    def create_object(attributes)
+      # NOTE: CSV imports are performed by workers, so we do not have a request context in order
+      # to create a SpamParams object to pass to the issuable create service.
+      spam_params = nil
+      create_service = create_service_class.new(project: @project,
+                                                current_user: @user,
+                                                params: attributes,
+                                                spam_params: spam_params).merge(create_service_params)
+
+      # For now, if create_service_class prepends RateLimitedService let's bypass rate limiting
+      if create_service_class < RateLimitedService
+        create_service.execute_without_rate_limiting
+      else
+        create_service.execute
+      end
+    end
+
+    def extra_create_service_params
+      {}
+    end
+
+    def email_results_to_user
+      # defined in ImportCsvService
+    end
+
+    def create_service_class
+      # defined in ImportCsvService
+    end
+  end
+end
diff --git a/app/services/issuable/import_csv/base_service.rb b/app/services/issuable/import_csv/base_service.rb
index e84d1032e41144..30a622621ca867 100644
--- a/app/services/issuable/import_csv/base_service.rb
+++ b/app/services/issuable/import_csv/base_service.rb
@@ -2,38 +2,13 @@
 
 module Issuable
   module ImportCsv
-    class BaseService
-      def initialize(user, project, csv_io)
-        @user = user
-        @project = project
-        @csv_io = csv_io
-        @results = { success: 0, error_lines: [], parse_error: false }
-      end
-
-      def execute
-        process_csv
-        email_results_to_user
-
-        @results
-      end
+    class BaseService < ::ImportCsv::BaseService
+      extend ::Gitlab::Utils::Override
 
       private
 
-      def process_csv
-        with_csv_lines.each do |row, line_no|
-          attributes = issuable_attributes_for(row)
-
-          if create_issuable(attributes)&.persisted?
-            @results[:success] += 1
-          else
-            @results[:error_lines].push(line_no)
-          end
-        end
-      rescue ArgumentError, CSV::MalformedCSVError
-        @results[:parse_error] = true
-      end
-
-      def issuable_attributes_for(row)
+      override :attributes_for
+      def attributes_for(row)
         {
           title: row[:title],
           description: row[:description],
@@ -41,57 +16,9 @@ def issuable_attributes_for(row)
         }
       end
 
-      def with_csv_lines
-        csv_data = @csv_io.open(&:read).force_encoding(Encoding::UTF_8)
-        validate_headers_presence!(csv_data.lines.first)
-
-        CSV.new(
-          csv_data,
-          col_sep: detect_col_sep(csv_data.lines.first),
-          headers: true,
-          header_converters: :symbol
-        ).each.with_index(2)
-      end
-
-      def validate_headers_presence!(headers)
-        headers.downcase! if headers
-        return if headers && headers.include?('title') && headers.include?('description')
-
-        raise CSV::MalformedCSVError
-      end
-
-      def detect_col_sep(header)
-        if header.include?(",")
-          ","
-        elsif header.include?(";")
-          ";"
-        elsif header.include?("\t")
-          "\t"
-        else
-          raise CSV::MalformedCSVError
-        end
-      end
-
-      def create_issuable(attributes)
-        # NOTE: CSV imports are performed by workers, so we do not have a request context in order
-        # to create a SpamParams object to pass to the issuable create service.
-        spam_params = nil
-        create_service = create_issuable_class.new(project: @project, current_user: @user, params: attributes, spam_params: spam_params)
-
-        # For now, if create_issuable_class prepends RateLimitedService let's bypass rate limiting
-        if create_issuable_class < RateLimitedService
-          create_service.execute_without_rate_limiting
-        else
-          create_service.execute
-        end
-      end
-
-      def email_results_to_user
-        # defined in ImportCsvService
-      end
-
-      def create_issuable_class
-        # defined in ImportCsvService
+      override :required_headers
+      def required_headers
+        %w[title description].freeze
       end
     end
   end
diff --git a/app/services/issues/import_csv_service.rb b/app/services/issues/import_csv_service.rb
index 83e550583f60e5..aa0bc5d1fe080d 100644
--- a/app/services/issues/import_csv_service.rb
+++ b/app/services/issues/import_csv_service.rb
@@ -14,11 +14,11 @@ def email_results_to_user
 
     private
 
-    def create_issuable(attributes)
+    def create_object(attributes)
       super[:issue]
     end
 
-    def create_issuable_class
+    def create_service_class
       Issues::CreateService
     end
 
diff --git a/app/services/work_items/import_csv_service.rb b/app/services/work_items/import_csv_service.rb
new file mode 100644
index 00000000000000..989b5cfe7b993e
--- /dev/null
+++ b/app/services/work_items/import_csv_service.rb
@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+module WorkItems
+  class ImportCsvService < ::ImportCsv::BaseService
+    def execute
+      raise Gitlab::Access::AccessDeniedError unless Ability.allowed?(@user, :create_work_item, @project)
+
+      super
+    end
+
+    private
+
+    override :attributes_for
+    def attributes_for(row)
+      {
+        # depends on work item type
+        # map key -> value
+      }
+    end
+
+    override :create_service_params
+    def extra_create_service_params
+      { widget_params: {} }
+    end
+
+    override :required_headers
+    def required_headers
+      # depends on work item type
+    end
+
+    def create_service_class
+      ::WorkItems::CreateService
+    end
+
+    def email_results_to_user
+      Notify.import_work_items_csv_email(@user.id, @project.id, @results).deliver_later
+    end
+  end
+end
diff --git a/app/workers/work_items/import_work_items_csv_worker.rb b/app/workers/work_items/import_work_items_csv_worker.rb
new file mode 100644
index 00000000000000..c20aaba3f3e0f9
--- /dev/null
+++ b/app/workers/work_items/import_work_items_csv_worker.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+module WorkItems
+  class ImportWorkItemsCsvWorker
+    include ApplicationWorker
+
+    data_consistency :always
+
+    sidekiq_options retry: 3
+    include Gitlab::Utils::StrongMemoize
+
+    idempotent!
+    feature_category :work_items
+
+    sidekiq_retries_exhausted do |job|
+      Upload.find(job['args'][2]).destroy
+    end
+
+    def perform(current_user_id, parent_id, upload_id)
+      upload = Upload.find(upload_id)
+      user = User.find(current_user_id)
+      parent_link = ::WorkItems::ParentLink.find(parent_id)
+
+      WorkItems::ImportCsvService.new(user, parent_link, upload.retrieve_uploader).execute
+      upload.destroy!
+    rescue ActiveRecord::RecordNotFound
+      # Resources have been removed, job should not be retried
+    end
+  end
+end
diff --git a/ee/app/services/requirements_management/import_csv_service.rb b/ee/app/services/requirements_management/import_csv_service.rb
index 07d3162a033dc5..406a425352214e 100644
--- a/ee/app/services/requirements_management/import_csv_service.rb
+++ b/ee/app/services/requirements_management/import_csv_service.rb
@@ -10,11 +10,11 @@ def execute
 
     private
 
-    def create_issuable(attributes)
+    def create_object(attributes)
       super[:issue]
     end
 
-    def create_issuable_class
+    def create_service_class
       ::Issues::CreateService
     end
 
@@ -22,7 +22,7 @@ def email_results_to_user
       Notify.import_requirements_csv_email(@user.id, @project.id, @results).deliver_later
     end
 
-    def issuable_attributes_for(row)
+    def attributes_for(row)
       super.merge(issue_type: 'requirement')
     end
   end
-- 
GitLab


From 987f021565ef11e129993e9b518a80d6d48ea2dc Mon Sep 17 00:00:00 2001
From: charlie ablett <cablett@gitlab.com>
Date: Tue, 4 Oct 2022 19:08:51 +1300
Subject: [PATCH 2/2] [skip-ci] First attempt at import csv

---
 app/mailers/emails/issues.rb                   | 10 ++++++++++
 app/services/import_csv/base_service.rb        | 18 ++++++++++++------
 app/services/work_items/import_csv_service.rb  |  4 +++-
 .../notify/import_issues_csv_email.html.haml   |  6 +++---
 .../notify/import_issues_csv_email.text.erb    |  4 ++--
 .../work_items/import_work_items_csv_worker.rb |  4 ++--
 config/sidekiq_queues.yml                      |  2 ++
 locale/gitlab.pot                              | 13 +++++++++----
 .../import_issues_csv_email.html.haml_spec.rb  |  6 +++---
 9 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/app/mailers/emails/issues.rb b/app/mailers/emails/issues.rb
index 6a5680c080bd36..7227a284e52329 100644
--- a/app/mailers/emails/issues.rb
+++ b/app/mailers/emails/issues.rb
@@ -99,6 +99,16 @@ def import_issues_csv_email(user_id, project_id, results)
         subject: subject('Imported issues'))
     end
 
+    def import_work_items_csv_email(user_id, project_id, results)
+      @user = User.find(user_id)
+      @project = Project.find(project_id)
+      @results = results
+
+      email_with_layout(
+        to: @user.notification_email_for(@project.group),
+        subject: subject('Imported work items'))
+    end
+
     def issues_csv_email(user, project, csv_data, export_status)
       @project = project
       @count = export_status.fetch(:rows_expected)
diff --git a/app/services/import_csv/base_service.rb b/app/services/import_csv/base_service.rb
index 51e91412986c68..5f5883a5860336 100644
--- a/app/services/import_csv/base_service.rb
+++ b/app/services/import_csv/base_service.rb
@@ -36,10 +36,14 @@ def attributes_for(row)
       raise NotImplementedError
     end
 
-    def headers_valid?
+    def headers_valid?(headers)
       required_headers.all? { |header| headers.include?(header) }
     end
 
+    def required_headers
+      NotImplementedError
+    end
+
     def with_csv_lines
       csv_data = @csv_io.open(&:read).force_encoding(Encoding::UTF_8)
       validate_headers_presence!(csv_data.lines.first)
@@ -54,7 +58,7 @@ def with_csv_lines
 
     def validate_headers_presence!(headers)
       headers.downcase! if headers
-      return if headers && headers_valid?
+      return if headers && headers_valid?(headers)
 
       raise CSV::MalformedCSVError
     end
@@ -75,10 +79,12 @@ def create_object(attributes)
       # NOTE: CSV imports are performed by workers, so we do not have a request context in order
       # to create a SpamParams object to pass to the issuable create service.
       spam_params = nil
-      create_service = create_service_class.new(project: @project,
-                                                current_user: @user,
-                                                params: attributes,
-                                                spam_params: spam_params).merge(create_service_params)
+      default_params = { project: @project,
+                         current_user: @user,
+                         params: attributes,
+                         spam_params: spam_params }
+
+      create_service = create_service_class.new(**default_params.merge(extra_create_service_params))
 
       # For now, if create_service_class prepends RateLimitedService let's bypass rate limiting
       if create_service_class < RateLimitedService
diff --git a/app/services/work_items/import_csv_service.rb b/app/services/work_items/import_csv_service.rb
index 989b5cfe7b993e..4313d49f8a456f 100644
--- a/app/services/work_items/import_csv_service.rb
+++ b/app/services/work_items/import_csv_service.rb
@@ -2,6 +2,8 @@
 
 module WorkItems
   class ImportCsvService < ::ImportCsv::BaseService
+    extend ::Gitlab::Utils::Override
+
     def execute
       raise Gitlab::Access::AccessDeniedError unless Ability.allowed?(@user, :create_work_item, @project)
 
@@ -18,7 +20,7 @@ def attributes_for(row)
       }
     end
 
-    override :create_service_params
+    override :extra_create_service_params
     def extra_create_service_params
       { widget_params: {} }
     end
diff --git a/app/views/notify/import_issues_csv_email.html.haml b/app/views/notify/import_issues_csv_email.html.haml
index 0008085025bd9f..5613d08142f391 100644
--- a/app/views/notify/import_issues_csv_email.html.haml
+++ b/app/views/notify/import_issues_csv_email.html.haml
@@ -5,12 +5,12 @@
   = s_('Notify|Your CSV import for project %{project_link} has been completed.').html_safe % { project_link: project_link }
 
 %p{ style: text_style }
-  - issues = n_('%d issue', '%d issues', @results[:success]) % @results[:success]
-  = s_('Notify|%{issues} imported.') % { issues: issues }
+  - work_items = n_('%d work item', '%d work items', @results[:success]) % @results[:success]
+  = s_('Notify|%{work_items} imported.') % { work_items: work_items }
 
 - if @results[:error_lines].present?
   %p{ style: text_style }
-    = s_('Notify|Errors found on %{singular_or_plural_line}: %{error_lines}. Please check if these lines have an issue title.') % { singular_or_plural_line: n_('line', 'lines', @results[:error_lines].size),
+    = s_('Notify|Errors found on %{singular_or_plural_line}: %{error_lines}. Please check if these lines have an work item title.') % { singular_or_plural_line: n_('line', 'lines', @results[:error_lines].size),
       error_lines: @results[:error_lines].join(', ') }
 
 - if @results[:parse_error]
diff --git a/app/views/notify/import_issues_csv_email.text.erb b/app/views/notify/import_issues_csv_email.text.erb
index 1117f90714dc89..454441e36b9534 100644
--- a/app/views/notify/import_issues_csv_email.text.erb
+++ b/app/views/notify/import_issues_csv_email.text.erb
@@ -1,9 +1,9 @@
 Your CSV import for project <%= @project.full_name %> (<%= project_url(@project) %>) has been completed.
 
-<%= pluralize(@results[:success], 'issue') %> imported.
+<%= pluralize(@results[:success], 'work_items') %> imported.
 
 <% if @results[:error_lines].present? %>
-Errors found on line <%= 'number'.pluralize(@results[:error_lines].size) %>: <%= @results[:error_lines].join(', ') %>. Please check if these lines have an issue title.
+Errors found on line <%= 'number'.pluralize(@results[:error_lines].size) %>: <%= @results[:error_lines].join(', ') %>. Please check if these lines have an work items title.
 <% end %>
 
 <% if @results[:parse_error] %>
diff --git a/app/workers/work_items/import_work_items_csv_worker.rb b/app/workers/work_items/import_work_items_csv_worker.rb
index c20aaba3f3e0f9..b9e3196d69127f 100644
--- a/app/workers/work_items/import_work_items_csv_worker.rb
+++ b/app/workers/work_items/import_work_items_csv_worker.rb
@@ -16,10 +16,10 @@ class ImportWorkItemsCsvWorker
       Upload.find(job['args'][2]).destroy
     end
 
-    def perform(current_user_id, parent_id, upload_id)
+    def perform(current_user_id, project_id, upload_id)
       upload = Upload.find(upload_id)
       user = User.find(current_user_id)
-      parent_link = ::WorkItems::ParentLink.find(parent_id)
+      parent_link = ::WorkItems::ParentLink.find(project_id)
 
       WorkItems::ImportCsvService.new(user, parent_link, upload.retrieve_uploader).execute
       upload.destroy!
diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml
index 548e643c7feb17..49d5c6eaac85bc 100644
--- a/config/sidekiq_queues.yml
+++ b/config/sidekiq_queues.yml
@@ -517,5 +517,7 @@
   - 1
 - - wikis_git_garbage_collect
   - 1
+- - work_items_import_work_items_csv
+  - 1
 - - x509_certificate_revoke
   - 1
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index a4ac7b42231a7e..aa0da50229a25d 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -468,6 +468,11 @@ msgid_plural "%d warnings found:"
 msgstr[0] ""
 msgstr[1] ""
 
+msgid "%d work item"
+msgid_plural "%d work items"
+msgstr[0] ""
+msgstr[1] ""
+
 msgid "%s additional commit has been omitted to prevent performance issues."
 msgid_plural "%s additional commits have been omitted to prevent performance issues."
 msgstr[0] ""
@@ -27484,9 +27489,6 @@ msgstr ""
 msgid "Notify|%{invited_user} has %{highlight_start}declined%{highlight_end} your invitation to join the %{target_link} %{target_name}."
 msgstr ""
 
-msgid "Notify|%{issues} imported."
-msgstr ""
-
 msgid "Notify|%{member_link} requested %{member_role} access to the %{target_source_link} %{target_type}."
 msgstr ""
 
@@ -27514,6 +27516,9 @@ msgstr ""
 msgid "Notify|%{updated_by_user_name} pushed new commits to merge request %{mr_link}"
 msgstr ""
 
+msgid "Notify|%{work_items} imported."
+msgstr ""
+
 msgid "Notify|A new GPG key was added to your account:"
 msgstr ""
 
@@ -27559,7 +27564,7 @@ msgstr ""
 msgid "Notify|Error parsing CSV file. Please make sure it has the correct format: a delimited text file that uses a comma to separate values."
 msgstr ""
 
-msgid "Notify|Errors found on %{singular_or_plural_line}: %{error_lines}. Please check if these lines have an issue title."
+msgid "Notify|Errors found on %{singular_or_plural_line}: %{error_lines}. Please check if these lines have an work item title."
 msgstr ""
 
 msgid "Notify|Fingerprint: %{fingerprint}"
diff --git a/spec/views/notify/import_issues_csv_email.html.haml_spec.rb b/spec/views/notify/import_issues_csv_email.html.haml_spec.rb
index 43dfab87ac9abc..9135bd7a3c407a 100644
--- a/spec/views/notify/import_issues_csv_email.html.haml_spec.rb
+++ b/spec/views/notify/import_issues_csv_email.html.haml_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'notify/import_issues_csv_email.html.haml' do
+RSpec.describe 'notify/import_work_items_csv_email.html.haml' do
   let(:user) { create(:user) }
   let(:project) { create(:project) }
   let(:correct_results) { { success: 3, valid_file: true } }
@@ -23,7 +23,7 @@
       render
 
       expect(rendered).to have_link(project.full_name, href: project_url(project))
-      expect(rendered).to have_content("3 issues imported")
+      expect(rendered).to have_content("3 work items imported")
       expect(rendered).not_to have_content("Errors found on line")
       expect(rendered).not_to have_content(
         "Error parsing CSV file. Please make sure it has the correct format: \
@@ -40,7 +40,7 @@
       render
 
       expect(rendered).to have_content("Errors found on lines: #{errored_results[:error_lines].join(", ")}. \
-Please check if these lines have an issue title.")
+Please check if these lines have an work item title.")
       expect(rendered).not_to have_content("Error parsing CSV file. Please make sure it has the correct format: \
 a delimited text file that uses a comma to separate values.")
     end
-- 
GitLab